Builder: the damaged-texture check after an unpack shows its progress #1764

Merged

nullifyshot merged 1 commit from builder/damaged-check-progress into main

2026-11-10 16:13:49 +00:00

nullifyshot commented

2026-11-10 16:10:13 +00:00

(Migrated from github.com)

What changed

nx2/nup.py: damaged_game_files lists the romfs .zs files first, then calls progress(done, total) after each file it opens. stand_in_damaged passes it on. Without a callback it works as before.
nx2/make_game.py:
- Before the check starts, it now prints: "checking your game files for damaged textures (once after an unpack): every .zs file is opened, which can take minutes".
- A new file_progress sends @@progress up to 4 times a second, plus once at the end. The Builder's bar moves within its Read your game files row.
- It prints a plain line at each 10%, for the log and for terminals, e.g. checked 40,874 of 102,185 files (40%, 6 s).
tests/test_damaged_check_progress.py (game-free) checks:
- the per-file reports and that the damaged file is still found
- how often @@progress goes out, and the last one at 1.000
- the 10% lines' text, including large numbers
- that stand_in_damaged_files announces the check and passes the progress on

The check itself, the .hookshot_romfs_checked marker and what gets replaced are unchanged. So is the protocol: the Builder already reads @@progress for the current stage.

Tested

Windows 11, Ryzen 9 9900X3D, using the v2.0.13 Builder's tools (Python 3.14.7), with a full dump freshly unpacked by the v2.0.13 Builder. That's 102,185 .zs files.

Before: the first v2.0.13 build after the unpack printed that line, then nothing for several minutes. The check opened about 300 files a second with almost no CPU in use. Windows Defender's real-time protection was on.
After: the changed check run on the same romfs, read-only:
```
checked 10,219 of 102,185 files (10%, 1 s)
...
checked 102,185 of 102,185 files (100%, 15 s)
```
It sent 60 @@progress lines, the last at 1.000, and found 0 damaged files. This run took 15 s because the files were already cached from the build's first pass.
Tests: tests/test_damaged_check_progress.py, tests/test_make_game_cache.py and tests/test_missing_nup.py pass. The two changed files byte-compile. I didn't run the full tests/run_all.py locally.

Not tested:

The bar in the Builder's window. Only the lines it reads were checked.
Linux.

After a fresh unpack, the first build looks stuck for minutes on "the game was read from this .nup before: reading it again is skipped". It is checking every game file for damage (#1002), but it says nothing while it does. ### What changed - `nx2/nup.py`: `damaged_game_files` lists the romfs `.zs` files first, then calls `progress(done, total)` after each file it opens. `stand_in_damaged` passes it on. Without a callback it works as before. - `nx2/make_game.py`: - Before the check starts, it now prints: "checking your game files for damaged textures (once after an unpack): every .zs file is opened, which can take minutes". - A new `file_progress` sends `@@progress` up to 4 times a second, plus once at the end. The Builder's bar moves within its **Read your game files** row. - It prints a plain line at each 10%, for the log and for terminals, e.g. `checked 40,874 of 102,185 files (40%, 6 s)`. - `tests/test_damaged_check_progress.py` (game-free) checks: - the per-file reports and that the damaged file is still found - how often `@@progress` goes out, and the last one at 1.000 - the 10% lines' text, including large numbers - that `stand_in_damaged_files` announces the check and passes the progress on The check itself, the `.hookshot_romfs_checked` marker and what gets replaced are unchanged. So is the protocol: the Builder already reads `@@progress` for the current stage. ### Tested Windows 11, Ryzen 9 9900X3D, using the v2.0.13 Builder's tools (Python 3.14.7), with a full dump freshly unpacked by the v2.0.13 Builder. That's 102,185 `.zs` files. - **Before:** the first v2.0.13 build after the unpack printed that line, then nothing for several minutes. The check opened about 300 files a second with almost no CPU in use. Windows Defender's real-time protection was on. - **After:** the changed check run on the same romfs, read-only: ``` checked 10,219 of 102,185 files (10%, 1 s) ... checked 102,185 of 102,185 files (100%, 15 s) ``` It sent 60 `@@progress` lines, the last at `1.000`, and found 0 damaged files. This run took 15 s because the files were already cached from the build's first pass. - **Tests:** `tests/test_damaged_check_progress.py`, `tests/test_make_game_cache.py` and `tests/test_missing_nup.py` pass. The two changed files byte-compile. I didn't run the full `tests/run_all.py` locally. Not tested: - The bar in the Builder's window. Only the lines it reads were checked. - Linux.

hckrman101 (Migrated from github.com) approved these changes

2026-11-10 16:12:50 +00:00

hckrman101 (Migrated from github.com) left a comment

Looks good. I read the diff against the call sites in make_game.py and the Builder's @@progress handling.

No UI calls from a worker thread: the check prints @@progress and the Builder reads it from the process output, as it already does. The check runs after @@stage read, so both the Tk bar and the WPF row (Weight[current]) pick it up.
Total of 0 files: handled in both places (1.0, and tenth = 10), and the test covers it. The last report is always sent (done >= total), so the bar ends at 1.000. The count is len(paths), which is exactly what gets opened.
Cadence: the 4 per second throttle and the 10 plain lines are sensible for the log. No callback gives the old behaviour.
If the check raises, nothing more is printed and the error goes up as before. The marker is only written after success, so it re-checks next time, same as before.
The tests are picked up by the runner's tests/test_*.py glob, so no registration is needed.

One thing to sort out before merging: #1738 (monsterlag) changes the same lines. It also adds a progress callback to nup.damaged_game_files and stand_in_damaged and prints in stand_in_damaged_files, with a different signature (progress(count) against progress(done, total)). They'll conflict, and both going in would leave two progress mechanisms. This one is better for the Builder because it drives the bar and has a total, so it's worth settling which one lands.

Nit: test_the_check_is_announced_and_reports greps the source text of stand_in_damaged_files, so it only breaks on a rename. It's fine as a guard, but a test that stubs nup.stand_in_damaged and checks the callback gets passed would be sturdier.

No CI checks are reported on this branch.

Looks good. I read the diff against the call sites in make_game.py and the Builder's `@@progress` handling. - No UI calls from a worker thread: the check prints `@@progress` and the Builder reads it from the process output, as it already does. The check runs after `@@stage read`, so both the Tk bar and the WPF row (`Weight[current]`) pick it up. - Total of 0 files: handled in both places (1.0, and tenth = 10), and the test covers it. The last report is always sent (`done >= total`), so the bar ends at 1.000. The count is `len(paths)`, which is exactly what gets opened. - Cadence: the 4 per second throttle and the 10 plain lines are sensible for the log. No callback gives the old behaviour. - If the check raises, nothing more is printed and the error goes up as before. The marker is only written after success, so it re-checks next time, same as before. - The tests are picked up by the runner's tests/test_*.py glob, so no registration is needed. One thing to sort out before merging: #1738 (monsterlag) changes the same lines. It also adds a progress callback to `nup.damaged_game_files` and `stand_in_damaged` and prints in `stand_in_damaged_files`, with a different signature (`progress(count)` against `progress(done, total)`). They'll conflict, and both going in would leave two progress mechanisms. This one is better for the Builder because it drives the bar and has a total, so it's worth settling which one lands. Nit: `test_the_check_is_announced_and_reports` greps the source text of `stand_in_damaged_files`, so it only breaks on a rename. It's fine as a guard, but a test that stubs `nup.stand_in_damaged` and checks the callback gets passed would be sturdier. No CI checks are reported on this branch.

Commenting is not possible because the repository is archived.

No reviewers

hckrman101