WIP: feat(gameplay): add experimental cutscene skipping #200
No reviewers
Labels
No labels
area/accessibility
area/audio
area/input
area/install
area/menu
area/renderer
area/saves
area/upscaling
good first issue
gpu/amd
gpu/igpu
gpu/intel
gpu/nvidia
help wanted
kind
bug
kind
chore
kind
docs
kind
feature
kind
performance
kind
question
needs measurements
platform/linux
platform/macos
platform/steam-deck
platform/windows
preview
priority
critical
priority
high
priority
low
status
blocked
status
confirmed
status
duplicate
status
invalid
status
needs-info
status
wontfix
testing wanted
ci: approved
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Longshot!200
Loading…
Reference in a new issue
No description provided.
Delete branch "brunobaccari/Longshot:feat/longshot-experimental-cutscene-skip"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Why
Some cutscenes do not expose the game's normal skip prompt. This adds an optional experimental way to request that prompt and confirm with its displayed button. It stays off by default.
What changed
skip_unskippable_cutscenes. English and PT-BR help warn that skipping can leave story progress incomplete or soft-lock a save; save first. Dialogue overlap and incomplete conversation/chest support remain explicit.Dm_Kokiri_0000andDm_Kokiri_0030, read from the active player's own resource. Unknown names, suffix matches and unreadable memory keep native permissions. The existing Skip opening remains independent.cutscene skip: override allowed for timeline ...once per timeline execution. The normal timeline-start hook resets the diagnostic even with Skip opening off. This logs permission being enabled, not proof that the player confirmed a skip.MkwVkHost/MkwVkRendererlayout change; API21 is unchanged.Flags and settings
skip_unskippable_cutscenes/ Gameplay → Skip cutscenes (Experimental)0)No environment/build switch is added. Keep
skip_opening=0when validating this interactive toggle.Measurements
main:
514591a3(2026-11-13)this PR: feat/longshot-experimental-cutscene-skip @
7d821fc0Hardware: Windows 11 Pro 26300, Ryzen 5 3600, 64 GiB RAM, Radeon RX 5500 XT, driver 32.0.21045.16010; Builder Python/llvm-mingw for directed checks. Linux focals use Alpine 3.22, GCC/G++ 14.2.0, Python 3.12.16 in Docker/WSL2. Current runtime measurements are pending.
Scene: Current-head opening movie and authored-unskippable 3D cutscene, followed by save/reload, are pending.
Method: API readback identifies current head
7d821fc0on main514591a3. No new validation result is added by this description update. At the earlier d3c79356 / maind9648863checkpoint, a source-only comparison (git range-diff) confirmed all five PR patches were unchanged from the previously checked 3b44463f / mainf3fe1763series. Upstream menu/input code changed, so earlier test results are kept with their original commits rather than relabeled as current-head runs. On 3b44463f, twelve of thirteen focused Windows scripts passed and the i18n script had one failure also reproduced on mainf3fe1763. Earlier Linux checks were on f2487bc7 / mainfbd42024. No complete current-head suite, application build, runtime or performance result is claimed.How to reproduce: Build both named revisions from the same supported dump, use physically copied saves and identical settings, keep Skip opening off, and compare the native dialogue-skip shortcut with the experimental toggle off/on. Confirm the actual destination scene and control, then save and reload. Warm the scene before two interleaved runs per revision; captures are outside timing samples.
514591a37d821fc0Previous Windows run on 3b44463f / main
f3fe1763(not a new run on7d821fc0):Older evidence on main
0c74253dand PR080c3873is historical only: a Windows build, 484 passed / 6 failed / 33 skipped with baseline failure checks, limited Linux fixtures/host-TU compilation, and scripted opening/3D skip observations. Later upstream renderer changes and this review follow-up invalidate transferring those results to the current head. Historical FPS runs varied too much to establish zero cost or a speed change.Testing
Tested: For current
7d821fc0, API readback verified the head and base514591a3. No completed source-test, application-build or runtime result is added by this metadata update. At the earlierd3c79356checkpoint, read-only API/Git checks verified its base, head and unchanged PR patch series. On prior3b44463f, twelve focused scripts passed:test_option_ids.py,test_cutscene_skip.py,test_opening_skip.py,test_oot_damage.py,test_damage_debug_menu.py,test_menu_keys.py,test_readme_menu_rows.py,test_audio_output_menu.py,test_stick_dead_zone.py,test_framegen_preset.py,test_menu_env_overrides.py, andtest_rumble_menu.py. These cover both feature IDs/callbacks, exact timeline allowlist, native/Off behavior, malformed pointers, log reset/deduplication, 24 readiness combinations, damage scaling and Debug visibility, menu navigation/persistence and source-reading fixtures.git diff --checkpasses. Independent comparison found no actionable issue: upstream damage functions and the PR skip functions were preserved, andMkwVkHost/MkwVkRenderermatch main exactly.Baseline failure:
test_menu_i18n.pyruns five tests: four pass andtest_texts_are_the_menusfails because an existing PT-BR Debug help key no longer matches the English menu text. The same five-test run fails identically on actual mainf3fe1763files. The conflict repair does not alter that upstream translation.Earlier evidence: On
f2487bc7/ mainfbd42024, eight Linux focal scripts and both changed host translation units passed with official standalone headers. Exact patch anchors also matched the preserved partialf647a072lift, with idempotence checked. These earlier checks are not a new full build or runtime result for the rebased head.Read-only inspection of the locally owned supported build
e0f3ccfaf50065ae4ea01eb5aeb13b1a45da3348confirmed the resource/name relationship used by the allowlist against both NSO and image. No game bytes, disassembly, generated code or binaries are included in this PR.Not tested: A complete current-head full suite (the attempt was interrupted during local build contention and is not counted as a full result), complete Windows application build, current UI screenshots, movie and authored-unskippable 3D skip followed by save/reload, second-movie playback/decoder continuity, and interleaved performance runs are pending. Earlier Linux compilation was limited to the changed host translation units and fixtures; no Linux rerun, full Linux application/renderer build or GPU execution is claimed for this rebased head. Audio is muted in local scripted runs, so audible playback has not been checked. Conversations/chests remain incomplete.
Needs other testers: None requested while author validation and review fixes are in progress. This is not a claim that untested platforms or paths are covered; no external test request was sent.
Screenshots
Current-head screenshots are pending. Existing attachments show the historical
080c3873implementation and must not be treated as evidence for this revised head:Risk and rollback
Skipping can omit authored story actions, flags, items, songs, age/time changes or warps and leave progress incomplete or a save soft-locked. Save first and keep the experimental option off by default. The native completion path alone does not prove story-state integrity. Disable the toggle (
skip_unskippable_cutscenes=0) to restore native permission; reverting this PR removes the option. Skip opening remains separate.Reviewer notes
Current state (2026-11-13): head
7d821fc0is based on514591a3. Prior patch-series comparisons remain evidence at their named commits. The separate author validation session is using this current head; no completed game result is claimed here. Both adjacent option IDs remain ingpu/vk/vk_api.h. The PR is WIP because full-suite, complete platform builds, current screenshots, save/reload and interleaved measurements remain pending. Existing review/comments are preserved; no checkbox is ticked solely to pass the description job.Review follow-ups in a dedicated worktree address the help/docs warning (13988), INI comment placement (13989), coverage restriction (13990) and diagnostic logging (13991). The atomic option snapshot is retained because it protects reads from game threads. Current runtime validation and save/reload remain prerequisites; no request-changes review is claimed as dismissed.
Issues
None.
Checklist
For everyone
type(scope): description).main. — API readback:514591a3, 2026-11-13.python -I -B tests/run_all.pyrun and passing (any failure that also fails onmainis named above).third_party/untouched (a new folder with aPROVENANCE.mdinstead).If applicable
Assisted-by:/Co-Authored-By:trailers.docs/tested-configurations.mdfor each measured run.mainand passing here.tests/that reads changed source updated in this PR, not weakened.Unchecked boxes name validation still pending on the revised head. This PR remains WIP; a successful description job in WIP does not establish readiness or complete these checks.
Ran the full Windows suite: 484 passed, 6 failed, 33 skipped, with source and fixture hashes unchanged. All six failures also reproduce on clean main and are named in the description. The actual new host C and directed fixtures passed on Linux. Release builds and in-game measurements are still in progress; this remains WIP.
The complete Windows standalone build linked the executable and Vulkan renderer with zero failures, and the generated output contains both guarded skip patches. The actual changed host translation unit compiled on Linux with the official runtime headers; no full Linux application or GPU run is claimed. Main advanced only in README/CONTRIBUTING; final rebase follows the ongoing scripted game comparison. The experimental setting remains off by default and the dialogue/chest limitations are retained. Functional skip and repeated measurements are still pending, so this remains WIP.
WIP: feat(gameplay): add experimental cutscene skippingto feat(gameplay): add experimental cutscene skipping080c38731199a0d64f0cThanks, this is a careful and small way in. The override changes only the authored bit and leaves the game's own input, confirmation and completion alone. Both lifted sites read the same flag (+952 bit 26 is +955 bit 2). The patches are guarded: an exact function, a unique anchor, idempotent, and a missing site is a
WARNINGinreport(). The row is off by default, saved asskip_unskippable_cutscenes, and the WebGPU path reads it as off (rx_option(56, 0)). It leaves the existing Skip opening hooks alone and doesn't touchMkwVkHost/MkwVkRenderer. Nothing in it depends on the frame rate, so I see no above-60 fps interaction.Must change
WIP:, though, so it reads as ready for review. Please put it back to WIP (pr wip 200) until the toggle has been seen working in the game, at least on the opening movie and one authored-unskippable cutscene, followed by a save and reload.f_83b63210orf_80ac8650. The opening-skip notes inhost/hos_main.cshow why some cutscenes may be locked. The game's flowcharts checkIsEventSkipedMostRecentand take a separate skip edge (warp Navi, music, flags, autosave), and the comment there makes a point of saying "Neither timeline writes save data", so other timelines can. A cutscene the designers made unskippable probably has no skip edge written for it. Anything its timeline does mid-play could then be lost on a skip: a flag, an item, a song, a change of age or time, a warp or a scene load. That can leave a save stuck. Please:hos_log, ashos_oot_openingdoes), so a bug report can show which cutscene was skipped;080c3873on0c74253dand leaves Rebased on current main unticked. The head is now5d2ef0af, which merges current main (1ef93e79), so that box can be ticked, but the tested commits and numbers need restating against this head.Should change
Finalize*Decoder.dec_finalizegives the decoder's ready event back (#110), so I see no leak in the code. But a decoder stopped with 16 submissions waiting hasn't been run. When you test the opening movie, please also skip a second movie in the same session and check that it plays, with its sound.ini_keyputs the new key under the comment "rows renamed since their values were first saved keep their keys". It's a new row, so that comment doesn't apply. Move the line above the comment, or just let the key come from the row name.cutscene_skip_nowatomic isn't needed: the other options the game threads read (opts.fov,opts.skip_opening) go straight through the same lambda. It's harmless, but it adds a stub to five test fixtures.Not checked: the anchors against the real lifted code (I have no
code_*.chere). The fixture intests/test_cutscene_skip.pyis put together from the patch's own strings, not taken from the lift, so it can't confirm the addresses either.@ -1257,6 +1257,9 @@ Row rows[] = {"during gameplay. Cutscenes keep their own view.", nullptr, false, true},{TAB_GAMEPLAY, "Text speed", &opts.text_speed, 0, 3, 1, tx_text_speed,"How fast the letters of dialogue and other messages appear.", hn_text_speed, false, true},{TAB_GAMEPLAY, "Skip cutscenes (Experimental)", &opts.allow_cutscene_skip, 0, 1, 1, tx_onoff,The help talks about overlapping dialogue, but the bigger risk is that a skipped cutscene leaves a flag, an item, a song, an age or time change, or a warp undone and soft-locks the save. Please say that, and suggest saving first.
@ -1741,6 +1745,7 @@ const char* ini_key(const Row& r) {if (r.value == &opts.vsync) return "vsync";// (rows renamed since their values were first saved keep their keys: an options file from before still loads)if (r.value == &opts.resolution) return "resolution";if (r.value == &opts.allow_cutscene_skip) return "skip_unskippable_cutscenes";This sits under the comment for rows renamed since their keys were first saved, which a new row isn't. Move it above that comment, or let the key come from the row name.
@ -0,0 +4,4 @@int rx_option(int id, int otherwise);uint64_t hos_oot_cutscene_skip_allowed(uint64_t native_allowed) {return rx_option(HOS_OPT_ALLOW_CUTSCENE_SKIP, 0) > 0 ? 1 : native_allowed;When the option turns a skip on that the game would have refused (
native_allowed == 0), log it once per event, ideally with the event or timeline name ashos_oot_openingdoes. Otherwise a bug report about a stuck save can't show which cutscene was skipped.@ -248,6 +248,72 @@ def patch_camera_centering(source):'#include "a64.h"\n', '#include "a64.h"\nextern int hos_disable_camera_centering(void);\n', 1)# The event's active-state tick extracts its authored skip permission here. Other conditions later disable the sameThis covers every event whose authored bit is clear, with no list. The opening notes in host/hos_main.c show the flowcharts use a separate skip edge (
IsEventSkipedMostRecent), which cutscenes made unskippable probably don't have. Could this start from an allowlist of cutscenes checked in the game, or at least a denylist for the song, item, age and warp ones?a7de9b74188fc74e076ffeat(gameplay): add experimental cutscene skippingto WIP: feat(gameplay): add experimental cutscene skipping8fc74e076f368c7da4e4Returned this PR to WIP and rebased onto main
f647a072without changing the two feature patches. Commit 368c7da4 adds the save/progress risk warning in English and PT-BR, the README and options reference, and moves the new INI key above the renamed-row comment (13988, 13989). Sixteen lightweight Python checks and diff whitespace validation pass. The description now separates historical evidence from the revised head; event logging, safe coverage limits and the requested save/reload validation remain in progress. The atomic snapshot is retained for cross-thread option reads.Addressed the remaining code requests in f2487bc7 (rebased onto main
fbd42024with identical feature patches): the override now accepts only exact Dm_Kokiri_0000/0030 names from the active player resource, validates mapped ranges, and keeps native permission for all other timelines (13990). It logs override allowed once per timeline execution; the normal starter resets the diagnostic even with Skip opening off (13991). UI/README/docs state the limited coverage and save risk. Windows focused fixtures, 24 readiness cases, eight Linux focal scripts and both changed host TUs with official 64-bit standalone headers pass; source hashes match after the rebase. The full Windows suite is running. This remains WIP: current game behavior, save/reload, decoder continuity, screenshots and interleaved measurements are still pending. No audible-playback or completed-skip claim is inferred from the diagnostic log.The API is available again and now confirms head f2487bc7, base/merge-base
fbd42024and draft=true. Both permission anchors matched newly lifted baseline source, and reapplying either hook was idempotent; this does not claim a completed build. The same menu-wiring test fails the expected option-presence assertion on actual main git objects and passes on this PR. The full Windows attempt was interrupted before completion to avoid concurrent local build contention, so no full-suite pass or regression is inferred. Existing focused Windows/Linux results remain as documented; application/runtime, save/reload and measurements remain pending.Repaired the broken-fork warning without recreating the PR. The fork and feature branch existed, but the internal
refs/pull/200/headwas still at2f253b57. Empty commitc53a10b4triggered Forgejo synchronization; the fork branch, PR API and internal PR reference now all point to it, and a fresh page fetch no longer shows the warning. Its tree is identical tof2487bc7; no source changed and no tests were rerun for this metadata repair. Existing comments/review are preserved. The PR remains WIP, with the previously documented full-suite and runtime checks still pending.WIP: feat(gameplay): add experimental cutscene skippingto feat(gameplay): add experimental cutscene skippingc53a10b4a43b44463fe8Resolved the
gpu/vk/vk_api.hconflict by rebasing onto mainf3fe1763, keeping upstream Damage taken ID55 and this PR cutscene-skip ID56. API21/interface layouts are unchanged. Published head3b44463f; Forgejo now reportsmergeable: trueand its internal PR reference matches. Twelve focused Windows scripts pass, covering both features and their menu/patch consumers. The thirteenth,test_menu_i18n.py, has one stale Debug-help translation failure reproduced identically on mainf3fe1763. No full build, full suite or game run was started for this conflict repair; those pending checks remain explicit in the updated description.3b44463fe8d3c79356b6Updated the description for current head d3c79356 / base
d9648863and returned the PR to WIP. The five patches are unchanged according to range-diff, but previous Windows/Linux evidence remains labeled with the commits actually tested. Full-suite, complete platform builds, current screenshots, save/reload and performance measurements remain pending and unchecked. The update triggered Description run 2234, which passed: https://git.magicbean.dev/MagicBeanDev/Longshot/actions/runs/2234 . Success in WIP is not a claim that those validations passed. A direct rerun of the historical run 2147 was denied with HTTP 403 because this fork account lacks upstream write permission; the new run is the current successful check. No source/branch change, build or game run was made for this description repair.feat(gameplay): add experimental cutscene skippingto WIP: feat(gameplay): add experimental cutscene skippingWIP: feat(gameplay): add experimental cutscene skippingto feat(gameplay): add experimental cutscene skippingd3c79356b67d821fc029feat(gameplay): add experimental cutscene skippingto WIP: feat(gameplay): add experimental cutscene skippingCheck status review: the Description job failed because this PR was marked ready with five validation boxes still open. Updated the current head/base references and restored WIP, preserving all unchecked boxes and the original commit identities of earlier test evidence. No code, branch or runtime result changed. The metadata edit retriggers the description check.
The eight other pending jobs belong to CI, third-party and website workflows and have need_approval=true; they have not run and require a repository maintainer to approve execution for this fork. The current account has read-only upstream permissions. No build or GPU run was started by this check review.
mooki referenced this pull request2026-11-14 09:14:12 +00:00
View command line instructions
Manual merge helper
Use this merge commit message when completing the merge manually.
Checkout
From your project repository, check out a new branch and test the changes.