fix(input): repair hold-to-toggle fixtures and menu reset docs #338

Open
simonwjackson wants to merge 2 commits from simonwjackson/Longshot:fix/hold-toggle-fixtures into main
Contributor

Why

The #137 merge broke two game-free tests on main. Restore the fixtures without weakening their assertions, and describe the Linux menu reset hook accurately.

What changed

  • tests/test_pad_layout.py: include the existing hold_toggle.h for the extracted binding table's HoldToggle.
  • tests/test_menu_keys.py: use C++ static_assert while including the C API header, then remove the macro. GCC now checks the same layout assertions as Clang.
  • host/oot_menu.c, README and development guide: Linux clears latches on any menu screen's pane-focus callback, not only the pause page. Individual title/save/game-over transitions remain explicitly unverified in play. No runtime statement changes.
  • Credit: mooki reported both compiler failures on #137; HumanBean identified the broader focus callback in review 1208.

Flags and settings

None. No runtime behavior or saved option changes.

Measurements

main: c0442078d3 (2026-11-14)
this PR: fix/hold-toggle-fixtures @ abecfa9
Current head: c734325a01 (same fixture fixes, updated main and run records)

The two measured pairs used the same fixes at abecfa9 against main c0442078. The follow-up now starts at f87557a; only the base and the run-record format changed. The fixture corrections are unchanged, so the deterministic compile results remain applicable; final focused checks and the confirmation full suite pass on the published head apart from the named inherited PATH failure.
Hardware: zao, Linux, native GCC and Clang; no GPU or game needed.
Scene: the real extracted binding/menu fixtures.
Method: two interleaved main/PR pairs. Each runs python -I -B tests/test_pad_layout.py, then tests/test_menu_keys.py --cxx g++, then the same menu check with --cxx clang++.
How to reproduce: run those three commands in each checkout with the project's C/C++ toolchain.

Check main @ c0442078, runs 1 / 2 this PR @ abecfa9, runs 1 / 2
Pad-layout fixture FAIL / FAIL PASS / PASS
Menu-keys, g++ FAIL / FAIL PASS / PASS
Menu-keys, clang++ PASS / PASS PASS / PASS
Median passing commands 1 of 3 3 of 3
main, pad layout: error: unknown type name ‘HoldToggle’
main, menu keys with g++: rx_nvn.h:83:15: error: expected constructor, destructor, or type conversion before ‘(’ token
this PR: pad-layout's two tests OK; menu keys PASS with g++ and clang++

Testing

Tested: Both failures reproduced on untouched main. Both checks pass with the fixes; menu keys passes with g++ and clang++. test_hold_toggle.py and git diff --check pass. The confirmation full suite on the published head has 533 passed, 1 failed, 16 skipped in 654 s under the shared lock with 8 jobs. The only failure, test_linux_tools.py, reproduces on untouched main (fixed /usr/bin:/bin misses Nix tools). The earlier stable run's one-second preparation-test timeout did not recur; its 19 tests also passed three separate focused repeats on both main and this PR. The final focused run passes 5/5, including lift-cache and the run-record validator. An earlier run during a main update is not used as final verification. There is no separate formatter or typechecker; the fixtures compile with compiler warnings enabled.

Not tested: No new gameplay run or Windows run. This changes tests, comments and documentation only. The Linux callback contract was read at nx2/oot_patches.py:patch_menu_focus and host/oot_menu.c:hos_oot_menu_focus; this is not a claim to have played every dialog.

Needs other testers: None for these fixture repairs. The underlying feature's Windows gameplay request T-NDNY remains open and is not a passing result.

Screenshots

Nothing on screen changes.

Risk and rollback

Only test compilation and documentation change. A bad fixture include could affect another compiler; GCC and Clang were both checked. Revert this follow-up to restore the previous fixtures, including their failures.

Reviewer notes

No product behavior, public ABI or third-party change. The C++ assertion spelling is scoped to the fixture's C-header include. No compatibility shim was added to production code.

Issues

See #137.

Checklist

For everyone

  • One topic.
  • The title is a Conventional Commit (type(scope): description).
  • Rebased on current main. New follow-up starts at f87557a; no published history rewritten.
  • python -I -B tests/run_all.py run and passing (any failure that also fails on main is named above).
  • Tested in the game, or what wasn't is named under Testing.
  • Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests).
  • Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported.
  • No game files, dumps, build output or generated files.
  • third_party/ untouched (a new folder with a PROVENANCE.md instead).
  • The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

If applicable

  • AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
  • A file in docs/tested/ for each measured run.
  • A check under tests/ that reads changed source updated in this PR, not weakened.
  • Docs updated.
