MagicBeanDev/Hookshot

Archived

Watch

Fork 0

fix(mods): merge prompts with the game's resource size table #2592

Closed

mooki000 wants to merge 1 commit from mooki000/fix/mods-opened-size-table into main

mooki000 commented

2026-11-12 00:41:22 +00:00

(Migrated from github.com)

What this is for: keep controller-button prompts working when a RomFS mod includes resource size tables for more than one game version, following hckrman101's review of #2270. What is measured: on Linux, GCC and Clang both pass 12 file-routing cases and the existing merge checks, including seven malformed-table refusals; both new open-order cases fail against main fce53453. Not measured: a new game run, Windows file enumeration, or any change to a normal single-table seed's gameplay.

Problem

The reviewer noted that:

build() picks the mod table with hos_mods_find, which returns the first match in hash order, while hos_mods_romfs passes the path actually opened.

host/prompts.c::sizes() merges once and remembers the source file. If the font is requested first, the first matching mod table can belong to another game version: the real table is subsequently served unmerged alongside the larger font. In the opposite order, the real table is merged, but the font's check asks about the other table and disables the replacement font.

The other review nit was about CI visibility, not another implementation change. At the 2026-11-11 17:13 CST check, #2270's check list still contained only two successful Conventional Commit title checks. That is not evidence that its GitHub regression suite ran. The current .github/workflows/source-regressions.yml already runs all discovered checks for pull requests to main; this PR needs those checks approved and green before merge.

What changed

host/prompts.c: discover the resource-table name in the game's own RomFS, using the same first-by-name rule as nx2/prompt_assets.py, before checking a mod's table for the font. This also works when the font is opened first.
host/mods.c: replace the prefix/suffix hash-order search with an exact, case/slash-normalized lookup of that name. It uses the same winning indexed entry as the normal RomFS open path.
tests/test_mods_size_table.py: add a second, wholly made-up table with different entries, exercise both table-first and font-first requests, and check both the returned files and merged bytes. --source <host-directory> runs the same fixtures against unmodified main.
readme.md: explain which version's table is used.

The merge algorithm, maximum-per-entry rule, malformed-table fallback, last-sorted-mod precedence and a mod's own font priority are unchanged. No game file or data made from the game is added.

How to try it

No switch or rebuild of a mod is needed. A mod containing two differently versioned size-table files should keep its matching table's entries and the controller font, irrespective of which file is requested first. The game-free reproduction is:

python -I -B tests/test_mods_size_table.py --cc gcc
python -I -B tests/test_mods_size_table.py --cc clang
# In this checkout, against another checkout's unmodified host sources:
python -I -B tests/test_mods_size_table.py --cc gcc --source /path/to/main/host

How it was tested

2026-11-11; Arch Linux, kernel 7.2.15-1-omarchy, Core i9-13900K; GCC 16.2.1 and Clang 23.1.3; deps Python 3.12 with -I -B. Base fce53453, branch fd017d3f. The machine was heavily loaded (load average about 287 during the focused checks). No GPU was used.
Final branch: bd4b1ec3, one commit on current main ef81aee7. The rebase from fce53453 was clean and its range-diff is =. Both compiler proofs, including the two failing open orders without the fix, were rerun after rebase; the relevant base host-source blobs are identical. The full-suite totals below are the recorded fce53453-based run, not a second full suite on the newer base.
The focused runs after the rebase had load snapshots of 71.63 to 75.04.

Both compilers print:

ok: merge_restbl: the larger entry of each file, entries of one side kept, sorted; 7 unmergeable tables refused
ok: 12 cases served as they should: no mod, a mod with a table, two table versions in both open orders, one that grows a file without, two mods, a table that cannot be merged, no table of ours, a mod's own font, compressed and size-less frames

Against the base, both compilers fail both new cases, rather than a source-text or compilation check:

Request order	Main's returned font	Main's returned actual table	Branch
Table, then font	The game's font; replacement disabled	Merged with the correct table	Correct merged table and controller font
Font, then table	Controller font	The unmerged mod table; the other version seeded the merge	Correct merged table and controller font

The normal single-table cases still compare all merged bytes to the independent expected table. No new game run was made: the ordinary seed's merge and resource sizes are unchanged, and the changed source-selection case is fully made up in the fixture. This does not claim a new gameplay result.

The whole tests/run_all.py suite ran once, in the idle scheduling class:

449 passed, 1 failed, 20 skipped in 288 s (linux, 32 at once)

