From 143ecd6e34ab83396755c4c4e946e409a7dd4ebf Mon Sep 17 00:00:00 2001 From: Ben Wallis Date: Mon, 16 Nov 2020 18:49:00 +0000 Subject: [PATCH] Moved waypoint persistence to new waypoint column on stats table --- .../down.sql | 38 ++++++++++++ .../up.sql | 21 +++++++ server/src/persistence/character.rs | 60 +++++++------------ .../src/persistence/character/conversions.rs | 35 +++++++++-- server/src/persistence/models.rs | 1 + server/src/persistence/schema.rs | 1 + 6 files changed, 112 insertions(+), 44 deletions(-) create mode 100644 server/src/migrations/2020-11-16-173524_move_waypoint_to_stats/down.sql create mode 100644 server/src/migrations/2020-11-16-173524_move_waypoint_to_stats/up.sql diff --git a/server/src/migrations/2020-11-16-173524_move_waypoint_to_stats/down.sql b/server/src/migrations/2020-11-16-173524_move_waypoint_to_stats/down.sql new file mode 100644 index 0000000000..7374a28efc --- /dev/null +++ b/server/src/migrations/2020-11-16-173524_move_waypoint_to_stats/down.sql @@ -0,0 +1,38 @@ +-- Put waypoint data back into item table +UPDATE item +SET position = ( SELECT s.waypoint + FROM stats s + WHERE s.stats_id = item.item_id + AND item.item_definition_id = 'veloren.core.pseudo_containers.character' + AND s.waypoint IS NOT NULL) +WHERE EXISTS ( SELECT s.waypoint + FROM stats s + WHERE s.stats_id = item.item_id + AND item.item_definition_id = 'veloren.core.pseudo_containers.character' + AND s.waypoint IS NOT NULL); + +-- SQLite does not support dropping columns on tables so the entire table must be +-- dropped and recreated without the 'waypoint' column +CREATE TABLE stats_new +( + stats_id INT NOT NULL + PRIMARY KEY + REFERENCES entity, + level INT NOT NULL, + exp INT NOT NULL, + endurance INT NOT NULL, + fitness INT NOT NULL, + willpower INT NOT NULL +); + +INSERT INTO stats_new (stats_id, level, exp, endurance, fitness, willpower) +SELECT stats_id, + level, + exp, + endurance, + fitness, + willpower +FROM stats; + +DROP TABLE stats; +ALTER TABLE stats_new RENAME TO stats; \ No newline at end of file diff --git a/server/src/migrations/2020-11-16-173524_move_waypoint_to_stats/up.sql b/server/src/migrations/2020-11-16-173524_move_waypoint_to_stats/up.sql new file mode 100644 index 0000000000..1edaa750e2 --- /dev/null +++ b/server/src/migrations/2020-11-16-173524_move_waypoint_to_stats/up.sql @@ -0,0 +1,21 @@ +-- Add a 'waypoint' column to the 'stats' table +ALTER TABLE stats ADD COLUMN waypoint TEXT NULL; + +-- Move any waypoints persisted into the item.position column into the new stats.waypoint column +UPDATE stats +SET waypoint = ( SELECT i.position + FROM item i + WHERE i.item_id = stats.stats_id + AND i.position != i.item_id + AND i.item_definition_id = 'veloren.core.pseudo_containers.character') +WHERE EXISTS ( SELECT i.position + FROM item i + WHERE i.item_id = stats.stats_id + AND i.position != i.item_id + AND i.item_definition_id = 'veloren.core.pseudo_containers.character'); + +-- Reset the 'item.position' column value for character pseudo-containers to the character id to +-- remove old waypoint data that has now been migrated to stats +UPDATE item +SET position = item_id +WHERE item_definition_id = 'veloren.core.pseudo_containers.character'; \ No newline at end of file diff --git a/server/src/persistence/character.rs b/server/src/persistence/character.rs index 6b9294f508..c52bacd89d 100644 --- a/server/src/persistence/character.rs +++ b/server/src/persistence/character.rs @@ -15,23 +15,21 @@ use crate::{ convert_character_from_database, convert_inventory_from_database_items, convert_items_to_database_items, convert_loadout_from_database_items, convert_stats_from_database, convert_stats_to_database, - convert_waypoint_to_database_json, + convert_waypoint_from_database_json, }, character_loader::{CharacterCreationResult, CharacterDataResult, CharacterListResult}, error::Error::DatabaseError, - json_models::CharacterPosition, PersistedComponents, }, }; use common::{ character::{CharacterId, CharacterItem, MAX_CHARACTERS_PER_PLAYER}, comp::item::tool::AbilityMap, - state::Time, }; use core::ops::Range; use diesel::{prelude::*, sql_query, sql_types::BigInt}; use std::sync::Arc; -use tracing::{error, trace}; +use tracing::{error, trace, warn}; /// Private module for very tightly coupled database conversion methods. In /// general, these have many invariants that need to be maintained when they're @@ -90,22 +88,27 @@ pub fn load_character_data( .filter(schema::body::dsl::body_id.eq(char_id)) .first::(&*connection)?; - let waypoint = item - .filter(item_id.eq(char_id)) - .first::(&*connection) - .ok() - .and_then(|it: Item| { - (serde_json::de::from_str::(it.position.as_str())) - .ok() - .map(|charpos| comp::Waypoint::new(charpos.waypoint, Time(0.0))) - }); + let char_waypoint = + stats_data + .waypoint + .as_ref() + .and_then(|x| match convert_waypoint_from_database_json(&x) { + Ok(w) => Some(w), + Err(e) => { + warn!( + "Error reading waypoint from database for character ID {}, error: {}", + char_id, e + ); + None + }, + }); Ok(( convert_body_from_database(&char_body)?, convert_stats_from_database(&stats_data, character_data.alias), convert_inventory_from_database_items(&inventory_items)?, convert_loadout_from_database_items(&loadout_items, map)?, - waypoint, + char_waypoint, )) } @@ -186,17 +189,14 @@ pub fn create_character( let character_id = new_entity_ids.next().unwrap(); let inventory_container_id = new_entity_ids.next().unwrap(); let loadout_container_id = new_entity_ids.next().unwrap(); - // by default the character's position is the id in textual form - let character_position = waypoint - .and_then(|waypoint| serde_json::to_string(&waypoint.get_pos()).ok()) - .unwrap_or_else(|| character_id.to_string()); + let pseudo_containers = vec![ Item { stack_size: 1, item_id: character_id, parent_container_item_id: WORLD_PSEUDO_CONTAINER_ID, item_definition_id: CHARACTER_PSEUDO_CONTAINER_DEF_ID.to_owned(), - position: character_position, + position: character_id.to_string(), }, Item { stack_size: 1, @@ -225,7 +225,7 @@ pub fn create_character( } // Insert stats record - let db_stats = convert_stats_to_database(character_id, &stats); + let db_stats = convert_stats_to_database(character_id, &stats, &waypoint)?; let stats_count = diesel::insert_into(stats::table) .values(&db_stats) .execute(&*connection)?; @@ -535,7 +535,7 @@ pub fn update( char_stats: comp::Stats, inventory: comp::Inventory, loadout: comp::Loadout, - waypoint: Option, + char_waypoint: Option, connection: VelorenTransaction, ) -> Result>, Error> { use super::schema::{item::dsl::*, stats::dsl::*}; @@ -558,22 +558,6 @@ pub fn update( next_id })?; - if let Some(waypoint) = waypoint { - match convert_waypoint_to_database_json(&waypoint) { - Ok(character_position) => { - diesel::update(item.filter(item_id.eq(char_id))) - .set(position.eq(character_position)) - .execute(&*connection)?; - }, - Err(err) => { - return Err(Error::ConversionError(format!( - "Error encoding waypoint: {:?}", - err - ))); - }, - } - } - // Next, delete any slots we aren't upserting. trace!("Deleting items for character_id {}", char_id); let existing_items = parent_container_item_id @@ -617,7 +601,7 @@ pub fn update( } } - let db_stats = convert_stats_to_database(char_id, &char_stats); + let db_stats = convert_stats_to_database(char_id, &char_stats, &char_waypoint)?; let stats_count = diesel::update(stats.filter(stats_id.eq(char_id))) .set(db_stats) .execute(&*connection)?; diff --git a/server/src/persistence/character/conversions.rs b/server/src/persistence/character/conversions.rs index 59574ff059..d49aeb64f8 100644 --- a/server/src/persistence/character/conversions.rs +++ b/server/src/persistence/character/conversions.rs @@ -9,8 +9,9 @@ use crate::persistence::{ }; use common::{ character::CharacterId, - comp::{item::tool::AbilityMap, Body as CompBody, *}, + comp::{item::tool::AbilityMap, Body as CompBody, Waypoint, *}, loadout_builder, + state::Time, }; use core::{convert::TryFrom, num::NonZeroU64}; use itertools::{Either, Itertools}; @@ -168,22 +169,44 @@ pub fn convert_body_to_database_json(body: &CompBody) -> Result { serde_json::to_string(&json_model).map_err(Error::SerializationError) } -pub fn convert_waypoint_to_database_json(waypoint: &Waypoint) -> Result { +fn convert_waypoint_to_database_json(waypoint: &Waypoint) -> Result { let charpos = CharacterPosition { waypoint: waypoint.get_pos(), }; - serde_json::to_string(&charpos).map_err(Error::SerializationError) + serde_json::to_string(&charpos) + .map_err(|err| Error::ConversionError(format!("Error encoding waypoint: {:?}", err))) } -pub fn convert_stats_to_database(character_id: CharacterId, stats: &common::comp::Stats) -> Stats { - Stats { +pub fn convert_waypoint_from_database_json(position: &str) -> Result { + let character_position = + serde_json::de::from_str::(position).map_err(|err| { + Error::ConversionError(format!( + "Error de-serializing waypoint: {} err: {}", + position, err + )) + })?; + Ok(Waypoint::new(character_position.waypoint, Time(0.0))) +} + +pub fn convert_stats_to_database( + character_id: CharacterId, + stats: &common::comp::Stats, + waypoint: &Option, +) -> Result { + let waypoint = match waypoint { + Some(w) => Some(convert_waypoint_to_database_json(&w)?), + None => None, + }; + + Ok(Stats { stats_id: character_id, level: stats.level.level() as i32, exp: stats.exp.current() as i32, endurance: stats.endurance as i32, fitness: stats.fitness as i32, willpower: stats.willpower as i32, - } + waypoint, + }) } pub fn convert_inventory_from_database_items(database_items: &[Item]) -> Result { diff --git a/server/src/persistence/models.rs b/server/src/persistence/models.rs index fb594a3cd5..e826ec0f91 100644 --- a/server/src/persistence/models.rs +++ b/server/src/persistence/models.rs @@ -46,6 +46,7 @@ pub struct Stats { pub endurance: i32, pub fitness: i32, pub willpower: i32, + pub waypoint: Option, } #[derive(Associations, Identifiable, Insertable, Queryable, Debug)] diff --git a/server/src/persistence/schema.rs b/server/src/persistence/schema.rs index d13e22bf22..8838656ecf 100644 --- a/server/src/persistence/schema.rs +++ b/server/src/persistence/schema.rs @@ -38,6 +38,7 @@ table! { endurance -> Integer, fitness -> Integer, willpower -> Integer, + waypoint -> Nullable, } }