Build: prepare the shaders at the end of the build, not at the first start #1026

Merged
klimovich008 merged 4 commits from prepare-shaders-at-build into main 2026-11-10 04:24:30 +00:00
klimovich008 commented 2026-11-09 06:53:28 +00:00 (Migrated from github.com)

Reopens #819, rebased on current main (GitHub wouldn't reopen it after the rebase). The "Prepare shaders now" tick requested in review is included. tests/test_linux_builder_jobs.py now passes: its harness window has the tick, the saved settings carry prepare=1, and a new case checks the unticked path (--no-prepare-shaders, prepare=0).

What

make_game.py now runs hookshot --prepare-shaders as its last step, so shader preparation happens during the build, under the builder's progress bar, instead of at the game's first start.

  • Relaying the output. The game's @@stage and @@progress lines go on to the builder unchanged. Its own @@done shaders and @@error ... lines go on as plain text, because both builders read those as the build itself finishing or failing.
  • Failures don't fail the build. If there's no usable Vulkan (exit 3), the game is missing, or anything else goes wrong, the build still succeeds. The game then prepares at its first start, as it does today.
  • Opt-out. Both builders have a "Prepare shaders now" tick, on by default. Unticked, they pass the new make_game.py --no-prepare-shaders and the game prepares at its first start, as today. The choice is remembered and used for updates. HOOKSHOT_NO_PREPARE=1 also skips the step.
  • Builder UI. The Windows builder gets a "Prepare the shaders" row and a share of the progress bar. The Linux builder already shows the stage's own text, so it needs no change.

Why

hookshot --prepare-shaders has been in the game since PR #320, but nothing runs it: not make_game.py, the Windows builder or the Linux builder. So every player gets the preparation at their first start, as minutes of a black game window. Its progress only shows in a console window that can open behind the game, which is why it looks like a hang (#26, #711, the "stuck at 99%" reports behind #701).

On my PC (RX 9070 XT, Ryzen 7 5700X3D), the first start after a fresh v2.0.3 build took 197 s before the first frame; it was 302 s on v2.0.2. A warm start takes 5.5–7.5 s. With this change, those minutes move into the build, where the player expects to wait and can see progress.

This PR doesn't change what gets prepared. It runs the same prepare_run the first start runs, so it neither fixes nor worsens the compute-layout bug that #772 fixes. #701, #702 and #772 change how the preparation works, not when it runs, so this combines with them.

Testing

  • New test, tests/test_prepare_at_build.py (4 cases):
    • a child that prints stage, progress and @@done: the first two are relayed, and @@done comes through as plain text;
    • a child that prints @@error and exits 3: no @@ line reaches the builder, the function returns 3, and the message says the game will prepare at first start;
    • a missing game exe: the build isn't failed;
    • an order check in main(): it runs after the game folder is swapped in, the shader list is written and the pipeline seed is copied, and before @@done.
  • Existing tests pass: test_finish_output.py, test_builder_status.py, test_build_log.py.
  • Windows piping: on Windows, hookshot.exe --prepare-shaders run with its stdout piped printed its @@progress and @@done shaders lines and exited 0. That confirms the relay gets the lines on Windows too.
  • Builder tests: tests/test_linux_builder_jobs.py (14 cases), test_builder_status.py, test_builder_scale.py, test_builder_python.py, test_make_game_cache.py, test_finish_output.py, test_build_log.py and test_prepare_at_build.py pass on current main.
  • Not tested: I haven't run a full builder build with this change or seen the new row in the Windows builder. The row only adds one entry to the existing Stages and Weight tables.

🤖 Generated with Claude Code

