Repository navigation
Conversation
…Insert menu
The Insert menu's settings could only be reached with a key (Insert, F1) or a
pad chord; a controller player now finds them in the pause menu, as a sixth
button on its Menu page below Title Screen, drawn and moved to as the game's own.
- nx2/pause_layout.py: Pa_Button_05 added to every PaSystemMenu_00 layout in
Common.blarc at build time, a copy of Pa_Button_04 188 below it (the page
draws the panes of its layout, so a copy made at run time is never drawn).
What grows is written after the end of the layout and of the archive: no
other offset moves, and the prompts' runs over Common.blarc stay valid.
- nx2/prompt_assets.py: the edited archive written to prompts/Common.blarc,
with prompts/recomp_button, and its size table entry raised.
- host/prompts.c: prompts/Common.blarc served with the button even without
runs over it.
- host/oot_pause_menu.c, nx2/oot_pause_menu.py: the sixth pane given a button
object as the others (f_80f99400), its text ("Opções Recomp" and the like in
the game's language) on its text boxes; A while the cursor is on it (its
state, noted at f_80ad5310) opens the Insert menu and is not passed to the
game, whose page has no end for a sixth button's decision.
- host/nn_hid.c: the buttons pass through hos_pausebtn_buttons.
- host/nn_applet.c: hos_game_language for the button's text.
- HOS_RECOMP_BUTTON=0 turns it off (docs/development.md).
- tests/test_oot_pause_menu.py: the edits all or none, unknown layouts and
archives left as they are; tests/test_input_background.py: a pass-through
for the new call in npad_states.
Tested in the game (Windows, RTX 2070 SUPER): the button shows below Title
Screen with its text, the cursor reaches it, A opens the Insert menu, and B
then leaves the pause menu. On Linux the button shows but does not open the
menu yet (the menu there reads the keyboard itself, with no queued key).
… of the commit before host/nn_applet.c, host/nn_hid.c, host/prompts.c, nx2/oot_patches.py and nx2/prompt_assets.py, as that commit's message describes them.
The code's comments and docstrings call the button "Recomp Options"; the Portuguese "Opções Recomp" stays only as the text a game in Portuguese shows.
HOS_RECOMP_BUTTON=0 is gone: the button is always shown with its text and always opens the Insert menu.
On Linux the Insert menu read the keyboard's state itself and had no queue of keys pressed, so the button's F1 request reached nothing there. - host/rx_backend.c: the menu's key queue on every platform. - host/posix/display_sdl.c: keys_update notes each key on the look it goes down, as host/display.c's WM_KEYDOWN does on Windows; the menu then reads its keys the same way on both. - host/oot_pause_menu.c: the F1 request on every platform. - tests/test_linux_menu_keys.py: a key reaches the queue once on its way down, again after it is let go, none while the window is not in front. Not run in the game on Linux: the files compile with gcc on Ubuntu 22.04 (WSL), where this check and tests/test_menu_keys.py pass.
This was referenced Nov 11, 2026
hckrman101
reviewed
Nov 11, 2026
hckrman101
left a comment
hckrman101
left a comment
Collaborator
There was a problem hiding this comment.
Read through it and I like the approach: the button is a real pane added to the layout at build time from the player's own files, nothing game-derived is committed, the lift edits are all-or-none with a warning, and swallowing A (instead of letting the game see a decision its script has no end for) is the right call given the pause-menu-can't-close problem you describe. Screenshots are there, and the Linux key-queue change is small and covered by a test. Nothing blocking in the code; a few things before this can go in:
- CI has not run. The three real checks (Windows builder, Linux AppImage, game-free regressions) are all
action_required, waiting on a maintainer to approve the workflow run. Only the title check is green, so the new tests are unproven in CI. - Overlap with #2366 (live prompts). It touches the same places:
build_layoutsinhost/prompts.c,size_table/maininnx2/prompt_assets.py,host/nn_hid.c,nx2/oot_patches.pyandhost/rx_backend.c. Both change howCommon.blarcis built and served (hereprompts/recomp_buttonplus a size/mtime hash; there the live reload), so whichever lands second needs a real merge, and the two should agree on who owns the served-archive key. Worth sorting out the order with the author of #2366 and @NotRexed. - #2197 (Esc opens the Insert menu) and #2378 (tuning bar) are about the menu itself, not the pause page, so no code conflict that I can see. #2197 is a nice companion (keyboard side) and this covers the controller side; the button's synthetic F1 goes through the same
hos_vk_note_key_pressqueue, so it keeps working if the shortcut default changes. - New UI in the game's own menu. This is a visible new entry in the pause menu, so it is @NotRexed's call whether he wants it there (and whether the label should be localized by us rather than by the game's message tables). I am not approving over that.
- Linux is untested (the PR says so). The
display_sdl.cchange now feeds every key-down into the Insert menu queue on Linux, which also means keys pressed while the menu is closed sit in the queue until it is next opened, same as on Windows today; fine, just worth a quick run by someone on Linux.
Nit: hos_pausebtn_buttons reads the button state with a raw GP(control + 0x38) while the rest of the file uses recomp_load*; consistent use would be a little safer if the object is ever freed while our_control is still set (it is cleared on the next page build, but not when the page is torn down).
… destroyed hos_pausebtn_buttons read the button's control state with a raw GP() read, and our_control was cleared only when the page was built again, not when it was torn down: after the page was freed the pad's A could read freed memory. - nx2/oot_pause_menu.py: f_80aa2ad0, the base of the page's destructor (shared with other screens), calls hos_pausebtn_gone on entry. - host/oot_pause_menu.c: hos_pausebtn_gone forgets the page, part, button and control when the page going is ours; the state is read with recomp_load8, as the rest of the file reads the game's memory. - tests/test_oot_pause_menu.py: no raw GP() read in hos_pausebtn_buttons, and the page destroyed clears the control. Tested in the game (Windows): A on the button opens the Insert menu; the pause menu closed with the cursor on the button, A in play is the game's; back to the title screen the page is destroyed and the button forgotten.
…tions Conflicts in host/prompts.c and nx2/prompt_assets.py, where both change how Common.blarc is built and served: - host/prompts.c: build_layouts now serves prompts/Common.blarc always (NotRexed#2366), so the Recomp Options button's own serving (the prompts/recomp_button marker, nruns || button) is dropped. The file's size and time are folded into the served file's key in layout_runs, so a build that changed the layout still makes it again. - nx2/prompt_assets.py: size_table takes len(font) (NotRexed#2366) and still raises the layouts' entry by the bytes the button added; the marker is no longer written.
Author
|
@hckrman101 Thanks for the review! Both points are addressed: Button state read (nit): fixed in 9d67875.
Overlap with #2366: merged
|
Joey-Magnus
suggested changes
Nov 11, 2026
Joey-Magnus
left a comment
Joey-Magnus
left a comment
Contributor
There was a problem hiding this comment.
The new test will fail on Linux once the workflows run: tests/test_oot_pause_menu.py:130 builds its include list without host/posix, so host/oot_pause_menu.c:17 (#include <windows.h>) isn't found. Add -I host/posix off Windows.
Also:
- The
display_sdl.cchange routes every key-down into the Insert menu's queue, which moves Linux menu input from polled state to a queue. Nobody has run it on Linux yet. - The earlier
GP()nit is only fixed on paper (recomp_load8ignores its context). The real fix is thehos_pausebtn_goneteardown hook, which looks right.
Whether a new entry belongs in the game's own pause menu is the maintainers' call.
Collaborator
|
This now conflicts with main in |
One conflict, in nx2/oot_patches.py: apply() is now patch_file per file and report() over the counts. The Recomp Options button's edits stay after report() there. The lift no longer goes through apply(): nx2/lift.py edits its C with nx2/chunks.py edit_c, a process per file. So edit_c now runs the button's edits too (apply_pause_menu, for the supported build, after report() and before the output size and page-mark groups), its counts over edit_c's processes (pause_menu_count, each). Without it the button was left out of the build with no warning. tests/test_oot_pause_menu.py: the counts are made through the caller's processes, and edit_c calls apply_pause_menu.
The compile check of host/oot_pause_menu.c (its #include <windows.h>, for VK_F1) had no -I host/posix off Windows, so it failed on Linux, as tests/test_display_pointer.py already adds it. Checked on Ubuntu 22.04 (WSL) with clang 14: fails before, passes after.
Author
|
@Joey-Magnus
@hckrman101 merged current |
hckrman101
pushed a commit
that referenced
this pull request
Nov 12, 2026
) **What this is for:** contributors rebuilding from source should not spend another minute drawing the same button prompts. **What is measured:** on an Intel Core i9-13900K running Arch Linux, the finish stage at main `c544828f` took 67.707 and 66.898 seconds; two unchanged runs took 0.603 and 0.651 seconds, and the final head took 0.616 seconds, clearing the run-time files as well. Other work was running throughout. **Not measured:** Windows, a packaged Builder update, a complete compile/lift, or gameplay performance. Refs #2636. ## Problem `nx2/make_game.py` removes the game folder's `prompts` directory and runs `prompt_assets.py` on every build when fontTools is available. Most source edits do not change anything those assets depend on. This avoids drawing unchanged assets; it does not make the drawing itself faster. #2636's A1 plans to measure the finish stage, but its A1–A10 items do not already propose this prompt cache. It complements #2674 and #2675, which address unnecessary retranslation rather than prompt generation. ## What changed - **`nx2/make_game.py`:** source-checkout builds request incremental prompt generation. Packaged builders still regenerate every time, using the finish stage's existing launcher distinction. `RX_RELEASE` cannot distinguish them: make_game sets it for source builds too. Missing fontTools still removes the replacement prompts, keeping the existing fallback to the game's icons. - **`nx2/prompt_assets.py`:** fingerprint the inputs before generating, draw into a fresh sibling directory, and record the successful outputs only if a second fingerprint still matches. Reuse requires both matching inputs and every recorded output's size and SHA-256. **A failed generation still removes the prompts, as before**, so the game shows its own icons. An interrupted builder can leave the previous complete set without a stamp; the next build regenerates it. The drawing, encoding and asset formats are unchanged. - **`tests/test_prompt_skip.py`:** game-free fixtures run the actual finish-stage prompt call and production cache/publication functions, with small authored files and a stand-in for expensive drawing. - **`docs/development.md`:** explain reuse, its limits, and `HOOKSHOT_FORCE_PROMPTS=1`. The force switch follows `env_on`: unset, empty, `0`, `false`, `off` and `no` are off, ignoring case and surrounding spaces. ### What prevents a wrong skip The input record covers: - The four prompt source modules, including controller, keyboard and binding tables. - The game's icon font and optional bold label font, compression dictionary, common texture/layout archives, and matching resource-size table names and contents. - The repository's controller and keyboard fonts, and added, changed or removed PNG overrides. - Python/platform; fontTools, Pillow, ASTC encoder, texture decoder, NumPy, zstandard and archspec availability and versions; Pillow's native library versions and the selected ASTC backend. - Generation parameters and relevant library switches, including `SOURCE_DATE_EPOCH`, `FREETYPE_PROPERTIES` and the encoder's custom CPU definitions. Languages and saved bindings are not build inputs here: the generator uses its source tables, and the runtime applies the selected controls. The output record includes files **and directories**: the keyboard directory itself is a runtime marker. Legitimately absent optional outputs stay absent; a missing, truncated or same-size-corrupted recorded file forces regeneration. The runtime's own compressed copies and control-specific keys are excluded before hashing, not mistaken for generator output. **The files the game derives at run time are still cleared by every build**, including an unchanged hit: their own keys do not cover changes to `host/prompts.c`. Only the known runtime filenames are removed; if one cannot be cleared, the build regenerates instead of keeping on doubt. An unreadable input cannot authorize reuse. The log gives the reason, even if a forced regeneration subsequently fails. A source edited during generation leaves no stamp, addressing the stale-input certification problem discussed in [#2674's review](#2674 (comment)). ## How it was tested 2026-11-12; Arch Linux, kernel `7.2.15-1-omarchy`, x86-64, Intel Core i9-13900K, 32 logical CPUs, Python 3.12.16. Base `c544828ff`; final head `2ec89de8`. No game or GPU program was launched for these checks. ### Finish-stage measurements These execute the real finish-stage statements through completion, including binary/image copies, the unchanged shader-list check, pipeline-list work and prompt generation, in a disposable copy of an existing built game folder. Compilation, translation and shader preparation are not included. They are **not whole rebuild times**. All optional prompt packages were present: fontTools 4.68.0, Pillow 13.0.0, astc-encoder-py 0.1.12, texture2ddecoder 1.0.6, archspec 0.2.7, NumPy 2.5.4 and zstandard 0.25.0. | Run | Source | Finish time | 1-minute load before → after | Top other work | |---|---|---:|---:|---| | Before 1 | `c544828f` | 67.707 s | 7.13 → 7.40 | Existing game 316 → 315% CPU; file indexing 66 → 79%; relay check also active | | Before 2 | `c544828f` | 66.898 s | 7.06 → 8.24 | Existing game 315 → 318%; terminal process about 41% | | First generation with cache | `63d5c965` | 66.400 s | 8.24 → 8.19 | Existing game 318 → 320%; terminal process about 42% | | Unchanged 1 | `63d5c965` | 0.603 s | 8.19 → 8.19 | Existing game 320%; terminal process about 42% | | Unchanged 2 | `63d5c965` | 0.651 s | 8.19 → 8.19 | Existing game 320%; terminal process about 42% | | Changed table source | `681208dd` | 63.147 s | 6.99 → 6.95 | Existing game 320 → 321%; terminal process about 42% | | Regenerate, final head | `2ec89de8` | 65.007 s | 5.77 → 5.86 | Existing game 317 → 316%; terminal process about 43% | | Unchanged, final head, with run-time files present | `2ec89de8` | 0.616 s | 5.79 → 5.79 | Existing game 316%; terminal process about 43% | Other work was running; none was a quiet measurement. CPU percentages are lifetime averages from snapshots, not interval measurements. The first generation is included to show that this is a skip, not a faster drawing algorithm. The unchanged runs print: ```text prompts: unchanged, kept ``` Adding a harmless comment to the button-table source forces a fresh generation: ```text prompts: regenerated (table changed) ``` The comment was removed afterwards. A timestamp-only touch does not regenerate: the key checks content. An additional run on the final head verifies an unchanged hit with all six runtime-derived files present: all six are removed, while the 16 generated originals and the stamp retain their sizes and SHA-256 values. The focused fixture also compares the retained originals byte for byte. The final timing rows above include that cleanup. **Asset parity:** all 16 regenerated files, 274,704,680 bytes, compare byte for byte with a full generation by main `c544828f`. Only the new stamp is excluded. Both sides used the same `SOURCE_DATE_EPOCH` because fontTools otherwise writes the current time into the OpenType header. This comparison setting is not introduced into normal builds. A development cache hit retains the earlier font timestamp; packaged builds retain their old full-generation behavior. ### Game-free checks - **23 focused tests pass** on the final change. They cover unchanged reuse, every input kind, package versions/presence, switch/parameter changes, missing/truncated/same-size-corrupted output, empty directories, clearing runtime derivatives while retaining byte-identical originals, regeneration when runtime cleanup fails, interruption, mid-generation edits, unreadable input, force-switch parsing, incomplete output, publication failure, and the packaged/no-fontTools paths. A non-zero generator result removes prompts for both source and packaged builds; interruption before that result leaves the old set unstamped. - The same unchanged-build test against main's finish stage fails behaviorally: `2 != 1: unchanged finish must not draw the prompts twice`. - Removing each of **18 guards/key components** in memory makes its corresponding behavior test fail on the final change. Three later corrections also fail against the earlier version of this patch: returned failure retained old prompts in both build paths, runtime files survived a hit, and an uncleared runtime file did not force regeneration. The unreadable-input diagnostic has a separate before-fails/after-passes proof. - The whole suite one commit earlier, at `f20ea58b` (before a failed generation was made to remove the prompts again and the run-time files to be cleared), `python -I -B tests/run_all.py --jobs 8`: ```text 472 passed, 0 failed, 20 skipped in 133 s (linux, 8 at once) ``` Load was 7.37 → 19.48. The first full run, at 32 checks at once, was **471 passed, 1 failed, 20 skipped**: a clang++ child compiling `texture_pass.cpp` for `nx2/test_depth_sample_types.py` died with SIGKILL. That fixture and its input trees are unchanged from main. No kernel OOM line was found, so the cause is not established. The complete run with eight workers passes, including that check. The failed run is retained, not counted as a pass. On the final head `2ec89de8` the focused checks, the baseline failure and all guard mutations were rerun, and the whole suite once more with eight workers: **471 passed, 1 failed, 20 skipped in 136 s** (load 11.14 → 22.62). The one failure is `tests/test_release_scripts.py`, `test_nightly_two_runs_at_once_end_at_the_newer_commit`, the intermittent race reported in #2626; this change touches none of its files. - No core dumps were recorded. Nothing generated from the game is in this change. ## Not tested - Windows filesystem behavior or a Windows build. - The packaged Windows Builder or Linux AppImage build/update end to end. Their always-regenerate selection is covered by a game-free test only. - A full compile/lift or a new gameplay run, other CPUs, or gameplay/frame-rate benefits. No screenshots: the regenerated asset bytes match, and no visual change is intended. ## Risks The input list must grow when the generator gains new file dependencies or outputs outside the prompt directory. A regression flags new local module imports, but it does not replace reviewing new file reads. Generation temporarily needs room for both old and new assets. Publication uses completed directories and rollback on a rename error; interruption cannot create a stamped half-set, though a process killed between the two directory renames may require regeneration on the next build. ## Overlaps The open-PR scan found 47 PRs touching one of these files. The actual conflict hunks were read: - **#2151:** adjacent item-button generation conflicts with the finish-stage prompt call/error branch. Preserve both steps and account for added prompt dependencies. - **#2261:** substantive overlap. It adds `--wheel-icons`, another input folder and output outside `prompts`. Keep that work outside this skip or extend the input/output proof and staging; accepting both hunks mechanically is not enough. - #1872, #1958, #2304, #2317, #2430, #2465, #2474, #2478, #2608, #2673, #2675 and #2686 share code files but merged cleanly in the scan. That is not proof that any newly added generator dependency is covered. Assisted-by: GPT-6 Astra Co-Authored-By: GPT-6 Astra <noreply@openai.com> Co-authored-by: GPT-6 Astra <noreply@openai.com>
Joey-Magnus
suggested changes
Nov 12, 2026
Joey-Magnus
left a comment
Joey-Magnus
left a comment
Contributor
There was a problem hiding this comment.
Thanks. The Linux include fix (5cd55e0) is right, and I'm fine with the recomp_load8 reasoning, since the teardown hook is the real fix.
One new problem from main: #2698 (merged this morning) keeps the button-prompt assets when nothing they're made from has changed, and prompt_key() in nx2/prompt_assets.py lists those sources. This PR's prompt_assets.py now imports pause_layout, which isn't in that list. So after a change to nx2/pause_layout.py, a source build keeps the old prompts/Common.blarc. Merged into main (51da046), tests/test_prompt_skip.py catches it:
AssertionError: 'pause_layout' not found in {'prompt_assets', 'prompt_faces', 'kit', 'prompt_draw', 'prompt_keys'} : new prompt dependency needs fingerprint coverage
The fix is to add "pause_layout.py" to source in prompt_key() and "pause_layout" to sources in that test. The rest of tests/run_all.py passes here, apart from a lift-cache check that fails on main too.
Still open from before: the display_sdl.c change moves Linux menu input from polled state to a queue, and nobody has said they ran it on Linux. A short run there (open the Insert menu from the new button, move around, bind a key) would settle it. CI hasn't built this head yet either (only the title check has run).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Insert menu's settings can only be reached with a key (Insert, F1) or a pad chord, which a controller player may never find. This adds a sixth button, "Recomp Options", to the pause menu's Menu page, below Title Screen, drawn and moved to like the game's own; A on it opens the Insert menu.
What changed
nx2/pause_layout.py(new): addsPa_Button_05to everyPaSystemMenu_00layout inCommon.blarcat build time, a copy ofPa_Button_04188 units below it. The page draws only the panes its layout has (a copy made at run time is never drawn), so the layout itself gets the sixth pane. Everything that grows is appended after the end of the layout and of the archive: no other offset moves, and the prompts' runs overCommon.blarcstay valid.nx2/prompt_assets.py: writes the edited archive toprompts/Common.blarc(with aprompts/recomp_buttonmarker) and raises its entry in the resource size table.host/prompts.c: servesprompts/Common.blarcwith the button even when no prompt runs are written over it.host/oot_pause_menu.c,nx2/oot_pause_menu.py(new): when the page is made (f_80f99400) the sixth pane gets a button object like the other five, and its text ("Recomp Options", localized for pt/es/fr/it/de) is put on its text boxes. A while the cursor is on it (its state, noted atf_80ad5310) opens the Insert menu and is not passed to the game: the page's script has no end for a sixth button's decision, and passing it left the pause menu impossible to close.host/nn_hid.c: the buttons pass throughhos_pausebtn_buttons.host/nn_applet.c:hos_game_language()for the button's text.host/rx_backend.c) now exists on every platform, andhost/posix/display_sdl.cnotes each key on the look it goes down, ashost/display.c'sWM_KEYDOWNdoes on Windows. The Insert menu then reads its keys the same way on both, and the button's F1 request reaches it on Linux too.tests/test_oot_pause_menu.py(the lifted-C edits all or none; unknown layouts and archives left as they are);tests/test_linux_menu_keys.py(Linux keys reach the queue once on their way down, none while the window is not in front);tests/test_input_background.pygets a pass-through stub for the new call innpad_states.No game data is in the change: the layout is edited on the player's PC from their own files, and the original romfs is not modified.
How it was tested
Screenshots