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

Closed

pbwraith wants to merge 2 commits from issue-6 into main

pull from: issue-6

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-7

pbwraith:issue-5

Conversation 3 Commits 2 Files Changed 39 +10785 -7

pbwraith commented 2026-08-25 07:45:01 +02:00

Owner

Copy Link

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

Problem

When GGRS fires DesyncDetected, the current handler only prints a message and shows an onscreen notification. There is no automatic recovery — the session continues in a desynced state.

Solution

When a desync is detected, the host (player_number==0) snapshots the current emulator state and ships it to the desynced peer(s) via the reliable channel. Both sides then reset their GGRS frame counters (using #5's reset_frame) and resume.

Implementation

1. ResyncState enum added to Netplay struct:

   • Idle — no resync in progress (default)
   • HostWaiting { state_data, state_frame, started_at, acked } — host has sent state, waiting for peer acks
   • ClientLoading { started_at } — non-host received state, loading it

2. DesyncDetected handler (handle_desync):

   • Host: creates in-memory savestate via create_savestate_in_memory, sends resync_state message to all peers, enters HostWaiting state
   • Non-host: ignores the event (state arrives as message)

3. Message handling (check_resync_messages):

   • Non-host: receives resync_state, enters ClientLoading
   • Host: receives resync_ready acks, marks peers as acked; when all acked, calls reset_frame and resumes

4. State loading (process_resync_load): non-host decompresses and loads the received state into the device (same swap pattern as savestates::load_savestate), calls reset_frame, sends resync_ready ack, resumes. Called from vi.rs update_screen before normal frame processing.

5. Frame loop pause (advance_frame): during resync, the frame loop pauses (pushes default AdvanceFrame with Disconnected inputs, like the existing disconnected path). 5-second timeout drops to disconnect behavior.

6. In-memory savestate (create_savestate_in_memory): serializes Device via postcard + compress, without file I/O. Suitable for sending over the reliable channel in 16KB chunks.

Dependencies

• #5 (frame reset): This branch is based on issue-5 and uses reset_frame and the patched ggrs set_current_frame. The PR includes #5's changes.
• #1 (message types): Uses existing NetplayMessage { name, data } struct with string names "resync_state" and "resync_ready". No new message type enum needed.
• #2 (in-memory state): create_savestate_in_memory implements a simplified version that serializes directly from the Device without the static clone buffer.

Tests

7 new tests:

• test_resync_state_default_is_idle — verifies Default trait
• test_resync_state_host_waiting — verifies HostWaiting fields
• test_resync_state_client_loading — verifies ClientLoading fields
• test_resync_message_serialization — resync_state message round-trip
• test_resync_ready_message_serialization — resync_ready message round-trip
• test_resync_timeout — 5s timeout logic for HostWaiting
• test_resync_all_acked — ack tracking logic

Note: Full P2P flow tests require networking and cannot be run in this environment. The tests verify the state machine logic, message serialization, and timeout/ack tracking.

Build note

The full gopher64 project cannot be compiled in this WSL environment due to missing SDL3/X11/Wayland development libraries. The ggrs crate tests (from #5) all pass.

Closes #6

## Desync auto-resync — host sends state on desync event (#6) ### Problem When GGRS fires `DesyncDetected`, the current handler only prints a message and shows an onscreen notification. There is no automatic recovery — the session continues in a desynced state. ### Solution When a desync is detected, the host (player_number==0) snapshots the current emulator state and ships it to the desynced peer(s) via the reliable channel. Both sides then reset their GGRS frame counters (using #5's `reset_frame`) and resume. ### Implementation 1. **ResyncState enum** added to `Netplay` struct: - `Idle` — no resync in progress (default) - `HostWaiting { state_data, state_frame, started_at, acked }` — host has sent state, waiting for peer acks - `ClientLoading { started_at }` — non-host received state, loading it 2. **DesyncDetected handler** (`handle_desync`): - Host: creates in-memory savestate via `create_savestate_in_memory`, sends `resync_state` message to all peers, enters `HostWaiting` state - Non-host: ignores the event (state arrives as message) 3. **Message handling** (`check_resync_messages`): - Non-host: receives `resync_state`, enters `ClientLoading` - Host: receives `resync_ready` acks, marks peers as acked; when all acked, calls `reset_frame` and resumes 4. **State loading** (`process_resync_load`): non-host decompresses and loads the received state into the device (same swap pattern as `savestates::load_savestate`), calls `reset_frame`, sends `resync_ready` ack, resumes. Called from `vi.rs` `update_screen` before normal frame processing. 5. **Frame loop pause** (`advance_frame`): during resync, the frame loop pauses (pushes default AdvanceFrame with Disconnected inputs, like the existing disconnected path). 5-second timeout drops to disconnect behavior. 6. **In-memory savestate** (`create_savestate_in_memory`): serializes Device via postcard + compress, without file I/O. Suitable for sending over the reliable channel in 16KB chunks. ### Dependencies - **#5 (frame reset)**: This branch is based on `issue-5` and uses `reset_frame` and the patched ggrs `set_current_frame`. The PR includes #5's changes. - **#1 (message types)**: Uses existing `NetplayMessage { name, data }` struct with string names "resync_state" and "resync_ready". No new message type enum needed. - **#2 (in-memory state)**: `create_savestate_in_memory` implements a simplified version that serializes directly from the Device without the static clone buffer. ### Tests 7 new tests: - `test_resync_state_default_is_idle` — verifies Default trait - `test_resync_state_host_waiting` — verifies HostWaiting fields - `test_resync_state_client_loading` — verifies ClientLoading fields - `test_resync_message_serialization` — resync_state message round-trip - `test_resync_ready_message_serialization` — resync_ready message round-trip - `test_resync_timeout` — 5s timeout logic for HostWaiting - `test_resync_all_acked` — ack tracking logic Note: Full P2P flow tests require networking and cannot be run in this environment. The tests verify the state machine logic, message serialization, and timeout/ack tracking. ### Build note The full gopher64 project cannot be compiled in this WSL environment due to missing SDL3/X11/Wayland development libraries. The ggrs crate tests (from #5) all pass. Closes #6

pbwraith added 2 commits 2026-08-25 07:45:03 +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

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:48 +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 referenced this pull request 2026-08-26 08:46:25 +02:00

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

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

Author

Owner

Copy Link

Parked on #23 -- and this PR is subsumed by !11

When #23 lands, rework !11 (which already contains this branch's commit
48598f4) and close this one as subsumed rather than repairing both.

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 this PR is subsumed by !11 When #23 lands, rework **!11** (which already contains this branch's commit `48598f4`) and close this one as subsumed rather than repairing both. ### 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:28 +02:00

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:23 +02:00

Author

Owner

Copy Link

Closing as subsumed, now that the thing this was parked on has landed.

#23 is CLOSED -- !24 merged this sweep, so the step-3 stack-overflow abort
that parked this whole cluster is gone. Re-verified the containment claim from
the earlier review rather than trusting the comment:

git merge-base --is-ancestor origin/issue-6 origin/issue-7  ->  TRUE

So !11 already carries every commit on issue-6; your desync auto-resync host half (#6) work is not
being dropped, it ships there. Repairing both branches would mean resolving the
same src/netplay.rs conflict twice and then reconciling two rebases of the
same commits.

The branch issue-6 is untouched -- I have not deleted it, because deleting a
Gitea PR source branch auto-closes the PR and recreating the branch does not
reopen it. If the containment ever needs re-checking, the branch is still there.

Follow the work on !11, which I have bounced with the specific rebase
it needs.

Closing as **subsumed**, now that the thing this was parked on has landed. `#23` is CLOSED -- `!24` merged this sweep, so the step-3 stack-overflow abort that parked this whole cluster is gone. Re-verified the containment claim from the earlier review rather than trusting the comment: git merge-base --is-ancestor origin/issue-6 origin/issue-7 -> TRUE So `!11` already carries every commit on `issue-6`; your desync auto-resync host half (#6) work is not being dropped, it ships there. Repairing both branches would mean resolving the same `src/netplay.rs` conflict twice and then reconciling two rebases of the same commits. **The branch `issue-6` is untouched** -- I have not deleted it, because deleting a Gitea PR source branch auto-closes the PR and recreating the branch does not reopen it. If the containment ever needs re-checking, the branch is still there. Follow the work on `!11`, which I have bounced with the specific rebase it needs.

pbwraith closed this pull request 2026-08-26 13:27:24 +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

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#10

No description provided.