diff --git a/server/src/lib.rs b/server/src/lib.rs index 9574b6e05d..7766160edf 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -610,11 +610,20 @@ impl Server { // 7 Persistence updates let before_persistence_updates = Instant::now(); - // Get character-related database responses and notify the requesting client - self.state + let character_loader = self + .state .ecs() - .read_resource::() + .read_resource::(); + + let character_updater = self + .state + .ecs() + .read_resource::(); + + // Get character-related database responses and notify the requesting client + character_loader .messages() + .chain(character_updater.messages()) .for_each(|query_result| match query_result.result { CharacterLoaderResponseKind::CharacterList(result) => match result { Ok(character_list_data) => self.notify_client( @@ -671,6 +680,9 @@ impl Server { }, }); + drop(character_loader); + drop(character_updater); + { // Check for new chunks; cancel and regenerate all chunks if the asset has been // reloaded. Note that all of these assignments are no-ops, so the diff --git a/server/src/persistence/character.rs b/server/src/persistence/character.rs index 81e8422657..a3d06c1e06 100644 --- a/server/src/persistence/character.rs +++ b/server/src/persistence/character.rs @@ -9,7 +9,7 @@ extern crate rusqlite; use super::{error::PersistenceError, models::*}; use crate::{ comp, - comp::{item::MaterialStatManifest, Inventory}, + comp::Inventory, persistence::{ character::conversions::{ convert_body_from_database, convert_body_to_database_json, @@ -101,7 +101,6 @@ pub fn load_character_data( requesting_player_uuid: String, char_id: CharacterId, connection: &mut Transaction, - msm: &MaterialStatManifest, ) -> CharacterDataResult { let character_containers = get_pseudo_containers(connection, char_id)?; let inventory_items = load_items_bfs(connection, character_containers.inventory_container_id)?; @@ -205,7 +204,6 @@ pub fn load_character_data( &inventory_items, character_containers.loadout_container_id, &loadout_items, - msm, )?, char_waypoint, )) @@ -221,7 +219,6 @@ pub fn load_character_data( pub fn load_character_list( player_uuid_: &str, connection: &mut Transaction, - msm: &MaterialStatManifest, ) -> CharacterListResult { let characters; { @@ -282,7 +279,7 @@ pub fn load_character_list( let loadout_items = load_items_bfs(connection, loadout_container_id)?; let loadout = - convert_loadout_from_database_items(loadout_container_id, &loadout_items, msm)?; + convert_loadout_from_database_items(loadout_container_id, &loadout_items)?; Ok(CharacterItem { character: char, @@ -298,7 +295,6 @@ pub fn create_character( character_alias: &str, persisted_components: PersistedComponents, connection: &mut Transaction, - msm: &MaterialStatManifest, ) -> CharacterCreationResult { check_character_limit(uuid, connection)?; @@ -445,7 +441,7 @@ pub fn create_character( } drop(stmt); - load_character_list(uuid, connection, msm).map(|list| (character_id, list)) + load_character_list(uuid, connection).map(|list| (character_id, list)) } /// Delete a character. Returns the updated character list. @@ -453,7 +449,6 @@ pub fn delete_character( requesting_player_uuid: &str, char_id: CharacterId, connection: &mut Transaction, - msm: &MaterialStatManifest, ) -> CharacterListResult { #[rustfmt::skip] let mut stmt = connection.prepare_cached(" @@ -549,7 +544,7 @@ pub fn delete_character( ))); } - load_character_list(requesting_player_uuid, connection, msm) + load_character_list(requesting_player_uuid, connection) } /// Before creating a character, we ensure that the limit on the number of diff --git a/server/src/persistence/character/conversions.rs b/server/src/persistence/character/conversions.rs index 4d83b04eb7..39def94143 100644 --- a/server/src/persistence/character/conversions.rs +++ b/server/src/persistence/character/conversions.rs @@ -22,6 +22,7 @@ use common::{ }; use core::{convert::TryFrom, num::NonZeroU64}; use hashbrown::HashMap; +use lazy_static::lazy_static; use std::{collections::VecDeque, sync::Arc}; use tracing::trace; @@ -31,6 +32,13 @@ pub struct ItemModelPair { pub model: Item, } +// Decoupled from the ECS resource because the plumbing is getting complicated; +// shouldn't matter unless someone's hot-reloading material stats on the live +// server +lazy_static! { + pub static ref MATERIAL_STATS_MANIFEST: MaterialStatManifest = MaterialStatManifest::default(); +} + /// Returns a vector that contains all item rows to upsert; parent is /// responsible for deleting items from the same owner that aren't affirmatively /// kept by this. @@ -230,7 +238,6 @@ pub fn convert_inventory_from_database_items( inventory_items: &[Item], loadout_container_id: i64, loadout_items: &[Item], - msm: &MaterialStatManifest, ) -> Result { // Loadout items must be loaded before inventory items since loadout items // provide inventory slots. Since items stored inside loadout items actually @@ -238,7 +245,7 @@ pub fn convert_inventory_from_database_items( // on populating the loadout items first, and then inserting the items into the // inventory at the correct position. // - let loadout = convert_loadout_from_database_items(loadout_container_id, loadout_items, msm)?; + let loadout = convert_loadout_from_database_items(loadout_container_id, loadout_items)?; let mut inventory = Inventory::new_with_loadout(loadout); let mut item_indices = HashMap::new(); @@ -310,7 +317,7 @@ pub fn convert_inventory_from_database_items( } } else if let Some(&j) = item_indices.get(&db_item.parent_container_item_id) { if let Some(Some(parent)) = inventory.slot_mut(slot(&inventory_items[j].position)?) { - parent.add_component(item, msm); + parent.add_component(item, &MATERIAL_STATS_MANIFEST); } else { return Err(PersistenceError::ConversionError(format!( "Parent slot {} for component {} was empty even though it occurred earlier in \ @@ -332,7 +339,6 @@ pub fn convert_inventory_from_database_items( pub fn convert_loadout_from_database_items( loadout_container_id: i64, database_items: &[Item], - msm: &MaterialStatManifest, ) -> Result { let loadout_builder = LoadoutBuilder::new(); let mut loadout = loadout_builder.build(); @@ -367,7 +373,7 @@ pub fn convert_loadout_from_database_items( } else if let Some(&j) = item_indices.get(&db_item.parent_container_item_id) { loadout .update_item_at_slot_using_persistence_key(&database_items[j].position, |parent| { - parent.add_component(item, msm); + parent.add_component(item, &MATERIAL_STATS_MANIFEST); }) .map_err(convert_error)?; } else { diff --git a/server/src/persistence/character_loader.rs b/server/src/persistence/character_loader.rs index 2f4bd580e6..360802cfee 100644 --- a/server/src/persistence/character_loader.rs +++ b/server/src/persistence/character_loader.rs @@ -3,12 +3,8 @@ use crate::persistence::{ error::PersistenceError, establish_connection, DatabaseSettings, PersistedComponents, }; -use common::{ - character::{CharacterId, CharacterItem}, - comp::inventory::item::MaterialStatManifest, -}; +use common::character::{CharacterId, CharacterItem}; use crossbeam_channel::{self, TryIter}; -use lazy_static::lazy_static; use rusqlite::Transaction; use std::sync::{Arc, RwLock}; use tracing::{error, trace}; @@ -83,13 +79,6 @@ pub struct CharacterLoader { update_tx: crossbeam_channel::Sender, } -// Decoupled from the ECS resource because the plumbing is getting complicated; -// shouldn't matter unless someone's hot-reloading material stats on the live -// server -lazy_static! { - pub static ref MATERIAL_STATS_MANIFEST: MaterialStatManifest = MaterialStatManifest::default(); -} - impl CharacterLoader { pub fn new(settings: Arc>) -> Result { let (update_tx, internal_rx) = crossbeam_channel::unbounded::(); @@ -163,7 +152,6 @@ impl CharacterLoader { &character_alias, persisted_components, &mut transaction, - &MATERIAL_STATS_MANIFEST, )), CharacterLoaderRequestKind::DeleteCharacter { player_uuid, @@ -172,25 +160,18 @@ impl CharacterLoader { &player_uuid, character_id, &mut transaction, - &MATERIAL_STATS_MANIFEST, )), CharacterLoaderRequestKind::LoadCharacterList { player_uuid } => { CharacterLoaderResponseKind::CharacterList(load_character_list( &player_uuid, &mut transaction, - &MATERIAL_STATS_MANIFEST, )) }, CharacterLoaderRequestKind::LoadCharacterData { player_uuid, character_id, } => { - let result = load_character_data( - player_uuid, - character_id, - &mut transaction, - &MATERIAL_STATS_MANIFEST, - ); + let result = load_character_data(player_uuid, character_id, &mut transaction); if result.is_err() { error!( ?result, diff --git a/server/src/persistence/character_updater.rs b/server/src/persistence/character_updater.rs index fb591146b8..f6de8c524d 100644 --- a/server/src/persistence/character_updater.rs +++ b/server/src/persistence/character_updater.rs @@ -2,9 +2,13 @@ use crate::comp; use common::character::CharacterId; use crate::persistence::{ - error::PersistenceError, establish_connection, DatabaseSettings, VelorenConnection, + character_loader::{CharacterLoaderResponse, CharacterLoaderResponseKind}, + error::PersistenceError, + establish_connection, DatabaseSettings, VelorenConnection, }; +use crossbeam_channel::TryIter; use rusqlite::DropBehavior; +use specs::Entity; use std::{ collections::HashMap, sync::{ @@ -18,6 +22,11 @@ pub type CharacterUpdateData = (comp::SkillSet, comp::Inventory, Option), + DeleteCharacter { + entity: Entity, + requesting_player_uuid: String, + character_id: CharacterId, + }, DisconnectedSuccess, } @@ -28,6 +37,7 @@ pub enum CharacterUpdaterEvent { /// such as inventory, loadout, etc... pub struct CharacterUpdater { update_tx: Option>, + response_rx: crossbeam_channel::Receiver, handle: Option>, pending_logout_updates: HashMap, /// Will disconnect all characters (without persistence) on the next tick if @@ -38,6 +48,8 @@ pub struct CharacterUpdater { impl CharacterUpdater { pub fn new(settings: Arc>) -> rusqlite::Result { let (update_tx, update_rx) = crossbeam_channel::unbounded::(); + let (response_tx, response_rx) = crossbeam_channel::unbounded::(); + let disconnect_all_clients_requested = Arc::new(AtomicBool::new(false)); let disconnect_all_clients_requested_clone = Arc::clone(&disconnect_all_clients_requested); @@ -68,6 +80,33 @@ impl CharacterUpdater { .store(true, Ordering::Relaxed); }; }, + CharacterUpdaterEvent::DeleteCharacter { + entity, + requesting_player_uuid, + character_id, + } => { + match execute_character_delete( + entity, + &requesting_player_uuid, + character_id, + &mut conn, + ) { + Ok(response) => { + if let Err(e) = response_tx.send(response) { + error!(?e, "Could not send character deletion response"); + } else { + debug!( + "Processed character delete for character ID {}", + character_id + ); + } + }, + Err(e) => error!( + "Error deleting character ID {}, error: {:?}", + character_id, e + ), + } + }, CharacterUpdaterEvent::DisconnectedSuccess => { info!( "CharacterUpdater received DisconnectedSuccess event, resuming \ @@ -84,6 +123,7 @@ impl CharacterUpdater { Ok(Self { update_tx: Some(update_tx), + response_rx, handle: Some(handle), pending_logout_updates: HashMap::new(), disconnect_all_clients_requested, @@ -125,6 +165,32 @@ impl CharacterUpdater { .load(Ordering::Relaxed) } + pub fn delete_character( + &mut self, + entity: Entity, + requesting_player_uuid: String, + character_id: CharacterId, + ) { + if let Err(e) = + self.update_tx + .as_ref() + .unwrap() + .send(CharacterUpdaterEvent::DeleteCharacter { + entity, + requesting_player_uuid, + character_id, + }) + { + error!(?e, "Could not send character deletion request"); + } else { + // Once a delete request has been sent to the channel we must remove any pending + // updates for the character in the event that it has recently logged out. + // Since the user has actively chosen to delete the character there is no value + // in the pending update data anyway. + self.pending_logout_updates.remove(&character_id); + } + } + /// Updates a collection of characters based on their id and components pub fn batch_update<'a>( &mut self, @@ -185,6 +251,9 @@ impl CharacterUpdater { future persistence batches from running", ); } + + /// Returns a non-blocking iterator over CharacterLoaderResponse messages + pub fn messages(&self) -> TryIter { self.response_rx.try_iter() } } fn execute_batch_update( @@ -205,6 +274,30 @@ fn execute_batch_update( Ok(()) } +fn execute_character_delete( + entity: Entity, + requesting_player_uuid: &str, + character_id: CharacterId, + connection: &mut VelorenConnection, +) -> Result { + let mut transaction = connection.connection.transaction()?; + + let response = CharacterLoaderResponse { + entity, + result: CharacterLoaderResponseKind::CharacterList(super::character::delete_character( + requesting_player_uuid, + character_id, + &mut transaction, + )), + }; + + if !response.is_err() { + transaction.commit()?; + }; + + Ok(response) +} + impl Drop for CharacterUpdater { fn drop(&mut self) { drop(self.update_tx.take()); diff --git a/server/src/sys/msg/character_screen.rs b/server/src/sys/msg/character_screen.rs index e3a984dadd..d5c1f46484 100644 --- a/server/src/sys/msg/character_screen.rs +++ b/server/src/sys/msg/character_screen.rs @@ -13,7 +13,7 @@ use common::{ }; use common_ecs::{Job, Origin, Phase, System}; use common_net::msg::{ClientGeneral, ServerGeneral}; -use specs::{Entities, Join, Read, ReadExpect, ReadStorage}; +use specs::{Entities, Join, Read, ReadExpect, ReadStorage, WriteExpect}; use std::sync::atomic::Ordering; use tracing::{debug, warn}; @@ -24,7 +24,7 @@ impl Sys { entity: specs::Entity, client: &Client, character_loader: &ReadExpect<'_, CharacterLoader>, - character_updater: &ReadExpect<'_, CharacterUpdater>, + character_updater: &mut WriteExpect<'_, CharacterUpdater>, uids: &ReadStorage<'_, Uid>, players: &ReadStorage<'_, Player>, presences: &ReadStorage<'_, Presence>, @@ -132,7 +132,7 @@ impl Sys { }, ClientGeneral::DeleteCharacter(character_id) => { if let Some(player) = players.get(entity) { - character_loader.delete_character( + character_updater.delete_character( entity, player.uuid().to_string(), character_id, @@ -154,7 +154,7 @@ impl<'a> System<'a> for Sys { Entities<'a>, Read<'a, EventBus>, ReadExpect<'a, CharacterLoader>, - ReadExpect<'a, CharacterUpdater>, + WriteExpect<'a, CharacterUpdater>, ReadStorage<'a, Uid>, ReadStorage<'a, Client>, ReadStorage<'a, Player>, @@ -173,7 +173,7 @@ impl<'a> System<'a> for Sys { entities, server_event_bus, character_loader, - character_updater, + mut character_updater, uids, clients, players, @@ -191,7 +191,7 @@ impl<'a> System<'a> for Sys { entity, client, &character_loader, - &character_updater, + &mut character_updater, &uids, &players, &presences,