Reopens #819, rebased on current main (GitHub wouldn't reopen it after the rebase). The "Prepare shaders now" tick requested in review is included. `tests/test_linux_builder_jobs.py` now passes: its harness window has the tick, the saved settings carry `prepare=1`, and a new case checks the unticked path (`--no-prepare-shaders`, `prepare=0`). ## What `make_game.py` now runs `hookshot --prepare-shaders` as its last step, so shader preparation happens during the build, under the builder's progress bar, instead of at the game's first start. - **Relaying the output.** The game's `@@stage` and `@@progress` lines go on to the builder unchanged. Its own `@@done shaders` and `@@error ...` lines go on as plain text, because both builders read those as the build itself finishing or failing. - **Failures don't fail the build.** If there's no usable Vulkan (exit 3), the game is missing, or anything else goes wrong, the build still succeeds. The game then prepares at its first start, as it does today. - **Opt-out.** Both builders have a **"Prepare shaders now"** tick, on by default. Unticked, they pass the new `make_game.py --no-prepare-shaders` and the game prepares at its first start, as today. The choice is remembered and used for updates. `HOOKSHOT_NO_PREPARE=1` also skips the step. - **Builder UI.** The Windows builder gets a "Prepare the shaders" row and a share of the progress bar. The Linux builder already shows the stage's own text, so it needs no change. ## Why `hookshot --prepare-shaders` has been in the game since PR #320, but nothing runs it: not `make_game.py`, the Windows builder or the Linux builder. So every player gets the preparation at their first start, as minutes of a black game window. Its progress only shows in a console window that can open behind the game, which is why it looks like a hang (#26, #711, the "stuck at 99%" reports behind #701). On my PC (RX 9070 XT, Ryzen 7 5700X3D), the first start after a fresh v2.0.3 build took **197 s** before the first frame; it was 302 s on v2.0.2. A warm start takes 5.5–7.5 s. With this change, those minutes move into the build, where the player expects to wait and can see progress. This PR doesn't change what gets prepared. It runs the same `prepare_run` the first start runs, so it neither fixes nor worsens the compute-layout bug that #772 fixes. #701, #702 and #772 change how the preparation works, not when it runs, so this combines with them. ## Testing - **New test, `tests/test_prepare_at_build.py`** (4 cases): - a child that prints stage, progress and `@@done`: the first two are relayed, and `@@done` comes through as plain text; - a child that prints `@@error` and exits 3: no `@@` line reaches the builder, the function returns 3, and the message says the game will prepare at first start; - a missing game exe: the build isn't failed; - an order check in `main()`: it runs after the game folder is swapped in, the shader list is written and the pipeline seed is copied, and before `@@done`. - **Existing tests pass:** `test_finish_output.py`, `test_builder_status.py`, `test_build_log.py`. - **Windows piping:** on Windows, `hookshot.exe --prepare-shaders` run with its stdout piped printed its `@@progress` and `@@done shaders` lines and exited 0. That confirms the relay gets the lines on Windows too. - **Builder tests:** `tests/test_linux_builder_jobs.py` (14 cases), `test_builder_status.py`, `test_builder_scale.py`, `test_builder_python.py`, `test_make_game_cache.py`, `test_finish_output.py`, `test_build_log.py` and `test_prepare_at_build.py` pass on current main. - **Not tested:** I haven't run a full builder build with this change or seen the new row in the Windows builder. The row only adds one entry to the existing `Stages` and `Weight` tables. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Cretezy (Migrated from github.com) reviewed 2026-11-09 07:27:31 +00:00
Cretezy (Migrated from github.com) left a comment

Thanks for this, preparing the shaders at build time is a nice improvement. A few issues below, the first three being the most important.

One more that falls outside the diff, in builder/linux/hookshot_builder.py (~line 500): compile_started and eta are set during the compile and never cleared on a later @@stage, so during the (now minutes-long) shaders stage the progress page keeps showing a growing "compiling M:SS · about N minutes left". That makes the build look stuck. Clearing them when a new stage starts would fix it.

🤖 Generated with Claude Code

Thanks for this, preparing the shaders at build time is a nice improvement. A few issues below, the first three being the most important. One more that falls outside the diff, in `builder/linux/hookshot_builder.py` (~line 500): `compile_started` and `eta` are set during the compile and never cleared on a later `@@stage`, so during the (now minutes-long) shaders stage the progress page keeps showing a growing "compiling M:SS · about N minutes left". That makes the build look stuck. Clearing them when a new stage starts would fix it. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Cretezy (Migrated from github.com) commented 2026-11-09 07:27:31 +00:00

