Skip to content

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

Open
mooki000 wants to merge 1 commit into
NotRexed:mainfrom
mooki000:fix/mods-opened-size-table
Open

mooki000 wants to merge 1 commit into
NotRexed:mainfrom
mooki000:fix/mods-opened-size-table

Conversation

@mooki000

Copy link
Copy Markdown
Contributor

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:

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

Assisted-by: GPT-6 Astra
Co-Authored-By: GPT-6 Astra noreply@openai.com

Follow hckrman101's review of NotRexed#2270. A mod can ship several versions'
size tables; do not seed the prompts' one-time merge from the first
match in hash order. Use the game's RomFS table name, selected as the
prompt-assets builder does, and look up the winning mod file exactly.
Keep the merge and mod precedence unchanged.

Made-up two-version tables cover both font-first and table-first opens.
The branch passes with GCC and Clang; both orders fail on main. No game
file is used by these fixtures.

Assisted-by: GPT-6 Astra
Co-Authored-By: GPT-6 Astra <noreply@openai.com>
@mooki000

Copy link
Copy Markdown
Contributor Author

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
mooki000 force-pushed the fix/mods-opened-size-table branch from bd4b1ec to 6cb337d Compare November 12, 2026 06:07

@Joey-Magnus Joey-Magnus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants