From 01df87bdb4081c98ac8573b12e11f52ac92432dd Mon Sep 17 00:00:00 2001 From: Imbris Date: Sat, 3 Jun 2023 15:59:16 -0400 Subject: [PATCH] Address/cleanup a couple TODOs, mainly comment improvements --- client/src/lib.rs | 2 +- common/net/src/sync/sync_ext.rs | 15 +++++++++------ common/src/uid.rs | 17 +++++++++-------- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/client/src/lib.rs b/client/src/lib.rs index a3e7346286..c19d73529b 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -2269,7 +2269,7 @@ impl Client { if self.uid() != Some(entity_uid) { self.state .ecs_mut() - .delete_entity_and_clear_from_id_maps(entity_uid); + .delete_entity_and_clear_uid_mapping(entity_uid); } }, ServerGeneral::Notification(n) => { diff --git a/common/net/src/sync/sync_ext.rs b/common/net/src/sync/sync_ext.rs index 0875b1e1fe..23895fa9f1 100644 --- a/common/net/src/sync/sync_ext.rs +++ b/common/net/src/sync/sync_ext.rs @@ -18,7 +18,7 @@ pub trait WorldSyncExt { where C::Storage: Default + specs::storage::Tracked; fn create_entity_synced(&mut self) -> specs::EntityBuilder; - fn delete_entity_and_clear_from_id_maps(&mut self, uid: Uid); + fn delete_entity_and_clear_uid_mapping(&mut self, uid: Uid); fn uid_from_entity(&self, entity: specs::Entity) -> Option; fn entity_from_uid(&self, uid: Uid) -> Option; fn apply_entity_package( @@ -66,11 +66,14 @@ impl WorldSyncExt for specs::World { /// This method should be used from the client-side when processing network /// messages that delete entities. - // TODO: rename method, document called from client only - fn delete_entity_and_clear_from_id_maps(&mut self, uid: Uid) { + /// + /// Only used on the client. + fn delete_entity_and_clear_uid_mapping(&mut self, uid: Uid) { // Clear from uid allocator - let maybe_entity = - self.write_resource::() + let maybe_entity = self.write_resource::() + // Note, rtsim entity and character id mappings don't exist on the client but it is + // easier to reuse the same structure here since there is shared code that needs to + // lookup the entity from a uid. .remove_entity(None, Some(uid), None, None); if let Some(entity) = maybe_entity { if let Err(e) = self.delete_entity(entity) { @@ -122,7 +125,7 @@ impl WorldSyncExt for specs::World { // there. Although the client might not get this anyway since it // should no longer be subscribed to any regions. if client_uid != Some(uid) { - self.delete_entity_and_clear_from_id_maps(uid); + self.delete_entity_and_clear_uid_mapping(uid); } }); } diff --git a/common/src/uid.rs b/common/src/uid.rs index 0b1185a47e..698b863329 100644 --- a/common/src/uid.rs +++ b/common/src/uid.rs @@ -53,13 +53,14 @@ mod not_wasm { } } + /// Mappings from various Id types to `Entity`s. #[derive(Debug)] pub struct IdMaps { /// "Universal" IDs (used to communicate entity identity over the /// network). uid_mapping: HashMap, - // -- Fields below only used on the server -- + // -- Fields below are only used on the server -- uid_allocator: UidAllocator, /// Character IDs. @@ -91,14 +92,14 @@ mod not_wasm { self.rid_mapping.get(&id).copied() } - // TODO: I think this is suitable to use on both the client and the server. - // NOTE: This is only used on the client? Do we not remove on the server? - // NOTE: We need UID mapping also on the client but we don't need the other - // mappings on the client! - // - // Useful for when a single entity is deleted because it doesn't reconstruct the - // entire hashmap + /// Removes mappings for the provided Id(s). + /// /// Returns the `Entity` that the provided `Uid` was mapped to. + /// + /// Used on both the client and the server when deleting entities, + /// although the client only ever provides a Some value for the + /// `Uid` parameter since the other mappings are not used on the + /// client. pub fn remove_entity( &mut self, expected_entity: Option,