From ede05c47b04438c1ee236f2459eb1584574664f4 Mon Sep 17 00:00:00 2001 From: Imbris Date: Tue, 31 Dec 2019 03:10:51 -0500 Subject: [PATCH] fix(char screen induced ghosts): Adds removing extra components and deleting entities clientside when going back to the character screen. Also, simplifies ClientState by removing the Dead variant and removing ClientMsg::StateRequest in favor of more specific ClientMsg variants. --- client/src/lib.rs | 55 +++++++++++++++++--------- common/src/event.rs | 5 ++- common/src/msg/client.rs | 49 +++-------------------- common/src/msg/mod.rs | 1 - common/src/msg/server.rs | 4 +- server/src/client.rs | 12 ++---- server/src/lib.rs | 61 ++++++++++++++++++++++------- server/src/sys/message.rs | 81 +++++++++++++++++---------------------- voxygen/src/session.rs | 4 +- 9 files changed, 135 insertions(+), 137 deletions(-) diff --git a/client/src/lib.rs b/client/src/lib.rs index 932add96d5..0ceed6b47b 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -5,7 +5,11 @@ pub mod error; // Reexports pub use crate::error::Error; -pub use specs::{join::Join, saveload::Marker, Entity as EcsEntity, ReadStorage, WorldExt}; +pub use specs::{ + join::Join, + saveload::{Marker, MarkerAllocator}, + Builder, Entity as EcsEntity, ReadStorage, WorldExt, +}; use common::{ comp::{self, ControlEvent, Controller, ControllerInputs, InventoryManip}, @@ -15,7 +19,7 @@ use common::{ }, net::PostBox, state::State, - sync::{Uid, WorldSyncExt}, + sync::{Uid, UidAllocator, WorldSyncExt}, terrain::{block::Block, TerrainChunk, TerrainChunkSize}, vol::RectVolSize, ChatType, @@ -84,7 +88,7 @@ impl Client { time_of_day, // world_map: /*(map_size, world_map)*/map_size, }) => { - // TODO: Voxygen should display this. + // TODO: Display that versions don't match in Voxygen if server_info.git_hash != common::util::GIT_HASH.to_string() { log::warn!( "Server is running {}[{}], you are running {}[{}], versions might be incompatible!", @@ -183,17 +187,15 @@ impl Client { self.client_state = ClientState::Pending; } - /// Request a state transition to `ClientState::Character`. + /// Send disconnect message to the server pub fn request_logout(&mut self) { - self.postbox - .send_message(ClientMsg::RequestState(ClientState::Connected)); + self.postbox.send_message(ClientMsg::Disconnect); self.client_state = ClientState::Pending; } - /// Request a state transition to `ClientState::Character`. + /// Request a state transition to `ClientState::Registered` from an ingame state. pub fn request_remove_character(&mut self) { - self.postbox - .send_message(ClientMsg::RequestState(ClientState::Registered)); + self.postbox.send_message(ClientMsg::ExitIngame); self.client_state = ClientState::Pending; } @@ -282,9 +284,9 @@ impl Client { } /// Send a chat message to the server. - pub fn send_chat(&mut self, msg: String) { - match validate_chat_msg(&msg) { - Ok(()) => self.postbox.send_message(ClientMsg::chat(msg)), + pub fn send_chat(&mut self, message: String) { + match validate_chat_msg(&message) { + Ok(()) => self.postbox.send_message(ClientMsg::ChatMsg { message }), Err(ChatMsgValidationError::TooLong) => log::warn!( "Attempted to send a message that's too long (Over {} bytes)", MAX_BYTES_CHAT_MSG @@ -331,7 +333,7 @@ impl Client { // 1) Handle input from frontend. // Pass character actions from frontend input to the player's entity. - if let ClientState::Character | ClientState::Dead = self.client_state { + if let ClientState::Character = self.client_state { self.state.write_component( self.entity, Controller { @@ -560,8 +562,8 @@ impl Client { .duration_since(self.last_server_ping) .as_secs_f64(); } - ServerMsg::ChatMsg { chat_type, message } => { - frontend_events.push(Event::Chat { chat_type, message }) + ServerMsg::ChatMsg { message, chat_type } => { + frontend_events.push(Event::Chat { message, chat_type }) } ServerMsg::SetPlayerEntity(uid) => { if let Some(entity) = self.state.ecs().entity_from_uid(uid) { @@ -591,6 +593,26 @@ impl Client { .delete_entity_and_clear_from_uid_allocator(entity); } } + // Cleanup for when the client goes back to the `Registered` state + ServerMsg::ExitIngameCleanup => { + // Get client entity Uid + let client_uid = self + .state + .read_component_cloned::(self.entity) + .map(|u| u.into()) + .expect("Client doesn't have a Uid!!!"); + // Clear ecs of all entities + self.state.ecs_mut().delete_all(); + self.state.ecs_mut().maintain(); + self.state.ecs_mut().insert(UidAllocator::default()); + // Recreate client entity with Uid + let entity_builder = self.state.ecs_mut().create_entity(); + let uid = entity_builder + .world + .write_resource::() + .allocate(entity_builder.entity, Some(client_uid)); + self.entity = entity_builder.with(uid).build(); + } ServerMsg::EntityPos { entity, pos } => { if let Some(entity) = self.state.ecs().entity_from_uid(entity) { self.state.write_component(entity, pos); @@ -641,9 +663,6 @@ impl Client { error, state ); } - ServerMsg::ForceState(state) => { - self.client_state = state; - } ServerMsg::Disconnect => { frontend_events.push(Event::Disconnect); } diff --git a/common/src/event.rs b/common/src/event.rs index ecdc180fe0..6dd8856f75 100644 --- a/common/src/event.rs +++ b/common/src/event.rs @@ -89,12 +89,15 @@ pub enum ServerEvent { Mount(EcsEntity, EcsEntity), Unmount(EcsEntity), Possess(Uid, Uid), - CreatePlayer { + CreateCharacter { entity: EcsEntity, name: String, body: comp::Body, main: Option, }, + ExitIngame { + entity: EcsEntity, + }, CreateNpc { pos: comp::Pos, stats: comp::Stats, diff --git a/common/src/msg/client.rs b/common/src/msg/client.rs index 2b031db3f1..ff2856bc81 100644 --- a/common/src/msg/client.rs +++ b/common/src/msg/client.rs @@ -1,6 +1,4 @@ -use super::ClientState; -use crate::terrain::block::Block; -use crate::{comp, ChatType}; +use crate::{comp, terrain::block::Block}; use vek::*; #[derive(Debug, Clone, Serialize, Deserialize)] @@ -14,16 +12,18 @@ pub enum ClientMsg { body: comp::Body, main: Option, // Specifier for the weapon }, + /// Request `ClientState::Registered` from an ingame state + ExitIngame, + /// Request `ClientState::Specator` from a registered or ingame state + Spectate, ControllerInputs(comp::ControllerInputs), ControlEvent(comp::ControlEvent), - RequestState(ClientState), SetViewDistance(u32), BreakBlock(Vec3), PlaceBlock(Vec3, Block), Ping, Pong, ChatMsg { - chat_type: ChatType, // This is unused afaik, TODO: remove message: String, }, PlayerPhysics { @@ -36,42 +36,3 @@ pub enum ClientMsg { }, Disconnect, } - -impl ClientMsg { - pub fn chat(message: String) -> ClientMsg { - ClientMsg::ChatMsg { - chat_type: ChatType::Chat, - message, - } - } - pub fn tell(message: String) -> ClientMsg { - ClientMsg::ChatMsg { - chat_type: ChatType::Tell, - message, - } - } - pub fn game(message: String) -> ClientMsg { - ClientMsg::ChatMsg { - chat_type: ChatType::GameUpdate, - message, - } - } - pub fn broadcast(message: String) -> ClientMsg { - ClientMsg::ChatMsg { - chat_type: ChatType::Broadcast, - message, - } - } - pub fn private(message: String) -> ClientMsg { - ClientMsg::ChatMsg { - chat_type: ChatType::Private, - message, - } - } - pub fn kill(message: String) -> ClientMsg { - ClientMsg::ChatMsg { - chat_type: ChatType::Private, - message, - } - } -} diff --git a/common/src/msg/mod.rs b/common/src/msg/mod.rs index 13fc0eeb64..f0c19f719d 100644 --- a/common/src/msg/mod.rs +++ b/common/src/msg/mod.rs @@ -13,7 +13,6 @@ pub enum ClientState { Connected, Registered, Spectator, - Dead, Character, } diff --git a/common/src/msg/server.rs b/common/src/msg/server.rs index 710d8a4dfa..f517b7790e 100644 --- a/common/src/msg/server.rs +++ b/common/src/msg/server.rs @@ -41,7 +41,9 @@ pub enum ServerMsg { }, PlayerListUpdate(PlayerListUpdate), StateAnswer(Result), - ForceState(ClientState), + /// Trigger cleanup for when the client goes back to the `Registered` state from an ingame + /// state + ExitIngameCleanup, Ping, Pong, ChatMsg { diff --git a/server/src/client.rs b/server/src/client.rs index 6d15deae2a..333599d82c 100644 --- a/server/src/client.rs +++ b/server/src/client.rs @@ -11,6 +11,7 @@ pub struct Client { pub client_state: ClientState, pub postbox: PostBox, pub last_ping: f64, + pub login_msg_sent: bool, } impl Component for Client { @@ -23,16 +24,13 @@ impl Client { } pub fn is_registered(&self) -> bool { match self.client_state { - ClientState::Registered - | ClientState::Spectator - | ClientState::Dead - | ClientState::Character => true, + ClientState::Registered | ClientState::Spectator | ClientState::Character => true, _ => false, } } pub fn is_ingame(&self) -> bool { match self.client_state { - ClientState::Spectator | ClientState::Character | ClientState::Dead => true, + ClientState::Spectator | ClientState::Character => true, _ => false, } } @@ -45,10 +43,6 @@ impl Client { self.postbox .send_message(ServerMsg::StateAnswer(Err((error, self.client_state)))); } - pub fn force_state(&mut self, new_state: ClientState) { - self.client_state = new_state; - self.postbox.send_message(ServerMsg::ForceState(new_state)); - } } // Distance from fuzzy_chunk before snapping to current chunk diff --git a/server/src/lib.rs b/server/src/lib.rs index 24429bd6c7..a67ced3508 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -28,7 +28,7 @@ use common::{ msg::{ClientMsg, ClientState, PlayerListUpdate, ServerError, ServerInfo, ServerMsg}, net::PostOffice, state::{BlockChange, State, TimeOfDay}, - sync::{Uid, WorldSyncExt}, + sync::{Uid,UidAllocator, WorldSyncExt}, terrain::{block::Block, TerrainChunkSize, TerrainGrid}, vol::{ReadVol, RectVolSize, Vox}, }; @@ -37,7 +37,7 @@ use metrics::ServerMetrics; use rand::Rng; use specs::{ join::Join, world::EntityBuilder as EcsEntityBuilder, Builder, Entity as EcsEntity, RunNow, - SystemData, WorldExt, + SystemData, WorldExt, saveload::MarkerAllocator, }; use std::{ i32, @@ -413,10 +413,8 @@ impl Server { } } - let mut remove = true; - if let Some(client) = state.ecs().write_storage::().get_mut(entity) { - remove = false; + if state.ecs().write_storage::().get_mut(entity).is_some() { state .ecs() .write_storage() @@ -431,10 +429,8 @@ impl Server { .map(|err| { error!("Failed to insert ForceUpdate on dead client: {:?}", err) }); - client.force_state(ClientState::Dead); - } - - if remove { + } else { + // If not a player delete the entity if let Err(err) = state.delete_entity_recorded(entity) { error!("Failed to delete destroyed entity: {:?}", err); } @@ -580,13 +576,12 @@ impl Server { ServerEvent::Respawn(entity) => { // Only clients can respawn - if let Some(client) = state.ecs().write_storage::().get_mut(entity) { + if state.ecs().write_storage::().get_mut(entity).is_some() { let respawn_point = state .read_component_cloned::(entity) .map(|wp| wp.get_pos()) .unwrap_or(state.ecs().read_resource::().0); - client.allow_state(ClientState::Character); state .ecs() .write_storage::() @@ -597,10 +592,13 @@ impl Server { .write_storage::() .get_mut(entity) .map(|pos| pos.0 = respawn_point); - let _ = state + state .ecs() .write_storage() - .insert(entity, comp::ForceUpdate); + .insert(entity, comp::ForceUpdate) + .err().map(|err| + error!("Error inserting ForceUpdate component when respawning client: {:?}", err) + ); } } @@ -730,7 +728,7 @@ impl Server { } } - ServerEvent::CreatePlayer { + ServerEvent::CreateCharacter { entity, name, body, @@ -747,6 +745,40 @@ impl Server { sys::subscription::initialize_region_subscription(state.ecs(), entity); } + ServerEvent::ExitIngame { entity } => { + // Create new entity with just `Clienti`, `Uid`, and `Player` components + // Easier than checking and removing all other known components + // Note: If other `ServerEvent`s are referring to this entity they will be + // disrupted + let maybe_client = + state.ecs().write_storage::().remove(entity); + let maybe_uid = + state.read_component_cloned::(entity); + let maybe_player = + state.ecs().write_storage::().remove(entity); + if let (Some(mut client), Some(uid), Some(player)) = ( + maybe_client, maybe_uid, maybe_player, + ) { + // Tell client its request was successful + client.allow_state(ClientState::Registered); + // Tell client to clear out other entities and its own components + client.notify(ServerMsg::ExitIngameCleanup); + + let entity_builder = + state.ecs_mut().create_entity().with(client).with(player); + // Ensure UidAllocator maps this uid to the new entity + let uid = entity_builder + .world + .write_resource::() + .allocate(entity_builder.entity, Some(uid.into())); + entity_builder.with(uid).build(); + } + // Delete old entity + if let Err(err) = state.delete_entity_recorded(entity) { + error!("Failed to delete entity when removing character: {:?}", err); + } + } + ServerEvent::CreateNpc { pos, stats, @@ -993,6 +1025,7 @@ impl Server { client_state: ClientState::Connected, postbox, last_ping: self.state.get_time(), + login_msg_sent: false, }; if self.server_settings.max_players diff --git a/server/src/sys/message.rs b/server/src/sys/message.rs index f2466c281f..30f063436f 100644 --- a/server/src/sys/message.rs +++ b/server/src/sys/message.rs @@ -1,7 +1,7 @@ use super::SysTimer; use crate::{auth_provider::AuthProvider, client::Client, CLIENT_TIMEOUT}; use common::{ - comp::{Admin, Body, CanBuild, Controller, ForceUpdate, Ori, Player, Pos, Vel}, + comp::{Admin, Body, CanBuild, Controller, ForceUpdate, Ori, Player, Pos, Stats, Vel}, event::{EventBus, ServerEvent}, msg::{ validate_chat_msg, ChatMsgValidationError, ClientMsg, ClientState, PlayerListUpdate, @@ -31,6 +31,7 @@ impl<'a> System<'a> for Sys { ReadStorage<'a, CanBuild>, ReadStorage<'a, Admin>, ReadStorage<'a, ForceUpdate>, + ReadStorage<'a, Stats>, WriteExpect<'a, AuthProvider>, Write<'a, BlockChange>, WriteStorage<'a, Pos>, @@ -54,6 +55,7 @@ impl<'a> System<'a> for Sys { can_build, admins, force_updates, + stats, mut accounts, mut block_changes, mut positions, @@ -98,40 +100,26 @@ impl<'a> System<'a> for Sys { // Process incoming messages. for msg in new_msgs { match msg { - ClientMsg::RequestState(requested_state) => match requested_state { - ClientState::Connected => disconnect = true, // Default state - ClientState::Registered => match client.client_state { - // Use ClientMsg::Register instead. - ClientState::Connected => { - client.error_state(RequestStateError::WrongMessage) - } - ClientState::Registered => { - client.error_state(RequestStateError::Already) - } - ClientState::Spectator | ClientState::Character | ClientState::Dead => { - // TODO: remove position etc here - client.allow_state(ClientState::Registered) - } - ClientState::Pending => {} - }, - ClientState::Spectator => match requested_state { - // Become Registered first. - ClientState::Connected => { - client.error_state(RequestStateError::Impossible) - } - ClientState::Spectator => { - client.error_state(RequestStateError::Already) - } - ClientState::Registered - | ClientState::Character - | ClientState::Dead => client.allow_state(ClientState::Spectator), - ClientState::Pending => {} - }, - // Use ClientMsg::Character instead. - ClientState::Character => { + // Go back to registered state (char selection screen) + ClientMsg::ExitIngame => match client.client_state { + // Use ClientMsg::Register instead. + ClientState::Connected => { client.error_state(RequestStateError::WrongMessage) } - ClientState::Dead => client.error_state(RequestStateError::Impossible), + ClientState::Registered => client.error_state(RequestStateError::Already), + ClientState::Spectator | ClientState::Character => { + server_emitter.emit(ServerEvent::ExitIngame { entity }); + } + ClientState::Pending => {} + }, + // Request spectator state + ClientMsg::Spectate => match client.client_state { + // Become Registered first. + ClientState::Connected => client.error_state(RequestStateError::Impossible), + ClientState::Spectator => client.error_state(RequestStateError::Already), + ClientState::Registered | ClientState::Character => { + client.allow_state(ClientState::Spectator) + } ClientState::Pending => {} }, // Valid player @@ -173,12 +161,12 @@ impl<'a> System<'a> for Sys { ClientMsg::Character { name, body, main } => match client.client_state { // Become Registered first. ClientState::Connected => client.error_state(RequestStateError::Impossible), - ClientState::Registered | ClientState::Spectator | ClientState::Dead => { - if let (Some(player), None) = ( + ClientState::Registered | ClientState::Spectator => { + if let (Some(player), false) = ( players.get(entity), - // Only send login message if the player didn't have a body + // Only send login message if it wasn't already sent // previously - bodies.get(entity), + client.login_msg_sent, ) { new_chat_msgs.push(( None, @@ -187,9 +175,10 @@ impl<'a> System<'a> for Sys { &player.alias )), )); + client.login_msg_sent = true; } - server_emitter.emit(ServerEvent::CreatePlayer { + server_emitter.emit(ServerEvent::CreateCharacter { entity, name, body, @@ -205,7 +194,7 @@ impl<'a> System<'a> for Sys { | ClientState::Spectator => { client.error_state(RequestStateError::Impossible) } - ClientState::Dead | ClientState::Character => { + ClientState::Character => { if let Some(controller) = controllers.get_mut(entity) { controller.inputs = inputs; } @@ -218,21 +207,19 @@ impl<'a> System<'a> for Sys { | ClientState::Spectator => { client.error_state(RequestStateError::Impossible) } - ClientState::Dead | ClientState::Character => { + ClientState::Character => { if let Some(controller) = controllers.get_mut(entity) { controller.events.push(event); } } ClientState::Pending => {} }, - ClientMsg::ChatMsg { chat_type, message } => match client.client_state { + ClientMsg::ChatMsg { message } => match client.client_state { ClientState::Connected => client.error_state(RequestStateError::Impossible), ClientState::Registered | ClientState::Spectator - | ClientState::Dead | ClientState::Character => match validate_chat_msg(&message) { - Ok(()) => new_chat_msgs - .push((Some(entity), ServerMsg::ChatMsg { chat_type, message })), + Ok(()) => new_chat_msgs.push((Some(entity), ServerMsg::chat(message))), Err(ChatMsgValidationError::TooLong) => log::warn!( "Recieved a chat message that's too long (max:{} len:{})", MAX_BYTES_CHAT_MSG, @@ -243,7 +230,9 @@ impl<'a> System<'a> for Sys { }, ClientMsg::PlayerPhysics { pos, vel, ori } => match client.client_state { ClientState::Character => { - if force_updates.get(entity).is_none() { + if force_updates.get(entity).is_none() + && stats.get(entity).map_or(true, |s| !s.is_dead) + { let _ = positions.insert(entity, pos); let _ = velocities.insert(entity, vel); let _ = orientations.insert(entity, ori); @@ -263,7 +252,7 @@ impl<'a> System<'a> for Sys { } } ClientMsg::TerrainChunkRequest { key } => match client.client_state { - ClientState::Connected | ClientState::Registered | ClientState::Dead => { + ClientState::Connected | ClientState::Registered => { client.error_state(RequestStateError::Impossible); } ClientState::Spectator | ClientState::Character => { diff --git a/voxygen/src/session.rs b/voxygen/src/session.rs index 8e7e0275c0..3deff8abd4 100644 --- a/voxygen/src/session.rs +++ b/voxygen/src/session.rs @@ -121,9 +121,7 @@ impl PlayState for SessionState { // Game loop let mut current_client_state = self.client.borrow().get_client_state(); - while let ClientState::Pending | ClientState::Character | ClientState::Dead = - current_client_state - { + while let ClientState::Pending | ClientState::Character = current_client_state { // Compute camera data let (view_mat, _, cam_pos) = self .scene