From 255ad9728908aae6ef4c1c57c26df113e5fa8cf5 Mon Sep 17 00:00:00 2001 From: Shane Handley Date: Tue, 16 Jun 2020 11:00:32 +1000 Subject: [PATCH] - Move the remaining character data loading to the message/thread formatharacter list actions - Handle/notify the client of errors during character load by returning to character select with the error, clean up client and server states - Add player_uuid check when loading character data. --- client/src/lib.rs | 50 +++++--- common/src/character.rs | 23 ++-- common/src/event.rs | 8 +- common/src/msg/client.rs | 5 +- common/src/msg/server.rs | 2 + server/src/events/entity_creation.rs | 19 +-- server/src/events/mod.rs | 11 +- server/src/lib.rs | 67 +++++++--- server/src/persistence/character.rs | 163 +++++++++++++++++-------- server/src/persistence/error.rs | 20 ++- server/src/state_ext.rs | 115 +++++++---------- server/src/sys/message.rs | 66 ++++++---- voxygen/src/menu/char_selection/mod.rs | 4 +- 13 files changed, 335 insertions(+), 218 deletions(-) diff --git a/client/src/lib.rs b/client/src/lib.rs index bfd16b278c..523f7fe7ba 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -242,9 +242,10 @@ impl Client { } /// Request a state transition to `ClientState::Character`. - pub fn request_character(&mut self, character_id: i32, body: comp::Body) { + pub fn request_character(&mut self, character_id: i32) { self.postbox - .send_message(ClientMsg::Character { character_id, body }); + .send_message(ClientMsg::Character(character_id)); + self.active_character_id = Some(character_id); self.client_state = ClientState::Pending; } @@ -861,23 +862,7 @@ impl Client { }, // 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(); + self.clean_state(); }, ServerMsg::InventoryUpdate(inventory, event) => { match event { @@ -934,6 +919,10 @@ impl Client { ServerMsg::Notification(n) => { frontend_events.push(Event::Notification(n)); }, + ServerMsg::CharacterDataLoadError(error) => { + self.clean_state(); + self.character_list.error = Some(error); + }, } } } else if let Some(err) = self.postbox.error() { @@ -978,6 +967,29 @@ impl Client { .cloned() .collect() } + + /// Clean client ECS state + fn clean_state(&mut self) { + 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(); + } } impl Drop for Client { diff --git a/common/src/character.rs b/common/src/character.rs index 03b0f8193b..6056b90017 100644 --- a/common/src/character.rs +++ b/common/src/character.rs @@ -1,3 +1,5 @@ +//! Structs representing a playable Character + use crate::comp; use serde_derive::{Deserialize, Serialize}; @@ -13,28 +15,23 @@ pub const MAX_CHARACTERS_PER_PLAYER: usize = 8; // Once we are happy that all characters have a loadout, or we manually // update/delete those that don't, it's no longer necessary and we can // remove this from here, as well as in the DB schema and persistence code. -#[derive(Clone, Debug, Serialize, Deserialize)] + +/// The minimum character data we need to create a new character on the server. +/// The `tool` field was historically used to persist the character's weapon +/// before Loadouts were persisted, and will be removed in the future. +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct Character { pub id: Option, pub alias: String, pub tool: Option, } -/// Represents a single character item in the character list presented during -/// character selection. This is a subset of the full character data used for -/// presentation purposes. -#[derive(Clone, Debug, Serialize, Deserialize)] +/// Data needed to render a single character item in the character list +/// presented during character selection. +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct CharacterItem { pub character: Character, pub body: comp::Body, pub level: usize, pub loadout: comp::Loadout, } - -/// The full representation of the data we store in the database for each -/// character -#[derive(Clone, Debug, Serialize, Deserialize)] -pub struct CharacterData { - pub body: comp::Body, - pub stats: comp::Stats, -} diff --git a/common/src/event.rs b/common/src/event.rs index ea4e9c4820..f848ae4c7e 100644 --- a/common/src/event.rs +++ b/common/src/event.rs @@ -93,10 +93,14 @@ pub enum ServerEvent { Unmount(EcsEntity), Possess(Uid, Uid), LevelUp(EcsEntity, u32), - SelectCharacter { + /// Inserts default components for a character when loading into the game + InitCharacterData { entity: EcsEntity, character_id: i32, - body: comp::Body, + }, + UpdateCharacterData { + entity: EcsEntity, + components: (comp::Body, comp::Stats, comp::Inventory, comp::Loadout), }, ExitIngame { entity: EcsEntity, diff --git a/common/src/msg/client.rs b/common/src/msg/client.rs index 550cf5de60..0ff1e38c91 100644 --- a/common/src/msg/client.rs +++ b/common/src/msg/client.rs @@ -14,10 +14,7 @@ pub enum ClientMsg { body: comp::Body, }, DeleteCharacter(i32), - Character { - character_id: i32, - body: comp::Body, - }, + Character(i32), /// Request `ClientState::Registered` from an ingame state ExitIngame, /// Request `ClientState::Spectator` from a registered or ingame state diff --git a/common/src/msg/server.rs b/common/src/msg/server.rs index 42501f5d7f..89a9d3a554 100644 --- a/common/src/msg/server.rs +++ b/common/src/msg/server.rs @@ -53,6 +53,8 @@ pub enum ServerMsg { time_of_day: state::TimeOfDay, world_map: (Vec2, Vec), }, + /// An error occurred while loading character data + CharacterDataLoadError(String), /// A list of characters belonging to the a authenticated player was sent CharacterListUpdate(Vec), /// An error occured while creating or deleting a character diff --git a/server/src/events/entity_creation.rs b/server/src/events/entity_creation.rs index 32e8a081a5..b1b628098d 100644 --- a/server/src/events/entity_creation.rs +++ b/server/src/events/entity_creation.rs @@ -9,16 +9,21 @@ use common::{ use specs::{Builder, Entity as EcsEntity, WorldExt}; use vek::{Rgb, Vec3}; -pub fn handle_create_character( - server: &mut Server, - entity: EcsEntity, - character_id: i32, - body: Body, -) { +pub fn handle_initialize_character(server: &mut Server, entity: EcsEntity, character_id: i32) { let state = &mut server.state; let server_settings = &server.server_settings; - state.create_player_character(entity, character_id, body, server_settings); + state.initialize_character_data(entity, character_id, server_settings); +} + +pub fn handle_loaded_character_data( + server: &mut Server, + entity: EcsEntity, + loaded_components: (comp::Body, comp::Stats, comp::Inventory, comp::Loadout), +) { + let state = &mut server.state; + + state.update_character_data(entity, loaded_components); sys::subscription::initialize_region_subscription(state.ecs(), entity); } diff --git a/server/src/events/mod.rs b/server/src/events/mod.rs index f12329275d..e541f22710 100644 --- a/server/src/events/mod.rs +++ b/server/src/events/mod.rs @@ -1,7 +1,8 @@ use crate::Server; use common::event::{EventBus, ServerEvent}; use entity_creation::{ - handle_create_character, handle_create_npc, handle_create_waypoint, handle_shoot, + handle_create_npc, handle_create_waypoint, handle_initialize_character, + handle_loaded_character_data, handle_shoot, }; use entity_manipulation::{ handle_damage, handle_destroy, handle_explosion, handle_land_on_ground, handle_level_up, @@ -70,11 +71,13 @@ impl Server { ServerEvent::Possess(possessor_uid, possesse_uid) => { handle_possess(&self, possessor_uid, possesse_uid) }, - ServerEvent::SelectCharacter { + ServerEvent::InitCharacterData { entity, character_id, - body, - } => handle_create_character(self, entity, character_id, body), + } => handle_initialize_character(self, entity, character_id), + ServerEvent::UpdateCharacterData { entity, components } => { + handle_loaded_character_data(self, entity, components); + }, ServerEvent::LevelUp(entity, new_level) => handle_level_up(self, entity, new_level), ServerEvent::ExitIngame { entity } => handle_exit_ingame(self, entity), ServerEvent::CreateNpc { diff --git a/server/src/lib.rs b/server/src/lib.rs index 1b6a6daf59..730e21d0b3 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -39,7 +39,7 @@ use common::{ vol::{ReadVol, RectVolSize}, }; use metrics::{ServerMetrics, TickMetrics}; -use persistence::character::{CharacterListUpdater, CharacterUpdater}; +use persistence::character::{CharacterLoader, CharacterLoaderResponseType, CharacterUpdater}; use specs::{join::Join, Builder, Entity as EcsEntity, RunNow, SystemData, WorldExt}; use std::{ i32, @@ -102,9 +102,9 @@ impl Server { state .ecs_mut() .insert(CharacterUpdater::new(settings.persistence_db_dir.clone())); - state.ecs_mut().insert(CharacterListUpdater::new( - settings.persistence_db_dir.clone(), - )); + state + .ecs_mut() + .insert(CharacterLoader::new(settings.persistence_db_dir.clone())); // System timers for performance monitoring state.ecs_mut().insert(sys::EntitySyncTimer::default()); @@ -383,20 +383,51 @@ impl Server { } // 7 Persistence updates - // Get completed updates to character lists and notify the client + let before_persistence_updates = Instant::now(); + + // Get character-related database responses and notify the requesting client self.state .ecs() - .read_resource::() + .read_resource::() .messages() - .for_each(|result| match result.result { - Ok(character_data) => self.notify_client( - result.entity, - ServerMsg::CharacterListUpdate(character_data), - ), - Err(error) => self.notify_client( - result.entity, - ServerMsg::CharacterActionError(error.to_string()), - ), + .for_each(|query_result| match query_result.result { + CharacterLoaderResponseType::CharacterList(result) => match result { + Ok(character_list_data) => self.notify_client( + query_result.entity, + ServerMsg::CharacterListUpdate(character_list_data), + ), + Err(error) => self.notify_client( + query_result.entity, + ServerMsg::CharacterActionError(error.to_string()), + ), + }, + CharacterLoaderResponseType::CharacterData(result) => { + let message = match *result { + Ok(character_data) => ServerEvent::UpdateCharacterData { + entity: query_result.entity, + components: character_data, + }, + 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 + self.notify_client( + query_result.entity, + ServerMsg::CharacterDataLoadError(error.to_string()), + ); + + // Clean up the entity data on the server + ServerEvent::ExitIngame { + entity: query_result.entity, + } + }, + }; + + self.state + .ecs() + .read_resource::>() + .emit_now(message); + }, }); let end_of_server_tick = Instant::now(); @@ -456,7 +487,11 @@ impl Server { self.tick_metrics .tick_time .with_label_values(&["entity cleanup"]) - .set((end_of_server_tick - before_entity_cleanup).as_nanos() as i64); + .set((before_persistence_updates - before_entity_cleanup).as_nanos() as i64); + self.tick_metrics + .tick_time + .with_label_values(&["persistence_updates"]) + .set((end_of_server_tick - before_persistence_updates).as_nanos() as i64); self.tick_metrics .tick_time .with_label_values(&["entity sync"]) diff --git a/server/src/persistence/character.rs b/server/src/persistence/character.rs index 160b439fc8..f6ccb5e2a0 100644 --- a/server/src/persistence/character.rs +++ b/server/src/persistence/character.rs @@ -1,4 +1,9 @@ -//! Database operations related to characters +//! Database operations related to character data +//! +//! Methods in this module should remain private - database updates and loading +//! are communicated via requests to the [`CharacterLoader`] and +//! [`CharacterUpdater`] while results/responses are polled and handled each +//! server tick. extern crate diesel; @@ -20,8 +25,10 @@ use crossbeam::{channel, channel::TryIter}; use diesel::prelude::*; use tracing::{error, warn}; +type CharacterLoaderRequest = (specs::Entity, CharacterLoaderRequestKind); + /// Available database operations when modifying a player's characetr list -enum CharacterListRequestKind { +enum CharacterLoaderRequestKind { CreateCharacter { player_uuid: String, character_alias: String, @@ -35,64 +42,98 @@ enum CharacterListRequestKind { LoadCharacterList { player_uuid: String, }, + LoadCharacterData { + player_uuid: String, + character_id: i32, + }, } -/// Common format dispatched in response to an update request +/// A tuple of the components that are persisted to the DB for each character +pub type PersistedComponents = (comp::Body, comp::Stats, comp::Inventory, comp::Loadout); + +type CharacterListResult = Result, Error>; +type CharacterDataResult = Result; + +/// Wrapper for results for character actions. Can be a list of +/// characters, or component data belonging to an individual character #[derive(Debug)] -pub struct CharacterListResponse { - pub entity: specs::Entity, - pub result: CharacterListResult, +pub enum CharacterLoaderResponseType { + CharacterList(CharacterListResult), + CharacterData(Box), } -/// A bi-directional messaging resource for making modifications to a player's -/// character list in a background thread. +/// Common message format dispatched in response to an update request +#[derive(Debug)] +pub struct CharacterLoaderResponse { + pub entity: specs::Entity, + pub result: CharacterLoaderResponseType, +} + +/// A bi-directional messaging resource for making requests to modify or load +/// character data in a background thread. /// -/// This is used exclusively during character selection, and handles loading the -/// player's character list, deleting characters, and creating new characters. -/// Operations not related to the character list (such as saving a character's -/// inventory, stats, etc..) are performed by the [`CharacterUpdater`] -pub struct CharacterListUpdater { - update_rx: Option>, - update_tx: Option>, +/// This is used on the character selection screen, and after character +/// selection when loading the components associated with a character. +/// +/// Requests messages are sent in the form of +/// [`CharacterLoaderRequestKind`] and are dispatched at the character select +/// screen. +/// +/// Responses are polled on each server tick in the format +/// [`CharacterLoaderResponse`] +pub struct CharacterLoader { + update_rx: Option>, + update_tx: Option>, handle: Option>, } -type CharacterListRequest = (specs::Entity, CharacterListRequestKind); - -impl CharacterListUpdater { +impl CharacterLoader { pub fn new(db_dir: String) -> Self { - let (update_tx, internal_rx) = channel::unbounded::(); - let (internal_tx, update_rx) = channel::unbounded::(); + let (update_tx, internal_rx) = channel::unbounded::(); + let (internal_tx, update_rx) = channel::unbounded::(); let handle = std::thread::spawn(move || { while let Ok(request) = internal_rx.recv() { let (entity, kind) = request; - if let Err(err) = internal_tx.send(CharacterListResponse { + if let Err(e) = internal_tx.send(CharacterLoaderResponse { entity, result: match kind { - CharacterListRequestKind::CreateCharacter { + CharacterLoaderRequestKind::CreateCharacter { player_uuid, character_alias, character_tool, body, - } => create_character( + } => CharacterLoaderResponseType::CharacterList(create_character( &player_uuid, - character_alias, + &character_alias, character_tool, &body, &db_dir, - ), - CharacterListRequestKind::DeleteCharacter { + )), + CharacterLoaderRequestKind::DeleteCharacter { player_uuid, character_id, - } => delete_character(&player_uuid, character_id, &db_dir), - CharacterListRequestKind::LoadCharacterList { player_uuid } => { - load_character_list(&player_uuid, &db_dir) + } => CharacterLoaderResponseType::CharacterList(delete_character( + &player_uuid, + character_id, + &db_dir, + )), + CharacterLoaderRequestKind::LoadCharacterList { player_uuid } => { + CharacterLoaderResponseType::CharacterList(load_character_list( + &player_uuid, + &db_dir, + )) }, + CharacterLoaderRequestKind::LoadCharacterData { + player_uuid, + character_id, + } => CharacterLoaderResponseType::CharacterData(Box::new( + load_character_data(&player_uuid, character_id, &db_dir), + )), }, }) { - log::error!("Could not send persistence request: {:?}", err); + error!(?e, "Could not send send persistence request"); } } }); @@ -105,7 +146,7 @@ impl CharacterListUpdater { } /// Create a new character belonging to the player identified by - /// `player_uuid`. + /// `player_uuid` pub fn create_character( &self, entity: specs::Entity, @@ -114,78 +155,91 @@ impl CharacterListUpdater { character_tool: Option, body: comp::Body, ) { - if let Err(err) = self.update_tx.as_ref().unwrap().send(( + if let Err(e) = self.update_tx.as_ref().unwrap().send(( entity, - CharacterListRequestKind::CreateCharacter { + CharacterLoaderRequestKind::CreateCharacter { player_uuid, character_alias, character_tool, body, }, )) { - log::error!("Could not send character creation request: {:?}", err); + error!(?e, "Could not send character creation request"); } } - /// Delete a character by `id` and `player_uuid`. + /// Delete a character by `id` and `player_uuid` pub fn delete_character(&self, entity: specs::Entity, player_uuid: String, character_id: i32) { - if let Err(err) = self.update_tx.as_ref().unwrap().send(( + if let Err(e) = self.update_tx.as_ref().unwrap().send(( entity, - CharacterListRequestKind::DeleteCharacter { + CharacterLoaderRequestKind::DeleteCharacter { player_uuid, character_id, }, )) { - log::error!("Could not send character deletion request: {:?}", err); + error!(?e, "Could not send character deletion request"); } } /// Loads a list of characters belonging to the player identified by /// `player_uuid` pub fn load_character_list(&self, entity: specs::Entity, player_uuid: String) { - if let Err(err) = self + if let Err(e) = self .update_tx .as_ref() .unwrap() - .send((entity, CharacterListRequestKind::LoadCharacterList { + .send((entity, CharacterLoaderRequestKind::LoadCharacterList { player_uuid, })) { - log::error!("Could not send character list load request: {:?}", err); + error!(?e, "Could not send character list load request"); } } - /// Returns a non-blocking iterator over CharacterListResponse messages - pub fn messages(&self) -> TryIter { + /// Loads components associated with a character + pub fn load_character_data( + &self, + entity: specs::Entity, + player_uuid: String, + character_id: i32, + ) { + if let Err(e) = self.update_tx.as_ref().unwrap().send(( + entity, + CharacterLoaderRequestKind::LoadCharacterData { + player_uuid, + character_id, + }, + )) { + error!(?e, "Could not send character data load request"); + } + } + + /// Returns a non-blocking iterator over CharacterLoaderResponse messages + pub fn messages(&self) -> TryIter { self.update_rx.as_ref().unwrap().try_iter() } } -impl Drop for CharacterListUpdater { +impl Drop for CharacterLoader { fn drop(&mut self) { drop(self.update_tx.take()); - if let Err(err) = self.handle.take().unwrap().join() { - log::error!("Error from joining character update thread: {:?}", err); + if let Err(e) = self.handle.take().unwrap().join() { + error!(?e, "Error from joining character loader thread"); } } } -type CharacterListResult = Result, Error>; - /// Load stored data for a character. /// /// After first logging in, and after a character is selected, we fetch this /// data for the purpose of inserting their persisted data for the entity. - -pub fn load_character_data( - character_id: i32, - db_dir: &str, -) -> Result<(comp::Stats, comp::Inventory, comp::Loadout), Error> { +fn load_character_data(player_uuid: &str, character_id: i32, db_dir: &str) -> CharacterDataResult { let connection = establish_connection(db_dir); let (character_data, body_data, stats_data, maybe_inventory, maybe_loadout) = schema::character::dsl::character .filter(schema::character::id.eq(character_id)) + .filter(schema::character::player_uuid.eq(player_uuid)) .inner_join(schema::body::table) .inner_join(schema::stats::table) .left_join(schema::inventory::table) @@ -193,6 +247,7 @@ pub fn load_character_data( .first::<(Character, Body, Stats, Option, Option)>(&connection)?; Ok(( + comp::Body::from(&body_data), comp::Stats::from(StatsJoinData { alias: &character_data.alias, body: &comp::Body::from(&body_data), @@ -300,7 +355,7 @@ fn load_character_list(player_uuid: &str, db_dir: &str) -> CharacterListResult { /// id for subsequent insertions fn create_character( uuid: &str, - character_alias: String, + character_alias: &str, character_tool: Option, body: &comp::Body, db_dir: &str, diff --git a/server/src/persistence/error.rs b/server/src/persistence/error.rs index 0086fa5cb4..93d56caa14 100644 --- a/server/src/persistence/error.rs +++ b/server/src/persistence/error.rs @@ -4,10 +4,14 @@ extern crate diesel; use std::fmt; -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub enum Error { // The player has already reached the max character limit CharacterLimitReached, + // An error occured while establish a db connection + DatabaseConnectionError(diesel::ConnectionError), + // An error occured while running migrations + DatabaseMigrationError(diesel_migrations::RunMigrationsError), // An error occured when performing a database action DatabaseError(diesel::result::Error), // Unable to load body or stats for a character @@ -17,8 +21,10 @@ pub enum Error { impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}", match self { - Self::DatabaseError(diesel_error) => diesel_error.to_string(), Self::CharacterLimitReached => String::from("Character limit exceeded"), + Self::DatabaseError(error) => error.to_string(), + Self::DatabaseConnectionError(error) => error.to_string(), + Self::DatabaseMigrationError(error) => error.to_string(), Self::CharacterDataError => String::from("Error while loading character data"), }) } @@ -27,3 +33,13 @@ impl fmt::Display for Error { impl From for Error { fn from(error: diesel::result::Error) -> Error { Error::DatabaseError(error) } } + +impl From for Error { + fn from(error: diesel::ConnectionError) -> Error { Error::DatabaseConnectionError(error) } +} + +impl From for Error { + fn from(error: diesel_migrations::RunMigrationsError) -> Error { + Error::DatabaseMigrationError(error) + } +} diff --git a/server/src/state_ext.rs b/server/src/state_ext.rs index 88abc1afb4..c95ad326c1 100644 --- a/server/src/state_ext.rs +++ b/server/src/state_ext.rs @@ -1,13 +1,11 @@ use crate::{ - client::Client, persistence, settings::ServerSettings, sys::sentinel::DeletedEntities, - SpawnPoint, + client::Client, persistence::character::PersistedComponents, settings::ServerSettings, + sys::sentinel::DeletedEntities, SpawnPoint, }; use common::{ comp, effect::Effect, - msg::{ - CharacterInfo, ClientState, PlayerListUpdate, RegisterError, RequestStateError, ServerMsg, - }, + msg::{CharacterInfo, ClientState, PlayerListUpdate, ServerMsg}, state::State, sync::{Uid, WorldSyncExt}, util::Dir, @@ -17,8 +15,11 @@ use tracing::warn; use vek::*; pub trait StateExt { + /// Push an item into the provided entity's inventory fn give_item(&mut self, entity: EcsEntity, item: comp::Item) -> bool; + /// Updates a component associated with the entity based on the `Effect` fn apply_effect(&mut self, entity: EcsEntity, effect: Effect); + /// Build a non-player character fn create_npc( &mut self, pos: comp::Pos, @@ -26,7 +27,9 @@ pub trait StateExt { loadout: comp::Loadout, body: comp::Body, ) -> EcsEntityBuilder; + /// Build a static object entity fn create_object(&mut self, pos: comp::Pos, object: comp::object::Body) -> EcsEntityBuilder; + /// Build a projectile fn create_projectile( &mut self, pos: comp::Pos, @@ -34,14 +37,19 @@ pub trait StateExt { body: comp::Body, projectile: comp::Projectile, ) -> EcsEntityBuilder; - fn create_player_character( + /// Insert common/default components for a new character joining the server + fn initialize_character_data( &mut self, entity: EcsEntity, character_id: i32, - body: comp::Body, server_settings: &ServerSettings, ); + /// Update the components associated with the entity's current character. + /// Performed after loading component data from the database + fn update_character_data(&mut self, entity: EcsEntity, components: PersistedComponents); + /// Iterates over registered clients and send each `ServerMsg` fn notify_registered_clients(&self, msg: ServerMsg); + /// Delete an entity, recording the deletion in [`DeletedEntities`] fn delete_entity_recorded( &mut self, entity: EcsEntity, @@ -82,7 +90,6 @@ impl StateExt for State { } } - /// Build a non-player character. fn create_npc( &mut self, pos: comp::Pos, @@ -115,7 +122,6 @@ impl StateExt for State { .with(loadout) } - /// Build a static object entity fn create_object(&mut self, pos: comp::Pos, object: comp::object::Body) -> EcsEntityBuilder { self.ecs_mut() .create_entity_synced() @@ -132,7 +138,6 @@ impl StateExt for State { .with(comp::Gravity(1.0)) } - /// Build a projectile fn create_projectile( &mut self, pos: comp::Pos, @@ -152,44 +157,14 @@ impl StateExt for State { .with(comp::Sticky) } - #[allow(clippy::unnecessary_operation)] // TODO: Pending review in #587 - fn create_player_character( + fn initialize_character_data( &mut self, entity: EcsEntity, character_id: i32, - body: comp::Body, server_settings: &ServerSettings, ) { - // Grab persisted character data from the db and insert their associated - // components. If for some reason the data can't be returned (missing - // data, DB error), kick the client back to the character select screen. - match persistence::character::load_character_data( - character_id, - &server_settings.persistence_db_dir, - ) { - Ok((stats, inventory, loadout)) => { - self.write_component(entity, stats); - self.write_component(entity, inventory); - self.write_component(entity, loadout); - }, - Err(e) => { - warn!( - ?e, - ?character_id, - "Failed to load character data for character_id" - ); - - if let Some(client) = self.ecs().write_storage::().get_mut(entity) { - client.error_state(RequestStateError::RegisterDenied( - RegisterError::InvalidCharacter, - )) - } - }, - } - let spawn_point = self.ecs().read_resource::().0; - self.write_component(entity, body); self.write_component(entity, comp::Energy::new(1000)); self.write_component(entity, comp::Controller::default()); self.write_component(entity, comp::Pos(spawn_point)); @@ -203,27 +178,19 @@ impl StateExt for State { self.write_component(entity, comp::Gravity(1.0)); self.write_component(entity, comp::CharacterState::default()); self.write_component(entity, comp::Alignment::Owned(entity)); - self.write_component( - entity, - comp::InventoryUpdate::new(comp::InventoryUpdateEvent::default()), - ); // Set the character id for the player // TODO this results in a warning in the console: "Error modifying synced // component, it doesn't seem to exist" // It appears to be caused by the player not yet existing on the client at this // point, despite being able to write the data on the server - &self - .ecs() + self.ecs() .write_storage::() .get_mut(entity) .map(|player| { player.character_id = Some(character_id); }); - // Make sure physics are accepted. - self.write_component(entity, comp::ForceUpdate); - // Give the Admin component to the player if their name exists in admin list if server_settings.admins.contains( &self @@ -236,30 +203,42 @@ impl StateExt for State { self.write_component(entity, comp::Admin); } - let uids = &self.ecs().read_storage::(); - let uid = uids - .get(entity) - .expect("Failed to fetch uid component for entity.") - .0; - - let stats = &self.ecs().read_storage::(); - let stat = stats - .get(entity) - .expect("Failed to fetch stats component for entity."); - - self.notify_registered_clients(ServerMsg::PlayerListUpdate( - PlayerListUpdate::SelectedCharacter(uid, CharacterInfo { - name: stat.name.to_string(), - level: stat.level.level(), - }), - )); - // Tell the client its request was successful. if let Some(client) = self.ecs().write_storage::().get_mut(entity) { client.allow_state(ClientState::Character); } } + fn update_character_data(&mut self, entity: EcsEntity, components: PersistedComponents) { + let (body, stats, inventory, loadout) = components; + + // Notify clients of a player list update + let client_uid = self + .read_component_cloned::(entity) + .map(|u| u.into()) + .expect("Client doesn't have a Uid!!!"); + + self.notify_registered_clients(ServerMsg::PlayerListUpdate( + PlayerListUpdate::SelectedCharacter(client_uid, CharacterInfo { + name: String::from(&stats.name), + level: stats.level.level(), + }), + )); + + self.write_component(entity, body); + self.write_component(entity, stats); + self.write_component(entity, inventory); + self.write_component(entity, loadout); + + self.write_component( + entity, + comp::InventoryUpdate::new(comp::InventoryUpdateEvent::default()), + ); + + // Make sure physics are accepted. + self.write_component(entity, comp::ForceUpdate); + } + fn notify_registered_clients(&self, msg: ServerMsg) { for client in (&mut self.ecs().write_storage::()) .join() diff --git a/server/src/sys/message.rs b/server/src/sys/message.rs index cfb22998d2..a4c63ec4f7 100644 --- a/server/src/sys/message.rs +++ b/server/src/sys/message.rs @@ -1,6 +1,6 @@ use super::SysTimer; use crate::{ - auth_provider::AuthProvider, client::Client, persistence::character::CharacterListUpdater, + auth_provider::AuthProvider, client::Client, persistence::character::CharacterLoader, CLIENT_TIMEOUT, }; use common::{ @@ -32,7 +32,7 @@ impl<'a> System<'a> for Sys { Entities<'a>, Read<'a, EventBus>, Read<'a, Time>, - ReadExpect<'a, CharacterListUpdater>, + ReadExpect<'a, CharacterLoader>, ReadExpect<'a, TerrainGrid>, Write<'a, SysTimer>, ReadStorage<'a, Uid>, @@ -60,7 +60,7 @@ impl<'a> System<'a> for Sys { entities, server_event_bus, time, - char_list, + character_loader, terrain, mut timer, uids, @@ -193,31 +193,45 @@ impl<'a> System<'a> for Sys { }, _ => {}, }, - ClientMsg::Character { character_id, body } => match client.client_state { + ClientMsg::Character(character_id) => match client.client_state { // Become Registered first. ClientState::Connected => client.error_state(RequestStateError::Impossible), ClientState::Registered | ClientState::Spectator => { - // Only send login message if it wasn't already - // sent previously - if let (Some(player), false) = - (players.get(entity), client.login_msg_sent) - { - new_chat_msgs.push(( - None, - ServerMsg::broadcast(format!( - "[{}] is now online.", - &player.alias - )), - )); + if let Some(player) = players.get(entity) { + // Send a request to load the character's component data from the + // DB. Once loaded, persisted components such as stats and inventory + // will be inserted for the entity + character_loader.load_character_data( + entity, + player.uuid().to_string(), + character_id, + ); - client.login_msg_sent = true; + // Start inserting non-persisted/default components for the entity + // while we load the DB data + server_emitter.emit(ServerEvent::InitCharacterData { + entity, + character_id, + }); + + // Only send login message if it wasn't already + // sent previously + if !client.login_msg_sent { + new_chat_msgs.push(( + None, + ServerMsg::broadcast(format!( + "[{}] is now online.", + &player.alias + )), + )); + + client.login_msg_sent = true; + } + } else { + client.notify(ServerMsg::CharacterDataLoadError(String::from( + "Failed to fetch player entity", + ))) } - - server_emitter.emit(ServerEvent::SelectCharacter { - entity, - character_id, - body, - }); }, ClientState::Character => client.error_state(RequestStateError::Already), ClientState::Pending => {}, @@ -332,12 +346,12 @@ impl<'a> System<'a> for Sys { }, ClientMsg::RequestCharacterList => { if let Some(player) = players.get(entity) { - char_list.load_character_list(entity, player.uuid().to_string()) + character_loader.load_character_list(entity, player.uuid().to_string()) } }, ClientMsg::CreateCharacter { alias, tool, body } => { if let Some(player) = players.get(entity) { - char_list.create_character( + character_loader.create_character( entity, player.uuid().to_string(), alias, @@ -348,7 +362,7 @@ impl<'a> System<'a> for Sys { }, ClientMsg::DeleteCharacter(character_id) => { if let Some(player) = players.get(entity) { - char_list.delete_character( + character_loader.delete_character( entity, player.uuid().to_string(), character_id, diff --git a/voxygen/src/menu/char_selection/mod.rs b/voxygen/src/menu/char_selection/mod.rs index 51c2de60f5..e57f919023 100644 --- a/voxygen/src/menu/char_selection/mod.rs +++ b/voxygen/src/menu/char_selection/mod.rs @@ -88,9 +88,7 @@ impl PlayState for CharSelectionState { char_data.get(self.char_selection_ui.selected_character) { if let Some(character_id) = selected_character.character.id { - self.client - .borrow_mut() - .request_character(character_id, selected_character.body); + self.client.borrow_mut().request_character(character_id); } }