Load was 111.79 at its start and 92.91 at its end. The only failure is the unrelated main regression test_env_on.py: HOS_FPS_LOG (host/oot_high_fps.c:15) still uses getenv(...) != NULL after #2470. That source and the guard test are unchanged here. test_updater_asset_digest.py passed via this machine's fallback; it is not counted as a failure. coredumpctl reported No coredumps found.
The same guard was then run against the untouched fce53453 checkout with GCC and Clang; both fail on that exact HOS_FPS_LOG line. This is the only remaining suite failure for B, not a mod-table regression.

Before pushing: one commit on current main, the commit identity and trailers as the project asks, and no game file or build output among the four changed files. Rerun on the pushed head by me: the test with gcc and with clang passes, and against main's host/ sources it fails with both.

Not tested

Windows runtime enumeration and a Windows game build.
A real mod containing multiple versions' tables, long play, or another game update's layout.
A game RomFS with ambiguous multiple applicable tables. Selection intentionally matches the prompt-assets builder; this does not invent a new version-selection policy.
GitHub CI for this not-yet-opened follow-up; the local results above do not stand in for it.

Risks

The font's initial preparation now enumerates the game's resource directory once per preparation rather than taking the first matching mod entry. It relies on hos_romfs_dir being the game's RomFS. Unsupported/ambiguous base layouts need a separate version-selection decision; ordinary supported single-table layouts and the merge's bounds are unchanged.

Overlaps

The final standalone overlap scan found 46 open PRs touching these files; the actual conflict hunks were read:

#2151: its punctuation helper is inserted beside game_table; retain both helpers.
#2317: the adjacent font-output comment changes to a cache path, plus the fixture's string.h include. Keep its path changes and this exact-table selection.
#2478, draft: substantive overlap. Its all-mod-table merger changes the old lookup's fallback semantics, the README and fixture sources. The exact-key selection must be carried into that resolver, including its merged-table/fallback behavior; do not resolve this by blindly keeping one side.
#2561: the README's large player/developer sections are replaced by documentation links. Move this table-selection explanation into its new mod documentation instead of retaining the old README section wholesale.
#2304, #2430, #2435 and #2583 touch the same code files but merged cleanly in the scan.

No separate claim issue: this is the review follow-up to #2270.

Assisted-by: GPT-6 Astra
Co-Authored-By: GPT-6 Astra [email protected]