This row is always added, and Ended() marks every row Done, so it gets a check mark even when the step was skipped (tick off / HOOKSHOT_NO_PREPARE) or failed (no Vulkan, exit 3). The player is told the shaders are ready and still gets the long black-window wait at first start. Maybe only add the row when preparation is on, and show it as skipped/failed when no @@stage shaders arrives or the game exits non-zero.

This row is always added, and `Ended()` marks every row Done, so it gets a check mark even when the step was skipped (tick off / `HOOKSHOT_NO_PREPARE`) or failed (no Vulkan, exit 3). The player is told the shaders are ready and still gets the long black-window wait at first start. Maybe only add the row when preparation is on, and show it as skipped/failed when no `@@stage shaders` arrives or the game exits non-zero.
Cretezy (Migrated from github.com) commented 2026-11-09 07:27:31 +00:00

On the launcher's Update button (LaunchArg -> ShowLauncher -> LaunchUpdateClick) and on the --update restart after a self-update (Updater.FromCommandLine), we return before ApplySettings() runs, so PrepareTick keeps its XAML default IsChecked=True. A player who unticked "Prepare shaders now" gets the preparation anyway on those updates, so the comment here doesn't hold for exactly the update flows. Reading the saved PrepareLater setting directly here (rather than the checkbox) would fix both paths.

On the launcher's Update button (`LaunchArg` -> `ShowLauncher` -> `LaunchUpdateClick`) and on the `--update` restart after a self-update (`Updater.FromCommandLine`), we return before `ApplySettings()` runs, so `PrepareTick` keeps its XAML default `IsChecked=True`. A player who unticked "Prepare shaders now" gets the preparation anyway on those updates, so the comment here doesn't hold for exactly the update flows. Reading the saved `PrepareLater` setting directly here (rather than the checkbox) would fix both paths.
Cretezy (Migrated from github.com) commented 2026-11-09 07:27:31 +00:00

There's no timeout here: if the game hangs in --prepare-shaders (driver deadlock, a Windows crash dialog left open on hookshot.exe), for line in p.stdout / p.wait() never return and the build sits at 92% forever. The game folder is already complete by then, so the only way out is cancelling, which reports failure. That goes against "failures don't fail the build". A watchdog (e.g. a reader thread plus p.wait(timeout=...) and p.kill()) would keep that promise.

There's no timeout here: if the game hangs in `--prepare-shaders` (driver deadlock, a Windows crash dialog left open on hookshot.exe), `for line in p.stdout` / `p.wait()` never return and the build sits at 92% forever. The game folder is already complete by then, so the only way out is cancelling, which reports failure. That goes against "failures don't fail the build". A watchdog (e.g. a reader thread plus `p.wait(timeout=...)` and `p.kill()`) would keep that promise.
Cretezy (Migrated from github.com) commented 2026-11-09 07:27:31 +00:00

This runs the game with make_game's full environment. hookshot-launch and play() strip HOOKSHOT_*, PYTHON*, APPDIR, APPIMAGE, ARGV0, OWD and LD_LIBRARY_PATH first, so the game uses its own SDL and the system libraries. Here, inside the AppImage, the Vulkan loader/driver can pick up the image's libraries: either the run fails (exit 3) and silently falls back to first start, or it prepares under a different environment than the player's game runs in. Reusing the same env-cleaning as the other launches would avoid that.

This runs the game with make_game's full environment. `hookshot-launch` and `play()` strip `HOOKSHOT_*`, `PYTHON*`, `APPDIR`, `APPIMAGE`, `ARGV0`, `OWD` and `LD_LIBRARY_PATH` first, so the game uses its own SDL and the system libraries. Here, inside the AppImage, the Vulkan loader/driver can pick up the image's libraries: either the run fails (exit 3) and silently falls back to first start, or it prepares under a different environment than the player's game runs in. Reusing the same env-cleaning as the other launches would avoid that.
Cretezy (Migrated from github.com) commented 2026-11-09 07:27:31 +00:00

nit: this largely re-implements run() (Popen with pipe, no_throttle, rstrip/print loop). Only the @@ line rewrite differs, and a line-filter argument to run() could cover it. As it stands the two copies already differ (no cwd=ROOT, no PYTHONIOENCODING).

