From 3ac18f73eab75442c4ddd5d892c18ffcc8cd24ec Mon Sep 17 00:00:00 2001
From: Ben Wallis <atomyc@gmail.com>
Date: Mon, 6 Jul 2020 22:06:03 +0100
Subject: [PATCH 1/3] 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<Inventory>, Option<Loadout>)>(&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<&<DB as diesel::backend::Backend>::RawValue>,
) -> diesel::deserialize::Result<Self> {
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<&<DB as diesel::backend::Backend>::RawValue>,
) -> diesel::deserialize::Result<Self> {
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)
}
}
From e83d0abd336d4f7b32ef429f06f41977f719cfdf Mon Sep 17 00:00:00 2001
From: Ben Wallis <atomyc@gmail.com>
Date: Tue, 7 Jul 2020 21:29:16 +0100
Subject: [PATCH 2/3] Added migration to create a default inventory and loadout
for characters without one
---
.../down.sql | 1 +
.../up.sql | 15 +++++++++++++++
2 files changed, 16 insertions(+)
create mode 100644 server/src/migrations/2020-07-07-201052_add_missing_inv_loadout/down.sql
create mode 100644 server/src/migrations/2020-07-07-201052_add_missing_inv_loadout/up.sql
diff --git a/server/src/migrations/2020-07-07-201052_add_missing_inv_loadout/down.sql b/server/src/migrations/2020-07-07-201052_add_missing_inv_loadout/down.sql
new file mode 100644
index 0000000000..7a6b1eb34b
--- /dev/null
+++ b/server/src/migrations/2020-07-07-201052_add_missing_inv_loadout/down.sql
@@ -0,0 +1 @@
+-- Nothing to undo since up.sql only creates missing inventory/loadout records
\ No newline at end of file
diff --git a/server/src/migrations/2020-07-07-201052_add_missing_inv_loadout/up.sql b/server/src/migrations/2020-07-07-201052_add_missing_inv_loadout/up.sql
new file mode 100644
index 0000000000..ade82ad00e
--- /dev/null
+++ b/server/src/migrations/2020-07-07-201052_add_missing_inv_loadout/up.sql
@@ -0,0 +1,15 @@
+-- This migration was added in conjunction with a change that no longer creates missing inventory/loadout records
+-- for characters that don't have one, to prevent such characters from being unable to log in.
+
+-- Create a default loadout for any characters that don't have one
+INSERT INTO loadout (character_id, items)
+SELECT c."id", "{""active_item"":{""item"":{""name"":""Battered Sword"",""description"":""Two-Hand Sword\n\nPower: 2-10\n\nHeld together by Rust and hope.\n\n<Right-Click to use>"",""kind"":{""Tool"":{""kind"":{""Sword"":""BasicSword""},""equip_time_millis"":300}}},""ability1"":{""TripleStrike"":{""base_damage"":5,""needs_timing"":false}},""ability2"":{""DashMelee"":{""energy_cost"":700,""buildup_duration"":{""secs"":0,""nanos"":500000000},""recover_duration"":{""secs"":0,""nanos"":500000000},""base_damage"":10}},""ability3"":null,""block_ability"":""BasicBlock"",""dodge_ability"":""Roll""},""second_item"":null,""shoulder"":null,""chest"":{""name"":""Rugged Shirt"",""description"":""Chest\n\nArmor: 0\n\nSmells like Adventure.\n\n<Right-Click to use>"",""kind"":{""Armor"":{""kind"":{""Chest"":""Rugged0""},""stats"":20}}},""belt"":null,""hand"":null,""pants"":{""name"":""Rugged Commoner's Pants"",""description"":""Legs\n\nArmor: 0\n\nThey remind you of the old days.\n\n<Right-Click to use>"",""kind"":{""Armor"":{""kind"":{""Pants"":""Rugged0""},""stats"":20}}},""foot"":{""name"":""Worn out Sandals"",""description"":""Feet\n\nArmor: 0\n\nLoyal companions.\n\n<Right-Click to use>"",""kind"":{""Armor"":{""kind"":{""Foot"":""Sandal0""},""stats"":20}}},""back"":null,""ring"":null,""neck"":null,""lantern"":{""name"":""Black Lantern"",""description"":""Used by city guards."",""kind"":{""Lantern"":{""kind"":""Black0"",""color"":{""r"":255,""g"":190,""b"":75},""strength_thousandths"":3000,""flicker_thousandths"":300}}},""head"":null,""tabard"":null}"
+FROM character c
+WHERE (SELECT COUNT(1) FROM loadout WHERE character_id = c.id) = 0;
+
+
+-- Create a default inventory for any characters that don't have one
+INSERT INTO inventory (character_id, items)
+SELECT c."id", "{""slots"":[{""name"":""Dwarven Cheese"",""description"":""Restores 15 Health\n\nAromatic and nutritious\n\n<Right-Click to use>"",""kind"":{""Consumable"":{""kind"":""Cheese"",""effect"":{""Health"":{""amount"":15,""cause"":""Item""}},""amount"":1}}},{""name"":""Apple"",""description"":""Restores 20 Health\n\nRed and juicy\n\n<Right-Click to use>"",""kind"":{""Consumable"":{""kind"":""Apple"",""effect"":{""Health"":{""amount"":20,""cause"":""Item""}},""amount"":1}}},null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null],""amount"":2}"
+FROM character c
+WHERE (SELECT COUNT(1) FROM inventory WHERE character_id = c.id) = 0;
\ No newline at end of file
From ddb66ec73b9215247c26a88cfba60271a6c2cd66 Mon Sep 17 00:00:00 2001
From: Ben Wallis <atomyc@gmail.com>
Date: Wed, 8 Jul 2020 08:04:28 +0100
Subject: [PATCH 3/3] Added CharacterDataError when character list fails to
load due to Loadout de-serialization error
---
server/src/persistence/character.rs | 72 ++++++++++++++---------------
1 file changed, 35 insertions(+), 37 deletions(-)
diff --git a/server/src/persistence/character.rs b/server/src/persistence/character.rs
index 13b9f30d95..1fe6f8f657 100644
--- a/server/src/persistence/character.rs
+++ b/server/src/persistence/character.rs
@@ -245,8 +245,8 @@ fn load_character_data(player_uuid: &str, character_id: i32, db_dir: &str) -> Ch
.inner_join(schema::loadout::table)
.first::<(Character, Body, Stats, Inventory, Loadout)>(&connection);
- if let Ok((character_data, body_data, stats_data, inventory, loadout)) = result {
- Ok((
+ match result {
+ Ok((character_data, body_data, stats_data, inventory, loadout)) => Ok((
comp::Body::from(&body_data),
comp::Stats::from(StatsJoinData {
alias: &character_data.alias,
@@ -255,13 +255,15 @@ fn load_character_data(player_uuid: &str, character_id: i32, db_dir: &str) -> Ch
}),
comp::Inventory::from(inventory),
comp::Loadout::from(&loadout),
- ))
- } else {
- error!(
- ?result,
- "Failed to load character data for character id {}", character_id
- );
- Err(Error::CharacterDataError)
+ )),
+ Err(e) => {
+ error!(
+ ?e,
+ ?character_id,
+ "Failed to load character data for character"
+ );
+ Err(Error::CharacterDataError)
+ },
}
}
@@ -273,40 +275,36 @@ fn load_character_data(player_uuid: &str, character_id: i32, db_dir: &str) -> Ch
/// stats, body, etc...) the character is skipped, and no entry will be
/// returned.
fn load_character_list(player_uuid: &str, db_dir: &str) -> CharacterListResult {
- let data = schema::character::dsl::character
+ let result = schema::character::dsl::character
.filter(schema::character::player_uuid.eq(player_uuid))
.order(schema::character::id.desc())
.inner_join(schema::body::table)
.inner_join(schema::stats::table)
- .left_join(schema::loadout::table)
- .load::<(Character, Body, Stats, Option<Loadout>)>(&establish_connection(db_dir))?;
+ .inner_join(schema::loadout::table)
+ .load::<(Character, Body, Stats, Loadout)>(&establish_connection(db_dir));
- Ok(data
- .iter()
- .map(|(character_data, body_data, stats_data, maybe_loadout)| {
- let character = CharacterData::from(character_data);
- let body = comp::Body::from(body_data);
- let level = stats_data.level as usize;
- let loadout = maybe_loadout.as_ref().map_or_else(
- || {
- LoadoutBuilder::new()
- .defaults()
- .active_item(LoadoutBuilder::default_item_config_from_str(
- character.tool.as_deref(),
- ))
- .build()
- },
- comp::Loadout::from,
- );
+ match result {
+ Ok(data) => Ok(data
+ .iter()
+ .map(|(character_data, body_data, stats_data, loadout)| {
+ let character = CharacterData::from(character_data);
+ let body = comp::Body::from(body_data);
+ let level = stats_data.level as usize;
+ let loadout = comp::Loadout::from(loadout);
- CharacterItem {
- character,
- body,
- level,
- loadout,
- }
- })
- .collect())
+ CharacterItem {
+ character,
+ body,
+ level,
+ loadout,
+ }
+ })
+ .collect()),
+ Err(e) => {
+ error!(?e, ?player_uuid, "Failed to load character list for player");
+ Err(Error::CharacterDataError)
+ },
+ }
}
/// Create a new character with provided comp::Character and comp::Body data.