WIP: feat(control): stream sampled game state through the bridge #58
No reviewers
Labels
No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
needs measurements
question
testing wanted
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Longshot!58
Loading…
Reference in a new issue
No description provided.
Delete branch "mooki/Longshot:play-harness"
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
A script steering Link needs current game state and the frame at which it was sampled, not an undated snapshot or a second output file. This adds read-only samples to the existing control bridge; kept as a draft here because quiet off/on frame-time cost has not been measured, despite the functional Longshot run below.
The original https://github.com/NotRexed/Hookshot/issues/2292 was approved by Joey-Magnus. His latest review says both requested changes were settled: bound the table lookup and use the existing bridge instead of a parallel file channel. That approval predates the later age-command rebase and this move; it is not a new forge approval.
What changed
host/oot_state_tap.creads bounded guest values only in Link's update under the existing debug lock. Copied JSON crosses to the socket thread; it does not follow guest pointers there.status.gamecarries 14 core fields, including sample frame/time, position, camera, life and the existing event/loading/age values. Unknown values are null. A retained sample during pause/loading is not fresh control.ootsubscriptions have independent cadence, watched flags and change baselines. At most 64 watched flags and 32 reported changes; both lookup and scan reject tables larger than 2^22 slots. Pending samples are not overwritten before consumption.watch --events oot --every 6 --flags ... --changes. Main's typed age-command accessor stays intact; the JSON accessor is separately namedoot_sample._CRT_SECURE_NO_WARNINGS, matching the repository's UTF-8 fixture convention. This applies the CRT declaration policy to Windows-target fixture compilation while retaining-Werrorand every behavioral assertion. No native runtime adaptation was needed beyond the project rename.Compared with GitHub head f2970916a626dc9f7a3eb1079063881c1687da2d, all four host files, the client,
tests/control_test.cand the client tests are byte-identical after the mechanical rename. Onlytests/test_control.pyandtests/test_oot_state_tap.pyadd that compiler definition; documentation adds current evidence. The historical fork checks passed the control/client checks on both systems, but the whole run had then-known unrelated failures; it was not globally green and is not new Windows evidence.Overlap with another open pull request: this and pull request 47 (command-line test sessions) both change
scripts/longshot_ctl.py,tests/test_longshot_ctl.pyand the results table. Each merges cleanly into main on its own; merged together they conflict in those three files, so whichever goes second needs a rebase, which I will do.Flags and settings
HOS_CONTROLwatch --eventswatch --everywatch --flagswatch --changestests --source-roottest_oot_state_tap.py --cctest_oot_state_tap.py --mutate-D_CRT_SECURE_NO_WARNINGSHOS_OOT_STATE_FILEHOS_OOT_STATE_EVERYHOS_OOT_STATE_FLAGSHOS_OOT_STATE_CHANGESMeasurements
Branch state: on main
d725b3631982ed0035ea3ac9cbe1677e9fad260c; head365b02c32d0f46e208e3475b0811bd0001c203c5; whole game-free suite on this head:511 passed, 1 failed, 20 skipped(maind725b3631982ed0035ea3ac9cbe1677e9fad260c:510 passed, 1 failed, 20 skipped); the commits are unchanged by rebases apart from the results table.The sole failure in both suites in the branch-state block is
tests/test_updater_asset_digest.py: local .NET SDK 10.0.113 cannot resolveMicrosoft.AspNetCore.App.Ref (= 10.0.13), errorNU1100. The historical suites below also include this failure.main:
d4a69456c9(2026-11-12)this PR: play-harness @
c26c0d5f76Measurement provenance: main @
d4a69456c9.c26c0d5f76, including the final documentation rows; native game build @4e63c76537.Hardware: Core i9-13900K, 96 GB RAM, Arch Linux 7.2.15; RTX 4090 / NVIDIA 615.78.08 for the real game.
Scene: synthetic-guest/real-control-bridge fixtures; separate copied-save Kokiri Forest run, child Link standing, windowless, muted, 1280×720 captures, frame generation off, deferred shader preparation.
Method: same sampler driver before/after/before/after after the initial whole suite, fresh HOME, repository-default gcc. Median below is successful fixture completion, not speed. One ordinary shared Longshot session supplies runtime state and image evidence. No quiet off/on overhead measurement.
How to reproduce: Run
python -I -B tests/test_oot_state_tap.py --source-root CHECKOUTwith pinned main and PR checkouts, interleaved twice. For gameplay use a disposable copied Kokiri Forest save with HOS_CONTROL enabled, wait for 30 fresh controllable frames, subscribe to oot every 6 presents, read samples, press B for 8 frames, capture and quit. Use game.frame to date buffered samples.Main
d725b3631adds only scripts, docs and tests, with no game-code change; the measuredd4a69456comparisons and pinned game evidence remain valid and were not repeated. Original results below retain their measured commits; the new suite is recorded in the branch-state block.Baseline: main @
d4a69456d4a69456c26c0d5fThe before failure is feature absence, not a regression in main. The full suite ran on the exact final head above, after the last commit, with
--jobs 6 --strict, fresh HOME and default compiler. Main's shared suite was run once for this day's moved pull requests ond4a69456. These checks have not yet been run on a forge runner.Main also failed two
tests/test_appimage_package_retry.pytiming/sequence cases under heavy load; its requested isolated rerun passed all 9 tests. It fails only under heavy load on this PC; timing-dependent. This branch passed it. Neither full suite is described as all green.The first Windows-target check reported missing CRT definitions for four translation units. After the separate fixture-only correction, all four passed. Native Windows execution of both the control and sampled-state fixtures remains untested.
Real Longshot session
Both native binaries were rebuilt from this branch, using a cloned immutable release-profile donor, with 0 failed compiles. Banner:
longshot v2.2.0-rc.1-26-g4e63c7653 (source 20775991).The standard-library session client started this game, waited for fresh controllable state sustained for 30 frames, read state, subscribed, pressed B, captured a picture and observed stop. Input came from a copied save; its input copy was unchanged. The 60.9 s run had load 33.94 → 51.29: not a performance comparison.
Selected fields from the actual replies:
All four samples carried all 14 core fields, at six-frame intervals. Sample 1512 was buffered during the press; 1518 is after its completion. Delivery order is not sample time. The inspected image shows Link, the forest, hearts and minimap. Fatal=0, crash=0, device-loss=0, kernel faults=0 and NVRM=0, with complete journal coverage. Save draining is unsupported.
Historical Hookshot evidence, not repeated or relabelled: the 2026-11-11 file/socket comparison on base
a5b955fb, builds d847c0cf/a57f35fe, found nine standing fields equal and a watched Bool transition also inchanged. Both exited 0. The socket picture was HUD on black while shaders remained queued, so no rendering-parity claim was made. A later Hookshot f2970916 run exercised child/adult state and the age command. Those are older builds, not this new run.The complete suite was rerun after every source/documentation commit, with the default compiler and fresh HOME. The earlier measurement rows remain labelled by their actual commits. Main's suite was run once for all branches on this commit. Runner execution took 182 s on the branch, not a performance measurement. No hosted CI or new game run is claimed.
Testing
Tested: on this PC (Arch Linux, RTX 4090 with NVIDIA 615.78.08, i9-13900K), the current-head whole game-free suite and the earlier two interleaved main/branch sampler-fixture pairs at their measured commits above. The fixture covered socket status, typed flags, nulls, cadence, subscription bounds, independent baselines and cleanup; main's missing sampler was a feature-absence negative. Four translation units compiled for the Windows target; not run on Windows. In the Longshot Kokiri Forest run at native build
4e63c765, waited for fresh controllable play for 30 frames, read state, subscribed every 6 presents, pressed B for 8 frames, checked sample frames, took a screenshot and observed the game exit. The source save copy stayed unchanged; save draining remains unsupported.Tested but not measured: The 60.9-second Longshot forest session and older Hookshot transport/age runs prove functionality, not sampling overhead, FPS or rendering parity.
Not tested: the explicitly listed platforms, integration and runtime cases below remain untested; only the bounded Linux/NVIDIA forest session above was run for Longshot, under shared load.
HOS_CONTROLis unset the source returns before reads or allocation; this source fact is not a measured zero-cost claim. With changes enabled, scanning still costs work despite the output cap. Keep draft pending the feature template's off/on cost evidence.Needs other testers:
tests/test_control.pyandtests/test_oot_state_tap.py, then subscribe toootevery 6 presents in controllable Kokiri Forest play and disconnect/reconnect, expecting passing fixtures, correctly dated samples and no stale watch after reconnect (request to open)Screenshots
Nothing on screen is changed by the read-only sampler. main @
d4a69456has no paired rendering capture; this PR @4e63c765is the native build pictured below (runtime identical atc26c0d5f). No rendering-parity claim.Risk and rollback
A stale sample can mislead an automated controller; callers must require fresh game.frame and known-false event/loading. changes=true scans tables on the guest update under a lock and has unmeasured cost. Unset HOS_CONTROL to disable the sampler, unsubscribe from oot to remove watch scans, or revert the commit. This does not write guest memory or promise save-safe shutdown.
Reviewer notes
Start with hos_oot_state_tap and its early return, range checks, table caps and state_lock handoff; then connection cleanup and pending-sample handling in control.c. Keep main's typed oot_state age callback distinct from oot_sample. Quiet off/on cost and native Windows execution remain unmeasured. The live description's PR47 overlap warning is retained; whichever lands second still needs integration.
Issues
Original pull request and reviews: https://github.com/NotRexed/Hookshot/issues/2292.
Original tracking report: https://github.com/NotRexed/Hookshot/issues/2335, closed to consolidate discussion in that pull request.
No corresponding issue exists in the forge's public issue inventory checked for this move. No old GitHub number is used as a forge closing reference.
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.docs/tested-configurations.mdfor each measured run.mainand passing here.tests/that reads changed source updated in this PR, not weakened.Moved from NotRexed/Hookshot#2292.
Assisted-by: GPT-6 Astra
Co-Authored-By: GPT-6 Astra [email protected]
On Windows longshot_ctl.py waited for a busy pipe only when the error carried winerror 231. Python's open() goes through the C runtime, which has no errno for ERROR_PIPE_BUSY: it raises "[Errno 22] Invalid argument" with no winerror, so the wait never happened. With one client it did not show. tests/test_oot_state_tap.py connects four clients in a row, and the Windows checks failed there with exactly that error when the bridge's next pipe instance was not up yet. The client now waits, up to its timeout, for either form. A pipe that does not exist (ENOENT) is still reported at once. Tested: Linux, Python 3.12: three cases in tests/test_longshot_ctl.py drive the Windows branch with open() stood in: busy in both forms then connected, a missing pipe not waited for, a pipe that stays busy fails at the deadline. With main's client the first case fails ("no game listens ... Invalid argument"), the other two pass. tests/test_longshot_ctl.py, tests/test_oot_state_tap.py and tests/test_control.py pass as before. Not tested: on Windows itself (the project's Windows job does that; it is where the failure was seen). Assisted-by: Claude Fable 5.2 Co-Authored-By: Claude Fable 5.2 <[email protected]>Discussion carried over from NotRexed/Hookshot#2292
Every comment that was readable on the original thread when it was copied on 2026-11-12, in order, unchanged except that issue and pull request numbers are written as links to the old repository and names carry no @. Comments by an account that has since been closed there could no longer be read and are missing; where a reply quotes them, the quote remains.
mooki000, 2026-11-11 17:08 UTC (comment, original):
Rebased onto main
b29967bfso the workflows run here: they had waited for an approval on the old head, and main's game-free tests are green again since NotRexed/Hookshot#2380. The patch is unchanged (git range-diff: the commit is equal), and its test passes on the new base.Joey-Magnus, 2026-11-11 20:07 UTC (review: approved, original):
Nice, self-contained addition. Some specifics I checked:
host/oot_state_tap.c).nx2/build.pycompiles everyhost/*.c, so nothing else needs wiring.hos_oot_lift_playerruns under the existingcheats_lock.HOS_OOT_STATE_FILEis unset, the cost is onegetenvon the first call and then an early return.hos_guest_range, and nothing in the game is written.0x84ca6bc0→ manager +32/+40 → camera +120) matchesoot_gameplay_viewinhost/hos_main.c.docs/development.md.fopenon a UTF-8 path is fine on Windows because of the exe's UTF-8 manifest;tests/test_utf8_paths.pypasses.tests/test_oot_state_tap.pypasses here (gcc,-Werror). The red Windows check istests/test_native_astc_texture_count.py, which main fixed in NotRexed/Hookshot#2397. Theappimagefailure was Launchpad timing out. Neither comes from this PR.Minor points, none of them blocking:
host/oot_state_tap.cstate_entry: the probe loop runs up tosizeiterations, wheresizecomes from guest memory.state_put_changescaps it at1 << 22, but this function doesn't. A torn or garbage read while a save loads could make one line walk billions of slots on a guest worker. Using the same cap here (if (size > (1u << 22)) return 0;) would keep a bad read from stalling the game.status(now on main) and NotRexed/Hookshot#2048's!cheat 998already report map, event, loading and age. You offered to move the fields intostatus. A maintainer should decide whether to keep two channels. I'm fine with this file as it is, because a file stream suitsHOS_INPUT_LIVEscripts.InterlockedCompareExchange64, whichhost/posixprovides.Joey-Magnus, 2026-11-11 21:25 UTC (review: comment, original):
The code is clean (opt-in, read-only, every guest read checked, and the test passes).
To answer the earlier question: it isn't integrated with the control bridge.
host/control.calready hasstatus(event, loading, age) andsubscribe(map, state), anddocs/control.mddescribes the bridge as the one connection for tools. Since you offered, moving these fields intostatus/subscribeseems better than a second channel.One fix either way:
state_entry(host/oot_state_tap.c:107) walks up tosizeslots read from guest memory with no upper limit. The sibling function caps it at1u<<22, and this one should too.mooki000, 2026-11-11 22:31 UTC (comment, original):
hckrman101 Joey-Magnus thanks both.
d847c0cf):state_entryreturns no entry for a table that reports more than1 << 22slots, the same limit asstate_put_changes. The test's fixture sets such a size on a table whose first slot matches the flag asked for and expects no entry; it fails without the line, with gcc and clang. Rebased ontoa5b955fb, so the Windows job has NotRexed/Hookshot#2397's fix.status/subscribeand drop the file switches, so the bridge stays the one connection for tools (unless NotRexed wants the file kept). I'll push that here once a run in the game shows the same values over the socket as the file gives.mooki000, 2026-11-12 00:07 UTC (comment, original):
hckrman101 Joey-Magnus the one-channel question, answered in code:
I moved the fields into the bridge in a57f35fe:
status.gameis the snapshot;subscribewithevents:["oot"],every,flagsandchangesstreams it. The four file switches and file writing are gone. Reads stay in Link's locked update; the socket thread receives copies. The lookup cap from d847c0cf remains, and d06e4956 strengthens the resize-baseline check.On Linux/RTX 4090, the same copied save gave identical map, position, facing, camera, life/max life, rupees, prompt and music over file versus socket. A watched Bool changed 0→1 and the same hash appeared in
changed. Both games exited cleanly; no driver faults. gcc/clang checks pass, with 38 behavioral regressions rejected by each.Windows and AMD are not tested. Nor is rendering parity: the socket capture showed HUD on black with shaders still queued. The full suite was 437 passed, 1 failed, 20 skipped; the failure was
test_bake_mimo.py's unreadable-.zsassertion, a test and a script this change does not touch, and rerun on its own it passes here and on the base's tree. Details and all three runs are in the updated description.NotRexed if you would rather keep the file stream for
HOS_INPUT_LIVEscripts, say so: the first two commits are still the file version as reviewed (d847c0cf), and the move is the three commits above them, so it can come out again or the file can stay beside the bridge.mooki000, 2026-11-12 06:07 UTC (comment, original):
Rebased the actual pushed head
045fc13donto current main87991bc8as723758ff. The only conflict was additive test-evidence rows; both contributors' records remain verbatim, with no runtime change, dropped commit or altered author/message/trailer. All 27 selected checks passed with GCC and Clang on the first rebase; the sampler, bridge and client controls failed against main's missing feature. Its suite was 470 passed / 1 inherited background-input fixture warning / 20 skipped. The final rebase adds only upstream NotRexed/Hookshot#2671 and changes no PR file blob; the three feature checks, final-main negatives, NotRexed/Hookshot#2671's offline check and Windows-target syntax all ran again. The final PR gate passes. The suite was not repeated, and no new gameplay, native Windows or performance evidence is claimed. Sources: the NotRexed/Hookshot#2292 evidence and final-handoff ledger above.Joey-Magnus, 2026-11-12 14:32 UTC (review: approved, original):
Both open points from the earlier round are settled. The
state_entrycap is in, and the fields now go throughstatus.gameand anootsubscription instead of a second file channel, as hckrman101 and I asked. Guest reads still happen only in Link's update undercheats_lock. Only copied JSON crossesstate_lock. Watch names are restricted to[A-Za-z0-9_.]or0xplus 8 hex digits before they go into JSON. Every path that drops a connection clears its watch. I merged it into current main:test_oot_state_tap,test_control,test_hookshot_ctl,test_utf8_pathsandtest_env_onpass.Minor points:
host/oot_state_tap.c:351-371: a subscription withchanges:truescans up to four GameData tables while holdingstate_lockon the game's worker, so astatuscall waits for that scan. Fine for a debug channel.status.game.event,loadingandagecan now benull. Before, they were always booleans or a string.docs/control.mdsays so, but scripts that test== falsewill notice.G.game->oot_state(&event, &loading, &child), which this PR replaces. Whichever lands second needs to adapt. NotRexed/Hookshot#2561 also movesdocs/control.md.mooki000, 2026-11-12 18:54 UTC (comment, original):
Rebased
45310b24onto86a9cda2; new headf2970916.The merged age command keeps main’s typed
oot_state; our JSON copy is nowoot_sample. No wire-protocol change.test_control.py,test_hookshot_ctl.pyandtest_oot_state_tap.pypass with GCC and Clang (6 invocations).Whole source check: 497 passed, 3 failed, 20 skipped. Each failure also occurs on untouched pinned main; this is not an all-green result.
Failures: the one-variant cache fixture’s missing ZSTD declarations, the missing Skip opening README row, and this isolated environment’s .NET reference pack. The first two are fixed on main since; this head merges cleanly with it.
Built this head from source; one windowless Linux/RTX 4090 run used a copied save.
statusshowed child then adult;age adult --arrive 60succeeded. The same watch delivered 2 checked samples before and 2 after the reload (49 events received overall).The run exited normally with no fatal, crash, device-loss or kernel-fault line. No performance claim.
A post-switch screenshot was captured and inspected; it is not a rendering-parity comparison.
The earlier approval predates this rebase. A full run of the project's checks in my fork, Windows included, ran on this head: the control and client tests pass on both systems, and the only failing tests were the ones failing on main at that time (since fixed there by NotRexed/Hookshot#2861 and NotRexed/Hookshot#2883). It also found one real Windows problem, fixed in the last commit: the client now waits for a busy pipe that
open()reports as EINVAL.Assisted-by: GPT-6 Astra
Co-Authored-By: GPT-6 Astra [email protected]
Rebased onto main
d725b3631as365b02c32dwith all eight patches unchanged; its scripts/docs/tests-only changes do not invalidate the earlier functional measurements or Longshot run. The new-template description keeps both uploaded pictures and the pull request 47 overlap warning, and adds the control/default table, reproduction steps, risks and rollback. The whole suite on this exact head finished 511 passed, 1 failed, 20 skipped, with only the known local .NET reference-pack failure; the WIP description check passes, with the quiet off/on cost measurement still explicitly outstanding.c26c0d5f76to365b02c32dAdded the Testing section and updated its checklist. Added one branch base/suite block, consolidated duplicate results and simplified the supporting wording; everything else is unchanged.
View command line instructions
Manual merge helper
Use this merge commit message when completing the merge manually.
Checkout
From your project repository, check out a new branch and test the changes.