## Why The #137 merge broke two game-free tests on main. Restore the fixtures without weakening their assertions, and describe the Linux menu reset hook accurately. ## What changed - `tests/test_pad_layout.py`: include the existing `hold_toggle.h` for the extracted binding table's `HoldToggle`. - `tests/test_menu_keys.py`: use C++ `static_assert` while including the C API header, then remove the macro. GCC now checks the same layout assertions as Clang. - `host/oot_menu.c`, README and development guide: Linux clears latches on any menu screen's pane-focus callback, not only the pause page. Individual title/save/game-over transitions remain explicitly unverified in play. No runtime statement changes. - Credit: mooki reported both compiler failures on #137; HumanBean identified the broader focus callback in review 1208. ## Flags and settings None. No runtime behavior or saved option changes. ## Measurements main: c0442078d32e13b7073a0451129296948e6fd1c8 (2026-11-14) this PR: fix/hold-toggle-fixtures @ abecfa9 Current head: c734325a013d065472df2cc0db4dce86e1c834c9 (same fixture fixes, updated main and run records) The two measured pairs used the same fixes at `abecfa9` against main `c0442078`. The follow-up now starts at `f87557a`; only the base and the run-record format changed. The fixture corrections are unchanged, so the deterministic compile results remain applicable; final focused checks and the confirmation full suite pass on the published head apart from the named inherited PATH failure. Hardware: zao, Linux, native GCC and Clang; no GPU or game needed. Scene: the real extracted binding/menu fixtures. Method: two interleaved main/PR pairs. Each runs `python -I -B tests/test_pad_layout.py`, then `tests/test_menu_keys.py --cxx g++`, then the same menu check with `--cxx clang++`. How to reproduce: run those three commands in each checkout with the project's C/C++ toolchain. | Check | main @ c0442078, runs 1 / 2 | this PR @ abecfa9, runs 1 / 2 | |---|---|---| | Pad-layout fixture | FAIL / FAIL | PASS / PASS | | Menu-keys, g++ | FAIL / FAIL | PASS / PASS | | Menu-keys, clang++ | PASS / PASS | PASS / PASS | | Median passing commands | 1 of 3 | 3 of 3 | ```text main, pad layout: error: unknown type name ‘HoldToggle’ main, menu keys with g++: rx_nvn.h:83:15: error: expected constructor, destructor, or type conversion before ‘(’ token this PR: pad-layout's two tests OK; menu keys PASS with g++ and clang++ ``` ## Testing **Tested:** Both failures reproduced on untouched main. Both checks pass with the fixes; menu keys passes with g++ and clang++. `test_hold_toggle.py` and `git diff --check` pass. The confirmation full suite on the published head has **533 passed, 1 failed, 16 skipped** in 654 s under the shared lock with 8 jobs. The only failure, `test_linux_tools.py`, reproduces on untouched main (fixed `/usr/bin:/bin` misses Nix tools). The earlier stable run's one-second preparation-test timeout did not recur; its 19 tests also passed three separate focused repeats on both main and this PR. The final focused run passes 5/5, including lift-cache and the run-record validator. An earlier run during a main update is not used as final verification. There is no separate formatter or typechecker; the fixtures compile with compiler warnings enabled. **Not tested:** No new gameplay run or Windows run. This changes tests, comments and documentation only. The Linux callback contract was read at `nx2/oot_patches.py:patch_menu_focus` and `host/oot_menu.c:hos_oot_menu_focus`; this is not a claim to have played every dialog. **Needs other testers:** None for these fixture repairs. The underlying feature's Windows gameplay request T-NDNY remains open and is not a passing result. ## Screenshots Nothing on screen changes. ## Risk and rollback Only test compilation and documentation change. A bad fixture include could affect another compiler; GCC and Clang were both checked. Revert this follow-up to restore the previous fixtures, including their failures. ## Reviewer notes No product behavior, public ABI or third-party change. The C++ assertion spelling is scoped to the fixture's C-header include. No compatibility shim was added to production code. ## Issues See #137. ## Checklist ### For everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`). - [x] Rebased on current `main`. New follow-up starts at f87557a; no published history rewritten. - [x] `python -I -B tests/run_all.py` run and passing (any failure that also fails on `main` is named above). - [x] Tested in the game, or what wasn't is named under Testing. - [x] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). - [x] Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### If applicable - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] A file in `docs/tested/` for each measured run. - [x] A check under `tests/` that reads changed source updated in this PR, not weakened. - [x] Docs updated.
Thanks to mooki for reporting both merged-fixture failures on #137: the extracted binding table needs HoldToggle, and GCC needs the C++ spelling of the C header’s layout assertions. Keep every assertion active.

Address HumanBean review 1208 by describing Linux’s menu pane-focus hook rather than claiming it is pause-only. Individual dialog gameplay is still explicitly unverified.

