diff --git a/CHANGELOG.md b/CHANGELOG.md index b1bbe39..4cfd8e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,10 +14,11 @@ In this document, all notable changes are listed, including bug fixes, breaking - fix: input delay changes now work when multiple local players share one endpoint; outgoing local inputs are queued by effective frame until every local player has input for that frame - fix: `SessionBuilder::with_num_players()` now revalidates already-registered handles, so changing the player count after adding players cannot leave invalid local, remote, or spectator handles in the builder - fix: `SessionBuilder::start_p2p_session()` now rejects `DesyncDetection::On { interval: 0 }`, which cannot produce a valid checksum reporting cadence -- fix: spectator catch-up now caps the number of frames advanced to the number of confirmed frames available from the host, so `catchup_speed` can safely exceed `max_frames_behind` - fix: spectator catch-up no longer attempts to read past `SPECTATOR_BUFFER_SIZE` frames when the spectator is very far behind; `frames_to_advance` is now also bounded by the buffer capacity +- fix: spectator catch-up now caps the number of frames advanced to the number of confirmed frames available from the host, so `catchup_speed` can safely exceed `max_frames_behind` - fix: `P2PSession` no longer leaks memory in all-local-player sessions; outgoing inputs were queued into an unbounded buffer that was never drained when there were no remote peers - fix: `P2PSession` with multiple local players at different input delays no longer stalls on the first frame; the outgoing queue now scans for the first complete frame rather than assuming the earliest buffered frame has inputs from all players +- fix: desync detection no longer panics with sparse saving when the configured checksum interval frame was not saved exactly (fixes [#107](https://github.com/gschup/ggrs/issues/107)) ### Improvements - feat: `P2PSession::set_input_delay()` allows changing the input delay for a local player mid-session (closes [#106](https://github.com/gschup/ggrs/issues/106)) diff --git a/src/network/protocol.rs b/src/network/protocol.rs index 9adaf25..81b97ea 100644 --- a/src/network/protocol.rs +++ b/src/network/protocol.rs @@ -917,6 +917,34 @@ mod protocol_tests { assert!(protocol.event_queue.is_empty()); } + #[test] + fn first_input_packet_with_nonzero_start_frame_is_accepted() { + // Regression: when the sender has input_delay > 0 its first packet has start_frame > 0. + // The protocol must accept it because the NULL_FRAME sentinel in recv_inputs serves as + // the delta-decode reference for any first packet, regardless of start_frame. + let mut protocol = running_protocol(vec![0], 1); + let reference = protocol + .recv_inputs + .get(&NULL_FRAME) + .expect("blank reference input should exist") + .bytes + .clone(); + let input_bytes = bincode::serialize(&TestInput { inp: 42 }).unwrap(); + let encoded = encode(&reference, [input_bytes].iter()); + let msg = input_message(Input { + peer_connect_status: vec![ConnectionStatus::default(); 1], + disconnect_requested: false, + start_frame: 2, // as if input_delay = 2 + ack_frame: NULL_FRAME, + bytes: encoded, + }); + + protocol.handle_message(&msg); + + assert_eq!(protocol.last_recv_frame(), 2); + assert!(!protocol.event_queue.is_empty()); + } + #[test] fn input_packet_with_wrong_player_byte_shape_is_dropped() { let mut protocol = running_protocol(vec![0, 1], 2); diff --git a/src/sessions/p2p_session.rs b/src/sessions/p2p_session.rs index 67ec1e6..a5ee15f 100644 --- a/src/sessions/p2p_session.rs +++ b/src/sessions/p2p_session.rs @@ -1107,28 +1107,35 @@ impl P2PSession { self.last_sent_checksum_frame + interval as i32 }; - if frame_to_send <= self.sync_layer.last_confirmed_frame() - && frame_to_send <= self.sync_layer.last_saved_frame() - { - let cell = self - .sync_layer - .saved_state_by_frame(frame_to_send) - .unwrap_or_else(|| panic!("cell not found!: frame {frame_to_send}")); + if frame_to_send <= self.sync_layer.last_confirmed_frame() { + let Some(cell) = + self.sync_layer + .saved_state_by_frame(frame_to_send) + .or_else(|| { + self.sync_layer.latest_saved_state_in_range( + frame_to_send, + self.sync_layer.last_confirmed_frame(), + ) + }) + else { + return; + }; if let Some(checksum) = cell.checksum() { + let checksum_frame = cell.frame(); for remote in self.player_reg.remotes.values_mut() { - remote.send_checksum_report(frame_to_send, checksum); + remote.send_checksum_report(checksum_frame, checksum); } - self.last_sent_checksum_frame = frame_to_send; + self.last_sent_checksum_frame = checksum_frame; // collect locally for later comparison - self.local_checksum_history.insert(frame_to_send, checksum); - } + self.local_checksum_history.insert(checksum_frame, checksum); - if self.local_checksum_history.len() > MAX_CHECKSUM_HISTORY_SIZE { - let oldest_frame_to_keep = frame_to_send - - (MAX_CHECKSUM_HISTORY_SIZE as i32 - 1) * interval as i32; - self.local_checksum_history - .retain(|&frame, _| frame >= oldest_frame_to_keep); + if self.local_checksum_history.len() > MAX_CHECKSUM_HISTORY_SIZE { + let oldest_frame_to_keep = checksum_frame + - (MAX_CHECKSUM_HISTORY_SIZE as i32 - 1) * interval as i32; + self.local_checksum_history + .retain(|&frame, _| frame >= oldest_frame_to_keep); + } } } } diff --git a/src/sync_layer.rs b/src/sync_layer.rs index fab3fef..77afd00 100644 --- a/src/sync_layer.rs +++ b/src/sync_layer.rs @@ -363,6 +363,27 @@ impl SyncLayer { } } + /// Returns the latest saved state whose frame falls within the given inclusive range. + pub(crate) fn latest_saved_state_in_range( + &self, + start: Frame, + end: Frame, + ) -> Option> { + if start > end { + return None; + } + + self.saved_states + .states + .iter() + .filter_map(|cell| { + let frame = cell.frame(); + (frame >= start && frame <= end).then_some((frame, cell.clone())) + }) + .max_by_key(|(frame, _)| *frame) + .map(|(_, cell)| cell) + } + /// Returns the latest saved frame pub(crate) fn last_saved_frame(&self) -> Frame { self.last_saved_frame @@ -528,6 +549,29 @@ mod sync_layer_tests { assert!(sync_layer.saved_state_by_frame(0).is_some()); } + #[test] + fn test_latest_saved_state_in_range_returns_latest_matching_frame() { + let mut sync_layer = SyncLayer::::new(1, 8); + for target_frame in [0, 4, 7] { + while sync_layer.current_frame() < target_frame { + sync_layer.advance_frame(); + } + + let req = sync_layer.save_current_state(); + if let GgrsRequest::SaveGameState { cell, frame } = req { + cell.save(frame, Some(frame as u8), None); + } + } + + let cell = sync_layer.latest_saved_state_in_range(1, 6).unwrap(); + assert_eq!(cell.frame(), 4); + + let cell = sync_layer.latest_saved_state_in_range(0, 3).unwrap(); + assert_eq!(cell.frame(), 0); + + assert!(sync_layer.latest_saved_state_in_range(1, 3).is_none()); + } + #[test] fn test_load_frame_rewinds_current_frame() { let mut sync_layer = SyncLayer::::new(1, 8); diff --git a/tests/test_p2p_session.rs b/tests/test_p2p_session.rs index e3292c1..73a04f1 100644 --- a/tests/test_p2p_session.rs +++ b/tests/test_p2p_session.rs @@ -302,6 +302,51 @@ fn test_desyncs_and_input_delay_no_panic() -> Result<(), GgrsError> { Ok(()) } +#[test] +#[serial] +fn test_desync_detection_with_sparse_saving_no_panic() -> Result<(), GgrsError> { + let addr1 = stubs::localhost(7731); + let addr2 = stubs::localhost(7732); + let desync_mode = DesyncDetection::On { interval: 6 }; + + let socket1 = UdpNonBlockingSocket::bind_to_port(7731).unwrap(); + let mut sess1 = SessionBuilder::::new() + .add_player(PlayerType::Local, 0)? + .add_player(PlayerType::Remote(addr2), 1)? + .with_sparse_saving_mode(true) + .with_desync_detection_mode(desync_mode) + .start_p2p_session(socket1)?; + + let socket2 = UdpNonBlockingSocket::bind_to_port(7732).unwrap(); + let mut sess2 = SessionBuilder::::new() + .add_player(PlayerType::Remote(addr1), 0)? + .add_player(PlayerType::Local, 1)? + .with_sparse_saving_mode(true) + .with_desync_detection_mode(desync_mode) + .start_p2p_session(socket2)?; + + stubs::sync_p2p_sessions(&mut sess1, &mut sess2); + + let mut stub1 = stubs::GameStub::new(); + let mut stub2 = stubs::GameStub::new(); + + for i in 0..40 { + sess1.poll_remote_clients(); + sess2.poll_remote_clients(); + + sess1.add_local_input(0, StubInput { inp: i }).unwrap(); + sess2.add_local_input(1, StubInput { inp: i }).unwrap(); + + let requests1 = sess1.advance_frame().unwrap(); + let requests2 = sess2.advance_frame().unwrap(); + + stub1.handle_requests(requests1); + stub2.handle_requests(requests2); + } + + Ok(()) +} + // ── set_input_delay ─────────────────────────────────────────────────────────── #[test]