diff --git a/common/net/src/sync/sync_ext.rs b/common/net/src/sync/sync_ext.rs index 54d5992da1..aefeffd194 100644 --- a/common/net/src/sync/sync_ext.rs +++ b/common/net/src/sync/sync_ext.rs @@ -53,16 +53,21 @@ impl WorldSyncExt for specs::World { fn create_entity_synced(&mut self) -> specs::EntityBuilder { let builder = self.create_entity(); + // TODO: if we split the UidAllocator and the IdMaps into two things then we + // need to fetch 2 resources when creating entities. let uid = builder .world .write_resource::() - .allocate(builder.entity, None); + .allocate(builder.entity); builder.with(uid) } + /// This method should be used from the client-side when processing network + /// messages that delete entities. + // TODO: rename method fn delete_entity_and_clear_from_uid_allocator(&mut self, uid: Uid) { // Clear from uid allocator - let maybe_entity = self.write_resource::().remove_entity(uid); + let maybe_entity = self.write_resource::().remove_entity_(uid); if let Some(entity) = maybe_entity { if let Err(e) = self.delete_entity(entity) { error!(?e, "Failed to delete entity"); diff --git a/common/src/comp/presence.rs b/common/src/comp/presence.rs index b63d4a6165..e8e4adfbb0 100644 --- a/common/src/comp/presence.rs +++ b/common/src/comp/presence.rs @@ -32,6 +32,9 @@ impl Component for Presence { pub enum PresenceKind { Spectator, Character(CharacterId), + // Note: we don't know if this character ID is valid and associated with the respective player + // until it the character has loaded successfully. + LoadingCharacter(CharacterId), Possessor, } diff --git a/common/src/uid.rs b/common/src/uid.rs index e09a95afc8..0324e357ca 100644 --- a/common/src/uid.rs +++ b/common/src/uid.rs @@ -1,10 +1,16 @@ -#[cfg(not(target_arch = "wasm32"))] -use hashbrown::HashMap; +// TODO: rename this module or create new one for ID maps use serde::{Deserialize, Serialize}; -#[cfg(not(target_arch = "wasm32"))] -use specs::{Component, Entity, FlaggedStorage, VecStorage}; use std::{fmt, u64}; +#[cfg(not(target_arch = "wasm32"))] +use { + crate::character::CharacterId, + crate::rtsim::RtSimEntity, + hashbrown::HashMap, + specs::{Component, Entity, FlaggedStorage, VecStorage}, +}; + +// TODO: could we switch this to `NonZeroU64`? #[derive(Copy, Clone, Debug, Eq, Hash, PartialEq, Serialize, Deserialize)] pub struct Uid(pub u64); @@ -20,45 +26,175 @@ impl fmt::Display for Uid { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{}", self.0) } } +pub use not_wasm::*; #[cfg(not(target_arch = "wasm32"))] -impl Component for Uid { - type Storage = FlaggedStorage>; -} +mod not_wasm { + use super::*; -#[cfg(not(target_arch = "wasm32"))] -#[derive(Debug)] -pub struct UidAllocator { - next_id: u64, - mapping: HashMap, -} + impl Component for Uid { + type Storage = FlaggedStorage>; + } -#[cfg(not(target_arch = "wasm32"))] -impl UidAllocator { - pub fn new() -> Self { - Self { - next_id: 0, - mapping: HashMap::new(), + // NOTE: This is technically only needed by the server code server. Keeping here + // for now since this is related to the other code here. + #[derive(Debug)] + pub struct UidAllocator { + /// Next Uid. + next_uid: u64, + } + + impl UidAllocator { + pub fn new() -> Self { Self { next_uid: 0 } } + + pub fn allocate(&mut self, entity: Entity, id: Option) -> Uid { + let id = id.unwrap_or_else(|| { + let id = self.next_uid; + self.next_uid += 1; + Uid(id) + }); + self.uid_mapping.insert(id, entity); + id } } - // Useful for when a single entity is deleted because it doesn't reconstruct the - // entire hashmap - pub fn remove_entity(&mut self, id: Uid) -> Option { self.mapping.remove(&id) } + #[derive(Debug)] + pub struct IdMaps { + /// "Universal" IDs (used to communicate entity identity over the + /// network). + uid_mapping: HashMap, - pub fn allocate(&mut self, entity: Entity, id: Option) -> Uid { - let id = id.unwrap_or_else(|| { - let id = self.next_id; - self.next_id += 1; - Uid(id) - }); - self.mapping.insert(id, entity); - id + // Maps below are only used on the server. + /// Character IDs. + cid_mapping: HashMap, + /// Rtsim Entities. + rid_mapping: HashMap, } - pub fn lookup_entity(&self, id: Uid) -> Option { self.mapping.get(&id).copied() } -} + impl IdManager { + pub fn new() -> Self { + Self { + uid_mapping: HashMap::new(), + cid_mapping: HashMap::new(), + rid_mapping: HashMap::new(), + } + } -#[cfg(not(target_arch = "wasm32"))] -impl Default for UidAllocator { - fn default() -> Self { Self::new() } + /// Given a `Uid` retrieve the corresponding `Entity` + pub fn uid_entity(&self, id: Uid) -> Option { self.uid_mapping.get(&id).copied() } + + /// Given a `CharacterId` retrieve the corresponding `Entity` + pub fn cid_entity(&self, id: CharacterId) -> Option { + self.uid_mapping.get(&id).copied() + } + + /// Given a `Uid` retrieve the corresponding `Entity` + pub fn rid_entity(&self, id: RtSimEntity) -> Option { + self.uid_mapping.get(&id).copied() + } + + // 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 + pub fn remove_entity( + &mut self, + expected_entity: Option, + uid: Uid, + cid: Option, + rid: Option, + ) -> Option { + #[cold] + #[inline(never)] + fn unexpected_entity() { + error!("{kind} mapped to an unexpected entity!"); + } + #[cold] + #[inline(never)] + fn not_present() { + error!("{kind} not mapped to any entity!"); + } + + fn remove( + mapping: &mut HashMap, + id: Option, + expected: Option, + ) -> Option { + if let Some(id) = id { + if let Some(e) = mapping.remove(id) { + if Some(expected) = expected && e != expected { + unexpected_entity::(); + } + Some(e) + } else { + not_present::(); + None + } + } else { + None + } + } + + let maybe_entity = remove(&mut self.uid_mapping, Some(uid), expected_entity); + let expected_entity = expected_entity.or(maybe_entity); + remove(&mut self.cid_mapping, cid, expected_entity); + remove(&mut self.rid_mapping, rid, expected_entity); + maybe_entity + } + + // TODO: we probably need separate methods here + // TODO: document what methods are called only on the client or the server + pub fn add_entity( + &mut self, + entity: Entity, + uid: Uid, + cid: Option, + rid: Option, + ) { + #[cold] + #[inline(never)] + fn already_present() { + let kind = core::any::type_name::(); + error!("{kind} already mapped to an entity!!!"); + } + + fn insert( + mapping: &mut HashMap, + new_id: ID, + entity: Entity, + ) { + if let Some(_previous_entity) = mapping.insert(new_id, entity) { + already_present::(); + } + } + + insert(&mut self.uid_mapping, uid, entity); + if let Some(cid) = cid { + insert(&mut self.cid_mapping, cid, entity); + } + if let Some(rid) = rid { + insert(&mut self.rid_mapping, rid, entity); + } + } + + pub fn allocate(&mut self, entity: Entity) -> Uid { + let id = id.unwrap_or_else(|| { + let id = self.next_uid; + self.next_uid += 1; + Uid(id) + }); + // TODO: not sure we want to insert here? + self.uid_mapping.insert(id, entity); + id + } + } + + impl Default for UidAllocator { + fn default() -> Self { Self::new() } + } + + impl Default for IdMaps { + fn default() -> Self { Self::new() } + } } diff --git a/server/src/events/entity_creation.rs b/server/src/events/entity_creation.rs index 151d7b37c8..1d66598dfd 100644 --- a/server/src/events/entity_creation.rs +++ b/server/src/events/entity_creation.rs @@ -83,12 +83,15 @@ pub fn handle_loaded_character_data( ))), ); } - server - .state - .update_character_data(entity, loaded_components); - sys::subscription::initialize_region_subscription(server.state.ecs(), entity); - // We notify the client with the metadata result from the operation. - server.notify_client(entity, ServerGeneral::CharacterDataLoadResult(Ok(metadata))); + let result_msg = if let Err(err) = server.state.update_character_data(entity, loaded_components) { + handle_exit_igname(entity, false); // remove client from in-game state + ServerGeneral::CharacterDataLoadResult(Err(err)) + } else { + sys::subscription::initialize_region_subscription(server.state.ecs(), entity); + // We notify the client with the metadata result from the operation. + ServerGeneral::CharacterDataLoadResult(Ok(metadata)) + }; + server.notify_client(entity, result_msg)); } pub fn handle_create_npc(server: &mut Server, pos: Pos, mut npc: NpcBuilder) -> EcsEntity { @@ -132,6 +135,7 @@ pub fn handle_create_npc(server: &mut Server, pos: Pos, mut npc: NpcBuilder) -> entity }; + // TODO: rtsim entity added here let entity = if let Some(rtsim_entity) = npc.rtsim_entity { entity.with(rtsim_entity).with(RepositionOnChunkLoad { needs_ground: false, diff --git a/server/src/events/player.rs b/server/src/events/player.rs index 12445347f4..a22e518547 100644 --- a/server/src/events/player.rs +++ b/server/src/events/player.rs @@ -439,7 +439,7 @@ pub fn handle_possess(server: &mut Server, possessor_uid: Uid, possessee_uid: Ui PresenceKind::Spectator => PresenceKind::Spectator, // This prevents persistence from overwriting original character info with stuff // from the new character. - PresenceKind::Character(_) => { + PresenceKind::Character(_) | PresenceKind::LoadingCharacter(_) => { delete_entity = Some(possessor); PresenceKind::Possessor }, diff --git a/server/src/lib.rs b/server/src/lib.rs index 2a6819fd18..714ef2320a 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -954,6 +954,8 @@ impl Server { active_abilities, map_marker, ); + // TODO: Does this need to be a server event? E.g. we could + // just handle it here. ServerEvent::UpdateCharacterData { entity: response.target_entity, components: character_data, @@ -962,9 +964,8 @@ impl Server { }, Err(error) => { // We failed to load data for the character from the DB. Notify - // the client to push the - // state back to character selection, with the error - // to display + // the client to push the state back to character selection, + // with the error to display self.notify_client( response.target_entity, ServerGeneral::CharacterDataLoadResult(Err( diff --git a/server/src/state_ext.rs b/server/src/state_ext.rs index 0ebccc739c..b404694355 100644 --- a/server/src/state_ext.rs +++ b/server/src/state_ext.rs @@ -640,7 +640,7 @@ impl StateExt for State { self.write_component_ignore_entity_dead( entity, - Presence::new(view_distances, PresenceKind::Character(character_id)), + Presence::new(view_distances, PresenceKind::LoadingCharacter(character_id)), ); // Tell the client its request was successful. @@ -675,7 +675,12 @@ impl StateExt for State { } } - fn update_character_data(&mut self, entity: EcsEntity, components: PersistedComponents) { + /// Returned error intended to be sent to the client. + fn update_character_data( + &mut self, + entity: EcsEntity, + components: PersistedComponents, + ) -> Result<(), String> { let PersistedComponents { body, stats, @@ -687,7 +692,26 @@ impl StateExt for State { map_marker, } = components; + // TODO: CharacterId established here (update id map) + if let Some(player_uid) = self.read_component_copied::(entity) { + let result = + if let Some(presence) = self.ecs().write_storage::().get_mut(entity) { + if let PresenceKind::LoadingCharacter(id) = presence.kind { + *presence.kind = PresenceKind::Character(id); + Ok(()) + } else { + Err("PresenceKind is not LoadingCharacter") + } + } else { + Err("Presence component missing") + }; + if let Err(err) = result { + let err = format!("Unexpected state when applying loaded character info: {err}"); + error!("{err}"); + return Err(err); + } + // Notify clients of a player list update self.notify_players(ServerGeneral::PlayerListUpdate( PlayerListUpdate::SelectedCharacter(player_uid, CharacterInfo { @@ -768,7 +792,7 @@ impl StateExt for State { restore_pet(self.ecs(), pet_entity, entity, pet); } } else { - warn!("Player has no pos, cannot load {} pets", pets.len()); + error!("Player has no pos, cannot load {} pets", pets.len()); } let presences = self.ecs().read_storage::(); @@ -801,6 +825,8 @@ impl StateExt for State { } } } + + Ok(()) } fn validate_chat_msg( @@ -1145,11 +1171,26 @@ impl StateExt for State { // Cancel extant trades events::cancel_trades_for(self, entity); - let (maybe_uid, maybe_pos) = ( + let (maybe_uid, maybe_presence, maybe_rtsim_entity, maybe_pos) = ( self.ecs().read_storage::().get(entity).copied(), + self.ecs() + .read_storage::() + .get(entity) + .map(|p| p.kind), + self.ecs() + .read_storage::() + .get(entity) + .copied(), self.ecs().read_storage::().get(entity).copied(), ); + if let Some(uid) = a { + if self.ecs().write_resource::() + .remove_entity().is_none() + } else { + warn!("Deleting entity without Uid component"); + } + let res = self.ecs_mut().delete_entity(entity); if res.is_ok() { if let (Some(uid), Some(pos)) = (maybe_uid, maybe_pos) {