From 0ef42e38545959226ef9288b77c31069a3b1e3d9 Mon Sep 17 00:00:00 2001 From: Avi Weinstock Date: Fri, 19 Mar 2021 19:53:17 -0400 Subject: [PATCH] Address MR 1945 review comments. --- client/src/lib.rs | 63 ++++++++++++++++------------------ voxygen/src/ecs/mod.rs | 5 +-- voxygen/src/ecs/sys/floater.rs | 23 ++++++------- voxygen/src/session.rs | 12 ------- 4 files changed, 40 insertions(+), 63 deletions(-) diff --git a/client/src/lib.rs b/client/src/lib.rs index 3f47bf8a1f..f64463fa01 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -175,7 +175,6 @@ pub struct Client { tick: u64, state: State, - entity: EcsEntity, view_distance: Option, // TODO: move into voxygen @@ -248,7 +247,6 @@ impl Client { let mut ping_interval = tokio::time::interval(core::time::Duration::from_secs(1)); let ( state, - entity, lod_base, lod_alt, lod_horizon, @@ -425,7 +423,6 @@ impl Client { Ok(( state, - entity, lod_base, lod_alt, Grid::from_raw(map_size.map(|e| e as i32), lod_horizon), @@ -493,7 +490,6 @@ impl Client { tick: 0, state, - entity, view_distance, loaded_distance: 0.0, @@ -775,7 +771,7 @@ impl Client { pub fn can_craft_recipe(&self, recipe: &str) -> bool { self.recipe_book .get(recipe) - .zip(self.inventories().get(self.entity)) + .zip(self.inventories().get(self.entity())) .map(|(recipe, inv)| inv.contains_ingredients(&*recipe).is_ok()) .unwrap_or(false) } @@ -883,7 +879,7 @@ impl Client { self.state .ecs() .read_storage::() - .get(self.entity) + .get(self.entity()) .is_some() } @@ -891,7 +887,7 @@ impl Client { self.state .ecs() .read_storage::() - .get(self.entity) + .get(self.entity()) .is_some() } @@ -908,7 +904,7 @@ impl Client { .state .ecs() .read_storage::() - .get(self.entity) + .get(self.entity()) .map_or(false, |h| h.is_dead) { self.send_msg(ClientGeneral::ControlEvent(ControlEvent::Respawn)); @@ -924,7 +920,7 @@ impl Client { .state .ecs() .read_storage::() - .get(self.entity) + .get(self.entity()) .map(|cs| cs.is_wield()); match is_wielding { @@ -939,7 +935,7 @@ impl Client { .state .ecs() .read_storage::() - .get(self.entity) + .get(self.entity()) .map(|cs| matches!(cs, comp::CharacterState::Sit)); match is_sitting { @@ -954,7 +950,7 @@ impl Client { .state .ecs() .read_storage::() - .get(self.entity) + .get(self.entity()) .map(|cs| matches!(cs, comp::CharacterState::Dance)); match is_dancing { @@ -969,7 +965,7 @@ impl Client { .state .ecs() .read_storage::() - .get(self.entity) + .get(self.entity()) .map(|cs| matches!(cs, comp::CharacterState::Sneak)); match is_sneaking { @@ -984,7 +980,7 @@ impl Client { .state .ecs() .read_storage::() - .get(self.entity) + .get(self.entity()) .map(|cs| { matches!( cs, @@ -1015,7 +1011,7 @@ impl Client { .state .ecs() .write_storage::() - .get_mut(self.entity) + .get_mut(self.entity()) { controller.actions.push(control_action); } @@ -1030,7 +1026,7 @@ impl Client { let chunk_pos = Vec2::from( self.state .read_storage::() - .get(self.entity) + .get(self.entity()) .cloned()? .0, ) @@ -1045,7 +1041,7 @@ impl Client { where C: Clone, { - Some(self.state.read_storage::().get(self.entity).cloned()?) + Some(self.state.read_storage::().get(self.entity()).cloned()?) } pub fn current_biome(&self) -> BiomeKind { @@ -1147,7 +1143,7 @@ impl Client { .state .ecs() .write_storage::() - .entry(self.entity) + .entry(self.entity()) .map(|entry| { entry .or_insert_with(|| Controller { @@ -1159,7 +1155,7 @@ impl Client { .inputs = inputs.clone(); }) { - let entry = self.entity; + let entry = self.entity(); error!( ?e, ?entry, @@ -1217,7 +1213,7 @@ impl Client { let pos = self .state .read_storage::() - .get(self.entity) + .get(self.entity()) .cloned(); if let (Some(pos), Some(view_distance)) = (pos, self.view_distance) { let chunk_pos = self.state.terrain().pos_key(pos.0.map(|e| e as i32)); @@ -1316,9 +1312,9 @@ impl Client { // 6) Update the server about the player's physics attributes. if self.presence.is_some() { if let (Some(pos), Some(vel), Some(ori)) = ( - self.state.read_storage().get(self.entity).cloned(), - self.state.read_storage().get(self.entity).cloned(), - self.state.read_storage().get(self.entity).cloned(), + self.state.read_storage().get(self.entity()).cloned(), + self.state.read_storage().get(self.entity()).cloned(), + self.state.read_storage().get(self.entity()).cloned(), ) { self.in_game_stream .send(ClientGeneral::PlayerPhysics { pos, vel, ori })?; @@ -1459,7 +1455,6 @@ impl Client { }, ServerGeneral::SetPlayerEntity(uid) => { if let Some(entity) = self.state.ecs().entity_from_uid(uid.0) { - self.entity = entity; *self.state.ecs_mut().write_resource() = PlayerEntity(Some(entity)); } else { return Err(Error::Other("Failed to find entity from uid.".to_owned())); @@ -1614,7 +1609,7 @@ impl Client { InventoryUpdateEvent::CollectFailed => {}, _ => { // Push the updated inventory component to the client - self.state.write_component(self.entity, inventory); + self.state.write_component(self.entity(), inventory); }, } @@ -1634,7 +1629,7 @@ impl Client { .ecs() .read_resource::>() .emit_now(LocalEvent::ApplyImpulse { - entity: self.entity, + entity: self.entity(), impulse, }); }, @@ -1809,14 +1804,14 @@ impl Client { } pub fn entity(&self) -> EcsEntity { - debug_assert_eq!( - self.state.ecs().read_resource::().0, - Some(self.entity) - ); - self.entity + self.state + .ecs() + .read_resource::() + .0 + .expect("Client::entity should always have PlayerEntity be Some") } - pub fn uid(&self) -> Option { self.state.read_component_copied(self.entity) } + pub fn uid(&self) -> Option { self.state.read_component_copied(self.entity()) } pub fn presence(&self) -> Option { self.presence } @@ -1869,7 +1864,7 @@ impl Client { pub fn is_admin(&self) -> bool { let client_uid = self .state - .read_component_copied::(self.entity) + .read_component_copied::(self.entity()) .expect("Client doesn't have a Uid!!!"); self.player_list @@ -1896,8 +1891,8 @@ impl Client { .write_resource::() .allocate(entity_builder.entity, Some(client_uid)); - self.entity = entity_builder.with(uid).build(); - self.state.ecs().write_resource::().0 = Some(self.entity); + let entity = entity_builder.with(uid).build(); + self.state.ecs().write_resource::().0 = Some(entity); } /// Change player alias to "You" if client belongs to matching player diff --git a/voxygen/src/ecs/mod.rs b/voxygen/src/ecs/mod.rs index 0f432ebf9e..701dbb334b 100644 --- a/voxygen/src/ecs/mod.rs +++ b/voxygen/src/ecs/mod.rs @@ -3,10 +3,7 @@ pub mod sys; use crate::audio::sfx::SfxEventItem; use common::{event::EventBus, slowjob::SlowJobPool}; -use specs::{Entity, World, WorldExt}; - -#[derive(Copy, Clone, Debug)] -pub struct MyEntity(pub Entity); +use specs::{World, WorldExt}; pub fn init(world: &mut World) { world.register::(); diff --git a/voxygen/src/ecs/sys/floater.rs b/voxygen/src/ecs/sys/floater.rs index 8929d0a839..9f5da5f6f8 100644 --- a/voxygen/src/ecs/sys/floater.rs +++ b/voxygen/src/ecs/sys/floater.rs @@ -1,14 +1,11 @@ -use crate::ecs::{ - comp::{HpFloater, HpFloaterList}, - MyEntity, -}; +use crate::ecs::comp::{HpFloater, HpFloaterList}; use common::{ comp::{Health, HealthSource, Pos}, - resources::DeltaTime, + resources::{DeltaTime, PlayerEntity}, uid::Uid, }; use common_ecs::{Job, Origin, Phase, System}; -use specs::{Entities, Join, Read, ReadExpect, ReadStorage, WriteStorage}; +use specs::{Entities, Join, Read, ReadStorage, WriteStorage}; // How long floaters last (in seconds) pub const HP_SHOWTIME: f32 = 3.0; @@ -21,7 +18,7 @@ impl<'a> System<'a> for Sys { #[allow(clippy::type_complexity)] type SystemData = ( Entities<'a>, - ReadExpect<'a, MyEntity>, + Read<'a, PlayerEntity>, Read<'a, DeltaTime>, ReadStorage<'a, Uid>, ReadStorage<'a, Pos>, @@ -53,7 +50,7 @@ impl<'a> System<'a> for Sys { } // Add hp floaters to all entities that have been damaged - let my_uid = uids.get(my_entity.0); + let my_uid = my_entity.0.and_then(|entity| uids.get(entity)); for (entity, health, hp_floater_list) in (&entities, &healths, &mut hp_floater_lists).join() { // Increment timer for time since last damaged by me @@ -82,11 +79,11 @@ impl<'a> System<'a> for Sys { if by_me { hp_floater_list.time_since_last_dmg_by_me = Some(0.0); } - my_entity.0 == entity || by_me + my_entity.0 == Some(entity) || by_me }, - HealthSource::Suicide => my_entity.0 == entity, - HealthSource::World => my_entity.0 == entity, - HealthSource::LevelUp => my_entity.0 == entity, + HealthSource::Suicide => my_entity.0 == Some(entity), + HealthSource::World => my_entity.0 == Some(entity), + HealthSource::LevelUp => my_entity.0 == Some(entity), HealthSource::Command => true, HealthSource::Item => true, _ => false, @@ -142,7 +139,7 @@ impl<'a> System<'a> for Sys { // Clear floaters if newest floater is past show time or health runs out if floaters.last().map_or(false, |f| { f.timer - > if entity != my_entity.0 { + > if Some(entity) != my_entity.0 { HP_SHOWTIME } else { MY_HP_SHOWTIME diff --git a/voxygen/src/session.rs b/voxygen/src/session.rs index b5d1425b42..5ad5a7cf6a 100644 --- a/voxygen/src/session.rs +++ b/voxygen/src/session.rs @@ -32,7 +32,6 @@ use common_net::{ use crate::{ audio::sfx::SfxEvent, controller::ControllerSettings, - ecs::MyEntity, hud::{DebugInfo, Event as HudEvent, Hud, HudInfo, PressBehavior, PromptDialogSettings}, i18n::{i18n_asset_key, Localization}, key_state::KeyState, @@ -266,17 +265,6 @@ impl PlayState for SessionState { (client.presence(), client.registered()) }; if client_presence.is_some() { - // Update MyEntity - // Note: Alternatively, the client could emit an event when the entity changes - // which may or may not be more elegant - { - let my_entity = self.client.borrow().entity(); - self.client - .borrow_mut() - .state_mut() - .ecs_mut() - .insert(MyEntity(my_entity)); - } // Compute camera data self.scene .camera_mut()