Desync auto-resync: client receives and loads full state (#7) #11

Open

pbwraith wants to merge 3 commits from issue-7 into main

pull from: issue-7

merge into: pbwraith:main

pbwraith:main

pbwraith:issue-23

pbwraith:issue-8

pbwraith:issue-20

pbwraith:issue-19

pbwraith:issue-4

pbwraith:issue-16

pbwraith:issue-2

pbwraith:issue-1

pbwraith:issue-3

pbwraith:issue-6

pbwraith:issue-5

Conversation 4 Commits 3 Files Changed 39 +10893 -7

pbwraith commented 2026-08-25 07:49:31 +02:00

Owner

Copy Link

Desync auto-resync — client receives and loads state (#7)

Problem

The #6 implementation (host-side resync) only swapped the device state on the client side, missing rdp_state and ra_state. The frame number was not encoded in the message, so the client used session.current_frame() as a fallback.

Solution

Update the client-side process_resync_load to:

1. Extract the host's frame number from the message data (4-byte big-endian i32 prefix)
2. Load all four state sections: device, saves, rdp_state, ra_state — full parity with disk savestates
3. Call reset_frame with the host's frame

Changes

• create_savestate_in_memory: Now includes rdp_state and ra_state in the compressed buffer (was only device + saves)
• handle_desync: Prepends the host's frame number as 4-byte big-endian i32 to the resync_state message data
• process_resync_load:
  • Extracts the frame from the first 4 bytes of the message data
  • Decompresses and loads all four sections: device, saves, rdp_state, ra_state
  • Performs full device state swap matching savestates::load_savestate exactly: includes vru, cheats, transferpaks, cart.rom, PIF channel reconnection, audio frequency update
  • Restores RDP state via ui::video::load_state and RA state via retroachievements::load_state
  • Calls reset_frame with the host's frame (not the client's current frame)

Dependencies

• #5 (frame reset): Uses reset_frame and patched ggrs set_current_frame
• #6 (host-side resync): This branch is based on issue-6 and extends the client-side loading. PR includes #5 and #6 changes.

Tests

3 new tests:

• test_resync_frame_encoding — frame number encoding/decoding round-trip through NetplayMessage serialization
• test_resync_frame_encoding_zero — edge case: frame 0
• test_resync_frame_encoding_negative — edge case: negative frame

Closes #7

## Desync auto-resync — client receives and loads state (#7) ### Problem The #6 implementation (host-side resync) only swapped the device state on the client side, missing rdp_state and ra_state. The frame number was not encoded in the message, so the client used `session.current_frame()` as a fallback. ### Solution Update the client-side `process_resync_load` to: 1. Extract the host's frame number from the message data (4-byte big-endian i32 prefix) 2. Load all four state sections: device, saves, rdp_state, ra_state — full parity with disk savestates 3. Call `reset_frame` with the host's frame ### Changes - **`create_savestate_in_memory`**: Now includes `rdp_state` and `ra_state` in the compressed buffer (was only device + saves) - **`handle_desync`**: Prepends the host's frame number as 4-byte big-endian i32 to the `resync_state` message data - **`process_resync_load`**: - Extracts the frame from the first 4 bytes of the message data - Decompresses and loads all four sections: device, saves, rdp_state, ra_state - Performs full device state swap matching `savestates::load_savestate` exactly: includes vru, cheats, transferpaks, cart.rom, PIF channel reconnection, audio frequency update - Restores RDP state via `ui::video::load_state` and RA state via `retroachievements::load_state` - Calls `reset_frame` with the host's frame (not the client's current frame) ### Dependencies - **#5 (frame reset)**: Uses `reset_frame` and patched ggrs `set_current_frame` - **#6 (host-side resync)**: This branch is based on `issue-6` and extends the client-side loading. PR includes #5 and #6 changes. ### Tests 3 new tests: - `test_resync_frame_encoding` — frame number encoding/decoding round-trip through NetplayMessage serialization - `test_resync_frame_encoding_zero` — edge case: frame 0 - `test_resync_frame_encoding_negative` — edge case: negative frame Closes #7

pbwraith added 3 commits 2026-08-25 07:49:32 +02:00

Fix GGRS frame counter reset after state load (#5) ... eb5cf56ee3

After a netplay savestate load, the GGRS session's internal frame counter
must be reset to match the savestate's frame, or desync detection fires
immediately on resume.

Changes:
- Patch vendored ggrs 0.13.0 to expose set_current_frame(frame) on
  P2PSession and SyncTestSession, with internal SyncLayer and InputQueue
  reset methods to clear stale saved states, checksum history, and input
  queues
- Add [patch.crates-io] ggrs = { path = 'vendor/ggrs' } in Cargo.toml
- Add netplay::reset_frame(session, frame) wrapper function
- Record current GGRS frame in Savestate.netplay_frame during
  create_savestate when in netplay context
- Call reset_frame after load_savestate in netplay context
- Add tests: advance 120 frames, reset to 60, verify current_frame()==60
  and clean resume with matching inputs; reset to 0; idempotent reset

Closes #5

Desync auto-resync: host sends state on desync event (#6) ... 48598f45ca

When GGRS fires DesyncDetected, the host (player_number==0) snapshots
the current emulator state, sends it to all peers via the reliable
channel as a 'resync_state' message, and pauses the frame loop until
all peers acknowledge with 'resync_ready'. Non-host peers ignore the
desync event and wait for the host's state.

Changes:
- Add ResyncState enum (Idle, HostWaiting, ClientLoading) to Netplay
- In poll_clients DesyncDetected handler: host creates in-memory
  savestate, sends resync_state message, enters HostWaiting state
- Add check_resync_messages: non-host receives resync_state and enters
  ClientLoading; host receives resync_ready acks and resets frame when
  all peers ack
- Add process_resync_load: non-host decompresses and loads received
  state, calls reset_frame, sends resync_ready ack, resumes
- Add resyncing state check in advance_frame: pauses frame loop during
  resync (like disconnected path), with 5s timeout -> disconnect
- Add create_savestate_in_memory: serializes Device via postcard +
  compress without file I/O
- Call process_resync_load from vi.rs update_screen before normal frame
  processing
- Add tests: ResyncState default/transitions, message serialization,
  timeout logic, ack tracking

Based on issue-5 branch (includes GGRS frame reset from #5).

Closes #6

Client receives and loads full state during desync resync (#7) ... 9c18fd09bb

Update the desync auto-resync client-side state loading to achieve full
parity with disk savestates:

- Include rdp_state and ra_state in create_savestate_in_memory (was
  only device + saves)
- Encode host's frame number as 4-byte big-endian i32 prefix in the
  resync_state message data so the client knows which frame to reset to
  (was using session.current_frame() as fallback)
- Update process_resync_load to extract the frame from the message and
  load all four state sections: device, saves, rdp_state, ra_state
- Add full device state swap in process_resync_load: includes vru,
  cheats, transferpaks, cart.rom, PIF channel reconnection, and audio
  frequency update — matching savestates::load_savestate exactly
- Restore RDP state via ui::video::load_state and RA state via
  retroachievements::load_state
- Add tests: frame encoding/decoding round-trip, zero frame, negative
  frame edge cases

Based on issue-6 branch (includes #5 frame reset and #6 host-side
resync).

Closes #7

pbwraith referenced this pull request 2026-08-25 12:34:36 +02:00

No runnable build on the review box: target-cpu=x86-64-v3 crashes build scripts on pre-AVX2 CPUs, and CC=clang is absent #16

pbwraith commented 2026-08-25 12:38:49 +02:00

Author

Owner

Copy Link

Not merged this sweep, and not bounced — this PR has not been found faulty.

Two things block a verdict, both recorded in gopher64-netplay#16:

1. There is no runnable gate on the review box. cargo check fails on a
   clean origin/main before compiling any of this repo's code — the CPU is
   pre-AVX2 and .cargo/config.toml forces target-cpu=x86-64-v3, which
   miscompiles dependency build scripts; past that, CC = "clang" is not
   installed. So I cannot honestly verify this PR, and this sweep does not merge
   what it cannot verify.
2. The seven open PRs do not compose. Four independently edit
   src/netplay.rs. Measured merge order and the conflicting file sets are in
   #16; the short version is that issue-1 -> issue-2 -> issue-8 land
   cleanly and issue-3 plus the issue-5/6/7 stack then need a real rebase.

No action needed from you until #16 is resolved. Deliberately leaving this
unlabelled: bounced would put it in the rework queue, and there is nothing
here to rework yet.

Not merged this sweep, and not bounced — **this PR has not been found faulty.** Two things block a verdict, both recorded in gopher64-netplay#16: 1. **There is no runnable gate on the review box.** `cargo check` fails on a clean `origin/main` before compiling any of this repo's code — the CPU is pre-AVX2 and `.cargo/config.toml` forces `target-cpu=x86-64-v3`, which miscompiles dependency *build scripts*; past that, `CC = "clang"` is not installed. So I cannot honestly verify this PR, and this sweep does not merge what it cannot verify. 2. **The seven open PRs do not compose.** Four independently edit `src/netplay.rs`. Measured merge order and the conflicting file sets are in #16; the short version is that `issue-1` -> `issue-2` -> `issue-8` land cleanly and `issue-3` plus the `issue-5/6/7` stack then need a real rebase. No action needed from you until #16 is resolved. Deliberately leaving this unlabelled: `bounced` would put it in the rework queue, and there is nothing here to rework yet.

pbwraith referenced this pull request 2026-08-25 17:40:09 +02:00

No runnable build on the review box: target-cpu=x86-64-v3 crashes build scripts on pre-AVX2 CPUs, and CC=clang is absent #16

pbwraith referenced this pull request 2026-08-25 22:43:17 +02:00

Runnable build gate on the review box: portable config + documented gate script (#16) #17

pbwraith referenced this pull request 2026-08-26 03:45:13 +02:00

Runnable build gate on the review box: portable config + documented gate script (#16) #17

pbwraith referenced this pull request 2026-08-26 08:39:55 +02:00

Allow savestate creation/loading during netplay (#2) #15

pbwraith referenced this pull request 2026-08-26 08:39:58 +02:00

netplay: session resume — reload saved state and broadcast to peers (#4) #18

pbwraith referenced this pull request 2026-08-26 08:40:00 +02:00

netplay: session save to disk (host) [issue-3] #13

pbwraith referenced this pull request 2026-08-26 08:40:04 +02:00

Fix GGRS frame counter reset after state load (#5) #9

pbwraith commented 2026-08-26 08:46:25 +02:00

Author

Owner

Copy Link

Parked on #23 (and you now need a rebase)

!11 is the one to carry this work forward -- it contains !10.

Two things to fix in ONE pass, not two

1. You now conflict with main. !14 (issue-1, state-message types)
merged a few minutes ago. Re-measured against the new main (c5609be) with
a real git merge in a throwaway worktree -- not the mergeable flag, which
still reads true:

git merge origin/issue-6  ->  CONFLICT  src/netplay.rs
git merge origin/issue-7  ->  CONFLICT  src/netplay.rs

Both branches merged cleanly before !14 landed, so this is a fresh, legitimate
rebase need and not something you got wrong.

2. A rebase alone will not make the gate green. cargo test --lib (gate
step 3) currently aborts the entire test binary with a stack overflow on any
test that constructs a Device -- measured on !15 and !18, root-caused and
filed as #23. Your branches touch src/device.rs and src/savestates.rs,
so this very likely reaches you too.

I did not gate !10/!11 -- I stopped once the conflict was established,
because a gate against a tree I would have had to hand-resolve proves nothing
about your branch. So treat point 2 as "expected, unverified for these two",
not as a measured failure of your code.

Waiting for #23 before reworking means one rebase against a main that can
actually run tests, instead of a rebase now and another after #23 lands.

Relationship between these two PRs

git merge-base --is-ancestor origin/issue-6 origin/issue-7 -> true.
!11 already contains !10's commit 48598f4. Rework !11 and close !10 as
subsumed; do not repair both.

## Parked on #23 (and you now need a rebase) !11 is the one to carry this work forward -- it contains !10. ### Two things to fix in ONE pass, not two **1. You now conflict with `main`.** !14 (`issue-1`, state-message types) merged a few minutes ago. Re-measured against the new `main` (`c5609be`) with a real `git merge` in a throwaway worktree -- not the `mergeable` flag, which still reads `true`: ``` git merge origin/issue-6 -> CONFLICT src/netplay.rs git merge origin/issue-7 -> CONFLICT src/netplay.rs ``` Both branches merged cleanly before !14 landed, so this is a fresh, legitimate rebase need and not something you got wrong. **2. A rebase alone will not make the gate green.** `cargo test --lib` (gate step 3) currently aborts the entire test binary with a stack overflow on any test that constructs a `Device` -- measured on !15 and !18, root-caused and filed as **#23**. Your branches touch `src/device.rs` and `src/savestates.rs`, so this very likely reaches you too. **I did not gate !10/!11** -- I stopped once the conflict was established, because a gate against a tree I would have had to hand-resolve proves nothing about your branch. So treat point 2 as "expected, unverified for these two", not as a measured failure of your code. Waiting for #23 before reworking means one rebase against a `main` that can actually run tests, instead of a rebase now and another after #23 lands. ### Relationship between these two PRs `git merge-base --is-ancestor origin/issue-6 origin/issue-7` -> **true**. !11 already contains !10's commit `48598f4`. Rework **!11** and close !10 as subsumed; do not repair both.

pbwraith added the

blocked

label 2026-08-26 08:46:26 +02:00

pbwraith referenced this pull request 2026-08-26 08:46:27 +02:00

Desync auto-resync: host sends state on desync event (#6) #10

pbwraith referenced this issue from a commit 2026-08-26 09:23:55 +02:00

ci: probe the Actions runner before writing the Rust gate

pbwraith commented 2026-08-26 13:27:25 +02:00

Author

Owner

Copy Link

Unblocked, and re-bounced with the specific thing left to do. blocked
cleared, bounced applied -- a PR carrying neither label is invisible to the
worker lane AND suppresses its own issue as a candidate, so this is the label
that makes the rework reachable.

The block is genuinely gone

#23 is CLOSED. !24 merged this sweep and I verified the gate now executes
Device-constructing tests instead of aborting:

step 3:  running 5 tests  (was 4)
  test_util::tests::device_round_trip_fits_under_the_8_mib_test_stack_budget ... ok
  test result: ok. 5 passed; 0 failed

So Desync auto-resync (host + client halves) can finally be gated on its merits.

What is actually left: a rebase, measured not guessed

Re-measured with a real git merge origin/main in the warm clone (NOT the
mergeable flag, which is unreliable in both directions):

branch is 13-14 commits behind origin/main
git merge origin/main  ->  CONFLICTS
    src/netplay.rs

!10 (issue-6) is closed as subsumed into this branch (--is-ancestor -> TRUE), so this PR is now the single carrier for that work.

Second thing, in the SAME pass

Your Device-constructing test bodies should route through the helper !24
added, not rely on the gate's env var:

use crate::test_util::with_big_stack;

#[test]
fn my_test() {
    with_big_stack(|| {
        // existing body, unchanged
    });
}

scripts/gate.sh now exports RUST_MIN_STACK=8388608, which immunises the
GATE -- but a bare cargo test --lib still gives each test thread the 2 MiB
libtest default, and a stack overflow SIGABRTs the whole binary rather than
failing one test. I verified with_big_stack carries a full Device round trip
at RUST_MIN_STACK=2097152, so wrapping the body makes the test independent of
how it is invoked. Do both in one pass -- a rebase now and a wrapper later is
two conflict resolutions on the same lines.

Note on the target

main is moving during this sweep (!24 landed; !15 is in the gate as I
write). Rebase onto whatever origin/main is when you pick this up and
re-measure the conflict set -- do not work from the file list above as if it
were fixed.

Unblocked, and re-bounced with the specific thing left to do. `blocked` cleared, `bounced` applied -- a PR carrying neither label is invisible to the worker lane AND suppresses its own issue as a candidate, so this is the label that makes the rework reachable. ## The block is genuinely gone `#23` is CLOSED. `!24` merged this sweep and I verified the gate now executes Device-constructing tests instead of aborting: step 3: running 5 tests (was 4) test_util::tests::device_round_trip_fits_under_the_8_mib_test_stack_budget ... ok test result: ok. 5 passed; 0 failed So Desync auto-resync (host + client halves) can finally be gated on its merits. ## What is actually left: a rebase, measured not guessed Re-measured with a real `git merge origin/main` in the warm clone (NOT the `mergeable` flag, which is unreliable in both directions): branch is 13-14 commits behind origin/main git merge origin/main -> CONFLICTS src/netplay.rs `!10` (`issue-6`) is closed as subsumed into this branch (`--is-ancestor` -> TRUE), so this PR is now the single carrier for that work. ## Second thing, in the SAME pass Your Device-constructing test bodies should route through the helper `!24` added, not rely on the gate's env var: use crate::test_util::with_big_stack; #[test] fn my_test() { with_big_stack(|| { // existing body, unchanged }); } `scripts/gate.sh` now exports `RUST_MIN_STACK=8388608`, which immunises the GATE -- but a bare `cargo test --lib` still gives each test thread the 2 MiB libtest default, and a stack overflow SIGABRTs the whole binary rather than failing one test. I verified `with_big_stack` carries a full Device round trip at `RUST_MIN_STACK=2097152`, so wrapping the body makes the test independent of how it is invoked. Do both in one pass -- a rebase now and a wrapper later is two conflict resolutions on the same lines. ## Note on the target `main` is moving during this sweep (`!24` landed; `!15` is in the gate as I write). Rebase onto whatever `origin/main` is when you pick this up and re-measure the conflict set -- do not work from the file list above as if it were fixed.

pbwraith added

bounced

and removed

blocked

labels 2026-08-26 13:27:26 +02:00

pbwraith referenced this pull request 2026-08-26 13:57:18 +02:00

Desync auto-resync — host sends state on desync event #6

pbwraith referenced this pull request 2026-08-26 14:52:37 +02:00

Fix GGRS frame counter reset after state load (#5) #9

pbwraith commented 2026-08-26 14:52:40 +02:00

Author

Owner

Copy Link

Ordering note -- land !9 first, and one conflict hunk is a trap

Not a new bounce; this PR keeps its existing bounced finding. Re-measured against
current main (c857fea) this sweep rather than trusting the last verdict:

CONFLICTS: 3  ->  src/device.rs, src/savestates.rs, src/netplay.rs

Adding what I learned reviewing the cluster as a whole:

1. Order. issue-5 is a strict ancestor of this branch
(git merge-base --is-ancestor origin/issue-5 origin/issue-7 -> TRUE), and all three
open PRs currently conflict on overlapping src/ files. I have bounced !9 with
a full resolution recipe and it should land first; rebasing this branch afterwards
means merging over content it already contains, which is much cheaper than
resolving the same hunks here independently. Wait for !9 unless you are told
otherwise.

2. The src/savestates.rs hunk is not additive. If you hit it, do not
"keep both sides": your side is the savestate-load success path, main's side
is show_savestate_load_failure(device, rewind); from #2's refactor of the
failure path. git aligned two unrelated regions. Keeping both splices the
netplay reset_frame call into the failure branch. Take main's structure and
re-insert the netplay block into the success branch only. Full detail on !9.

3. src/netplay.rs now has a #[cfg(test)] mod tests on main. A second
mod tests in the same file will not compile -- fold into the existing one.

## Ordering note -- land !9 first, and one conflict hunk is a trap Not a new bounce; this PR keeps its existing `bounced` finding. Re-measured against current `main` (`c857fea`) this sweep rather than trusting the last verdict: CONFLICTS: 3 -> src/device.rs, src/savestates.rs, src/netplay.rs Adding what I learned reviewing the cluster as a whole: **1. Order.** `issue-5` is a strict ancestor of this branch (`git merge-base --is-ancestor origin/issue-5 origin/issue-7` -> TRUE), and all three open PRs currently conflict on overlapping `src/` files. I have bounced **!9** with a full resolution recipe and it should land first; rebasing this branch afterwards means merging over content it already contains, which is much cheaper than resolving the same hunks here independently. **Wait for !9 unless you are told otherwise.** **2. The `src/savestates.rs` hunk is not additive.** If you hit it, do not "keep both sides": your side is the savestate-load **success** path, `main`'s side is `show_savestate_load_failure(device, rewind);` from `#2`'s refactor of the **failure** path. git aligned two unrelated regions. Keeping both splices the netplay `reset_frame` call into the failure branch. Take `main`'s structure and re-insert the netplay block into the success branch only. Full detail on !9. **3. `src/netplay.rs` now has a `#[cfg(test)] mod tests` on `main`.** A second `mod tests` in the same file will not compile -- fold into the existing one.

pbwraith referenced this pull request 2026-08-26 19:48:24 +02:00

Desync auto-resync — host sends state on desync event #6

pbwraith added the

manual-only

label 2026-08-26 20:25:29 +02:00

pbwraith referenced this pull request 2026-08-26 23:54:29 +02:00

Role boundary unrecorded: every open item in this repo is `manual-only`, so 3 bounced PRs carry findings no lane can ever act on #26

This repo is archived. You cannot comment on pull requests.

Reviewers

No reviewers

Labels

Clear labels   

blocked

Has a named, verified prerequisite that is not yet met

  

bounced

Reviewed and sent back with a specific finding; makes the PR claimable by the worker lane (priority 0).

  

in-progress

  

in-review

  

manual-only

Worker lanes must not claim this. Rust builds materialise 8-17 GB target/ dirs per worktree and exhaust the review box; the gate runs on the Actions runner instead.

  

needs-guidance

Requires an owner/human decision before further work

  

netplay

  

p1

  

p2

  

p3

  

rust

No Label

blocked

bounced

in-progress

in-review

manual-only

needs-guidance

netplay

p1

p2

p3

rust

Milestone

Clear milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

No Assignees

1 Participants

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: pbwraith/gopher64-netplay#11

No description provided.