Build: prepare the shaders at the end of the build, not at the first start #1026
No reviewers
Labels
No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
No milestone
No assignees
1 participant
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Hookshot!1026
Loading…
Reference in a new issue
No description provided.
Delete branch "prepare-shaders-at-build"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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.pynow passes: its harness window has the tick, the saved settings carryprepare=1, and a new case checks the unticked path (--no-prepare-shaders,prepare=0).What
make_game.pynow runshookshot --prepare-shadersas its last step, so shader preparation happens during the build, under the builder's progress bar, instead of at the game's first start.@@stageand@@progresslines go on to the builder unchanged. Its own@@done shadersand@@error ...lines go on as plain text, because both builders read those as the build itself finishing or failing.make_game.py --no-prepare-shadersand the game prepares at its first start, as today. The choice is remembered and used for updates.HOOKSHOT_NO_PREPARE=1also skips the step.Why
hookshot --prepare-shadershas been in the game since PR #320, but nothing runs it: notmake_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_runthe 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
tests/test_prepare_at_build.py(4 cases):@@done: the first two are relayed, and@@donecomes through as plain text;@@errorand exits 3: no@@line reaches the builder, the function returns 3, and the message says the game will prepare at first start;main(): it runs after the game folder is swapped in, the shader list is written and the pipeline seed is copied, and before@@done.test_finish_output.py,test_builder_status.py,test_build_log.py.hookshot.exe --prepare-shadersrun with its stdout piped printed its@@progressand@@done shaderslines and exited 0. That confirms the relay gets the lines on Windows too.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.pyandtest_prepare_at_build.pypass on current main.StagesandWeighttables.🤖 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_startedandetaare 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
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 shadersarrives or the game exits non-zero.On the launcher's Update button (
LaunchArg->ShowLauncher->LaunchUpdateClick) and on the--updaterestart after a self-update (Updater.FromCommandLine), we return beforeApplySettings()runs, soPrepareTickkeeps its XAML defaultIsChecked=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 savedPrepareLatersetting directly here (rather than the checkbox) would fix both paths.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 plusp.wait(timeout=...)andp.kill()) would keep that promise.This runs the game with make_game's full environment.
hookshot-launchandplay()stripHOOKSHOT_*,PYTHON*,APPDIR,APPIMAGE,ARGV0,OWDandLD_LIBRARY_PATHfirst, 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.nit: this largely re-implements
run()(Popen with pipe,no_throttle, rstrip/print loop). Only the@@line rewrite differs, and a line-filter argument torun()could cover it. As it stands the two copies already differ (nocwd=ROOT, noPYTHONIOENCODING).os.environ.get("HOOKSHOT_NO_PREPARE")is a truthiness check, soHOOKSHOT_NO_PREPARE=0also skips preparation. The game'sHOOKSHOT_NO_UPDATE_CHECKhandling compares against"0"; matching that would be more consistent.With
RX_TARGET=windowson Linux (CROSS),BESIDE[0]ishookshot.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 whenCROSSis set.minor: preparation runs before
total_seconds/record_times, sobuild_times.jsonand 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,These tests check the source text (substring positions, exact call text, exactly 2
--no-prepare-shadersper builder file) rather than behaviour. Harmless refactors break them (renaminggame_dir, wrapping the guard, a third use of the flag), while the launcher/--updateissue above still passes. Couldprepare_shadersbe tested against a stub executable that prints@@lines / hangs / exits 3 instead?Thanks for the thorough review. All ten points are addressed in f1432f8 (rebased on current main):
--updaterestart and the main window's Update now go throughBeginUpdate, which reads the savedPrepareLatersetting (BuilderSettings.PrepareNow) instead of the checkbox.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 shadersarrived and that line didn't; otherwise it shows as skipped, with the reason.run()has asilence=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@@progressoutput keeps it alive.make_game.game_env()is used for the preparation and by the Linux builder's Play, and a test keepshookshot-launch's list in sync. The game now also runs from its own folder, like Play.run().prepare_shadersgoes throughrun()with ashow=line filter (pluscwd=andbase=), so cwd andPYTHONIOENCODINGhandling is shared.HOOKSHOT_NO_PREPAREskips unless it's"0".record_timesand@@times, so the timings stay comparable.prepare_shadersagainst stub executables: relay, exit 3, crash, silent hang, missing exe, and environment/cwd. The Windows update-flag and row decisions are covered in thebuilder_workersharness.@@stageclearscompile_startedandeta.Checks:
test_prepare_at_build(17 tests),test_linux_builder_jobs, and the other builder and make_game tests pass.dotnet build -c Releasegives 0 warnings and 0 errors, andbuilder_workerspasses 80/80.test_hookshot_launchandtest_linux_updatecan't run on Windows (shell scripts); they fail there with or without this change.Thanks, that's a thorough rework. I went through all ten points and they're addressed:
BeginUpdatecovers all three update entry points, the row only gets a check mark when the shaders really were prepared,game_env()matcheshookshot-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-gamemode (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 readsprepare=1again. Windows saves the settings beforeUpdateFolder, so the two builders differ here.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. SoShowReadynever 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_exitshows "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.This reads the saved choice correctly, but on the launcher path
ApplySettingsstill never runs, soPrepareTickkeeps its default (ticked). If the player then goes Back and presses Build in that window,SaveSettingswritesPrepareLater = falseover their savedtrue, and every later update prepares again. Loading the tick fromprepareNowhere (or callingApplySettingson that path) would keep them in sync.minor: on Linux "built in" is wall time including the preparation, while "last time" is the
@@timestotal, 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 usesbuiltInfor this; comparing againstself.times[0]here would match it.nit: with
base=game_env()this putsPYTHONIOENCODINGback in, so the game's environment isn't quite the onegame_env()/hookshot-launchgive. Harmless for the game, but the env test doesn't notice because that name isn't in its list. Adding it only whenbase is Nonewould make the claim exact.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). Thenwait()blocks forever and we're back to a build that never ends. Something likep.wait(timeout=30), then giving up on the process and carrying on, would close that gap.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 shadersline inshow_prepared/on_lineand treating it as success (or switching to a short silence after it) would handle that.Thanks for the update. Checked
d6594bfagainst my last review at f1432f8: the new commit is mostly the earlier rework plus tests (the BuilderSettings/ShaderStep split,BeginUpdatereading the saved choice,game_env(), the cleared compile clock on a later@@stageon Linux are all fine), but none of the 7 points I raised have changed. Still open:nx2/make_game.pyrun(): afterp.kill()thep.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.@@done shadersfollowed by a hang at driver teardown is still judged by the exit code only (stopped after 15 min, reported as not prepared).MainWindow.xaml.csEnded: a Cancel during the shaders step still takes the"cancelled"branch back to the files page, soShowReadynever runs although the game is installed (Linuxon_exitlikewise).MainWindow.xaml.cs: on the launcher pathPrepareTickis still never loaded from the saved settings, so Back then Build writesPrepareLater = falseover the saved choice.hookshot_builder.py: "built in" is still wall time incl. the shaders while "last time" is the@@timestotal (compare againstself.times[0]).run()withbase=game_env()still addsPYTHONIOENCODINGback in.hookshot_builder.pybuild()inupdate-gamemode still doesn'tsave_config, so unticking "Prepare shaders now" there only applies to that update (Windows saves beforeUpdateFolder).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.
This landed in the v2.0.12 branch (merged there for testing; it reaches
mainwith that release), so closing. Thanks!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.