**What this is for:** keep controller-button prompts working when a RomFS mod includes resource size tables for more than one game version, following [hckrman101's review of #2270](https://git.magicbean.dev/MagicBeanDev/Hookshot/pulls/2270). **What is measured:** on Linux, GCC and Clang both pass 12 file-routing cases and the existing merge checks, including seven malformed-table refusals; both new open-order cases fail against main `fce53453`. **Not measured:** a new game run, Windows file enumeration, or any change to a normal single-table seed's gameplay. ## Problem The reviewer noted that: > `build()` picks the mod table with `hos_mods_find`, which returns the first match in hash order, while `hos_mods_romfs` passes the path actually opened. `host/prompts.c::sizes()` merges once and remembers the source file. If the font is requested first, the first matching mod table can belong to another game version: the real table is subsequently served unmerged alongside the larger font. In the opposite order, the real table is merged, but the font's check asks about the other table and disables the replacement font. The other review nit was about CI visibility, not another implementation change. At the 2026-11-11 17:13 CST check, #2270's check list still contained only two successful Conventional Commit title checks. That is not evidence that its GitHub regression suite ran. The current `.github/workflows/source-regressions.yml` already runs all discovered checks for pull requests to main; this PR needs those checks approved and green before merge. ## What changed - `host/prompts.c`: discover the resource-table name in the game's own RomFS, using the same first-by-name rule as `nx2/prompt_assets.py`, before checking a mod's table for the font. This also works when the font is opened first. - `host/mods.c`: replace the prefix/suffix hash-order search with an exact, case/slash-normalized lookup of that name. It uses the same winning indexed entry as the normal RomFS open path. - `tests/test_mods_size_table.py`: add a second, wholly made-up table with different entries, exercise both table-first and font-first requests, and check both the returned files and merged bytes. `--source <host-directory>` runs the same fixtures against unmodified main. - `readme.md`: explain which version's table is used. The merge algorithm, maximum-per-entry rule, malformed-table fallback, last-sorted-mod precedence and a mod's own font priority are unchanged. No game file or data made from the game is added. ## How to try it No switch or rebuild of a mod is needed. A mod containing two differently versioned size-table files should keep its matching table's entries and the controller font, irrespective of which file is requested first. The game-free reproduction is: ```sh python -I -B tests/test_mods_size_table.py --cc gcc python -I -B tests/test_mods_size_table.py --cc clang # In this checkout, against another checkout's unmodified host sources: python -I -B tests/test_mods_size_table.py --cc gcc --source /path/to/main/host ``` ## How it was tested 2026-11-11; Arch Linux, kernel `7.2.15-1-omarchy`, Core i9-13900K; GCC 16.2.1 and Clang 23.1.3; deps Python 3.12 with `-I -B`. Base `fce53453`, branch `fd017d3f`. The machine was heavily loaded (load average about 287 during the focused checks). No GPU was used. Final branch: `bd4b1ec3`, one commit on current main `ef81aee7`. The rebase from `fce53453` was clean and its range-diff is `=`. Both compiler proofs, including the two failing open orders without the fix, were rerun after rebase; the relevant base host-source blobs are identical. The full-suite totals below are the recorded `fce53453`-based run, not a second full suite on the newer base. The focused runs after the rebase had load snapshots of 71.63 to 75.04. Both compilers print: ```text ok: merge_restbl: the larger entry of each file, entries of one side kept, sorted; 7 unmergeable tables refused ok: 12 cases served as they should: no mod, a mod with a table, two table versions in both open orders, one that grows a file without, two mods, a table that cannot be merged, no table of ours, a mod's own font, compressed and size-less frames ``` Against the base, **both compilers fail both new cases**, rather than a source-text or compilation check: | Request order | Main's returned font | Main's returned actual table | Branch | |---|---|---|---| | Table, then font | The game's font; replacement disabled | Merged with the correct table | Correct merged table and controller font | | Font, then table | Controller font | The unmerged mod table; the other version seeded the merge | Correct merged table and controller font | The normal single-table cases still compare all merged bytes to the independent expected table. No new game run was made: the ordinary seed's merge and resource sizes are unchanged, and the changed source-selection case is fully made up in the fixture. This does not claim a new gameplay result. The whole `tests/run_all.py` suite ran once, in the idle scheduling class: ```text 449 passed, 1 failed, 20 skipped in 288 s (linux, 32 at once) ``` Load was 111.79 at its start and 92.91 at its end. The only failure is the unrelated main regression `test_env_on.py`: `HOS_FPS_LOG (host/oot_high_fps.c:15)` still uses `getenv(...) != NULL` after #2470. That source and the guard test are unchanged here. `test_updater_asset_digest.py` **passed** via this machine's fallback; it is not counted as a failure. `coredumpctl` reported `No coredumps found.` The same guard was then run against the untouched `fce53453` checkout with GCC and Clang; both fail on that exact `HOS_FPS_LOG` line. This is the only remaining suite failure for B, not a mod-table regression. Before pushing: one commit on current main, the commit identity and trailers as the project asks, and no game file or build output among the four changed files. Rerun on the pushed head by me: the test with gcc and with clang passes, and against main's `host/` sources it fails with both. ## Not tested - Windows runtime enumeration and a Windows game build. - A real mod containing multiple versions' tables, long play, or another game update's layout. - A game RomFS with ambiguous multiple applicable tables. Selection intentionally matches the prompt-assets builder; this does not invent a new version-selection policy. - GitHub CI for this not-yet-opened follow-up; the local results above do not stand in for it. ## Risks The font's initial preparation now enumerates the game's resource directory once per preparation rather than taking the first matching mod entry. It relies on `hos_romfs_dir` being the game's RomFS. Unsupported/ambiguous base layouts need a separate version-selection decision; ordinary supported single-table layouts and the merge's bounds are unchanged. ## Overlaps The final standalone overlap scan found **46 open PRs** touching these files; the actual conflict hunks were read: - [#2151](https://git.magicbean.dev/MagicBeanDev/Hookshot/pulls/2151): its punctuation helper is inserted beside `game_table`; retain both helpers. - [#2317](https://git.magicbean.dev/MagicBeanDev/Hookshot/pulls/2317): the adjacent font-output comment changes to a cache path, plus the fixture's `string.h` include. Keep its path changes and this exact-table selection. - [#2478](https://git.magicbean.dev/MagicBeanDev/Hookshot/pulls/2478), draft: substantive overlap. Its all-mod-table merger changes the old lookup's fallback semantics, the README and fixture sources. The exact-key selection must be carried into that resolver, including its merged-table/fallback behavior; do not resolve this by blindly keeping one side. - [#2561](https://git.magicbean.dev/MagicBeanDev/Hookshot/pulls/2561): the README's large player/developer sections are replaced by documentation links. Move this table-selection explanation into its new mod documentation instead of retaining the old README section wholesale. - #2304, #2430, #2435 and #2583 touch the same code files but merged cleanly in the scan. No separate claim issue: this is the review follow-up to #2270. Assisted-by: GPT-6 Astra Co-Authored-By: GPT-6 Astra <[email protected]>

github-sync commented

2026-11-12 06:23:26 +00:00

Owner

@mooki000 commented on GitHub · 2026-11-12 06:07 UTC · original

Rebased bd4b1ec3 onto current main 87991bc8 as 6cb337df. The declaration conflict keeps this PR's exact game-table lookup together with main's keyboard/XInput prompts; the native-Windows font fix and no-controller fixture repair remain. Merge/fallback rules, both open-order assertions and the original commit message/trailers are unchanged. All 21 selected checks passed with GCC and Clang, while the feature control failed on main; the suite was 469 passed / 1 inherited background-input fixture warning / 20 skipped. The last rebase adds only upstream #2671 and leaves every PR file byte-identical; size-table/prompt checks, final-main negatives, #2671's offline check and Windows-target syntax ran again, with zero syntax errors using the production zstd include. The final PR gate passes. The suite was not repeated; no new gameplay or native Windows execution is claimed. Sources: the #2592 evidence and final-handoff ledger above.

> [@mooki000](https://github.com/mooki000) commented on GitHub · 2026-11-12 06:07 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2592#issuecomment-6816119973) Rebased `bd4b1ec3` onto current main `87991bc8` as `6cb337df`. The declaration conflict keeps this PR's exact game-table lookup together with main's keyboard/XInput prompts; the native-Windows font fix and no-controller fixture repair remain. Merge/fallback rules, both open-order assertions and the original commit message/trailers are unchanged. All 21 selected checks passed with GCC and Clang, while the feature control failed on main; the suite was 469 passed / 1 inherited background-input fixture warning / 20 skipped. The last rebase adds only upstream #2671 and leaves every PR file byte-identical; size-table/prompt checks, final-main negatives, #2671's offline check and Windows-target syntax ran again, with zero syntax errors using the production zstd include. The final PR gate passes. The suite was not repeated; no new gameplay or native Windows execution is claimed. Sources: the #2592 evidence and final-handoff ledger above.

github-sync commented

2026-11-12 14:43:14 +00:00

Owner

@Joey-Magnus approved on GitHub · 2026-11-12 14:33 UTC · original

I merged this into current main. tests/test_mods_size_table.py passes, including both new open orders, and so does tests/test_prompt_skip.py. game_table() (host/prompts.c:1321) picks the game's table first-by-name, the same rule nx2/prompt_assets.py uses. hos_mods_file() (host/mods.c:200) then looks up that exact key through the same find() the RomFS open uses. Both orders now merge the same table, which fixes what hckrman101 pointed out on #2270. hos_mods_find has no other callers, so removing it is safe.

FindFirstFileA on the RomFS path is fine on Windows because of the UTF-8 manifest (tests/test_utf8_paths.py passes), and on Linux it goes through host/posix like mods.c's existing scans. It runs once per font preparation, not per frame.

This will need care when #2478 (draft) is rebased, since it rewrites the same lookup. Your description already says so.

> [@Joey-Magnus](https://github.com/Joey-Magnus) approved on GitHub · 2026-11-12 14:33 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2592#pullrequestreview-5876219170) I merged this into current main. tests/test_mods_size_table.py passes, including both new open orders, and so does tests/test_prompt_skip.py. `game_table()` (host/prompts.c:1321) picks the game's table first-by-name, the same rule nx2/prompt_assets.py uses. `hos_mods_file()` (host/mods.c:200) then looks up that exact key through the same `find()` the RomFS open uses. Both orders now merge the same table, which fixes what hckrman101 pointed out on #2270. `hos_mods_find` has no other callers, so removing it is safe. FindFirstFileA on the RomFS path is fine on Windows because of the UTF-8 manifest (tests/test_utf8_paths.py passes), and on Linux it goes through host/posix like mods.c's existing scans. It runs once per font preparation, not per frame. This will need care when #2478 (draft) is rebased, since it rewrites the same lookup. Your description already says so.

github-sync referenced this pull request from a commit

2026-11-12 17:57:17 +00:00

fix(mods): merge prompts with the game's resource size table (#2592)

github-sync referenced this pull request

2026-11-12 17:58:03 +00:00

docs: rewrite the docs: player docs in docs/user/, developer docs in docs/dev/, the readme as a front page #2561

github-sync closed this pull request

2026-11-12 17:58:11 +00:00

Commenting is not possible because the repository is archived.

No reviewers