Repository navigation
feat(netplay): implement GGPO-style rollback netplay (T-82-002) - #57
Conversation
Implements a new rustysnes-netplay crate: two-player rollback netcode, ported from RustyNES's own proven RollbackSession shape (scoped to 2 players -- the SNES core has no multitap emulation, no N-player mesh/NAT-traversal/spectator breadth ported, out of this ticket's scope). Two real bugs found and fixed during implementation, not just reasoned about: - A desync checksum race: computing/sending a checksum from "live" (possibly still-predicted) state instead of only once a frame is fully confirmed produced false-positive desyncs between two peers that were, in fact, converging correctly. - No resend path for dropped Input packets, permanently stalling a session under any nonzero packet loss. tests/determinism.rs proves bit-identical rollback resimulation against a fresh no-rollback reference, under both ideal conditions and real synthetic latency+jitter+10% packet loss. Frontend wiring (native/UDP only -- the browser WebRTC transport is itself complete and wasm32-clippy-verified, but SDP-negotiation UI is a separate, deferred scope): a Tools -> Netplay window, a new `netplay` feature, and NetplayState::drive dispatched from the per-frame render loop via an early continue that skips the single-player path entirely -- its own drive loop, independent of emu-thread. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces GGPO-style 2-player rollback netplay to RustySNES via a new rustysnes-netplay crate, featuring a core rollback loop, native UDP and browser WebRTC transports, and a deterministic MemoryTransport for testing. The frontend has been integrated with a native-only Netplay UI window and a dedicated drive loop that bypasses the single-player path when connected. The review feedback highlights critical security vulnerabilities regarding missing input validation on untrusted network messages (which could lead to OOM panics, permanent session stalls, or memory exhaustion), opportunities for performance optimization (such as avoiding duplicate state serialization and linear history scans), and style guide violations concerning the use of const fn on complex structs containing heap-allocated fields.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Pull request overview
Adds a new rollback netplay subsystem to RustySNES via a dedicated rustysnes-netplay crate (GGPO-style, 2-player), plus native-frontend wiring behind a netplay feature flag, with determinism-focused tests and documentation updates.
Changes:
- Introduces
rustysnes-netplay(protocol/messages, session/rollback logic, deterministic test transport, UDP + wasm WebRTC transports). - Integrates native UDP netplay into the frontend (Tools → Netplay…, connect/disconnect actions, dedicated per-frame drive loop, and a split “present current frame” path).
- Updates project docs/STATUS/CHANGELOG and sprint acceptance criteria to reflect the netplay implementation and validation approach.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| to-dos/phase-8-reach/sprint-2-community.md | Marks acceptance criteria as completed with references to tests/architecture notes. |
| docs/STATUS.md | Updates overall project status to reflect netplay as implemented. |
| docs/frontend.md | Documents frontend netplay behavior and drive-loop ownership boundary. |
| crates/rustysnes-netplay/tests/determinism.rs | Adds determinism proof tests comparing rollback sessions to a reference run. |
| crates/rustysnes-netplay/src/webrtc.rs | Adds wasm32 WebRTC RtcDataChannel transport wrapper. |
| crates/rustysnes-netplay/src/udp.rs | Adds native nonblocking “connected UDP socket” transport + loopback tests. |
| crates/rustysnes-netplay/src/transport.rs | Adds Transport trait and deterministic MemoryTransport for tests. |
| crates/rustysnes-netplay/src/session.rs | Implements RollbackSession core logic (prediction, rollback, resend, checksums). |
| crates/rustysnes-netplay/src/rng.rs | Adds deterministic PRNG for synthetic network conditions in tests. |
| crates/rustysnes-netplay/src/message.rs | Defines/encodes/decodes the wire protocol with bounds-checked parsing. |
| crates/rustysnes-netplay/src/lib.rs | Exposes crate API and target-gated transports. |
| crates/rustysnes-netplay/Cargo.toml | Adds crate metadata and dependencies (core/savestate + wasm deps). |
| crates/rustysnes-frontend/src/ui_shell.rs | Adds Netplay window UI state and connect/disconnect menu actions. |
| crates/rustysnes-frontend/src/netplay.rs | Adds frontend-side session wrapper (NetplayState) and drive routine. |
| crates/rustysnes-frontend/src/lib.rs | Conditionally exports netplay module behind feature/target gates. |
| crates/rustysnes-frontend/src/emu.rs | Splits out present_current_frame to support netplay-driven stepping. |
| crates/rustysnes-frontend/src/app.rs | Integrates netplay drive path into per-frame loop + action dispatch. |
| crates/rustysnes-frontend/Cargo.toml | Adds netplay feature and optional dependency on rustysnes-netplay. |
| CHANGELOG.md | Adds release notes describing the rollback netplay feature and scope. |
| Cargo.lock | Adds the new crate and its dependency closure to the lockfile. |
…(T-82-002) Addresses PR #57 review findings: bound history growth against a hostile/ corrupted peer's frame index, cap the pending-checksum queue, gate all non-Sync messages on a verified handshake, fix a misprediction-detection condition that never actually fired, collapse a duplicate save_state() call, wire the previously-dead input_delay knob, and replace an O(frame) prediction scan with an O(1) read. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
RUSTDOCFLAGS="-D warnings" cargo doc rejects a public item's doc comment linking to a private one (rustdoc::private_intra_doc_links); swap the three new [`Self::...`]/[`ITEM`] links to plain code spans, matching this project's existing convention for links a default doc build can't resolve. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Summary
Implements T-82-002, a new
rustysnes-netplaycrate: two-player rollback netcode, ported from RustyNES's own provenrustynes-netplay::session::RollbackSessionshape (the N-player mesh/Roster/spectator/NAT-traversal breadth RustyNES also carries is deliberately not ported — out of this ticket's stated scope, and the SNES core only has two physical controller ports, no multitap emulation, so 2 players is the core's real ceiling).Two real bugs found and fixed during implementation (caught by the determinism test suite, not just reasoned about):
Inputpacket, which permanently stalled a session under any nonzero packet loss. Fixed with a cumulative-ack-based resend on everyadvance()call.Proof, not assertion:
tests/determinism.rsdrives two sessions over a seeded, deterministicMemoryTransport— one run under ideal (zero-latency) conditions, one under real synthetic latency + jitter + 10% packet loss — and asserts both sessions' per-frame framebuffer hash sequence matches a fresh, no-rollback reference run exactly, under both conditions.Transports:
udp.rs'sUdpTransportis a realstd::net::UdpSocket, proven by a genuine OS-level loopback round-trip test.webrtc.rs'sWebRtcTransportwraps aweb_sys::RtcDataChannel, wasm32-clippy-verified against the real API.Honest scope note: the frontend UI wiring is native/UDP only this pass — the browser-side SDP offer/answer/ICE negotiation glue needed to actually establish a
RtcDataChannelis a genuinely separate scope of async signaling work, not half-wired in.Frontend integration: a new
netplayfeature (native-only) adds a Tools → Netplay… window and aNetplayState.Active::render's per-frame loop dispatches toNetplayState::drivevia an earlycontinuethat skips the entire single-playerapply_frame_input/cheats/rewind/script/run_framepath whenever a session is connected — netplay's own drive loop, verified independent ofemu-thread. A newEmuCore::present_current_framesplitsrun_frame's framebuffer-decode/audio-drain half out, sinceRollbackSession::advancedrives the core crate'sSystemdirectly. Known limitation, shared with rollback netplay generally: a rollback event may audibly glitch (audio already sent to the output device can't be "unplayed"), even though video always reflects the corrected state cleanly.Test plan
cargo test -p rustysnes-netplay(16 tests: unit + the 3 determinism tests, including the adverse-conditions stress test)cargo test --workspace/cargo test -p rustysnes-frontend --features netplay,scripting,cheatscargo clippy --workspace --all-targets -- -D warnings/cargo clippy -p rustysnes-frontend --all-targets --features netplay,scripting,cheats -- -D warningscargo clippy -p rustysnes-netplay --target wasm32-unknown-unknown --lib -- -D warnings(the WebRTC transport)cargo fmt --checkcargo build -p rustysnes-core --target thumbv7em-none-eabihf --no-default-features(no_std gate, unaffected)RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-depsand with--features netplay,scripting,cheatsnetplayoff, full default-feature workspace build/test/clippy/fmt/doc verified unaffected🤖 Generated with Claude Code