Skip to content

A batch in a game that's already open links up to the game at once, not at its first marker - #1126

Merged
ccoventry merged 3 commits into
devfrom
fix/events-pipe-reconnect
Oct 29, 2026
Merged

ccoventry merged 3 commits into
devfrom
fix/events-pipe-reconnect

Conversation

@ccoventry

Copy link
Copy Markdown
Owner

Release note: none

Follow-up to #1075.

What changes

In a game left open between batches (#725), Studio's events-pipe reader for the next batch could only connect when the game sent that batch's first marker, 9-30 s after Start Capture Batch (#1075's retest). The hook's pipe server (goldsrc-hooks/src/events.rs, its own thread) only noticed the previous batch's reader had gone when a write failed. Until then the game's "Studio connected" flag stayed stale.

Now, on each 500 ms idle timeout, the server asks the pipe whether its reader has gone and, if so, lets it go and listens again, so the next reader gets in within about half a second.

  • How it checks: ConnectNamedPipe on an instance that's already connected returns at once, with ERROR_PIPE_CONNECTED while the reader is there and ERROR_NO_DATA once it has closed its end. I measured this in an i686 test. PeekNamedPipe returns ERROR_ACCESS_DENIED on the outbound server end, so it can't be used.
  • No lost or doubled markers: the check only runs when the server has nothing to send. Its retry slot is empty at that point, so nothing is held back, and nothing has been written to the reader that's leaving.
  • batch_end (A capture batch outlives DoD Studio closing, and Studio catches up when it reopens #545) needed a change too: it quits the game 5 s after BATCH_COMPLETE when no reader is connected. Until now a reused game survived only because "connected" stayed stale after Studio's reader stopped at the end of the batch. With the faster check, the game would have quit after every batch. Now the batch end is forgotten once the pipe has actually written BATCH_COMPLETE to a reader (batch_end::studio_saw_end). That is what "Studio had its say" means. A Studio that closed mid-batch never gets that line, so the game still quits by itself as before.
  • Comments that said the pipe notices a gone reader only on a failed write are updated (batch.rs HELD_BEAT, native/src/obs/pipe_tail.rs).

Decisions

  • Release note none. The live test showed the footer's "Starting the game…" wait is not caused by the pipe. "Capturing clip 1 of 1" needs the game's NEXT_CLIP, which comes when the demo has loaded, ~30 s after Start in a reused game, before and after this fix. The old reader got every queued marker on connect, so it never lagged behind. What's fixed: Studio's reader is connected within a second, and the hook's "Studio connected" flag is accurate within half a second. That flag decides whether a held batch gives up on a closed Studio (Render and clean up clips while a capture batch is still running, so a long batch never fills the drive #861) and whether review-lock refusals are sent. Players won't see a change. The ~30 s demo load in a reused game is a separate question, not looked at here.
  • I used ConnectNamedPipe's ERROR_NO_DATA instead of a zero-byte write. A zero-byte write can reach the reader as a 0-byte read, which looks like EOF.
  • A reader counts as gone only on ERROR_NO_DATA. Any other result keeps the old behaviour, where a failed write notices it.

Test done

  • New real-pipe test (i686, events::tests::readers_get_everything_once_and_the_next_one_gets_in_at_once): a reader gets the hello, then the backlog in order. It drops while the server is idle, and the next reader gets in within 1.5 s (measured well under that). That reader gets each new line exactly once, and "connected" clears within 2 s of it leaving. With the check disabled, the test fails: "All pipe instances are busy" after 2 s. batch_end's test now also covers studio_saw_end.
  • cargo fmt --check is clean. Clippy -D warnings (1.98.1) is clean on the workspace (host) and on goldsrc-hooks i686. Hooks i686: 943 passed. Workspace tests (excluding hooks and web-analyzer) all pass, native 944.
    • A first workspace run, made at low priority while a game was capturing, had 5 OBS fake-server/pipe timing tests time out (os error 10060). They pass on an idle machine (obs:: 63/63), and so did the full rerun.
  • Live, Brain account, game lock held. Studio was built from this branch with its own settings folder; the user's settings.json was unchanged (sha256 c18fbc447751fe69 before and after). Hook DLL from this branch. Direct-to-video, Render While Capturing on, keep the game behind. Demo wsod25_grp5_h1_m00cat.dem (D:, folder with a space), highlight 4 (14:17, 3 kills). I added a temporary activity-log line for every marker Studio received, not committed.
Install Batch Game Start → pipe connected Markers received
POST 1 fresh from Launch Game 1 s all 10, once each, via pipe
POST 2 reused < 1 s (first marker came 31 s in) all 10, once each
PRE 3 fresh from Launch Game 1 s all 10, once each
PRE 4 reused < 1 s (first marker came 28 s in) all 10, once each

The 10 markers are DEMO_START, NEXT_CLIP, SPEED_FLUSH, AUDIO_SYNC, START_RECORD, CUSTOM_CMD1_BEFORE, STOP_RECORD, CUSTOM_CMD2_AFTER, FAST_FORWARD and BATCH_COMPLETE. Before, from #1075's retest with the same setup on POST, reused-game batches connected 14 s, 29 s, 30 s and 14 s after Start, each at the batch's first marker. After every batch the game was still open 15 s later, well past batch_end's 5 s grace, with the same PID throughout (54188 POST, 41396 PRE). So the studio_saw_end change keeps a reused game open as before.

🤖 Generated with Claude Code

ccoventry and others added 2 commits October 29, 2026 03:53
…atch's reader connects at once

On a game left open between batches (#725) the events pipe only noticed the
previous batch's reader had gone at its next write, so the next batch's
reader waited until the game's first marker (9-25 s in #1075's retest).
While idle, the server now asks ConnectNamedPipe whether its reader has
gone (ERROR_NO_DATA) and listens again. Nothing is held back at that point,
so no line is lost or sent twice.

batch_end used the stale "connected" to leave a reused game open after its
batch; it now forgets the batch's end once BATCH_COMPLETE has been written
to a reader, so the faster notice doesn't make the game quit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ccoventry

Copy link
Copy Markdown
Owner Author

Independent review (head e6ebcbb)

Verdict: no bugs found, no changes pushed. Ready to merge.

Batch-end paths

Case What happens now Compared with dev
(a) Fresh game, Studio connected, normal end BATCH_COMPLETE is written to Studio's reader, so studio_saw_end clears the marker. Studio kills hl.exe when it sees the marker (pipe/trigger). Same. On dev the flag was still true at the end of the grace period, so the hook forgot the marker then.
(b) Studio closed or crashed mid-batch (#545) The reader handle closes with the process. The pipe notices within 0.5 s and listens again. BATCH_COMPLETE waits in the queue and is never written, so SEEN_AT stays set. At the end of the grace period the flag is false, so the game quits. on_marker runs before emit, so the quit still happens when the queue is full (MAX_PENDING). Same result, noticed sooner
(c) Reused game (#725/#1075) BATCH_COMPLETE is written to the batch's reader and the marker is cleared. The reader then closes and is let go within 0.5 s. The game stays open. The game stays open, but now because of studio_saw_end, not because the flag was stale
(d) Cancel LEAVE_LINES starts with ds_capture_batch stop, so no BATCH_COMPLETE is sent and the game stays open (reused). A fresh game is killed. Same
(e) Written, then the reader dies before reading it The marker is cleared, so the game does not quit by itself. Same as dev: the flag stayed true until the next write failed, so the hook forgot the marker. Only a tiny window in which Studio would have to die.
(f) Review lock refuse_from_studio The refused label is sent only while a reader is really connected. Better: on dev a stale flag queued the label for the next reader, which is the case the comment there warns about
(g) Held batch (#861/#885) gone_since now starts within 0.5 s of the reader closing, not at the next HELD_BEAT write that fails (≤5 s). HELD_ALONE (10 s) still decides. PipeTailer stays connected for the whole batch and reconnects in a loop, so a held batch with Studio present never looks gone. Same rule, noticed sooner

How the end of the batch is ordered: the schedule echoes BATCH_COMPLETE at s_end and the _next alias (exit trigger) at s_end + 1. The pipe thread is waiting in recv_timeout, so it writes the marker almost at once, well before Studio's 16 or 500 ms poll could see the trigger and cancel the reader. If the reader did close first, the write fails and the game quits 5 s later. That matches dev, where a failed write also cleared the flag.

ConnectNamedPipe on the connected instance

  • The handle is synchronous (PIPE_WAIT, not overlapped) and one thread owns it. Underneath, a listen on an instance that is connected returns STATUS_PIPE_CONNECTED (535) at once, and on one that is closing it returns STATUS_PIPE_CLOSING (232, ERROR_NO_DATA) at once. It blocks only on a listening or disconnected instance, which this loop never passes it. The call changes nothing about the instance and cannot take a connection away from anyone.
  • After ERROR_NO_DATA the loop breaks, then calls DisconnectNamedPipe and CloseHandle, then CreateNamedPipeW and a blocking connect. The order is correct.
  • With nMaxInstances = 1, a new reader gets ERROR_PIPE_BUSY for up to 0.5 s. Both readers retry every 250 ms, so that is fine.
  • CONNECTED and SEEN_AT_MS are atomics. studio_saw_end runs on the pipe thread and does a single store. Nothing new runs on the game frame: batch_end::poll is still one atomic load until a batch ends.

Markers across a reconnect

reader_gone is checked only when retry is empty and the channel has timed out, so no line is in flight. A line whose write fails still goes back on retry for the next reader. No marker is lost or sent twice beyond what dev already does: a partial write is resent whole, which is older than this PR.

Minor, not changed

  • The test for studio_saw_end only shows that it equals reset. The real-pipe test leaves out BATCH_COMPLETE on purpose, because SEEN_AT_MS is a global another test sets. That is a reasonable trade.
  • studio_saw_end stores 0 with no condition. A batch_ended() could slip in between only if a hold gave up after BATCH_COMPLETE, which cannot happen.

Test done

  • Local i686 hook tests (worktree on e6ebcbb): events:: and batch_end:: 7/7 passed. I ran the whole i686 hook suite 5 times to look for timing flakes in the new 1.5 s assert: 943 passed every time, about 2.3 s per run.
  • CI on e6ebcbb: 6/6 green. I pushed nothing, so I did not re-run it.
  • No game launched. The builder's live runs (POST and PRE, fresh and reused, 10/10 markers) already cover the behaviour, and no finding needed a live check.

🤖 Generated with Claude Code

@ccoventry
ccoventry merged commit 82eef63 into dev Oct 29, 2026
5 checks passed
@ccoventry
ccoventry deleted the fix/events-pipe-reconnect branch October 29, 2026 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant