pbwraith/gopher64-netplay
Archived
1
0
Fork 0
Code Issues Pull Requests 3 Actions Packages Projects Releases Wiki Activity

GGRS frame counter reset after state load #5

New Issue
Closed
opened 2026-08-25 06:03:46 +02:00 by pbwraith · 2 comments
pbwraith commented 2026-08-25 06:03:46 +02:00
Owner
Copy Link

Repo: gopher64-netplay (fork of https://github.com/gopher64/gopher64)
Context: Full scope doc at upstream analysis — see commit history. Netplay uses GGRS over WebRTC (matchbox_socket), reliable channel carries NetplayMessage { name, data } in 16KB chunks. Savestates serialize full Device via postcard + compress.

Goal: After a netplay state load, both clients' GGRS sessions must agree on the current frame, else desync detection fires immediately.

Background:

  • GGRS P2PSession tracks internal frame counters; loading a savestate replaces emulator state but NOT the GGRS session frame
  • The resume flow (issue #4) and resync flow (issue #6/7) both depend on this
  • src/netplay.rs init() builds the session with SessionBuilder — check if GGRS exposes current-frame mutation, or whether a new session must be built with an offset

Acceptance:

  • Investigate ggrs::P2PSession (check the Cargo.lock version and docs.rs for ggrs used by this fork) for frame reset; if no public API:
    • Fork/patch the ggrs crate (vendored or [patch] in Cargo.toml) to expose set_current_frame(i32) or equivalent — keep the change minimal and upstream-compatible
  • Add netplay::reset_frame(session, frame) — safe no-op if unsupported, documented
  • After load_savestate_from_data in a netplay context, call reset so session.current_frame() == state frame
  • Desync detection must NOT fire within 60 frames of a legit state load
  • Tests: create session, advance 120 frames, save state metadata, reset to frame 60, verify current_frame()==60 and no immediate desync event when resuming with matching inputs

Verification: cargo test. Note in the PR body whether ggrs had to be patched and how.

**Repo:** gopher64-netplay (fork of https://github.com/gopher64/gopher64) **Context:** Full scope doc at upstream analysis — see commit history. Netplay uses GGRS over WebRTC (matchbox_socket), reliable channel carries `NetplayMessage { name, data }` in 16KB chunks. Savestates serialize full Device via postcard + compress. **Goal:** After a netplay state load, both clients' GGRS sessions must agree on the current frame, else desync detection fires immediately. **Background:** - GGRS `P2PSession` tracks internal frame counters; loading a savestate replaces emulator state but NOT the GGRS session frame - The resume flow (issue #4) and resync flow (issue #6/7) both depend on this - `src/netplay.rs` `init()` builds the session with `SessionBuilder` — check if GGRS exposes current-frame mutation, or whether a new session must be built with an offset **Acceptance:** - Investigate `ggrs::P2PSession` (check the Cargo.lock version and docs.rs for ggrs used by this fork) for frame reset; if no public API: - Fork/patch the ggrs crate (vendored or `[patch]` in Cargo.toml) to expose `set_current_frame(i32)` or equivalent — keep the change minimal and upstream-compatible - Add `netplay::reset_frame(session, frame)` — safe no-op if unsupported, documented - After `load_savestate_from_data` in a netplay context, call reset so `session.current_frame() == state frame` - Desync detection must NOT fire within 60 frames of a legit state load - Tests: create session, advance 120 frames, save state metadata, reset to frame 60, verify `current_frame()==60` and no immediate desync event when resuming with matching inputs **Verification:** `cargo test`. Note in the PR body whether ggrs had to be patched and how.
pbwraith added the
netplay
p1
rust
 labels 2026-08-25 06:04:34 +02:00
pbwraith added the
in-progress
 label 2026-08-25 07:16:47 +02:00
pbwraith added
in-review
 and removed
in-progress
 labels 2026-08-25 07:38:13 +02:00
pbwraith commented 2026-08-25 07:39:01 +02:00
Author
Owner
Copy Link

/tmp/comment_5.md

/tmp/comment_5.md
pbwraith commented 2026-08-26 14:11:37 +02:00
Author
Owner
Copy Link

[lane5] orphaned_review invariant repaired — PR reopened, kept in-review

Orphaned-review repair for pbwraith/gopher64-netplay#5 ("GGRS frame counter reset
after state load"):

  • Claimed with orphaned_review=true: issue-5 carried in-review but had no
    reviewable open PR. Its own PR (!9) was closed/unmerged, marked "subsumed by
    !18".
  • Verified locally from the remote (bare clone):
    • origin/issue-5 still exists @ eb5cf56ee3
    • git merge-base --is-ancestor origin/issue-5 origin/main -> FALSE (work not
      landed; non-empty diff vs main: 38 files, +10392/-2)
    • git merge-base --is-ancestor origin/issue-5 origin/issue-4 -> TRUE (contained
      in !18's head)
  • However !18 (issue-4, PR #18) is OPEN but BOUNCED, not merged — the subsumption
    never actually completed, so issue-5's work currently has NO delivered review
    channel. Per the orphaned-review invariant: restored head has a non-empty diff,
    so the SAME closed/unmerged PR (!9) is reopened (no duplicate opened).
  • Reopened PR !9: state=open, merged=false, head=issue-5@eb5cf56, base=main
    (c857fea), changed_files=38, additions=10392, deletions=2. Labels: none (the
    stale blocked tied to closed #23 is gone).
  • Issue-5 stays in-review; a reviewable open PR now exists for it. Do NOT close
    or merge this manually — leave for the manual review lane; if !18's bounce is
    fixed first, coordinate the merge order (this branch is a strict subset of !18).
[lane5] orphaned_review invariant repaired — PR reopened, kept in-review Orphaned-review repair for pbwraith/gopher64-netplay#5 ("GGRS frame counter reset after state load"): - Claimed with orphaned_review=true: issue-5 carried `in-review` but had no reviewable open PR. Its own PR (!9) was closed/unmerged, marked "subsumed by !18". - Verified locally from the remote (bare clone): - `origin/issue-5` still exists @ eb5cf56ee3b8daca58fce25234f454c4586aea7d - `git merge-base --is-ancestor origin/issue-5 origin/main` -> FALSE (work not landed; non-empty diff vs main: 38 files, +10392/-2) - `git merge-base --is-ancestor origin/issue-5 origin/issue-4` -> TRUE (contained in !18's head) - However !18 (issue-4, PR #18) is OPEN but BOUNCED, not merged — the subsumption never actually completed, so issue-5's work currently has NO delivered review channel. Per the orphaned-review invariant: restored head has a non-empty diff, so the SAME closed/unmerged PR (!9) is reopened (no duplicate opened). - Reopened PR !9: state=open, merged=false, head=issue-5@eb5cf56, base=main (c857fea), changed_files=38, additions=10392, deletions=2. Labels: none (the stale `blocked` tied to closed #23 is gone). - Issue-5 stays `in-review`; a reviewable open PR now exists for it. Do NOT close or merge this manually — leave for the manual review lane; if !18's bounce is fixed first, coordinate the merge order (this branch is a strict subset of !18).
pbwraith added the
manual-only
 label 2026-08-26 20:25:36 +02:00
This repo is archived. You cannot comment on issues.
No Branch/Tag Specified
Branches Tags
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#5
No description provided.
Delete Branch "%!s()"

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?