WIP: fix: Hookshot's small player-facing fixes for v3.0.0-rc.4, restored #286
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/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
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Longshot!286
Loading…
Reference in a new issue
No description provided.
Delete branch "TrashPanda/Longshot:restore/rc4-player-fixes"
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
Players on v3.0.0-rc.3 still hit bugs Hookshot fixed after Longshot branched: the game closing at Windows' commit limit,
Mouse 4/5 not bindable on Linux, a control's second key lost on a rebind, the gyro and a background controller moving
Link with the menu open, a failed options save emptying the file, Linux sound not reopening, 5.1 counted wrong. These are
the small, low-risk ones, for v3.0.0-rc.4. Split out of #209.
What changed
Split out of #209 as its reviewer asked (review on #209, point 4, group c). Each commit is the merged Hookshot pull request under its original author, carried through Longshot's rename; a
Restored-from:trailer names it, and the bare Hookshot numbers in the messages and the restored text are now full URLs.Credit to the authors of the restored work: jarynclouatre, NotRexed, Jiro131, bluetoe-99, David B, Jordan Ella.
vk_texture.cpp,vk_memory.cpp'smemory_table_flush), building on Longshot's commit rule (#77)controller or the menu open, the mouse camera while the mouse is released, the touchpad click as a binding
(
HOS_PAD_TOUCHPADmoves insideHOS_PAD_N:MKWVK_API_VERSION21 to 22)a stalled preparation is not promised; the setup retries a rename a scanner blocked (
builder/setup/setup.c)Split siblings: tools, checks and docs #284, test-request pinning #285, renderer fixes #287, new defaults and speed-ups #288.
Commits, in order:
350ed8fbfix(setup): a rename blocked by a scanner is tried again, and the message carries Windows' error (Hookshot 3021, by jarynclouatre)91bd08f0fix(memory): a failed allocation at the commit limit no longer closes the game (Hookshot issue 2941) (Hookshot 3017, by NotRexed)498c7d22fix(menu): a control's second key survives a rebind, with a column of its own (Hookshot 2933, by jarynclouatre)50651b0bfix(input): custom ocarina note buttons count with the controller in the background (Hookshot 2944, by jarynclouatre)f7d1a3cefix(input): the native mouse camera leaves the view to the game while the mouse is released (Hookshot 2947, by jarynclouatre)d1571435fix(menu): a bug report whose report.txt could not be written out is not shown as saved (Hookshot 2948, by jarynclouatre)2af6de34fix(input): Mouse 4 and Mouse 5 can be bound on Linux (Hookshot 2961, by jarynclouatre)240c669efix(input): the gyro is at rest while the Insert menu is open (Hookshot 2962, by jarynclouatre)cb60f53bfix(menu): a save that fails leaves the options file as it was (Hookshot 2966, by jarynclouatre)99827e14fix(linux): the sound output is closed before it is opened again (Hookshot 2970, by jarynclouatre)f84a838afix(menu): Windows' reason for a bug report that could not be saved is UTF-8 whatever the code page (Hookshot issue 2954) (Hookshot 2978, by Jiro131)6d293860fix(builder): a preparation the GPU driver stalled is not promised for the first start (Hookshot 2985, by jarynclouatre)45637584feat(input): a DualShock 4 or DualSense touchpad click can be bound in Controls (Hookshot 2989, by jarynclouatre)474f52f9fix(oot): the scarecrow's recall of the recorded song takes the note remaps too (kind 11) (Hookshot 3018, by bluetoe-99)99a45bd6fix(build): a direct lift removes an earlier lift's split parts before it writes its own (Hookshot 2953, by jarynclouatre)89ada153fix(builder): a refused extracted dump leaves the saved copy of the game code alone (Hookshot 2968, by jarynclouatre)b0c4ba56fix(audio): Auto counts 5.1 speakers, not channels (Hookshot 3044, by David B)bad3e030fix(linux): the log says why the renderer did not load (Hookshot 2875, by Jordan Ella)5395d066fix(input): the controller in the background stays out of the game while the Insert menu is open (Hookshot 2963, by jarynclouatre)093cdd71test(menu): the upscaler round trip's fixture declares logf and the stat calls save() now uses (mine)986db4c5docs: the restored Hookshot references as full URLs (mine)Left out of this group on purpose: the floor web above 60 fps (https://github.com/NotRexed/Hookshot/pull/2929) is
already on main as #49.
Flags and settings
HOS_INPUT_BACKGROUND=1HOS_AUDIO_CHANNELS=2/6Measurements
No numbers of my own: each fix was measured or shown upstream (in its commit message). Here the change is checked by
the game-free checks and a build on both platforms, main and this PR on the same PC.
main:
d9648863(2026-11-13)this PR: restore/rc4-player-fixes @ b4c98fb5 (the runs; now
986db4c5, after the docs-only rebase onto main's514591a3)Hardware: RTX 4070 Ti (driver 595.99.02), Ryzen 7 9800X3D, Ubuntu 26.04.1
Scene: none (no game run)
Method:
python3 -I -B tests/run_all.py --jobs 6, once on each; the builds belowHow to reproduce: the same command on each commit
d9648863run_all.py: passed / failed / skippedThe numbers still hold on
514591a3: main's #274 changed a Discord link in 11 docs files and nothing these checks or builds compile; the checks that read those docs were run again on this head.Testing
Tested:
python3 -I -B tests/run_all.py --jobs 6on Linux (Ubuntu 26.04.1, Python 3.14, Vulkan and SPIR-V headers found): this PR @b4c98fb5: 519 passed, 2 failed, 18 skipped (test_lift_cache.py, test_menu_i18n.py); main @d9648863on the same PC: 513 passed, 2 failed, 18 skipped (test_lift_cache.py, test_menu_i18n.py).test_lift_cachefails on main the same way (the lift's scripts do not import here), andtest_menu_i18nfails on that main too. Main then gained514591a3(#274, a Discord link in 11 docs files only); after the rebase onto it, the checks that read those docs passed again at986db4c5.Built at
b4c98fb5with no game, on Linux (system clang 21, lld) and for Windows (cross-built here with tools.lock's llvm-mingw 20261104 and its windows-x64 CPM cache, which has FFmpeg and dav1d): (before the docs-only rebase onto #274)nx2/build.py --standalonewith an empty lift compiled everything in the repository that the game links, the host, its libraries and the Vulkan back end (406 files on Linux, 405 for Windows), with no failure, and its link stopped only at the symbols the game's lifted code provides (18 on Linux, 19 on Windows: the same list as main @d9648863).RX_VK_ONLY=1 RX_RELEASE=1linkedlongshot_vk.soandlongshot_vk.dll, andbuilder/setup/setup.clinked asLongshotBuilder-Setup.exewith build-windows.yml's command. The five split branches merged together locally (the API version taken as 23) compiled and linked the same way, so they also build together.Not tested: a run in the game, on either platform: the game build needs the game's lifted code, and I had no run
here. The setup's retry (https://github.com/NotRexed/Hookshot/pull/3021) has not run on Windows.
Needs other testers:
make_game.py, then a short run (title, a save, Kokiri Forest): rebind acontrol's second key, open and close the Insert menu with a controller connected, save the options; expecting no
crash and the options file intact (T-DV9Q, claimable once a maintainer adds the
testing wantedlabel)Screenshots
None: no run in the game yet (the Controls page's Key 1 / Key 2 columns change on screen).
Risk and rollback
Input and menu changes are the most visible; each commit reverts on its own.
MKWVK_API_VERSIONgoes to 22 here: therenderer fixes (#287) take 23 and are meant to merge after this one; should they merge first, they take 22 and this one 23.
Reviewer notes
Please merge with rebase, not squash, so that each author keeps their commit. One commit of mine fixes the upscaler
round-trip check from #55:
save()now useslogfandlstat, which its fixture did not declare.Issues
See #209
Checklist
For everyone
type(scope): description).main.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.MKWVK_API_VERSIONbumped (gpu/vk/vk_api.h'sMkwVkHostorMkwVkRendererchanged).tests/that reads changed source updated in this PR, not weakened.In Hookshot Builder, a game folder that was built from a supported `.nup` can be rebuilt from an extracted dump. When that dump is of another game version, the build is refused ("This is build X. Hookshot supports build Y"), which is right, but `nx2/make_game.py` checked the version only after the dump's modules had replaced the folder's saved copy (`.hookshot_exefs`). The stamp of the earlier `.nup` stayed. Choosing that `.nup` again then said "reading it again is skipped", built from the refused modules and was refused too, until the folder was deleted. ## Change In the extracted-dump branch of `main()`: - `check_supported_build` runs right after the dump's modules are unpacked, before the romfs is linked in and before `.hookshot_exefs` is touched. A refused dump leaves the folder as it was. - An accepted dump removes the `.nup` stamp (`.hookshot_romfs`), since the saved copies are no longer that `.nup`'s. The next build from the `.nup` reads it again. The `.nup` branch already checks the build before it replaces anything; it is unchanged. ## Testing - `tests/test_make_game_cache.py`, two new cases (the cache test's fake `.nup` and modules, no game): - A refused extracted dump: the folder's files are byte for byte as before, and the earlier `.nup` then reuses its copy and reaches the lift with the supported build. - An accepted extracted dump: its modules are saved, the `.nup` stamp is gone, and the earlier `.nup` is read again. - Both fail on main and pass here. - Full suite on Linux: 473 passed, 53 skipped, 2 failed, both the same on main on this machine (`test_bake_portability`: `zstandard` is not installed; `nx2/test_texture_init_barrier.py`: `clang++` is not installed). - No real game build was run. Restored-from: https://github.com/NotRexed/Hookshot/pull/2968proci referenced this pull request2026-11-14 00:01:43 +00:00
Review of 286 at
986db4c5. I read the code only; I didn't build or run anything.v3.0.0: the commit-limit crash fix and the options-save fix should go in. Their code reads correctly:
memory_table_flushcatchesbad_allocand writes the table at once.save()writes a.newfile and renames it, and writes through links.The PR as a whole can't merge yet, though, because
mainhas since taken two of its commits and reworked the Controls grid:Two commits are already on
main.474f52f9(scarecrow kind 11, https://github.com/NotRexed/Hookshot/pull/3018) is byte for byte #281.bad3e030(https://github.com/NotRexed/Hookshot/pull/2875) is an older form of #34. Onmainthat fix nameslongshot_vk.sorather than.dll, strips dlerror's leading path, and says "nothing is drawn" in place of "WebGPU". It also has its owntest_renderer_load_error.py.Please drop both commits, and the
test_renderer_load_error.pypart of986db4c5. Keepingbad3e030would conflict, and if resolved toward this side it would revert #34's message.The Controls grid collides with #137 (hold-to-toggle, now on
main).mainhasenum CtlColumn { kCtlPrimary, kCtlAlternate, kCtlKeyboard, kCtlToggle, kCtlColumns }.498c7d22putskCtlKeyboard2in the same slot, and itsctl_key_slot/ctl_steptreat every column fromkCtlKeyboardon as a key column. Merged mechanically, the Toggle cell would become "key slot 2". This needs a real port onto #137:ctl_key_slotlimited to the two key columns;Hit::kMax(128) checked against 5 columns times the rows.45637584(touchpad bindable,HOS_PAD_N+ 1) touches the same page andvk_api.h, and should be checked with it."Rebased on current
main" is false.mainis25ee6a83. Besides the above, there are textual conflicts in:vk_menu.cppsave()(#137'stoggle_lines), the Controls drawing andmenu_tick;vk_bug_report.cpp(#103);nn_audio.c(#136 Mono: a comment only, butau_menu_outputnow clamps to 3);README.md.CI run 2339 at
986db4c5was still queued when I looked.No run in the game yet (T-DV9Q is waiting for the
testing wantedlabel). The Key 1/Key 2 columns and the touchpad binding show on screen. After the re-port onto #137, the Controls page needs a run more than anything else here.If the Controls re-port will take a while, splitting the low-risk fixes into their own PR would let them make rc.4 without waiting on it. Those are: commit limit, options save, bug-report errors and UTF-8, the setup retry, Linux audio reopen, Mouse 4/5, the gyro and background-controller fixes, 5.1 counting, and the two
make_game.pyfixes.Checked and fine:
move_retry:fail()exits the thread, so the twofailcalls don't both run, andpublish_installrestores the error after rollback.au_layout_channelsspeaker masks.MKWVK_API_VERSIONorder with #287.Restored-from:trailers) and licence (GPL, from Hookshot).@ -21,3 +21,3 @@kButton, // the restart question's buttons: index 0 Later, 1 Restart now};enum CtlColumn { kCtlPrimary, kCtlAlternate, kCtlKeyboard, kCtlColumns };enum CtlColumn { kCtlPrimary, kCtlAlternate, kCtlKeyboard, kCtlKeyboard2, kCtlColumns };main(#137) haskCtlTogglein this slot:{ kCtlPrimary, kCtlAlternate, kCtlKeyboard, kCtlToggle, kCtlColumns }. With both,ctl_key_slotbelow (col >= kCtlKeyboard) would give the Toggle column key slot 2, andctl_step's 'round all four' would also be wrong. This needs a port onto #137, not a conflict pick.@ -296,1 +296,4 @@#ifdef _WIN32hos_log("rx: RX_BACKEND=vulkan, but %s did not load (%lu): WebGPU", path, GetLastError());#else /* (why: a missing file, or a symbol it needs that this executable lacks, as one from another build) */hos_log("rx: RX_BACKEND=vulkan, but %s did not load (%s): WebGPU", path, hos_load_error());This one is already on
mainas #34, reworked: there the line nameslongshot_vk.so, strips dlerror's<path>:prefix and saysnothing is drawn(Linux has no WebGPU). Please drop this commit (bad3e030) rather than resolve the conflict.@ -743,3 +743,3 @@OC_DECL = ("extern volatile int hos_oc_wait;\nextern volatile int hos_oc_last_kind;\n""extern volatile unsigned hos_oc_beat;\n""#define HOS_OC_BUTTONS(k) ((k) == 1 || (k) == 2 || (k) == 3 || (k) == 10)\n")"#define HOS_OC_BUTTONS(k) ((k) == 1 || (k) == 2 || (k) == 3 || (k) == 10 || (k) == 11)\n")Identical to #281, which is already on
main(b7a46d6b). Drop474f52f9.Reviewed head
986db4c5against maincf605c85: all 47 changed files, the restoration commits, the existing review and its inline comments; on Linux, 42 targeted check invocations passed and 1 failed with Clang, with 11 additional main-side comparison invocations. No game or full build ran.Separate changes restored
In commit order:
350ed8fb: retry Windows setup renames blocked by a scanner.91bd08f0: release emergency commit memory and handle a failed page-table allocation.498c7d22: edit the two keyboard bindings separately.50651b0b: allow remapped ocarina buttons on a background controller.f7d1a3ce: stop the native mouse camera overriding the view while capture is released.d1571435: report a failed close ofreport.txtas a failed save.2af6de34: publish Mouse 4 and Mouse 5 on Linux.240c669e: hold the gyro at rest while the Insert menu is open.cb60f53b: replace the options file only after a successful write.99827e14: close Linux's SDL audio device before reopening it.f84a838a: convert Windows' bug-report error text to UTF-8.6d293860: correct the message after shader preparation exits with status 4.45637584: make the PlayStation touchpad click bindable.474f52f9: apply note remaps during scarecrow recall, kind 11.99a45bd6: remove old split parts before a direct relift.89ada153: validate an extracted dump before replacing the saved code and invalidate its old package stamp.b0c4ba56: distinguish surround speakers from an interface's channel count.bad3e030: log the Linux renderer loader's reason for failure.5395d066: apply the Insert-menu input hold to background controllers too.093cdd71adapts the upscaler test to the options writer;986db4c5changes references, not runtime behaviour.What's good
The changes mostly address small, identifiable paths. The new Mouse 4/5, released-camera, relift, dump-cache and preparation-message cases fail against current main for the intended reasons and pass here. The SDL device handle is carried through its caller and closed before its callback's storage is freed. The touchpad value reaches the binding reader, writer, gameplay mapping and menu routing, while the prompt-art count stays unchanged.
Existing review: still outstanding
The head has not changed since #286 (comment).
testing wantedlabel is now present, so that part of the earlier note is out of date; the missing run is not.Must change
tests/test_controls_key_slots.py:59declareslistening, and the extracted binding branch assigns it, but the fixture never reads it. With Clang 23.1.3,python3 -I -B tests/test_controls_key_slots.py --cxx clang++stops atvariable 'listening' set but not used [-Werror,-Wunused-but-set-global], before any binding assertion runs. GCC passes. Assert that a completed binding leaves listening mode, keeping the warning checks enabled, then run this driver with both compilers. This is separate from the already requested Controls port.Each restoration against current main
“Separate” below means it can be its own change without the other restorations, with its tests and the stated main adaptations. It is not a claim that unrun gameplay is proven.
publish_installandplace_toolstill try the relevant move once.move_retryto both updated setup fixtures. A scanner-held Windows install remains untested.memory_table_flushstill grows its temporary vector without this catch or reserve. Main's lazy guest commitment does not handle this C++ allocation.oc_pad_wordrejects the background unconditionally.fwritebut ignoresfclose.logs/paths, and combine both sides oftest_bug_report.py."w".toggle_lines and their assertions. Carry the upscaler fixture adaptation in093cdd71with it. Ordinary files and resolved Linux symlinks use replacement; Windows reparse points and unresolved links deliberately keep the old direct-write behaviour.waveOutCloseresets the queue but never closes SDL.FormatMessageA.nChannelsalone..soname, single path, “nothing is drawn” wording and Vulkan-loader diagnostic.pads-insertcase with main's hold-toggle, mouse and background cases, rather than choosing either fixture wholesale.I would separate the builder-only items 1, 12, 15 and 16 first, then the small input, report, options and audio fixes. Keep item 3 on its own until the grid is ported. Item 13 does not need to wait for Key 2. Item 2 also deserves its own review and evidence rather than being hidden in the input bundle.
Smaller points and evidence limits
tests/test_sdl_audio_reopen.py:44-65checks that another open works, not that the previous device closed. It passed against unmodified main with GCC and Clang, both with normal library selection and withHOS_SDL_VERSION=2. A device that accepts several opens lets the leak pass. Add a deterministic close/callback-lifetime check for the SDL2 and SDL3 paths; keep the real dummy-device test as a smoke test.Not checked
No full suite, full game build, Windows execution or Windows fixture syntax check ran here. No real audio device, keyboard, mouse, controller, commit-limit condition or game scene was exercised. The Controls port needs pictures and interaction checks at all menu sizes, including both keys, Toggle, touchpad binding, reset and restart. The Windows memory and save paths need their own failure-path evidence. There is no measured performance gain in this review.
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.