Skip to content

Commit

Permalink
Fix rollback for pre-predicted entities (#788)
Browse files Browse the repository at this point in the history 
* avoid log

* fix pre-prediction rollback
  • Loading branch information
@cBournhonesque
cBournhonesque authored Jan 9, 2025
1 parent 37f0b54 commit eb07354
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 40 deletions.
7 changes: 7 additions & 0 deletions lightyear/src/client/prediction/pre_prediction.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,13 +152,15 @@ 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};
use crate::tests::protocol::{ComponentClientToServer, ComponentSyncModeFull};
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()
Expand Down Expand Up @@ -260,6 +262,11 @@ mod tests {
.0,
2.0
);
assert!(stepper
.client_app
.world()
.get::<PredictionHistory<ComponentSyncModeFull>>(predicted_entity)
.is_some());
}

// TODO: test that pre-predicted works in host-server mode
Expand Down
75 changes: 39 additions & 36 deletions lightyear/src/client/prediction/predicted_history.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,9 @@ pub(crate) fn add_non_networked_component_history<C: Component + PartialEq + Clo

/// Add component history for entities that are predicted
/// There is extra complexity because the component could get added on the Confirmed entity (received from the server), or added to the Predicted entity directly
/// Also:
/// - handle PrePredicted entities (the Predicted entity might already have the component)
/// - handle entity that become Predicted after authority transfer (the Confirmed entity might already have the component)
#[allow(clippy::type_complexity)]
pub(crate) fn add_component_history<C: SyncComponent>(
component_registry: Res<ComponentRegistry>,
Expand All @@ -130,7 +133,7 @@ pub(crate) fn add_component_history<C: SyncComponent>(
With<Predicted>,
),
>,
confirmed_entities: Query<(Entity, &Confirmed, Option<Ref<C>>)>,
confirmed_entities: Query<(Entity, Ref<Confirmed>, Option<Ref<C>>)>,
) {
let kind = std::any::type_name::<C>();
let tick = tick_manager.tick();
Expand All @@ -146,48 +149,48 @@ pub(crate) fn add_component_history<C: SyncComponent>(
&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::<C>() {
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::<C>::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::<C>() {
ComponentSyncMode::Full => {
debug!("Adding history for {:?}", std::any::type_name::<C>());
// 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::<C>::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);
}
}
_ => {}
}
_ => {}
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions lightyear/src/client/prediction/rollback.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -150,7 +150,7 @@ pub(crate) fn check_rollback<C: SyncComponent>(
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::<C>()
Expand All @@ -164,7 +164,7 @@ pub(crate) fn check_rollback<C: SyncComponent>(
// 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,
Expand Down
8 changes: 7 additions & 1 deletion lightyear/src/shared/replication/authority.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -1013,9 +1014,14 @@ mod tests {
.0,
2.0
);
assert!(stepper
.client_app_1
.world()
.get::<PredictionHistory<ComponentSyncModeFull>>(predicted_1)
.is_some());
assert_eq!(
stepper
.client_app_2
.client_app_1
.world()
.get::<ComponentSyncModeFull>(predicted_1)
.unwrap()
Expand Down

0 comments on commit eb07354

Please sign in to comment.