Stand-ins: check once per unpack, on --update too, skippable frames (follow-up to #1308) #1373

Merged
Joey-Magnus merged 2 commits from fix/stand-ins-once-per-unpack into main 2026-11-10 04:39:09 +00:00
Joey-Magnus commented 2026-11-09 23:39:59 +00:00 (Migrated from github.com)

Follow-up to #1308. The commit for its review nits landed on the branch after #1308 was merged, so it is here on its own:

  • Once per unpack: .hookshot_romfs_checked stores the .hookshot_romfs stamp the check ran under, so a rebuild over the same .nup no longer walks the ~102k romfs files again (about 34 s with a cold cache). A fresh unpack deletes the note first, and the note goes through check_cache_paths like the other generated paths.
  • --update: the check runs there too, but only on a romfs the builder unpacked itself (the .hookshot_romfs stamp must exist).
  • Skippable frames: a .zs file that starts with a zstd skippable frame (0x184D2A50 to 0x184D2A5F) now counts as whole (nup.zstd_start).
  • test_make_game_cache (test_same_source_reuses_the_cache and test_update_replaces_clearable_scratch_exefs): a reused unpack may add only that note, holding the same stamp.

Tested: nx2/test_nup_stand_in.py (new cases for skippable frames and for checking once per stamp) and the full tests/run_all.py on Linux: 186 passed, 19 skipped for missing tools.

🤖 Generated with Claude Code

Follow-up to #1308. The commit for its review nits landed on the branch after #1308 was merged, so it is here on its own: - **Once per unpack:** `.hookshot_romfs_checked` stores the `.hookshot_romfs` stamp the check ran under, so a rebuild over the same `.nup` no longer walks the ~102k romfs files again (about 34 s with a cold cache). A fresh unpack deletes the note first, and the note goes through `check_cache_paths` like the other generated paths. - **`--update`:** the check runs there too, but only on a romfs the builder unpacked itself (the `.hookshot_romfs` stamp must exist). - **Skippable frames:** a `.zs` file that starts with a zstd skippable frame (`0x184D2A50` to `0x184D2A5F`) now counts as whole (`nup.zstd_start`). - **`test_make_game_cache`** (`test_same_source_reuses_the_cache` and `test_update_replaces_clearable_scratch_exefs`): a reused unpack may add only that note, holding the same stamp. Tested: `nx2/test_nup_stand_in.py` (new cases for skippable frames and for checking once per stamp) and the full `tests/run_all.py` on Linux: 186 passed, 19 skipped for missing tools. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
NotRexed commented 2026-11-10 03:49:39 +00:00 (Migrated from github.com)

This landed in the v2.0.12 branch (merged there for testing; it reaches main with that release), so closing. Thanks!

This landed in the v2.0.12 branch (merged there for testing; it reaches `main` with that release), so closing. Thanks!
NotRexed commented 2026-11-10 03:54:31 +00:00 (Migrated from github.com)

Reopened: closing this was a mistake on my side. It was tested in the v2.0.12 branch, and it will be merged into main here, so it shows as merged. Sorry for the noise.

Reopened: closing this was a mistake on my side. It was tested in the v2.0.12 branch, and it will be merged into main here, so it shows as merged. Sorry for the noise.
hckrman101 (Migrated from github.com) approved these changes 2026-11-10 04:17:58 +00:00
hckrman101 (Migrated from github.com) left a comment

Looks good. I read it against main and the description matches the code.

  • Once per unpack: the note holds the .hookshot_romfs stamp, and a fresh unpack removes it before extracting, so a new .nup is always checked again. The note goes through check_cache_paths like the other generated paths. A crash mid-check leaves no note, so it just runs again.
  • --update: the call sits after check_modules, and the stamp guard means a romfs the builder didn't unpack is left alone. out and romfs are both in scope there.
  • Skippable frames: zstd_start is right (low byte 0x50-0x5F, then 2a 4d 18), and it is used in both damaged_game_files and stand_in_for, so a skippable-first file is neither flagged nor offered as a stand-in source wrongly.
  • Tests: both files are picked up by the nx2/test_*.py / tests/test_*.py glob in run_all.py, so nothing to register. The test_make_game_cache changes pin the note to the stamp, which is what you want.

Nits, no need to act: test_nup_stand_in.py swaps sys.modules["nup"] for a stub and never restores it, which is fine now as it is the last thing in the file but would bite if someone adds a real-nup case below it. CI: everything has passed except the Windows jobs (Vulkan back end, Python source checks windows-2022), which were still pending when I looked.

Looks good. I read it against main and the description matches the code. - Once per unpack: the note holds the `.hookshot_romfs` stamp, and a fresh unpack removes it before extracting, so a new .nup is always checked again. The note goes through `check_cache_paths` like the other generated paths. A crash mid-check leaves no note, so it just runs again. - `--update`: the call sits after `check_modules`, and the stamp guard means a romfs the builder didn't unpack is left alone. `out` and `romfs` are both in scope there. - Skippable frames: `zstd_start` is right (low byte 0x50-0x5F, then `2a 4d 18`), and it is used in both `damaged_game_files` and `stand_in_for`, so a skippable-first file is neither flagged nor offered as a stand-in source wrongly. - Tests: both files are picked up by the `nx2/test_*.py` / `tests/test_*.py` glob in run_all.py, so nothing to register. The `test_make_game_cache` changes pin the note to the stamp, which is what you want. Nits, no need to act: `test_nup_stand_in.py` swaps `sys.modules["nup"]` for a stub and never restores it, which is fine now as it is the last thing in the file but would bite if someone adds a real-nup case below it. CI: everything has passed except the Windows jobs (Vulkan back end, Python source checks windows-2022), which were still pending when I looked.
Commenting is not possible because the repository is archived.
No description provided.