From 3ac18f73eab75442c4ddd5d892c18ffcc8cd24ec Mon Sep 17 00:00:00 2001 From: Ben Wallis Date: Mon, 6 Jul 2020 22:06:03 +0100 Subject: [PATCH] Removed de-serialization failure tolerance to prevent player inventory/loadout wipes --- server/src/persistence/character.rs | 90 +++++++++-------------------- server/src/persistence/models.rs | 29 +--------- 2 files changed, 29 insertions(+), 90 deletions(-) diff --git a/server/src/persistence/character.rs b/server/src/persistence/character.rs index 92d506b219..13b9f30d95 100644 --- a/server/src/persistence/character.rs +++ b/server/src/persistence/character.rs @@ -236,71 +236,33 @@ impl Drop for CharacterLoader { 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) - .left_join(schema::loadout::table) - .first::<(Character, Body, Stats, Option, Option)>(&connection)?; + let result = 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) + .inner_join(schema::inventory::table) + .inner_join(schema::loadout::table) + .first::<(Character, Body, Stats, Inventory, Loadout)>(&connection); - Ok(( - comp::Body::from(&body_data), - comp::Stats::from(StatsJoinData { - alias: &character_data.alias, - body: &comp::Body::from(&body_data), - stats: &stats_data, - }), - maybe_inventory.map_or_else( - || { - // If no inventory record was found for the character, create it now - let row = Inventory::from((character_data.id, comp::Inventory::default())); - - if let Err(error) = diesel::insert_into(schema::inventory::table) - .values(&row) - .execute(&connection) - { - warn!( - "Failed to create an inventory record for character {}: {}", - &character_data.id, error - ) - } - - comp::Inventory::default() - }, - comp::Inventory::from, - ), - maybe_loadout.map_or_else( - || { - // Create if no record was found - let default_loadout = LoadoutBuilder::new() - .defaults() - .active_item(LoadoutBuilder::default_item_config_from_str( - character_data.tool.as_deref(), - )) - .build(); - - let row = NewLoadout::from((character_data.id, &default_loadout)); - - if let Err(e) = diesel::insert_into(schema::loadout::table) - .values(&row) - .execute(&connection) - { - let char_id = character_data.id; - warn!( - ?e, - ?char_id, - "Failed to create an loadout record for character", - ) - } - - default_loadout - }, - |data| comp::Loadout::from(&data), - ), - )) + if let Ok((character_data, body_data, stats_data, inventory, loadout)) = result { + Ok(( + comp::Body::from(&body_data), + comp::Stats::from(StatsJoinData { + alias: &character_data.alias, + body: &comp::Body::from(&body_data), + stats: &stats_data, + }), + comp::Inventory::from(inventory), + comp::Loadout::from(&loadout), + )) + } else { + error!( + ?result, + "Failed to load character data for character id {}", character_id + ); + Err(Error::CharacterDataError) + } } /// Loads a list of characters belonging to the player. This data is a small diff --git a/server/src/persistence/models.rs b/server/src/persistence/models.rs index d5267c24a2..1d7e3e7043 100644 --- a/server/src/persistence/models.rs +++ b/server/src/persistence/models.rs @@ -2,7 +2,7 @@ extern crate serde_json; use super::schema::{body, character, inventory, loadout, stats}; use crate::comp; -use common::{character::Character as CharacterData, LoadoutBuilder}; +use common::character::Character as CharacterData; use diesel::sql_types::Text; use serde::{Deserialize, Serialize}; use tracing::warn; @@ -212,14 +212,7 @@ where bytes: Option<&::RawValue>, ) -> diesel::deserialize::Result { let t = String::from_sql(bytes)?; - - match serde_json::from_str(&t) { - Ok(data) => Ok(Self(data)), - Err(e) => { - warn!(?e, "Failed to deserialise inventory data"); - Ok(Self(comp::Inventory::default())) - }, - } + serde_json::from_str(&t).map_err(Box::from) } } @@ -298,23 +291,7 @@ where bytes: Option<&::RawValue>, ) -> diesel::deserialize::Result { let t = String::from_sql(bytes)?; - - match serde_json::from_str(&t) { - Ok(data) => Ok(Self(data)), - Err(e) => { - warn!(?e, "Failed to deserialise loadout data"); - - // We don't have a weapon reference here, so we default to sword - let loadout = LoadoutBuilder::new() - .defaults() - .active_item(LoadoutBuilder::default_item_config_from_str(Some( - "common.items.weapons.sword.starter_sword", - ))) - .build(); - - Ok(Self(loadout)) - }, - } + serde_json::from_str(&t).map_err(Box::from) } }