feat(dev): test a change against a base, as a pack of what it changes #147
No reviewers
Labels
No labels
area/accessibility
area/audio
area/input
area/install
area/menu
area/renderer
area/saves
area/upscaling
good first issue
gpu/amd
gpu/igpu
gpu/intel
gpu/nvidia
help wanted
kind
bug
kind
chore
kind
docs
kind
feature
kind
performance
kind
question
needs measurements
platform/linux
platform/steam-deck
platform/windows
preview
priority
critical
priority
high
priority
low
status
blocked
status
confirmed
status
duplicate
status
invalid
status
needs-info
status
wontfix
testing wanted
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Longshot!147
Loading…
Reference in a new issue
No description provided.
Delete branch "klimovich008/Longshot:feat/dev-packs"
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?
Why
Testing a change in the game needed a full build per change: about 20 minutes, then the shader preparation, and a new
folder of tens of GB. This lets a change be tested as a small pack over one shared base, in seconds for a renderer-only
change and minutes for a host change. It was merged into Hookshot as https://github.com/NotRexed/Hookshot/pull/2732
(ac117196, reviewed by Joey-Magnus) after Longshot forked; this is that change with Longshot's names.
What changed
scripts/dev_packs.py(new):basebuilds a commit once (a full build, kept;--seedtakes a current game folder'sshader caches,
--predict-sbudgets the predicted pipelines,--language),adoptuses an existing full build as thebase,
packbuilds only what a change touches andrtmakes a runtime from the base with packs over it;warmruns awarm-up command in a base's runtime and keeps its caches;
lslists bases and packs.gpu/vk/files the game is not built from buildslongshot_vk.dllalone (RX_VK_ONLY);anything else builds the game and keeps each module whose linked objects differ from the base's (never compared by
bytes: both carry a fingerprint of every source).
nx2/build.pyhashes into the stamp) or a new shader seed makes the runtimeprepare again.
one job per base at a time (a busy marker).
tests/test_dev_packs.py(new, game-free): routing, the stamp, stamped objects, caches,module_changed, linkinputs, generated files, runtimes, guards, the predict budget, warm-up, busy marker; each rule has a broken variant
that must fail.
docs/development.md: "Testing a change against a base".AGENTS.md: one line in "Make the change".longshot.exe,longshot_vk.dll, thelongshot_*caches and log files,LONGSHOT_NO_PREPARE,LONGSHOT_KEEP_BUILD_FILES), and AGENTS.md's line moved to this repository's list.Flags and settings
LONGSHOT_NO_PREPARE,LONGSHOT_KEEP_BUILD_FILES,RX_VK_ONLY,RX_RELEASE,RX_TARGET(read),RX_VK_PREDICT_S(with--predict-s)Measurements
A development script: nothing in the game changes, so no frame rate is compared between main and this PR (main has no
such script). Each table names what it compares. RX 9070 XT (AMD 26.10.2), Ryzen 7 5700X3D, Windows 11.
main:
839e85ff(2026-11-13)this PR: feat/dev-packs @
2e898648Hardware: RX 9070 XT (AMD 26.10.2), Ryzen 7 5700X3D, 64 GB, Windows 11
Our first Longshot base, main @
839e85ff, was built with the script at33bb3594: compile 340 s (0 failed), 36 min in allwith the preparation (Most), 25,303 noted and 9,687 compute pipelines built, 0 missed. The PR has since merged main
d956c9ea, whose changes to nx2/make_game.py and nx2/build.py since839e85ffare AMD FSR 4 on Linux and the progressbar's step shares: a Windows build without
--fsr, as this base was, runs the same steps.The predicted-pipeline budget (
--predict-s), on Hookshot (where the script was merged as ac117196):46780503, full base46780503,--predict-s 60baseBaseline: Hookshot main @
46780503, full base. Also measured on Hookshot: a host-only pack's game module was byte-identical to the same change'sfull build apart from 2 PE header bytes.
Testing
Tested:
python -I tests/test_dev_packs.pypasses on this branch (Windows 11, merged with Longshot maind956c9ea).python -I -B tests/run_all.py(llvm-mingw 20261104): 497 passed, 4 failed, 36 skipped. The 4 failures(
nx2/test_nup_paths.py,tests/test_check_third_party.py,tests/test_display_pointer.py,tests/test_release.py)fail the same way on unmodified main
d956c9eaon this PC.In use: our first Longshot base was built with this script (
dev_packs.py base, at33bb3594) from main839e85ffon anRX 9070 XT / Ryzen 7 5700X3D, Windows 11: the build (compile 340 s, 0 failed) and the shader preparation (Most: 25,303
noted and 9,687 compute pipelines built, 0 missed), 36 min in all, with
base.jsonrecording the commit, the language andthe build's outputs. Packs and runtimes over it follow in our next A/B runs.
Codex's reviews (comments above) found the cache-family, recovery-marker and workflow-folder cases fixed here. At
8254a11bthey found that a base whose shader preparation fails (no usable Vulkan, a stuck driver, a game that can't start) lost its
compiled build, and that
LONGSHOT_NO_PREPAREno longer skipped it: both fixed (e6f67e69,243143a3), each with a case intest_prepare_outcomesthat fails on the code before; a sol review of 8254a11b..243143a3 found nothing more.Not tested: a pack and a runtime over the Longshot base yet (next), and Linux.
Needs other testers:
baseon main, then one host-only and one renderer-only pack andrtover them; the host pack should keeponly
longshot, the renderer pack onlylongshot_vk.so(T-PYS2).Screenshots
Nothing on screen.
Risk and rollback
A script and its test only; nothing in the game, the builders or a release reads it. Reverting the commit removes it.
Bases, packs and runtimes are built from the game and never leave the PC (the docs section says so).
Reviewer notes
The routing (
route,renderer_read_by_game) andmodule_changedare the parts worth reading first.Issues
See https://github.com/NotRexed/Hookshot/pull/2732
Checklist
For everyone
type(scope): description).main.python -I -B tests/run_all.pyrun and passing (any failure that also fails onmainis named above).third_party/untouched (a new folder with aPROVENANCE.mdinstead).If applicable
Assisted-by:/Co-Authored-By:trailers.tests/that reads changed source updated in this PR, not weakened.🤖 Generated with Claude Code
The port reads cleanly, and the routing and
module_changedhold up againstnx2/build.pyas it is now: the stamp files and dirs,host_hos_main.o/vk_vk_menu.o, thehost_depslines, theobjs.rsp/vk_objs.rsplink lists and the include walk fromhost/all match.One thing has to change before this merges.
cmd_baserecordsoutputsafter the base's shader prepare has run in the game folder, so the files the prepare writes beside the game and thatCACHES/WRITTEN_IN_PLAYdon't list count as build outputs.mkw_vk_pipelines.stampis the one that matters. Depending on the GPU,longshot_compute_variants.binandlongshot_module_ids.binare others. A game pack builds withLONGSHOT_NO_PREPARE=1, so none of them are ingame-out.generated_changesthen lists them asdeletedandrtremoves them from the runtime. With no stamp file,pcache_stamp::checkdropsmkw_vk_pipelines.binand every play file at the first start (vk_draw.cpppcache_drop_stale), so every game-pack runtime starts with a cold pipeline cache. The no-pack runtime keeps its cache, which breaks the "one difference" A/B the docs promise. Details are inline. A case for this intest_generated_and_runtime(a stamp file written by the base's prepare survives a game pack'srt) would keep it fixed.The other inline comments are nits.
For the description (not blocking review, but
pr statusflags it): the Measurements table is missing, and the Linux test request is still to open.@ -0,0 +63,4 @@# nx2/version.py's describe(), a fingerprint of every source, so those two objects differ in every build of another tree)MODULES = ((("longshot.exe", "longshot.map", "objs.rsp", "host_hos_main.o"),("longshot_vk.dll", "longshot_vk.map", "vk_objs.rsp", "vk_vk_menu.o")) if WINDOWS else(("longshot", None, "objs.rsp", "host_hos_main.o"), ("longshot_vk.so", None, "vk_objs.rsp", "vk_vk_menu.o")))Nit: on Linux, build.py links with
-Map=too (longshot.map,longshot_vk.so.mapinsa/build), so the maps could be kept here as on Windows, for crash offsets in a Linux pack. This isn't needed for this PR.@ -0,0 +72,4 @@TRANSLATOR = tuple("gpu/vk/%s/" % d for d in STAMP_DIRS) + tuple("gpu/vk/" + f for f in STAMP_FILES)RENDERER_ONLY = ("gpu/vk/", "third_party/imgui/", "third_party/fidelityfx")INCLUDE = re.compile(r'#\s*include\s+"([^"]+)"')NO_MODULE = ("docs/", "tests/", ".github/", ".claude/")Nit: Longshot keeps its forge files in
.forgejo/, so a change there routes asgameand runs a full game pack for nothing. Add.forgejo/(and.github/can stay for the upstream merges).@ -0,0 +76,4 @@HEADERS = (".h", ".hpp", ".hh", ".inc", ".inl")SOURCES = (".c", ".cc", ".cpp") + HEADERS# (files the game writes beside itself while it runs, besides the caches: never a build's output, so never "deleted" by a pack)WRITTEN_IN_PLAY = ("longshot_options.ini", "longshot_log.txt", "longshot_driver_faults.txt", "longshot_prepared.bin.warming")Related to the comment on
b["outputs"]: the game also writeslongshot_log.1.txt,longshot_load_start.txt(.waited),longshot_frames.csvandlongshot_pipelines.bin.builtbeside itself. If outputs are recorded before the prepare, this list matters much less. If you keep the exclusion approach instead, it needs those too.@ -0,0 +387,4 @@if a.predict_s > 0:prepare(game, b)b["build_s"] = round(time.time() - t)b["outputs"] = build_outputs(game)Blocking: this runs after the prepare has run in
game(make_game.py's own prepare when--predict-sis 0,prepare(game, b)above otherwise). Anything the prepare writes beside the game that isn't inCACHESorWRITTEN_IN_PLAYgets recorded as a build output. That includesmkw_vk_pipelines.stamp(pcache_stamp.h),longshot_compute_variants.bin,longshot_module_ids.bin, andlongshot_prepare_skippedafter a skipped prepare. A game pack builds withLONGSHOT_NO_PREPARE=1, sogenerated_changesreports all of them as gone, andrtdeletes them from the runtime. Without the stamp,pcache_stamp::checktreats the folder as unstamped and dropsmkw_vk_pipelines.binand the play files at the first start, so every game-pack runtime runs with a cold pipeline cache against a warm no-pack runtime.The simplest fix is to always build the base with
LONGSHOT_NO_PREPARE=1, recordoutputshere before the prepare, and then callprepare(game, b)(with or without the budget). A stricter exclusion list would also work, but it can drift as the renderer adds files.@ -0,0 +578,4 @@else:cmd_rt(argparse.Namespace(dir=d, runtime=rt, packs=[]))for cmd in a.run:argv = [x.replace("{runtime}", rt) for x in shlex.split(cmd)]Nit:
shlex.splitin POSIX mode treats backslashes as escapes, so on Windows--run "C:\Games\rt\longshot.exe"becomesC:Gamesrtlongshot.exe. The docstring says it's split POSIX-style, but a Windows user will trip over it. Considershlex.split(cmd, posix=os.name != "nt")(and stripping the quotes it leaves), or say in docs/development.md to use forward slashes.Pushed
5dc34020: changes under .forgejo/ (Longshot's workflows and templates) now build nothing, as .github/ already didn't; before, a CI-only change rebuilt the game. New route case, which fails without the fix. Found in Codex's review. test_dev_packs passes.Pushed
896b76ec, from Codex's two source reviews: --seed now copies the play cache files too (mkw_vk_pipelines_play_.bin; is_cache() already knew them), game_language uses make_game.py's folder_language (indented comments and unknown lines skipped), and the test's link lists are written as UTF-8. New checks for the seed and the language, each failing on the previous code. test_dev_packs passes.Pushed
defd599a, from Codex's final source pass: a base's recorded outputs and a pack's generated changes now skip every cache file (is_cache()), the play caches too, so a seeded play file is never listed as gone and dropped from a pack's runtime. The runtime test seeds one before the outputs are recorded and gives the pack another; each half of the fix fails the test when reverted. test_dev_packs passes.Pushed
220791ed, from Codex's spec pass: the pipeline cache's stamp (mkw_vk_pipelines.stamp, gpu/vk/pcache_stamp.h) is now one of the caches, so a seed copies it and a base's outputs and a pack's changes never list it; without it the renderer drops the copied pipeline cache and its play files. The tests seed it and check both; they fail without it. test_dev_packs passes.Pushed
94170793, from Codex's review: a pack's list of files gone now skips caches too, so an outputs list a base recorded before the caches' family was complete (play files, the stamp) can't make a pack delete them. Test: such a list with the stamp, a play file and a generated file the build no longer makes; only the generated file is gone. It fails without the filter. test_dev_packs passes.Pushed
636c445e, from Codex's final reviews: the renderer's whole cache family in one pass (the compressed program cache and its index, module ids, the built pipeline notes, compute variants, the older state list), the files the game writes while it runs kept out of a base's outputs, and a check in both directions (every .bin/.idx the renderer names must be a cache or a build's own file, so a new cache file fails the test until it is classified). The docs now say a renderer-only change keeps the renderer alone, as the routing does, with a route case for a shader. test_dev_packs passes.Pushed
33bb3594, from Codex's standards review: the preparation's recovery markers (longshot_pipelines.bin.predicting, .running, .built.warming) are files written while the game runs, so a base never records them as outputs and a pack never deletes them, also from an older base's list; a crash mid-prediction keeps its marker for the next start. Test extended; each half fails when reverted. test_dev_packs passes.From Codex's review: Final source review and the game-free dev-packs fixture pass at
33bb3594. Restoring the earlier cache family, marker output classification or marker deletion handling makes the corresponding check fail. This covers cache/stamp preservation, legacy output lists, recovery markers and the documented build routes. Symlink-specific cases were skipped on this Windows account. Fresh Longshot base construction and shader preparation have not been checked by Codex; no measured stutter or build-time gain is claimed from these fixtures.9a186f71into feat/dev-packs a58c307982Merged main
9a186f71(no force-push) and pushed33f821aa: main's new longshot_pipelines.bin.predicted_keys (#158) joins the cache family. run_all: 497 passed, 4 failed, 36 skipped; the 4 fail the same on unmodified main here. In use: our first Longshot base (main839e85ff) was built with this script: compile 340 s, 0 failed, Most preparation 25,303 + 9,687 pipelines, 0 missed, 36 min.Thanks for the careful review, HumanBean. Each point:
8254a11b. The base builds withLONGSHOT_NO_PREPARE=1, recordsoutputs, then runsprepare(game, b), with the budget when one is set and with make_game.py's Compile shaders level as make_game.py passes it. Nothing the prepare writes can be taken for an output any more.test_outputs_before_preparehas a fake prepare writemkw_vk_pipelines.stampand a file no list names. Neither ends up inoutputs, while the shader seed does. The test fails when the outputs are recorded after the prepare.220791ed,636c445e,33f821aa): the stamp,longshot_compute_variants.bin,longshot_module_ids.bin, the compressed program cache and its index,.built, and.predicted_keys.94170793,33bb3594)..bin/.idxthe renderer names must be classified.longshot_log.1.txt,longshot_load_start.txt(.waited),longshot_frames.csv,longshot_prepare_skippedand the recovery markers are inWRITTEN_IN_PLAY, and.builtis a cache (636c445e,33bb3594). With the outputs now recorded before the prepare, this list matters much less, as you said, but it stays as a second guard.shlex.splitand Windows paths: docs/development.md now says--runis split POSIX-style, so Windows paths take forward slashes (8254a11b). Our own caller passescygpath -mpaths. If you'd rather haveposix=Falseon Windows, I can switch it..forgejo/: routes as nothing to build now,.github/kept, with a route case (5dc34020).Description: the Measurements table is in. A Linux test request (host-only and renderer-only packs over one base) is being opened.
WIP: feat(dev): test a change against a base, as a pack of what it changesto feat(dev): test a change against a base, as a pack of what it changesPushed
e6f67e69and243143a3, from Codex's sol reviews of8254a11b: a base whose shader preparation fails (exit 3 no usable Vulkan, exit 4 a stuck driver, or a game that cannot start at all) now keeps its compiled build, with the reason in base.json's not_prepared, as make_game.py's own preparation leaves the work to the first start; a runtime's preparation still stops on failure. LONGSHOT_NO_PREPARE (set and not 0) skips a base's preparation again. The predicted-key file is named literally in the seed and outputs checks. New test_prepare_outcomes fails on8254a11b; a sol review of 8254a11b..243143a3 found nothing more. run_all: 497 passed, 4 failed (the same 4 as unmodified main here), 36 skipped.d956c9eainto feat/dev-packsMerged main
d956c9ea(no force-push) as2e898648: main's new "Build times" section in docs/development.md had been added where this PR's section is; both are kept, this PR's first. run_all on the merge: 497 passed, 4 failed, 36 skipped; the 4 (test_nup_paths, test_check_third_party, test_display_pointer, test_release) fail the same way on unmodifiedd956c9eahere (test_builder_dlss_update passes now, after #179). test_dev_packs passes.@HumanBean could you take another look when you have time? Your blocking point (outputs recorded after the preparation) is fixed in
8254a11b, withtest_outputs_before_prepare; the other four are answered in my reply above, each with its commit. Since then: a base whose preparation fails now keeps its build (e6f67e69,243143a3,test_prepare_outcomes), and maind956c9eais merged in (2e898648). run_all on the head: 497 passed, and the 4 that fail on unmodified main here fail the same way. Thanks!