From 3603b95c99f4e720326d4d0ffa60160333d1a7a0 Mon Sep 17 00:00:00 2001 From: cBournhonesque Date: Wed, 8 Jan 2025 12:25:24 -0500 Subject: [PATCH 1/2] avoid log --- lightyear/src/client/input/leafwing.rs | 7 ++++++ .../src/client/prediction/pre_prediction.rs | 1 - lightyear/src/server/prediction.rs | 22 +++++++++---------- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/lightyear/src/client/input/leafwing.rs b/lightyear/src/client/input/leafwing.rs index 2662803e9..d72022bb4 100644 --- a/lightyear/src/client/input/leafwing.rs +++ b/lightyear/src/client/input/leafwing.rs @@ -590,6 +590,13 @@ fn prepare_input_message( // maybe if it's pre-predicted, we send the original entity (pre-predicted), and the server will apply the conversion // on their end? if pre_predicted.is_some() { + // wait until the client receives the PrePredicted entity confirmation to send inputs + // otherwise we get failed entity_map logs + // TODO: the problem is that we wait until we have received the server answer. Ideally we would like + // to wait until the server has received the PrePredicted entity + if predicted.is_none() { + continue; + } trace!( ?tick, "sending inputs for pre-predicted entity! Local client entity: {:?}", diff --git a/lightyear/src/client/prediction/pre_prediction.rs b/lightyear/src/client/prediction/pre_prediction.rs index ba6fc0de8..4cb3157a8 100644 --- a/lightyear/src/client/prediction/pre_prediction.rs +++ b/lightyear/src/client/prediction/pre_prediction.rs @@ -90,7 +90,6 @@ impl PrePredictionPlugin { .as_ref() .unwrap() }; - let confirmed = trigger.entity(); // PrePredicted was replicated from the server: // When we receive an update from the server that confirms a pre-predicted entity, // we will add the Predicted component diff --git a/lightyear/src/server/prediction.rs b/lightyear/src/server/prediction.rs index d375524b3..128475689 100644 --- a/lightyear/src/server/prediction.rs +++ b/lightyear/src/server/prediction.rs @@ -61,6 +61,7 @@ pub(crate) fn compute_hash( pub(crate) fn handle_pre_predicted( trigger: Trigger, mut commands: Commands, + mut manager: ResMut, // add `With` bound for host-server mode; so that we don't trigger this system // for local client entities q: Query<(Entity, &PrePredicted, &Replicated)>, @@ -71,20 +72,17 @@ pub(crate) fn handle_pre_predicted( replicated.from ); let sending_client = replicated.from.unwrap(); + let confirmed_entity = pre_predicted.confirmed_entity.unwrap(); + // update the mapping so that when we send updates, the server entity gets mapped + // to the client's confirmed entity + manager + .connection_mut(sending_client) + .unwrap() + .replication_receiver + .remote_entity_map + .insert(confirmed_entity, local_entity); commands .entity(local_entity) .transfer_authority(AuthorityPeer::Server); - let confirmed_entity = pre_predicted.confirmed_entity.unwrap(); - commands.queue(move |world: &mut World| { - // update the mapping so that when we send updates, the server entity gets mapped - // to the client's confirmed entity - world - .resource_mut::() - .connection_mut(sending_client) - .unwrap() - .replication_receiver - .remote_entity_map - .insert(confirmed_entity, local_entity) - }) } } From 5c98a965a56dd6550570b23df435bf79f1f20084 Mon Sep 17 00:00:00 2001 From: cBournhonesque Date: Wed, 8 Jan 2025 21:09:08 -0500 Subject: [PATCH 2/2] fix pre-prediction rollback --- .../src/client/prediction/pre_prediction.rs | 7 ++ .../client/prediction/predicted_history.rs | 75 ++++++++++--------- lightyear/src/client/prediction/rollback.rs | 6 +- lightyear/src/shared/replication/authority.rs | 8 +- 4 files changed, 56 insertions(+), 40 deletions(-) diff --git a/lightyear/src/client/prediction/pre_prediction.rs b/lightyear/src/client/prediction/pre_prediction.rs index 4cb3157a8..f1aa0355c 100644 --- a/lightyear/src/client/prediction/pre_prediction.rs +++ b/lightyear/src/client/prediction/pre_prediction.rs @@ -152,6 +152,7 @@ impl PrePredictionPlugin { #[cfg(test)] mod tests { use super::*; + use crate::client::prediction::predicted_history::PredictionHistory; use crate::prelude::server; use crate::prelude::server::AuthorityPeer; use crate::prelude::{client, ClientId}; @@ -159,6 +160,7 @@ mod tests { use crate::tests::stepper::{BevyStepper, TEST_CLIENT_ID}; /// Simple preprediction case + /// Also check that the PredictionHistory is correctly added to the PrePredicted entity #[test] fn test_pre_prediction() { // tracing_subscriber::FmtSubscriber::builder() @@ -260,6 +262,11 @@ mod tests { .0, 2.0 ); + assert!(stepper + .client_app + .world() + .get::>(predicted_entity) + .is_some()); } // TODO: test that pre-predicted works in host-server mode diff --git a/lightyear/src/client/prediction/predicted_history.rs b/lightyear/src/client/prediction/predicted_history.rs index 2bb392b54..4df423358 100644 --- a/lightyear/src/client/prediction/predicted_history.rs +++ b/lightyear/src/client/prediction/predicted_history.rs @@ -116,6 +116,9 @@ pub(crate) fn add_non_networked_component_history( component_registry: Res, @@ -130,7 +133,7 @@ pub(crate) fn add_component_history( With, ), >, - confirmed_entities: Query<(Entity, &Confirmed, Option>)>, + confirmed_entities: Query<(Entity, Ref, Option>)>, ) { let kind = std::any::type_name::(); let tick = tick_manager.tick(); @@ -146,48 +149,48 @@ pub(crate) fn add_component_history( &mut commands, ); - // if component got added on confirmed side + // if component got added on confirmed side, or if confirmed itself got added (this is useful to handle cases + // where the confirmed entity already exists and had the component, for example when authority was transferred + // away from the client and the client needs to add prediction) // - full: sync component and add history // - simple/once: sync component if let Some(confirmed_component) = confirmed_component { - // We check this instead of using confirmed_component.is_added() - // in case there were existing components on the confirmed entity - // when the predicted entity was spawned (for example in case of an authority transfer) - if predicted_component.is_some() { - // component already added on predicted side, no need to do anything - continue; - } - trace!(?kind, "Component added on confirmed side"); - // safety: we know the entity exists - let mut predicted_entity_mut = commands.get_entity(predicted_entity).unwrap(); - // map any entities from confirmed to predicted - let mut new_component = confirmed_component.deref().clone(); - let _ = manager.map_entities(&mut new_component, component_registry.as_ref()); - match component_registry.prediction_mode::() { - ComponentSyncMode::Full => { - // insert history, it will be quickly filled by a rollback (since it starts empty before the current client tick) - // or will it? because the component just got spawned anyway.. - // TODO: then there's no need to add the component here, since it's going to get added during rollback anyway? - let mut history = PredictionHistory::::default(); - history.add_update(tick, confirmed_component.deref().clone()); - predicted_entity_mut.insert((new_component, history)); - } - ComponentSyncMode::Simple => { - debug!( - ?kind, - "Component simple synced between confirmed and predicted" - ); - // we only sync the components once, but we don't do rollback so no need for a component history - predicted_entity_mut.insert(new_component); - } - ComponentSyncMode::Once => { - // if this was a prespawned entity, don't override SyncMode::Once components! - if predicted_component.is_none() { + if confirmed_component.is_added() || confirmed.is_added() { + trace!(?kind, "Component added on confirmed side"); + // safety: we know the entity exists + let mut predicted_entity_mut = + commands.get_entity(predicted_entity).unwrap(); + // map any entities from confirmed to predicted + let mut new_component = confirmed_component.deref().clone(); + let _ = + manager.map_entities(&mut new_component, component_registry.as_ref()); + match component_registry.prediction_mode::() { + ComponentSyncMode::Full => { + debug!("Adding history for {:?}", std::any::type_name::()); + // insert history, it will be quickly filled by a rollback (since it starts empty before the current client tick) + // or will it? because the component just got spawned anyway.. + // TODO: then there's no need to add the component here, since it's going to get added during rollback anyway? + let mut history = PredictionHistory::::default(); + history.add_update(tick, confirmed_component.deref().clone()); + predicted_entity_mut.insert((new_component, history)); + } + ComponentSyncMode::Simple => { + debug!( + ?kind, + "Component simple synced between confirmed and predicted" + ); // we only sync the components once, but we don't do rollback so no need for a component history predicted_entity_mut.insert(new_component); } + ComponentSyncMode::Once => { + // if this was a prespawned entity, don't override SyncMode::Once components! + if predicted_component.is_none() { + // we only sync the components once, but we don't do rollback so no need for a component history + predicted_entity_mut.insert(new_component); + } + } + _ => {} } - _ => {} } } } diff --git a/lightyear/src/client/prediction/rollback.rs b/lightyear/src/client/prediction/rollback.rs index dc6704048..154e1ee90 100644 --- a/lightyear/src/client/prediction/rollback.rs +++ b/lightyear/src/client/prediction/rollback.rs @@ -11,7 +11,7 @@ use bevy::prelude::{ use bevy::reflect::Reflect; use bevy::time::{Fixed, Time}; use parking_lot::RwLock; -use tracing::{debug, error, trace, trace_span}; +use tracing::{debug, error, trace, trace_span, warn}; use crate::client::components::{Confirmed, SyncComponent}; use crate::client::config::ClientConfig; @@ -150,7 +150,7 @@ pub(crate) fn check_rollback( continue; }; let Ok(mut predicted_history) = predicted_query.get_mut(p) else { - debug!( + warn!( "Predicted entity {:?} was not found when checking rollback for {:?}", confirmed.predicted, std::any::type_name::() @@ -164,7 +164,7 @@ pub(crate) fn check_rollback( // get the tick that the confirmed entity is at let tick = confirmed.tick; if tick > current_tick { - debug!( + warn!( "Confirmed entity {:?} is at a tick in the future: {:?} compared to client timeline. Current tick: {:?}", confirmed_entity, tick, diff --git a/lightyear/src/shared/replication/authority.rs b/lightyear/src/shared/replication/authority.rs index 5819063a0..575f832cf 100644 --- a/lightyear/src/shared/replication/authority.rs +++ b/lightyear/src/shared/replication/authority.rs @@ -51,6 +51,7 @@ impl MapEntities for AuthorityChange { #[cfg(test)] mod tests { + use crate::client::prediction::predicted_history::PredictionHistory; use crate::prelude::client::{Confirmed, ConfirmedHistory}; use crate::prelude::server::{Replicate, SyncTarget}; use crate::prelude::{client, server, ClientId, NetworkTarget, Replicated}; @@ -1013,9 +1014,14 @@ mod tests { .0, 2.0 ); + assert!(stepper + .client_app_1 + .world() + .get::>(predicted_1) + .is_some()); assert_eq!( stepper - .client_app_2 + .client_app_1 .world() .get::(predicted_1) .unwrap()