diff --git a/client/src/lib.rs b/client/src/lib.rs index 554af175fc..25411c1615 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -47,7 +47,7 @@ use common::{ TerrainGrid, }, trade::{PendingTrade, SitePrices, TradeAction, TradeId, TradeResult}, - uid::{Uid, IdMaps}, + uid::{IdMaps, Uid}, vol::RectVolSize, weather::{Weather, WeatherGrid}, }; diff --git a/common/net/src/sync/mod.rs b/common/net/src/sync/mod.rs index 35c624ebfc..e6c03b25ab 100644 --- a/common/net/src/sync/mod.rs +++ b/common/net/src/sync/mod.rs @@ -7,7 +7,7 @@ mod sync_ext; mod track; // Reexports -pub use common::uid::{Uid, IdMaps}; +pub use common::uid::{IdMaps, Uid}; pub use net_sync::{NetSync, SyncFrom}; pub use packet::{ handle_insert, handle_interp_insert, handle_interp_modify, handle_interp_remove, handle_modify, diff --git a/common/src/combat.rs b/common/src/combat.rs index f5eed7e4e7..5edcbd8bd4 100644 --- a/common/src/combat.rs +++ b/common/src/combat.rs @@ -19,7 +19,7 @@ use crate::{ outcome::Outcome, resources::Secs, states::utils::StageSection, - uid::{Uid, IdMaps}, + uid::{IdMaps, Uid}, util::Dir, }; diff --git a/common/src/uid.rs b/common/src/uid.rs index 8f3825bc17..f0847e7f56 100644 --- a/common/src/uid.rs +++ b/common/src/uid.rs @@ -147,7 +147,8 @@ mod not_wasm { } /// Only used on the client (server solely uses `Self::allocate` to - /// allocate and add Uid mappings). + /// allocate and add Uid mappings and `Self::remap` to move the `Uid` to + /// a different entity). pub fn add_entity(&mut self, uid: Uid, entity: Entity) { Self::insert(&mut self.uid_mapping, uid, entity); } @@ -171,6 +172,18 @@ mod not_wasm { uid } + /// Links an existing `Uid` to a new entity. + /// + /// Only used on the server. + /// + /// Used for `handle_exit_ingame` which moves the same `Uid` to a new + /// entity. + pub fn remap_entity(&mut self, uid: Uid, new_entity: Entity) { + if self.uid_mapping.insert(uid, new_entity).is_none() { + error!("Uid {uid:?} remaped but there was no existing entry for it!"); + } + } + #[cold] #[inline(never)] fn already_present() { diff --git a/common/state/src/plugin/memory_manager.rs b/common/state/src/plugin/memory_manager.rs index 08ed50b9e8..f8bd0ad254 100644 --- a/common/state/src/plugin/memory_manager.rs +++ b/common/state/src/plugin/memory_manager.rs @@ -8,7 +8,7 @@ use wasmer::{Function, Memory, Value}; use common::{ comp::{Health, Player}, - uid::{Uid, IdMaps}, + uid::{IdMaps, Uid}, }; use super::errors::{MemoryAllocationError, PluginModuleError}; diff --git a/common/state/src/plugin/module.rs b/common/state/src/plugin/module.rs index d588bbadde..a38591313a 100644 --- a/common/state/src/plugin/module.rs +++ b/common/state/src/plugin/module.rs @@ -238,13 +238,12 @@ fn retrieve_action( EcsAccessError::EcsPointerNotAvailable, ))? }; - let player = - world - .id_maps - .uid_entity(e) - .ok_or(RetrieveError::EcsAccessError( - EcsAccessError::EcsEntityNotFound(e), - ))?; + let player = world + .id_maps + .uid_entity(e) + .ok_or(RetrieveError::EcsAccessError( + EcsAccessError::EcsEntityNotFound(e), + ))?; Ok(RetrieveResult::GetPlayerName( world @@ -267,13 +266,12 @@ fn retrieve_action( EcsAccessError::EcsPointerNotAvailable, ))? }; - let player = - world - .id_maps - .uid_entity(e) - .ok_or(RetrieveError::EcsAccessError( - EcsAccessError::EcsEntityNotFound(e), - ))?; + let player = world + .id_maps + .uid_entity(e) + .ok_or(RetrieveError::EcsAccessError( + EcsAccessError::EcsEntityNotFound(e), + ))?; Ok(RetrieveResult::GetEntityHealth( world .health diff --git a/common/systems/src/aura.rs b/common/systems/src/aura.rs index c5f68129fb..201326d5b3 100644 --- a/common/systems/src/aura.rs +++ b/common/systems/src/aura.rs @@ -8,7 +8,7 @@ use common::{ }, event::{Emitter, EventBus, ServerEvent}, resources::Time, - uid::{Uid, IdMaps}, + uid::{IdMaps, Uid}, }; use common_ecs::{Job, Origin, Phase, System}; use specs::{ diff --git a/common/systems/src/beam.rs b/common/systems/src/beam.rs index ccd6b7e36a..0dcc55736a 100644 --- a/common/systems/src/beam.rs +++ b/common/systems/src/beam.rs @@ -9,7 +9,7 @@ use common::{ outcome::Outcome, resources::{DeltaTime, Time}, terrain::TerrainGrid, - uid::{Uid, IdMaps}, + uid::{IdMaps, Uid}, vol::ReadVol, GroupTarget, }; diff --git a/common/systems/src/buff.rs b/common/systems/src/buff.rs index 412739c5e2..ef0f85303e 100644 --- a/common/systems/src/buff.rs +++ b/common/systems/src/buff.rs @@ -15,7 +15,7 @@ use common::{ event::{Emitter, EventBus, ServerEvent}, resources::{DeltaTime, Secs, Time}, terrain::SpriteKind, - uid::{Uid, IdMaps}, + uid::{IdMaps, Uid}, Damage, DamageSource, }; use common_base::prof_span; @@ -275,27 +275,26 @@ impl<'a> System<'a> for Sys { } }) .for_each(|(buff_id, buff, uid, aura_key)| { - let replace = - if let Some(aura_entity) = read_data.id_maps.uid_entity(*uid) { - if let Some(aura) = read_data - .auras - .get(aura_entity) - .and_then(|auras| auras.auras.get(*aura_key)) - { - if let (Some(pos), Some(aura_pos)) = ( - read_data.positions.get(entity), - read_data.positions.get(aura_entity), - ) { - pos.0.distance_squared(aura_pos.0) > aura.radius.powi(2) - } else { - true - } + let replace = if let Some(aura_entity) = read_data.id_maps.uid_entity(*uid) { + if let Some(aura) = read_data + .auras + .get(aura_entity) + .and_then(|auras| auras.auras.get(*aura_key)) + { + if let (Some(pos), Some(aura_pos)) = ( + read_data.positions.get(entity), + read_data.positions.get(aura_entity), + ) { + pos.0.distance_squared(aura_pos.0) > aura.radius.powi(2) } else { true } } else { true - }; + } + } else { + true + }; if replace { expired_buffs.push(*buff_id); server_emitter.emit(ServerEvent::Buff { diff --git a/common/systems/src/melee.rs b/common/systems/src/melee.rs index 4478aa0452..24b4eb6c27 100644 --- a/common/systems/src/melee.rs +++ b/common/systems/src/melee.rs @@ -10,7 +10,7 @@ use common::{ outcome::Outcome, resources::Time, terrain::TerrainGrid, - uid::{Uid, IdMaps}, + uid::{IdMaps, Uid}, util::{find_dist::Cylinder, Dir}, vol::ReadVol, GroupTarget, diff --git a/common/systems/src/projectile.rs b/common/systems/src/projectile.rs index 3eaf9d5b51..4ef507d864 100644 --- a/common/systems/src/projectile.rs +++ b/common/systems/src/projectile.rs @@ -8,7 +8,7 @@ use common::{ event::{Emitter, EventBus, ServerEvent}, outcome::Outcome, resources::{DeltaTime, Time}, - uid::{Uid, IdMaps}, + uid::{IdMaps, Uid}, util::Dir, GroupTarget, }; diff --git a/common/systems/src/shockwave.rs b/common/systems/src/shockwave.rs index a67d733722..f22440bcb2 100644 --- a/common/systems/src/shockwave.rs +++ b/common/systems/src/shockwave.rs @@ -8,7 +8,7 @@ use common::{ event::{EventBus, ServerEvent}, outcome::Outcome, resources::{DeltaTime, Time}, - uid::{Uid, IdMaps}, + uid::{IdMaps, Uid}, util::Dir, GroupTarget, }; diff --git a/server/agent/src/data.rs b/server/agent/src/data.rs index a6f617e08f..254d8bae58 100644 --- a/server/agent/src/data.rs +++ b/server/agent/src/data.rs @@ -21,7 +21,7 @@ use common::{ rtsim::{Actor, RtSimEntity}, states::utils::{ForcedMovement, StageSection}, terrain::TerrainGrid, - uid::{Uid, IdMaps}, + uid::{IdMaps, Uid}, }; use specs::{ shred::ResourceId, Entities, Entity as EcsEntity, Join, Read, ReadExpect, ReadStorage, diff --git a/server/src/cmd.rs b/server/src/cmd.rs index 177e72f32a..f12d0c71df 100644 --- a/server/src/cmd.rs +++ b/server/src/cmd.rs @@ -47,7 +47,7 @@ use common::{ resources::{BattleMode, PlayerPhysicsSettings, Secs, Time, TimeOfDay, TimeScale}, rtsim::{Actor, Role}, terrain::{Block, BlockKind, CoordinateConversions, SpriteKind, TerrainChunkSize}, - uid::{Uid, IdMaps}, + uid::{IdMaps, Uid}, vol::ReadVol, weather, Damage, DamageKind, DamageSource, Explosion, LoadoutBuilder, RadiusEffect, }; diff --git a/server/src/events/entity_manipulation.rs b/server/src/events/entity_manipulation.rs index 7e43495205..61b8759fdb 100644 --- a/server/src/events/entity_manipulation.rs +++ b/server/src/events/entity_manipulation.rs @@ -32,7 +32,7 @@ use common::{ states::utils::StageSection, terrain::{Block, BlockKind, TerrainGrid}, trade::{TradeResult, Trades}, - uid::{Uid, IdMaps}, + uid::{IdMaps, Uid}, util::Dir, vol::ReadVol, Damage, DamageKind, DamageSource, Explosion, GroupTarget, RadiusEffect, @@ -1040,13 +1040,8 @@ pub fn handle_explosion(server: &Server, pos: Vec3, explosion: Explosion, o .and_then(|cs| cs.attack_immunities()) .map_or(false, |i| i.explosions); // PvP check - let may_harm = combat::may_harm( - alignments, - players, - id_maps, - owner_entity, - entity_b, - ); + let may_harm = + combat::may_harm(alignments, players, id_maps, owner_entity, entity_b); let attack_options = combat::AttackOptions { target_dodging, may_harm, diff --git a/server/src/events/group_manip.rs b/server/src/events/group_manip.rs index 5be89d4bea..3dbc1c1fd1 100644 --- a/server/src/events/group_manip.rs +++ b/server/src/events/group_manip.rs @@ -305,7 +305,7 @@ pub fn handle_group(server: &mut Server, entity: Entity, manip: GroupManip) { )); } // Tell the old leader that the transfer was succesful - if let Some(client) = clients.get(target) { + if let Some(client) = clients.get(entity) { client.send_fallible(ServerGeneral::server_msg( ChatType::Meta, "You are no longer the group leader.", diff --git a/server/src/events/player.rs b/server/src/events/player.rs index 9d7745369e..61b5bdd5a6 100644 --- a/server/src/events/player.rs +++ b/server/src/events/player.rs @@ -8,7 +8,7 @@ use common::{ comp, comp::{group, pet::is_tameable, Presence, PresenceKind}, resources::Time, - uid::{Uid, IdMaps}, + uid::{IdMaps, Uid}, }; use common_base::span; use common_net::msg::{PlayerListUpdate, ServerGeneral}; @@ -61,6 +61,10 @@ pub fn handle_exit_ingame(server: &mut Server, entity: EcsEntity, skip_persisten // // Easier than checking and removing all other known components. // + // Also, allows clients to not update their Uid based references to this + // client (e.g. for this specific client's knowledge of its own Uid and for + // groups since exiting in-game does not affect group membership) + // // Note: If other `ServerEvent`s are referring to this entity they will be // disrupted. @@ -89,30 +93,24 @@ pub fn handle_exit_ingame(server: &mut Server, entity: EcsEntity, skip_persisten // Tell client its request was successful client.send_fallible(ServerGeneral::ExitInGameSuccess); - let entity_builder = state.ecs_mut().create_entity().with(client).with(player); - - // Preserve group component if present - let entity_builder = match maybe_group { - Some(group) => entity_builder.with(group), - None => entity_builder, - }; - - // Preserve admin component if present - let entity_builder = match maybe_admin { - Some(admin) => entity_builder.with(admin), - None => entity_builder, - }; + let new_entity = state + .ecs_mut() + .create_entity() + .with(client) + .with(player) + // Preserve group component if present + .maybe_with(maybe_group) + // Preserve admin component if present + .maybe_with(maybe_admin) + .with(uid) + .build(); // Ensure IdMaps maps this uid to the new entity - let uid = entity_builder - .world - .write_resource::() - .allocate(entity_builder.entity, Some(uid.into())); - let new_entity = entity_builder.with(uid).build(); + ecs.write_resource::().remap_entity(uid, new_entity); - // Note, since the Uid has been removed from the old entity, that prevents - // `delete_entity_recorded` from making any changes to the group (TODO double check this - // logic) + // Note, since the Uid has been removed from the old entity and the + // Group component will be removed below, that prevents + // `delete_entity_recorded` from making any changes to the group. if let Some(group) = maybe_group { let mut group_manager = state.ecs().write_resource::(); if group_manager @@ -131,10 +129,10 @@ pub fn handle_exit_ingame(server: &mut Server, entity: EcsEntity, skip_persisten ); } } - } - // Erase group component to avoid group restructure when deleting the entity - state.ecs().write_storage::().remove(entity); + // Erase group component to avoid group restructure when deleting the entity + state.ecs().write_storage::().remove(entity); + } // Delete old entity if let Err(e) = state.delete_entity_recorded(entity) { error!( diff --git a/server/src/state_ext.rs b/server/src/state_ext.rs index 0a7b5dcfd2..eb0ef32019 100644 --- a/server/src/state_ext.rs +++ b/server/src/state_ext.rs @@ -1212,7 +1212,11 @@ impl StateExt for State { if res.is_ok() { if let (Some(uid), Some(pos)) = (maybe_uid, maybe_pos) { // TODO: exit_ingame for player doesn't hit this path since Uid is removed, not - // sure if that is correct. + // sure if that is correct. However, we don't want recording the UID in deleted + // entities to end up overwriting the rejoined client with that UID... + // + // What is the difference between leaving a region and the deleted entities + // list? if let Some(region_key) = self .ecs() .read_resource::() diff --git a/server/src/sys/entity_sync.rs b/server/src/sys/entity_sync.rs index b73b18690f..dd8cbe1f9d 100644 --- a/server/src/sys/entity_sync.rs +++ b/server/src/sys/entity_sync.rs @@ -191,6 +191,8 @@ impl<'a> System<'a> for Sys { .map(|key| !regions.contains(key)) .unwrap_or(true) { + // TODO: I suspect it would be more efficient (in terms of + // bandwidth) to batch messages like this. client.send_fallible(ServerGeneral::DeleteEntity(uid)); } } diff --git a/server/src/sys/msg/register.rs b/server/src/sys/msg/register.rs index 1c695ee551..0ccc723e8b 100644 --- a/server/src/sys/msg/register.rs +++ b/server/src/sys/msg/register.rs @@ -11,7 +11,7 @@ use common::{ recipe::{default_component_recipe_book, default_recipe_book, default_repair_recipe_book}, resources::TimeOfDay, shared_server_config::ServerConstants, - uid::{Uid, IdMaps}, + uid::{IdMaps, Uid}, }; use common_base::prof_span; use common_ecs::{Job, Origin, Phase, System}; @@ -54,7 +54,7 @@ pub struct ReadData<'a> { trackers: TrackedStorages<'a>, _healths: ReadStorage<'a, Health>, // used by plugin feature _plugin_mgr: ReadPlugin<'a>, // used by plugin feature - _id_maps: Read<'a, IdMaps>, // used by plugin feature + _id_maps: Read<'a, IdMaps>, // used by plugin feature } /// This system will handle new messages from clients diff --git a/server/src/sys/pets.rs b/server/src/sys/pets.rs index 9682ca2117..d444b1b73c 100644 --- a/server/src/sys/pets.rs +++ b/server/src/sys/pets.rs @@ -38,16 +38,14 @@ impl<'a> System<'a> for Sys { _ => None, }) .filter_map(|(pet_entity, pet_pos, owner_uid)| { - id_maps - .uid_entity(owner_uid) - .and_then(|owner_entity| { - match (positions.get(owner_entity), physics.get(owner_entity)) { - (Some(position), Some(physics)) => { - Some((pet_entity, position, physics, pet_pos)) - }, - _ => None, - } - }) + id_maps.uid_entity(owner_uid).and_then(|owner_entity| { + match (positions.get(owner_entity), physics.get(owner_entity)) { + (Some(position), Some(physics)) => { + Some((pet_entity, position, physics, pet_pos)) + }, + _ => None, + } + }) }) .filter(|(_, owner_pos, owner_physics, pet_pos)| { // Don't teleport pets to the player if they're in the air, nobody wants diff --git a/voxygen/src/hud/group.rs b/voxygen/src/hud/group.rs index 3fdf13c37c..07ae9e574b 100644 --- a/voxygen/src/hud/group.rs +++ b/voxygen/src/hud/group.rs @@ -18,7 +18,7 @@ use common::{ combat, comp::{group::Role, inventory::item::MaterialStatManifest, invite::InviteKind, Stats}, resources::Time, - uid::{Uid, IdMaps}, + uid::{IdMaps, Uid}, }; use common_net::sync::WorldSyncExt; use conrod_core::{ diff --git a/voxygen/src/scene/particle.rs b/voxygen/src/scene/particle.rs index a1aa46843e..6c643443fb 100644 --- a/voxygen/src/scene/particle.rs +++ b/voxygen/src/scene/particle.rs @@ -287,9 +287,7 @@ impl ParticleMgr { if target.is_some() { let ecs = scene_data.state.ecs(); if target - .and_then(|target| { - ecs.read_resource::().uid_entity(target) - }) + .and_then(|target| ecs.read_resource::().uid_entity(target)) .and_then(|entity| { ecs.read_storage::() .get(entity)