nit: this largely re-implements `run()` (Popen with pipe, `no_throttle`, rstrip/print loop). Only the `@@` line rewrite differs, and a line-filter argument to `run()` could cover it. As it stands the two copies already differ (no `cwd=ROOT`, no `PYTHONIOENCODING`).
Cretezy (Migrated from github.com) commented 2026-11-09 07:27:32 +00:00

os.environ.get("HOOKSHOT_NO_PREPARE") is a truthiness check, so HOOKSHOT_NO_PREPARE=0 also skips preparation. The game's HOOKSHOT_NO_UPDATE_CHECK handling compares against "0"; matching that would be more consistent.

`os.environ.get("HOOKSHOT_NO_PREPARE")` is a truthiness check, so `HOOKSHOT_NO_PREPARE=0` also skips preparation. The game's `HOOKSHOT_NO_UPDATE_CHECK` handling compares against `"0"`; matching that would be more consistent.
Cretezy (Migrated from github.com) commented 2026-11-09 07:27:32 +00:00

With RX_TARGET=windows on Linux (CROSS), BESIDE[0] is hookshot.exe, so this tries to exec a PE binary on the build host. Without binfmt it fails with a misleading "prepared at first start" message. With Wine registered through binfmt_misc, it prepares a cache for the build machine's GPU instead of the target PC's. I think this should be skipped when CROSS is set.

With `RX_TARGET=windows` on Linux (`CROSS`), `BESIDE[0]` is `hookshot.exe`, so this tries to exec a PE binary on the build host. Without binfmt it fails with a misleading "prepared at first start" message. With Wine registered through binfmt_misc, it prepares a cache for the build machine's GPU instead of the target PC's. I think this should be skipped when `CROSS` is set.
Cretezy (Migrated from github.com) commented 2026-11-09 07:27:32 +00:00

minor: preparation runs before total_seconds / record_times, so build_times.json and the "the build took X (the last one Y)" comparison mix in a GPU-dependent step that can be switched off. Recording it after (or separately) keeps the timings comparable between runs.

