diff --git a/client/src/lib.rs b/client/src/lib.rs index 5ccb65ff65..c19d73529b 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -2257,17 +2257,10 @@ impl Client { .apply_entity_sync_package(entity_sync_package, uid); }, ServerGeneral::CompSync(comp_sync_package, force_counter) => { - // TODO: Test with and without sync_me = true commented out to see if results are - // as expected. (client shouldn't normally receive updates if client physics is - // enabled) - let b = self.position(); self.force_update_counter = force_counter; self.state .ecs_mut() .apply_comp_sync_package(comp_sync_package); - if b != self.position() { - println!("{b:?}"); - } }, ServerGeneral::CreateEntity(entity_package) => { self.state.ecs_mut().apply_entity_package(entity_package); diff --git a/common/src/region.rs b/common/src/region.rs index dda545ab3a..d6905eb94f 100644 --- a/common/src/region.rs +++ b/common/src/region.rs @@ -209,6 +209,13 @@ impl RegionMap { } } + /// Must be called immediately after succesfully deleting an entity from the + /// ecs (i.e. when deleting the entity did not generate a WrongGeneration + /// error). + pub fn entity_deleted(&mut self, entity: specs::Entity) { + self.tracked_entities.remove(entity.id()); + } + fn add_entity(&mut self, id: u32, pos: Vec3, from: Option>) { let key = Self::pos_key(pos); if let Some(region) = self.regions.get_mut(&key) { @@ -247,6 +254,8 @@ impl RegionMap { } } } + } else if !self.tracked_entities.contains(id) { + return None; } else { // Check neighbors for o in &NEIGHBOR_OFFSETS { @@ -269,26 +278,9 @@ impl RegionMap { None } - /// Checks if this entity is located in the `RegionMap` using the provided - /// position to limit which regions are checked. - /// - /// May produce false negatives (e.g. if for some reason the entity is not - /// in a region near the provided position). - pub fn in_region_map_relaxed(&self, entity: specs::Entity, pos: Vec3) -> bool { - let id = entity.id(); - let pos = pos.map(|e| e as i32); - - // Compute key for most likely region - let key = Self::pos_key(pos); - if let Some(region) = self.regions.get(&key) && region.entities().contains(id) { return true } - - // Get the base of the four nearest regions. - let quad_base = Self::pos_key(pos - (REGION_SIZE / 2) as i32); - [(0, 0), (0, 1), (1, 0), (1, 1)] - .iter() - .map(|&offset| Vec2::::from(offset) + quad_base) - // skip key we already checked - .any(|k| k != key && self.regions.get(&key).is_some_and(|r| r.entities().contains(id))) + /// Checks if this entity is located in the `RegionMap`. + pub fn in_region_map(&self, entity: specs::Entity) -> bool { + self.tracked_entities.contains(entity.id()) } fn key_index(&self, key: Vec2) -> Option { diff --git a/server/src/events/player.rs b/server/src/events/player.rs index f329c3bada..7f7bdc2536 100644 --- a/server/src/events/player.rs +++ b/server/src/events/player.rs @@ -127,15 +127,16 @@ pub fn handle_exit_ingame(server: &mut Server, entity: EcsEntity, skip_persisten error!("handle_exit_ingame called with entity that is missing expected components"); } - let maybe_character = state + let (maybe_character, sync_me) = state .read_storage::() .get(entity) - .and_then(|p| p.kind.character_id()); + .map(|p| (p.kind.character_id(), p.sync_me)) + .unzip(); let maybe_rtsim = state.read_component_copied::(entity); state.mut_resource::().remove_entity( Some(entity), None, // Uid re-mapped, we don't want to remove the mapping - maybe_character, + maybe_character.flatten(), maybe_rtsim, ); @@ -143,7 +144,9 @@ pub fn handle_exit_ingame(server: &mut Server, entity: EcsEntity, skip_persisten // Uid to a new entity (and e.g. don't want it to be unmapped). // // Delete old entity - if let Err(e) = crate::state_ext::delete_entity_common(state, entity, maybe_uid) { + if let Err(e) = + crate::state_ext::delete_entity_common(state, entity, maybe_uid, sync_me.unwrap_or(true)) + { error!( ?e, ?entity, diff --git a/server/src/state_ext.rs b/server/src/state_ext.rs index 30746a90c3..2e499896d8 100644 --- a/server/src/state_ext.rs +++ b/server/src/state_ext.rs @@ -709,7 +709,7 @@ impl StateExt for State { self.ecs() .write_resource::() .add_character(id, entity); - //presence.sync_me = true; + presence.sync_me = true; Ok(()) } else { Err("PresenceKind is not LoadingCharacter") @@ -1196,20 +1196,21 @@ impl StateExt for State { // NOTE: We expect that these 3 components are never removed from an entity (nor // mutated) (at least not without updating the relevant mappings)! let maybe_uid = self.read_component_copied::(entity); - let maybe_character = self + let (maybe_character, sync_me) = self .read_storage::() .get(entity) - .and_then(|p| p.kind.character_id()); + .map(|p| (p.kind.character_id(), p.sync_me)) + .unzip(); let maybe_rtsim = self.read_component_copied::(entity); self.mut_resource::().remove_entity( Some(entity), maybe_uid, - maybe_character, + maybe_character.flatten(), maybe_rtsim, ); - delete_entity_common(self, entity, maybe_uid) + delete_entity_common(self, entity, maybe_uid, sync_me.unwrap_or(true)) } fn entity_as_actor(&self, entity: EcsEntity) -> Option { @@ -1247,6 +1248,7 @@ pub(crate) fn delete_entity_common( state: &mut State, entity: EcsEntity, maybe_uid: Option, + sync_me: bool, ) -> Result<(), specs::error::WrongGeneration> { if maybe_uid.is_none() { // For now we expect all entities have a Uid component. @@ -1254,8 +1256,24 @@ pub(crate) fn delete_entity_common( } let maybe_pos = state.read_component_copied::(entity); - let res = state.ecs_mut().delete_entity(entity); + // TODO: workaround for https://github.com/amethyst/specs/pull/766 + let actual_gen = state.ecs().entities().entity(entity.id()).gen(); + let res = if actual_gen == entity.gen() { + state.ecs_mut().delete_entity(entity) + } else { + Err(specs::error::WrongGeneration { + action: "delete", + actual_gen, + entity, + }) + }; + if res.is_ok() { + let region_map = state.mut_resource::(); + let uid_pos_region_key = maybe_uid + .zip(maybe_pos) + .map(|(uid, pos)| (uid, pos, region_map.find_region(entity, pos.0))); + region_map.entity_deleted(entity); // Note: Adding the `Uid` to the deleted list when exiting "in-game" relies on // the client not being able to immediately re-enter the game in the // same tick (since we could then mix up the ordering of things and @@ -1263,16 +1281,14 @@ pub(crate) fn delete_entity_common( // // The client will ignore requests to delete its own entity that are triggered // by this. - if let Some((uid, pos)) = maybe_uid.zip(maybe_pos) { - let region_key = state - .ecs() - .read_resource::() - .find_region(entity, pos.0); + if let Some((uid, pos, region_key)) = uid_pos_region_key { if let Some(region_key) = region_key { state .mut_resource::() .record_deleted_entity(uid, region_key); - } else { + // If there is a position and sync_me is true, but the entity is not + // in a region, something might be wrong. + } else if sync_me { // Don't panic if the entity wasn't found in a region, maybe it was just created // and then deleted before the region manager had a chance to assign it a region warn!( diff --git a/server/src/sys/entity_sync.rs b/server/src/sys/entity_sync.rs index 898731efd8..e3e85d6de7 100644 --- a/server/src/sys/entity_sync.rs +++ b/server/src/sys/entity_sync.rs @@ -325,8 +325,7 @@ impl<'a> System<'a> for Sys { // Include additional components for clients that aren't in a region (e.g. due // to having no position or have sync_me as `false`) since those // won't be synced above. - let include_all_comps = - !maybe_pos.is_some_and(|pos| region_map.in_region_map_relaxed(entity, pos.0)); + let include_all_comps = region_map.in_region_map(entity); let mut comp_sync_package = trackers.create_sync_from_client_package( &tracked_storages,