Assisted-by: OpenAI GPT-6 Astra
Co-Authored-By: OpenAI GPT-6 Astra <[email protected]>
docs: record input-fixture regression checks in run fragments
Some checks are pending
CI / Conventional Commit title (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
Third-party parts / As their notes say (pull_request) Blocked by required conditions
Third-party parts / The same as upstream (pull_request) Blocked by required conditions
Website / Website on Cloudflare (pull_request) Blocked by required conditions
Website / Website on Cloudflare (pull_request_target) Has been skipped
PR description / Description follows the template (pull_request_target) Successful in 3s
c734325a01
Main now freezes the old ledger table; use the new per-run files.

Assisted-by: OpenAI GPT-6 Astra
Co-Authored-By: OpenAI GPT-6 Astra <[email protected]>
Author
Contributor

Confirmation suite at c734325: 533 passed, 1 inherited Nix PATH failure, 16 skipped in 654 s under the shared lock. Both reported fixture failures remain fixed, with menu-keys checked using g++ and clang++; no new failures. The earlier preparation-test timeout did not recur.

Confirmation suite at c734325: 533 passed, 1 inherited Nix PATH failure, 16 skipped in 654 s under the shared lock. Both reported fixture failures remain fixed, with menu-keys checked using g++ and clang++; no new failures. The earlier preparation-test timeout did not recur.
simonwjackson changed title from WIP: fix(input): repair hold-to-toggle fixtures and menu reset docs to fix(input): repair hold-to-toggle fixtures and menu reset docs 2026-11-14 02:53:52 +00:00
bibiburl approved these changes 2026-11-14 07:18:01 +00:00
bibiburl left a comment

Reviewed c734325a01 against current main 0caf06184b. No blocking finding in the fixture repairs and hook documentation.

Independently reproduced the HoldToggle compile failure and GCC _Static_assert failure on main. With current main integrated locally, the two pad-layout tests pass, menu-key checks pass with GCC and Clang, and the hold-toggle test passes. The tested-configuration validator also passes. No assertions were removed: the C-header layout assertions are retained using C++ static_assert, with the macro undefined immediately after the include.

Checked patch_menu_focus and Linux hos_oot_menu_focus: the injected callback passes screen/pane/return-address through, and the Linux hook clears toggles without filtering for the pause screen. The development guide correctly separates that source contract from unverified individual gameplay transitions. The host-file edit is comment-only. Current-main merge simulation is clean.

Merge coordination: this PR already includes #331's one-line fix. Prefer this PR rather than merging both unchanged; combining the branches currently conflicts only over the trailing comment on that include, which should be retained once.

Scope: focused Linux checks and source review, not a rerun of the author's full suite or playthrough. No Windows/game validation performed by this reviewer. Assisted-by: Codex (OpenAI).

Reviewed c734325a013d065472df2cc0db4dce86e1c834c9 against current main 0caf06184b391a5259d3741bb006ef5826eb6417. No blocking finding in the fixture repairs and hook documentation. Independently reproduced the HoldToggle compile failure and GCC _Static_assert failure on main. With current main integrated locally, the two pad-layout tests pass, menu-key checks pass with GCC and Clang, and the hold-toggle test passes. The tested-configuration validator also passes. No assertions were removed: the C-header layout assertions are retained using C++ static_assert, with the macro undefined immediately after the include. Checked patch_menu_focus and Linux hos_oot_menu_focus: the injected callback passes screen/pane/return-address through, and the Linux hook clears toggles without filtering for the pause screen. The development guide correctly separates that source contract from unverified individual gameplay transitions. The host-file edit is comment-only. Current-main merge simulation is clean. Merge coordination: this PR already includes #331's one-line fix. Prefer this PR rather than merging both unchanged; combining the branches currently conflicts only over the trailing comment on that include, which should be retained once. Scope: focused Linux checks and source review, not a rerun of the author's full suite or playthrough. No Windows/game validation performed by this reviewer. Assisted-by: Codex (OpenAI).
Some checks are pending
CI / Conventional Commit title (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
Third-party parts / As their notes say (pull_request) Blocked by required conditions
Third-party parts / The same as upstream (pull_request) Blocked by required conditions
Website / Website on Cloudflare (pull_request) Blocked by required conditions
Website / Website on Cloudflare (pull_request_target) Has been skipped
PR description / Description follows the template (pull_request_target) Successful in 3s
This pull request has changes conflicting with the target branch.
  • docs/development.md
  • tests/test_menu_keys.py
Some workflows are waiting to be reviewed.
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.
git fetch -u 'fix/hold-toggle-fixtures:simonwjackson-fix/hold-toggle-fixtures'
git switch 'simonwjackson-fix/hold-toggle-fixtures'
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
MagicBeanDev/Longshot!338
No description provided.