minor: preparation runs before `total_seconds` / `record_times`, so `build_times.json` and the "the build took X (the last one Y)" comparison mix in a GPU-dependent step that can be switched off. Recording it after (or separately) keeps the timings comparable between runs.
@ -0,0 +1,195 @@
"""make_game.py's last step, hookshot --prepare-shaders, run against stub games: its stage and progress lines go on to the builder,
Cretezy (Migrated from github.com) commented 2026-11-09 07:27:32 +00:00

These tests check the source text (substring positions, exact call text, exactly 2 --no-prepare-shaders per builder file) rather than behaviour. Harmless refactors break them (renaming game_dir, wrapping the guard, a third use of the flag), while the launcher/--update issue above still passes. Could prepare_shaders be tested against a stub executable that prints @@ lines / hangs / exits 3 instead?

These tests check the source text (substring positions, exact call text, exactly 2 `--no-prepare-shaders` per builder file) rather than behaviour. Harmless refactors break them (renaming `game_dir`, wrapping the guard, a third use of the flag), while the launcher/`--update` issue above still passes. Could `prepare_shaders` be tested against a stub executable that prints `@@` lines / hangs / exits 3 instead?
klimovich008 commented 2026-11-09 10:22:50 +00:00 (Migrated from github.com)

Thanks for the thorough review. All ten points are addressed in f1432f8 (rebased on current main):

  1. Updates use the saved choice. Launcher Update, the --update restart and the main window's Update now go through BeginUpdate, which reads the saved PrepareLater setting (BuilderSettings.PrepareNow) instead of the checkbox.
  2. The row tells the truth. It's only added when preparation is on. When make_game skips or falls back, it prints the shaders were not prepared now (<why>): … as a plain line, so older builders just log it. The row gets a check mark only if @@stage shaders arrived and that line didn't; otherwise it shows as skipped, with the reason.
  3. Watchdog. run() has a silence= option (reader thread, then kill). Preparation stops after 15 minutes with no output and falls back to the first start without failing the build. Steady @@progress output keeps it alive.
  4. Environment. A new make_game.game_env() is used for the preparation and by the Linux builder's Play, and a test keeps hookshot-launch's list in sync. The game now also runs from its own folder, like Play.
  5. No more duplicate of run(). prepare_shaders goes through run() with a show= line filter (plus cwd= and base=), so cwd and PYTHONIOENCODING handling is shared.
  6. HOOKSHOT_NO_PREPARE skips unless it's "0".
  7. Cross builds skip the step, with a message saying why.
  8. Build times. Preparation now runs after record_times and @@times, so the timings stay comparable.
  9. Behavioural tests. The tests now run prepare_shaders against stub executables: relay, exit 3, crash, silent hang, missing exe, and environment/cwd. The Windows update-flag and row decisions are covered in the builder_workers harness.
  10. Linux builder. A new @@stage clears compile_started and eta.

Checks: test_prepare_at_build (17 tests), test_linux_builder_jobs, and the other builder and make_game tests pass. dotnet build -c Release gives 0 warnings and 0 errors, and builder_workers passes 80/80. test_hookshot_launch and test_linux_update can't run on Windows (shell scripts); they fail there with or without this change.

Thanks for the thorough review. All ten points are addressed in f1432f8 (rebased on current main): 1. **Updates use the saved choice.** Launcher Update, the `--update` restart and the main window's Update now go through `BeginUpdate`, which reads the saved `PrepareLater` setting (`BuilderSettings.PrepareNow`) instead of the checkbox. 2. **The row tells the truth.** It's only added when preparation is on. When make_game skips or falls back, it prints `the shaders were not prepared now (<why>): …` as a plain line, so older builders just log it. The row gets a check mark only if `@@stage shaders` arrived and that line didn't; otherwise it shows as skipped, with the reason. 3. **Watchdog.** `run()` has a `silence=` option (reader thread, then kill). Preparation stops after 15 minutes with no output and falls back to the first start without failing the build. Steady `@@progress` output keeps it alive. 4. **Environment.** A new `make_game.game_env()` is used for the preparation and by the Linux builder's Play, and a test keeps `hookshot-launch`'s list in sync. The game now also runs from its own folder, like Play. 5. **No more duplicate of `run()`.** `prepare_shaders` goes through `run()` with a `show=` line filter (plus `cwd=` and `base=`), so cwd and `PYTHONIOENCODING` handling is shared. 6. **`HOOKSHOT_NO_PREPARE`** skips unless it's `"0"`. 7. **Cross builds** skip the step, with a message saying why. 8. **Build times.** Preparation now runs after `record_times` and `@@times`, so the timings stay comparable. 9. **Behavioural tests.** The tests now run `prepare_shaders` against stub executables: relay, exit 3, crash, silent hang, missing exe, and environment/cwd. The Windows update-flag and row decisions are covered in the `builder_workers` harness. 10. **Linux builder.** A new `@@stage` clears `compile_started` and `eta`. Checks: `test_prepare_at_build` (17 tests), `test_linux_builder_jobs`, and the other builder and make_game tests pass. `dotnet build -c Release` gives 0 warnings and 0 errors, and `builder_workers` passes 80/80. `test_hookshot_launch` and `test_linux_update` can't run on Windows (shell scripts); they fail there with or without this change.
hckrman101 (Migrated from github.com) reviewed 2026-11-09 10:40:21 +00:00
hckrman101 (Migrated from github.com) left a comment

Thanks, that's a thorough rework. I went through all ten points and they're addressed: BeginUpdate covers all three update entry points, the row only gets a check mark when the shaders really were prepared, game_env() matches hookshot-launch, the stub-exe tests are a big improvement, and cross builds / HOOKSHOT_NO_PREPARE=0 / the Linux stage clock are fixed. A few things in the new code, mostly around the watchdog and Cancel:

One outside the diff: in the Linux builder's update-game mode (build(), ~line 434), the update starts without saving the config, so unticking "Prepare shaders now" there only affects this update. The next launcher or self-update reads prepare=1 again. Windows saves the settings before UpdateFolder, so the two builders differ here.

Thanks, that's a thorough rework. I went through all ten points and they're addressed: `BeginUpdate` covers all three update entry points, the row only gets a check mark when the shaders really were prepared, `game_env()` matches `hookshot-launch`, the stub-exe tests are a big improvement, and cross builds / `HOOKSHOT_NO_PREPARE=0` / the Linux stage clock are fixed. A few things in the new code, mostly around the watchdog and Cancel: One outside the diff: in the Linux builder's `update-game` mode (`build()`, ~line 434), the update starts without saving the config, so unticking "Prepare shaders now" there only affects this update. The next launcher or self-update reads `prepare=1` again. Windows saves the settings before `UpdateFolder`, so the two builders differ here.
hckrman101 (Migrated from github.com) commented 2026-11-09 10:40:21 +00:00

A Cancel during the optional shaders step goes through the "cancelled" branch just below, back to the files page, even though the new game is already installed by then. So ShowReady never runs: no shortcuts, and the launcher doesn't learn the folder is ready. On an AMD GPU where preparation takes 20+ minutes, pressing Cancel there is quite likely. Linux has the same issue (on_exit shows "The build stopped"). Ending a cancel that comes after the finish stage as "ready, shaders left to the first start" would fit the step being optional.

A Cancel during the optional shaders step goes through the `"cancelled"` branch just below, back to the files page, even though the new game is already installed by then. So `ShowReady` never runs: no shortcuts, and the launcher doesn't learn the folder is ready. On an AMD GPU where preparation takes 20+ minutes, pressing Cancel there is quite likely. Linux has the same issue (`on_exit` shows "The build stopped"). Ending a cancel that comes after the finish stage as "ready, shaders left to the first start" would fit the step being optional.
hckrman101 (Migrated from github.com) commented 2026-11-09 10:40:21 +00:00

This reads the saved choice correctly, but on the launcher path ApplySettings still never runs, so PrepareTick keeps its default (ticked). If the player then goes Back and presses Build in that window, SaveSettings writes PrepareLater = false over their saved true, and every later update prepares again. Loading the tick from prepareNow here (or calling ApplySettings on that path) would keep them in sync.

This reads the saved choice correctly, but on the launcher path `ApplySettings` still never runs, so `PrepareTick` keeps its default (ticked). If the player then goes Back and presses Build in that window, `SaveSettings` writes `PrepareLater = false` over their saved `true`, and every later update prepares again. Loading the tick from `prepareNow` here (or calling `ApplySettings` on that path) would keep them in sync.
hckrman101 (Migrated from github.com) commented 2026-11-09 10:40:21 +00:00

minor: on Linux "built in" is wall time including the preparation, while "last time" is the @@times total, which now leaves it out. So a build that took exactly as long as last time reads 10 minutes slower when preparation took 10. Windows uses builtIn for this; comparing against self.times[0] here would match it.

minor: on Linux "built in" is wall time including the preparation, while "last time" is the `@@times` total, which now leaves it out. So a build that took exactly as long as last time reads 10 minutes slower when preparation took 10. Windows uses `builtIn` for this; comparing against `self.times[0]` here would match it.
hckrman101 (Migrated from github.com) commented 2026-11-09 10:40:21 +00:00

nit: with base=game_env() this puts PYTHONIOENCODING back in, so the game's environment isn't quite the one game_env() / hookshot-launch give. Harmless for the game, but the env test doesn't notice because that name isn't in its list. Adding it only when base is None would make the claim exact.

nit: with `base=game_env()` this puts `PYTHONIOENCODING` back in, so the game's environment isn't quite the one `game_env()` / `hookshot-launch` give. Harmless for the game, but the env test doesn't notice because that name isn't in its list. Adding it only when `base is None` would make the claim exact.
hckrman101 (Migrated from github.com) commented 2026-11-09 10:40:21 +00:00

After the silence timeout this p.wait() has no timeout. The case the watchdog is mainly for is a game stuck inside a GPU driver call, and such a process often can't die right away (D state on Linux, a pending kernel wait on Windows). Then wait() blocks forever and we're back to a build that never ends. Something like p.wait(timeout=30), then giving up on the process and carrying on, would close that gap.

After the silence timeout this `p.wait()` has no timeout. The case the watchdog is mainly for is a game stuck inside a GPU driver call, and such a process often can't die right away (D state on Linux, a pending kernel wait on Windows). Then `wait()` blocks forever and we're back to a build that never ends. Something like `p.wait(timeout=30)`, then giving up on the process and carrying on, would close that gap.
hckrman101 (Migrated from github.com) commented 2026-11-09 10:40:21 +00:00

Success is judged only by the exit code. If the game prints @@done shaders (cache saved, marker removed) and then hangs in Vulkan/driver teardown at exit, which some drivers do, the 15-minute silence kills it, and the build reports "not prepared" 15 minutes late although the work is done. Noting the @@done shaders line in show_prepared/on_line and treating it as success (or switching to a short silence after it) would handle that.

Success is judged only by the exit code. If the game prints `@@done shaders` (cache saved, marker removed) and then hangs in Vulkan/driver teardown at exit, which some drivers do, the 15-minute silence kills it, and the build reports "not prepared" 15 minutes late although the work is done. Noting the `@@done shaders` line in `show_prepared`/`on_line` and treating it as success (or switching to a short silence after it) would handle that.
hckrman101 (Migrated from github.com) reviewed 2026-11-09 23:23:55 +00:00
hckrman101 (Migrated from github.com) left a comment

Thanks for the update. Checked d6594bf against my last review at f1432f8: the new commit is mostly the earlier rework plus tests (the BuilderSettings/ShaderStep split, BeginUpdate reading the saved choice, game_env(), the cleared compile clock on a later @@stage on Linux are all fine), but none of the 7 points I raised have changed. Still open:

  1. nx2/make_game.py run(): after p.kill() the p.wait() still has no timeout, so a game stuck in a driver call (D state) still hangs the build forever. p.wait(timeout=30) and carry on.
  2. @@done shaders followed by a hang at driver teardown is still judged by the exit code only (stopped after 15 min, reported as not prepared).
  3. MainWindow.xaml.cs Ended: a Cancel during the shaders step still takes the "cancelled" branch back to the files page, so ShowReady never runs although the game is installed (Linux on_exit likewise).
  4. MainWindow.xaml.cs: on the launcher path PrepareTick is still never loaded from the saved settings, so Back then Build writes PrepareLater = false over the saved choice.
  5. hookshot_builder.py: "built in" is still wall time incl. the shaders while "last time" is the @@times total (compare against self.times[0]).
  6. nit: run() with base=game_env() still adds PYTHONIOENCODING back in.
  7. hookshot_builder.py build() in update-game mode still doesn't save_config, so unticking "Prepare shaders now" there only applies to that update (Windows saves before UpdateFolder).

CI is green and the branch merges cleanly with main. Nothing here is a regression, but 3 and 1 are the ones I'd fix before merging.

Thanks for the update. Checked d6594bf against my last review at f1432f8: the new commit is mostly the earlier rework plus tests (the BuilderSettings/ShaderStep split, `BeginUpdate` reading the saved choice, `game_env()`, the cleared compile clock on a later `@@stage` on Linux are all fine), but none of the 7 points I raised have changed. Still open: 1. `nx2/make_game.py` `run()`: after `p.kill()` the `p.wait()` still has no timeout, so a game stuck in a driver call (D state) still hangs the build forever. `p.wait(timeout=30)` and carry on. 2. `@@done shaders` followed by a hang at driver teardown is still judged by the exit code only (stopped after 15 min, reported as not prepared). 3. `MainWindow.xaml.cs` `Ended`: a Cancel during the shaders step still takes the `"cancelled"` branch back to the files page, so `ShowReady` never runs although the game is installed (Linux `on_exit` likewise). 4. `MainWindow.xaml.cs`: on the launcher path `PrepareTick` is still never loaded from the saved settings, so Back then Build writes `PrepareLater = false` over the saved choice. 5. `hookshot_builder.py`: "built in" is still wall time incl. the shaders while "last time" is the `@@times` total (compare against `self.times[0]`). 6. nit: `run()` with `base=game_env()` still adds `PYTHONIOENCODING` back in. 7. `hookshot_builder.py` `build()` in `update-game` mode still doesn't `save_config`, so unticking "Prepare shaders now" there only applies to that update (Windows saves before `UpdateFolder`). CI is green and the branch merges cleanly with main. Nothing here is a regression, but 3 and 1 are the ones I'd fix before merging.
NotRexed commented 2026-11-10 03:49:33 +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:55:48 +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.
Commenting is not possible because the repository is archived.
No description provided.