Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
28 changes: 28 additions & 0 deletions src/network/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
39 changes: 23 additions & 16 deletions src/sessions/p2p_session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1107,28 +1107,35 @@ impl<T: Config> P2PSession<T> {
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);
}
}
}
}
Expand Down
44 changes: 44 additions & 0 deletions src/sync_layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,27 @@ impl<T: Config> SyncLayer<T> {
}
}

/// 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<GameStateCell<T::State>> {
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
Expand Down Expand Up @@ -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::<TestConfig>::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::<TestConfig>::new(1, 8);
Expand Down
45 changes: 45 additions & 0 deletions tests/test_p2p_session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<StubConfig>::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::<StubConfig>::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]
Expand Down
Loading