From 1bb409f085cbb68f2dbd0fa9ff24d92dbda9f397 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 11:50:33 -0500 Subject: [PATCH] netplay: resume both sides at the saved GGRS frame (issue #5 reset_frame) --- src/netplay.rs | 128 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 105 insertions(+), 23 deletions(-) diff --git a/src/netplay.rs b/src/netplay.rs index 08be0f2..b2e9b75 100644 --- a/src/netplay.rs +++ b/src/netplay.rs @@ -500,19 +500,19 @@ fn wait_for_ready_all( /// RNG/RTC/savestate handshake and before the GGRS frame loop starts: /// /// - host (player 0): reads the `.netplay-state` file, validates its ROM -/// hash, broadcasts the raw session-save bytes to every peer via -/// `netplay_state` and waits for each peer's `netplay_state_ready` ack -/// (30 s timeout, then the session is disconnected); +/// hash, loads the embedded savestate into its own device, broadcasts the +/// session-save bytes to every peer via `netplay_state` and waits for each +/// peer's `netplay_state_ready` ack (30 s timeout, then the lagging peers +/// are disconnected and the session is abandoned); /// - peers: receive `netplay_state`, decode it with the same /// `decode_netplay_session` the host used to read the file, load the /// embedded savestate via `SavestateData::load_from_memory` + /// `savestates::load_savestate_from_data`, then acknowledge with /// `netplay_state_ready`. /// -/// The GGRS session frame counter is intentionally NOT reset here: that is -/// issue #5's `netplay::reset_frame`. Both sides record the saved frame in -/// `netplay.resume_frame` so the frame-reset hook has the value ready; the -/// PR body documents the gap until #5 lands. +/// Both sides then reset the GGRS session frame counter to the saved frame +/// with issue #5's `netplay::reset_frame`, so the frame loop resumes at the +/// saved frame instead of 0. /// /// On any failure the session is flagged disconnected (mirroring the /// existing peer-disconnect path) so the frame loop never starts desynced. @@ -531,6 +531,24 @@ pub fn resume_session(device: &mut device::Device) -> Result<(), String> { } } +/// Load a decoded `.netplay-state` envelope into the local device (its +/// savestate payload via `load_savestate_from_data`) and reset the GGRS +/// session frame counter to the saved frame (issue #5's `reset_frame`). +fn apply_resume_state(device: &mut device::Device, save: &NetplaySessionSave) -> Result<(), String> { + let state_data = savestates::SavestateData::load_from_memory(&save.state) + .ok_or_else(|| "Session state could not be deserialized".to_string())?; + + if !savestates::load_savestate_from_data(device, &state_data) { + return Err("Session state could not be applied".to_string()); + } + + if let Some(netplay) = device.netplay.as_mut() { + netplay.resume_frame = Some(save.frame); + reset_frame(&mut netplay.session, save.frame); + } + Ok(()) +} + fn resume_session_host(device: &mut device::Device) -> Result<(), String> { ui::video::onscreen_message( "Syncing session state…", @@ -561,13 +579,17 @@ fn resume_session_host(device: &mut device::Device) -> Result<(), String> { )); } + // The host resumes at the saved state too: load the savestate into the + // local device and reset the frame counter before the handshake, exactly + // as every peer will. + apply_resume_state(device, &save)?; + // Broadcast the file bytes unmodified: `encode_netplay_session` produces // exactly the on-disk format, so every peer decodes what the host read. let broadcast = encode_netplay_session(&save) .ok_or_else(|| "Could not encode session state for broadcast".to_string())?; let netplay = device.netplay.as_mut().unwrap(); - netplay.resume_frame = Some(save.frame); send_netplay_state(netplay, &broadcast); if !wait_for_ready_all( @@ -613,15 +635,9 @@ fn resume_session_peer(device: &mut device::Device) -> Result<(), String> { let save = decode_netplay_session(&state_bytes) .ok_or_else(|| "Could not decode session state received from host".to_string())?; - let state_data = savestates::SavestateData::load_from_memory(&save.state) - .ok_or_else(|| "Session state received from host could not be deserialized".to_string())?; - - if !savestates::load_savestate_from_data(device, &state_data) { - return Err("Session state received from host could not be applied".to_string()); - } + apply_resume_state(device, &save)?; let netplay = device.netplay.as_mut().unwrap(); - netplay.resume_frame = Some(save.frame); send_ready(netplay, NETPLAY_STATE_READY_MESSAGE); ui::video::onscreen_message( @@ -1218,7 +1234,9 @@ mod tests { /// Mock 2-peer resume flow (issue #4): the host's `.netplay-state` /// envelope travels over the `netplay_state` reliable message, the peer /// decodes it and loads the embedded savestate, and both sides agree on - /// the frame metadata. + /// the frame metadata. The peer's GGRS session frame counter is reset to + /// the saved frame (issue #5's `reset_frame`) so the frame loop resumes + /// there, not at 0. #[test] fn resume_state_reaches_peer_with_same_frame_metadata() { use crate::savestates::{create_savestate_in_memory, load_savestate_from_data}; @@ -1265,15 +1283,12 @@ mod tests { assert_eq!(peer_save.rom_hash, "DEADBEEF-C01DF00D"); assert_eq!(peer_save.state, save.state, "state payload travels unmodified"); - // Peer loads the embedded savestate into a fresh device (no disk). + // Peer loads the embedded savestate into a fresh device (no disk), + // exactly like resume_session_peer: load_from_memory -> + // load_savestate_from_data, then issue #5's frame reset. let mut peer_device = crate::device::Device::new(false); peer_device.netplay = Some(mock_netplay(1)); - let state_data = crate::savestates::SavestateData::load_from_memory(&peer_save.state) - .expect("peer deserializes the savestate"); - assert!( - load_savestate_from_data(&mut peer_device, &state_data), - "peer applies the state" - ); + apply_resume_state(&mut peer_device, &peer_save).expect("peer applies the state"); assert_eq!(peer_device.cpu.gpr[1], 0x1234_5678_9abc_def0); assert_eq!(peer_device.cpu.gpr[3], 0x0fed_cba9_8765_4321); @@ -1284,6 +1299,73 @@ mod tests { peer_device.netplay.is_some(), "the live netplay session survives the state load" ); + + // Both sides agree on the frame metadata: the resumed frame is the + // saved frame. + let peer_netplay = peer_device.netplay.as_ref().unwrap(); + assert_eq!(peer_netplay.resume_frame, Some(12_345)); + assert_eq!( + peer_netplay.session.current_frame(), + 12_345, + "GGRS frame counter resumes at the saved frame, not 0" + ); + } + + /// Host resume path (issue #4): a `.netplay-state` file on disk is + /// decoded by `resume_session_host`, its savestate loaded into the + /// host's own device, and the GGRS frame counter reset to the saved + /// frame before broadcast. + #[test] + fn resume_host_loads_state_from_disk_and_resets_frame() { + use crate::device::Device; + + let mut host_device = Device::new(false); + host_device.cpu.gpr[5] = 0xDEAD_BEEF; + host_device.rdram.mem.resize(0x800, 0); + host_device.rdram.mem[0x100] = 0x42; + host_device.netplay = Some(mock_netplay(0)); + host_device.ui.game_hash = "DEADBEEF-C01DF00D".to_string(); + + let snapshot = + create_savestate_in_memory(&mut host_device).expect("create in-memory savestate"); + let save = NetplaySessionSave { + state: snapshot.save_to_memory(), + frame: 77_007, + input_delay: 2, + rom_hash: "DEADBEEF-C01DF00D".to_string(), + players: 2, + }; + + static COUNTER: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0); + let path = std::env::temp_dir().join(format!( + "gopher64-netplay-resume-test-{}-{}.netplay-state", + std::process::id(), + COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed) + )); + write_netplay_session(&path, &save).expect("write session save"); + + // Fresh device stands in for the host's just-launched session; the + // savestate was captured from a different Device instance above. + let mut host_session = Device::new(false); + host_session.ui.game_hash = "DEADBEEF-C01DF00D".to_string(); + host_session.netplay = Some(mock_netplay(0)); + host_session.netplay.as_mut().unwrap().state_path = Some(path.clone()); + + let frame_before = host_session.netplay.as_ref().unwrap().session.current_frame(); + resume_session_host(&mut host_session).expect("host resume succeeds"); + + assert_eq!( + host_session.netplay.as_ref().unwrap().resume_frame, + Some(77_007) + ); + assert_eq!( + host_session.netplay.as_ref().unwrap().session.current_frame(), + 77_007, + "host frame counter resumes at the saved frame (was {frame_before})" + ); + assert_eq!(host_session.cpu.gpr[5], 0xDEAD_BEEF); + assert_eq!(host_session.rdram.mem[0x100], 0x42); + let _ = std::fs::remove_file(&path); } /// The host's 30s ack wait succeeds once every peer acknowledged and