Rework storage of achievement progress for characters, now just storing

a single row for each character with a JSON object with information on
achievement progress.

Work on the process of merging a character's achievement progress with
the master list of achievements when it is returned to the client.

common/src/comp/achievement.rs

@@ -1,13 +1,13 @@
 use crate::comp::item::{Consumable, Item, ItemKind};
 use hashbrown::HashMap;
 use serde::{Deserialize, Serialize};
-use specs::{Component, Entity, FlaggedStorage};
+use specs::{Component, Entity};
 use specs_idvs::IdvStorage;
 
 /// Used for in-game events that contribute towards player achievements.
 ///
-/// For example, when an `InventoryManip` is detected, we record that event in
+/// For example, when an item is collected in the world, we a trigger
-/// order to process achievements which depend on collecting items.
+/// to process an entities achievements which depend on collecting items.
 pub struct AchievementTrigger {
     pub entity: Entity,
     pub event: AchievementEvent,
@@ -75,8 +75,18 @@ pub struct CharacterAchievement {
     pub progress: usize,
 }
 
+impl From<&Achievement> for CharacterAchievement {
+    fn from(achievement: &Achievement) -> Self {
+        Self {
+            achievement: achievement.clone(),
+            completed: false,
+            progress: 0,
+        }
+    }
+}
+
 impl CharacterAchievement {
-    /// Increment the progress of this Achievement based on its type
+    /// Increment the progress of this item based on its type
     ///
     /// By default, when an achievement is incremented, its `progress` value is
     /// incremented by 1. This covers many cases, but using this method allows
@@ -104,18 +114,6 @@ impl CharacterAchievement {
     }
 }
 
-/// For initialisation of a new CharacterAchievement item when a new achievement
-/// is progressed
-impl From<Achievement> for CharacterAchievement {
-    fn from(achievement: Achievement) -> Self {
-        Self {
-            achievement,
-            completed: false,
-            progress: 0,
-        }
-    }
-}
-
 /// Each character is assigned an achievement list, which holds information
 /// about which achievements that the player has made some progress on, or
 /// completed.
@@ -127,11 +125,13 @@ impl Default for AchievementList {
 }
 
 impl AchievementList {
-    pub fn from(data: HashMap<String, CharacterAchievement>) -> Self { Self(data) }
+    pub fn new(items: HashMap<String, CharacterAchievement>) -> Self { Self(items) }
+
+    pub fn items(&self) -> HashMap<String, CharacterAchievement> { self.0.clone() }
 }
 
 impl Component for AchievementList {
-    type Storage = FlaggedStorage<Self, IdvStorage<Self>>; // TODO check
+    type Storage = IdvStorage<Self>;
 }
 
 impl AchievementList {
@@ -150,11 +150,17 @@ impl AchievementList {
         achievement: &Achievement,
         event: &AchievementEvent,
     ) -> Option<CharacterAchievement> {
+        tracing::info!(?achievement, "Processing achievement");
+
         let uuid = achievement.uuid.clone();
 
         self.0
             .entry(uuid)
-            .or_insert(CharacterAchievement::from(achievement.clone()))
+            .or_insert(CharacterAchievement {
+                achievement: achievement.clone(),
+                completed: false,
+                progress: 0,
+            })
             .increment_progress(event)
             .cloned()
     }

common/src/state.rs

@@ -158,6 +158,7 @@ impl State {
         ecs.register::<comp::ChatMode>();
         ecs.register::<comp::Group>();
         ecs.register::<comp::Faction>();
+        ecs.register::<comp::AchievementList>();
 
         // Register synced resources used by the ECS.
         ecs.insert(TimeOfDay(0.0));

server/src/events/player.rs

@@ -67,17 +67,25 @@ pub fn handle_client_disconnect(server: &mut Server, entity: EcsEntity) -> Event
     }
 
     // Sync the player's character data to the database
-    if let (Some(player), Some(stats), Some(inventory), Some(loadout), updater) = (
+    if let (
+        Some(player),
+        Some(stats),
+        Some(inventory),
+        Some(loadout),
+        Some(achievement_list),
+        updater,
+    ) = (
         state.read_storage::<Player>().get(entity),
         state.read_storage::<comp::Stats>().get(entity),
         state.read_storage::<comp::Inventory>().get(entity),
         state.read_storage::<comp::Loadout>().get(entity),
+        state.read_storage::<comp::AchievementList>().get(entity),
         state
             .ecs()
             .read_resource::<persistence::character::CharacterUpdater>(),
     ) {
         if let Some(character_id) = player.character_id {
-            updater.update(character_id, stats, inventory, loadout);
+            updater.update(character_id, stats, inventory, loadout, achievement_list);
         }
     }
 

server/src/lib.rs

@@ -40,6 +40,7 @@ use common::{
 use futures_executor::block_on;
 use futures_timer::Delay;
 use futures_util::{select, FutureExt};
+use hashbrown::HashSet;
 use metrics::{ServerMetrics, TickMetrics};
 use network::{Address, Network, Pid};
 use persistence::{
@@ -90,8 +91,8 @@ pub struct Server {
     metrics: ServerMetrics,
     tick_metrics: TickMetrics,
 
-    /// Holds a list of all available achievements
+    /// A list of all achievements available to characetrs on this server.
-    achievement_data: Vec<comp::Achievement>,
+    achievement_data: HashSet<comp::CharacterAchievement>,
 }
 
 impl Server {
@@ -266,11 +267,14 @@ impl Server {
         debug!("Syncing Achievement data...");
 
         let achievement_data = match persistence::achievement::sync(&settings.persistence_db_dir) {
-            Ok(achievements) => achievements,
+            Ok(achievements) => achievements
+                .iter()
+                .map(comp::CharacterAchievement::from)
+                .collect::<HashSet<comp::CharacterAchievement>>(),
             Err(e) => {
-                warn!(?e, "Achievement data migration error");
+                warn!(?e, "Achievement data migration or load error");
 
-                Vec::new()
+                HashSet::new()
             },
         };
 
@@ -445,8 +449,8 @@ impl Server {
 
         for trigger in achievement_events {
             // Get the achievement that matches this event
-            for achievement in &self.achievement_data {
+            for item in &self.achievement_data {
-                if achievement.matches_event(&trigger.event) {
+                if item.achievement.matches_event(&trigger.event) {
                     // Calls to `process_achievement` return true to indicate that the
                     // achievement is complete. In this case, we notify the client to notify them of
                     // completing the achievement
@@ -457,7 +461,7 @@ impl Server {
                         .get_mut(trigger.entity)
                     {
                         if let Some(character_achievement) =
-                            achievement_list.process_achievement(achievement, &trigger.event)
+                            achievement_list.process_achievement(&item.achievement, &trigger.event)
                         {
                             self.notify_client(
                                 trigger.entity,
@@ -526,10 +530,22 @@ impl Server {
                     entity,
                     result,
                 )) => match result {
-                    Ok(achievement_data) => self.notify_client(
+                    Ok(character_achievements) => {
-                        entity,
+                        // Merge the character's achievement data with the server list
-                        ServerMsg::CharacterAchievementDataLoaded(achievement_data),
+
-                    ),
+                        tracing::info!(?character_achievements, "character_achievements");
+                        tracing::info!(?self.achievement_data, "achievement_data");
+
+                        self.notify_client(
+                            entity,
+                            ServerMsg::CharacterAchievementDataLoaded(comp::AchievementList::new(
+                                self.achievement_data
+                                    .union(&character_achievements)
+                                    .map(|ca| (ca.achievement.uuid.to_owned(), ca.clone()))
+                                    .collect(),
+                            )),
+                        )
+                    },
                     Err(error) => self.notify_client(
                         entity,
                         ServerMsg::CharacterAchievementDataError(error.to_string()),

server/src/migrations/2020-06-25-172916_character_achievement/down.sql

@@ -1 +0,0 @@
-DROP TABLE IF EXISTS "character_achievement";

server/src/migrations/2020-06-25-172916_character_achievement/up.sql

@@ -1,8 +0,0 @@
-CREATE TABLE IF NOT EXISTS "character_achievement" (
-    character_id INTEGER PRIMARY KEY NOT NULL,
-    achievement_uuid TEXT NOT NULL,
-    completed INTEGER NOT NULL DEFAULT 0,
-    progress INTEGER NOT NULL DEFAULT 0,
-    FOREIGN KEY(character_id) REFERENCES "character"(id) ON DELETE CASCADE,
-    FOREIGN KEY(achievement_uuid) REFERENCES "achievement"(uuid) ON DELETE CASCADE
-);

server/src/migrations/2020-06-25-172916_character_achievements/down.sql

@@ -0,0 +1 @@
+DROP TABLE IF EXISTS "character_achievements";

server/src/migrations/2020-06-25-172916_character_achievements/up.sql

@@ -0,0 +1,5 @@
+CREATE TABLE IF NOT EXISTS "character_achievements" (
+    character_id INTEGER PRIMARY KEY NOT NULL,
+    items TEXT NOT NULL,
+    FOREIGN KEY(character_id) REFERENCES "character"(id) ON DELETE CASCADE
+);

server/src/persistence/achievement.rs

@@ -4,8 +4,7 @@ use super::{
     error::Error,
     establish_connection,
     models::{
-        Achievement as AchievementModel, CharacterAchievement, CharacterAchievementJoinData,
+        Achievement as AchievementModel, CharacterAchievements, DataMigration, NewDataMigration,
-        DataMigration, NewDataMigration,
     },
     schema,
 };
@@ -15,6 +14,7 @@ use diesel::{
     prelude::*,
     result::{DatabaseErrorKind, Error as DieselError},
 };
+use hashbrown::HashSet;
 use std::{
     collections::hash_map::DefaultHasher,
     hash::{Hash, Hasher},
@@ -29,7 +29,10 @@ enum AchievementLoaderRequestKind {
     },
 }
 
-type LoadCharacterAchievementsResult = (specs::Entity, Result<comp::AchievementList, Error>);
+type LoadCharacterAchievementsResult = (
+    specs::Entity,
+    Result<HashSet<comp::CharacterAchievement>, Error>,
+);
 
 /// Wrapper for results
 #[derive(Debug)]
@@ -100,26 +103,15 @@ impl Drop for AchievementLoader {
 fn load_character_achievement_list(
     character_id: i32,
     db_dir: &str,
-) -> Result<comp::AchievementList, Error> {
+) -> Result<HashSet<comp::CharacterAchievement>, Error> {
-    let character_achievements = schema::character_achievement::dsl::character_achievement
+    let character_achievements = schema::character_achievements::dsl::character_achievements
-        .filter(schema::character_achievement::character_id.eq(character_id))
+        .filter(schema::character_achievements::character_id.eq(character_id))
-        .inner_join(schema::achievements::table)
+        .first::<CharacterAchievements>(&establish_connection(db_dir))?;
-        .load::<(CharacterAchievement, AchievementModel)>(&establish_connection(db_dir))?;
 
-    Ok(comp::AchievementList::from(
+    let result: HashSet<comp::CharacterAchievement> =
-        character_achievements
+        character_achievements.items.0.iter().cloned().collect();
-            .iter()
+
-            .map(|(character_achievement, achievement)| {
+    Ok(result)
-                (
-                    achievement.uuid.clone(),
-                    comp::CharacterAchievement::from(CharacterAchievementJoinData {
-                        character_achievement,
-                        achievement,
-                    }),
-                )
-            })
-            .collect(),
-    ))
 }
 
 pub fn sync(db_dir: &str) -> Result<Vec<comp::Achievement>, Error> {

server/src/persistence/character.rs

@@ -11,8 +11,8 @@ use super::{
     error::Error,
     establish_connection,
     models::{
-        Body, Character, Inventory, InventoryUpdate, Loadout, LoadoutUpdate, NewCharacter,
+        Body, Character, CharacterAchievements, Inventory, InventoryUpdate, Loadout, LoadoutUpdate,
-        NewLoadout, Stats, StatsJoinData, StatsUpdate,
+        NewCharacter, NewLoadout, Stats, StatsJoinData, StatsUpdate,
     },
     schema,
 };
@@ -446,7 +446,12 @@ fn check_character_limit(uuid: &str, db_dir: &str) -> Result<(), Error> {
     }
 }
 
-type CharacterUpdateData = (StatsUpdate, InventoryUpdate, LoadoutUpdate);
+type CharacterUpdateData = (
+    StatsUpdate,
+    InventoryUpdate,
+    LoadoutUpdate,
+    CharacterAchievements,
+);
 
 /// A unidirectional messaging resource for saving characters in a
 /// background thread.
@@ -476,16 +481,25 @@ impl CharacterUpdater {
     /// Updates a collection of characters based on their id and components
     pub fn batch_update<'a>(
         &self,
-        updates: impl Iterator<Item = (i32, &'a comp::Stats, &'a comp::Inventory, &'a comp::Loadout)>,
+        updates: impl Iterator<
+            Item = (
+                i32,
+                &'a comp::Stats,
+                &'a comp::Inventory,
+                &'a comp::Loadout,
+                &'a comp::AchievementList,
+            ),
+        >,
     ) {
         let updates = updates
-            .map(|(id, stats, inventory, loadout)| {
+            .map(|(id, stats, inventory, loadout, achievements)| {
                 (
                     id,
                     (
                         StatsUpdate::from(stats),
                         InventoryUpdate::from(inventory),
                         LoadoutUpdate::from((id, loadout)),
+                        CharacterAchievements::from((id, achievements)),
                     ),
                 )
             })
@@ -503,8 +517,15 @@ impl CharacterUpdater {
         stats: &comp::Stats,
         inventory: &comp::Inventory,
         loadout: &comp::Loadout,
+        achievements: &comp::AchievementList,
     ) {
-        self.batch_update(std::iter::once((character_id, stats, inventory, loadout)));
+        self.batch_update(std::iter::once((
+            character_id,
+            stats,
+            inventory,
+            loadout,
+            achievements,
+        )));
     }
 }
 
@@ -513,12 +534,16 @@ fn batch_update(updates: impl Iterator<Item = (i32, CharacterUpdateData)>, db_di
 
     if let Err(e) = connection.transaction::<_, diesel::result::Error, _>(|| {
         updates.for_each(
-            |(character_id, (stats_update, inventory_update, loadout_update))| {
+            |(
+                character_id,
+                (stats_update, inventory_update, loadout_update, achievements_update),
+            )| {
                 update(
                     character_id,
                     &stats_update,
                     &inventory_update,
                     &loadout_update,
+                    &achievements_update,
                     &connection,
                 )
             },
@@ -535,6 +560,7 @@ fn update(
     stats: &StatsUpdate,
     inventory: &InventoryUpdate,
     loadout: &LoadoutUpdate,
+    achievements: &CharacterAchievements,
     connection: &SqliteConnection,
 ) {
     // Update Stats
@@ -569,6 +595,19 @@ fn update(
     {
         warn!(?e, ?character_id, "Failed to update loadout for character",)
     }
+
+    // Update Achievements. Use `replace_into` as 'insert or update' as the
+    // characetr may not have a row
+    if let Err(e) = diesel::replace_into(schema::character_achievements::table)
+        .values(achievements)
+        .execute(connection)
+    {
+        warn!(
+            ?e,
+            ?character_id,
+            "Failed to update achievements for character",
+        )
+    }
 }
 
 impl Drop for CharacterUpdater {

server/src/persistence/models.rs

@@ -1,7 +1,8 @@
 extern crate serde_json;
 
 use super::schema::{
-    achievements, body, character, character_achievement, data_migration, inventory, loadout, stats,
+    achievements, body, character, character_achievements, data_migration, inventory, loadout,
+    stats,
 };
 use crate::comp;
 use chrono::NaiveDateTime;
@@ -436,32 +437,77 @@ impl From<&Achievement> for comp::Achievement {
 /// In the interest of storing as little achievement data per-character as
 /// pssible, we only store a 'character_achievement' entry when a character has
 /// made some progress towards the completion of an achievement.
-#[derive(Queryable, Debug, Identifiable)]
+#[derive(Associations, AsChangeset, Identifiable, Queryable, Debug, Insertable)]
+#[belongs_to(Character)]
 #[primary_key(character_id)]
-#[table_name = "character_achievement"]
+#[table_name = "character_achievements"]
-pub struct CharacterAchievement {
+pub struct CharacterAchievements {
-    pub character_id: i32,
+    character_id: i32,
-    pub achievement_uuid: String,
+    pub items: CharacterAchievementData,
-    pub completed: i32,
-    pub progress: i32,
 }
 
-/// The required elements to build comp::CharacterAchievement from database data
+/// The representation of each achievement in the Vec of achievement items
-pub struct CharacterAchievementJoinData<'a> {
+/// within achievement data. The structure of that data follows the format:
-    pub character_achievement: &'a CharacterAchievement,
+/// {
-    pub achievement: &'a Achievement,
+///     character_id,
+///     items: [ CharacterAchievementItem ]
+/// }
+#[derive(Debug, Deserialize, Serialize, PartialEq)]
+pub struct CharacterAchievementItem {
+    achievement_uuid: String,
+    progress: i32,
 }
 
-impl From<CharacterAchievementJoinData<'_>> for comp::CharacterAchievement {
+/// When persisting the component to the database
-    fn from(data: CharacterAchievementJoinData) -> comp::CharacterAchievement {
+impl From<(i32, &comp::AchievementList)> for CharacterAchievements {
-        comp::CharacterAchievement {
+    fn from(data: (i32, &comp::AchievementList)) -> CharacterAchievements {
-            achievement: comp::Achievement::from(data.achievement),
+        let (character_id, achievements) = data;
-            completed: data.character_achievement.completed == 1,
+
-            progress: data.character_achievement.progress as usize,
+        let items = achievements
+            .items()
+            .iter()
+            .map(|(_, char_achievement)| char_achievement.clone())
+            .collect::<Vec<_>>();
+
+        CharacterAchievements {
+            character_id,
+            items: CharacterAchievementData(items),
         }
     }
 }
 
+/// A wrapper type for Inventory components used to serialise to and from JSON
+/// If the column contains malformed JSON, a default inventory is returned
+#[derive(SqlType, AsExpression, Debug, Deserialize, Serialize, FromSqlRow, PartialEq)]
+#[sql_type = "Text"]
+pub struct CharacterAchievementData(pub Vec<comp::CharacterAchievement>);
+
+impl<DB> diesel::deserialize::FromSql<Text, DB> for CharacterAchievementData
+where
+    DB: diesel::backend::Backend,
+    String: diesel::deserialize::FromSql<Text, DB>,
+{
+    fn from_sql(
+        bytes: Option<&<DB as diesel::backend::Backend>::RawValue>,
+    ) -> diesel::deserialize::Result<Self> {
+        let t = String::from_sql(bytes)?;
+        serde_json::from_str(&t).map_err(Box::from)
+    }
+}
+
+impl<DB> diesel::serialize::ToSql<Text, DB> for CharacterAchievementData
+where
+    DB: diesel::backend::Backend,
+{
+    fn to_sql<W: std::io::Write>(
+        &self,
+        out: &mut diesel::serialize::Output<W, DB>,
+    ) -> diesel::serialize::Result {
+        let s = serde_json::to_string(&self.0)?;
+        <String as diesel::serialize::ToSql<Text, DB>>::to_sql(&s, out)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

server/src/persistence/schema.rs

@@ -32,14 +32,11 @@ table! {
 }
 
 table! {
-    character_achievement (character_id) {
+    character_achievements (character_id) {
         character_id -> Integer,
-        achievement_uuid -> Text,
+        items -> Text,
-        completed -> Integer,
-        progress -> Integer,
     }
 }
-
 table! {
     data_migration (id) {
         id -> Integer,
@@ -77,8 +74,7 @@ table! {
 }
 
 joinable!(body -> character (character_id));
-joinable!(character_achievement -> character (character_id));
+joinable!(character_achievements -> character (character_id));
-joinable!(character_achievement -> achievements (achievement_uuid));
 joinable!(inventory -> character (character_id));
 joinable!(loadout -> character (character_id));
 joinable!(stats -> character (character_id));
@@ -87,7 +83,7 @@ allow_tables_to_appear_in_same_query!(
     achievements,
     body,
     character,
-    character_achievement,
+    character_achievements,
     inventory,
     loadout,
     stats,

server/src/sys/persistence.rs

@@ -2,7 +2,7 @@ use crate::{
     persistence::character,
     sys::{SysScheduler, SysTimer},
 };
-use common::comp::{Inventory, Loadout, Player, Stats};
+use common::comp::{AchievementList, Inventory, Loadout, Player, Stats};
 use specs::{Join, ReadExpect, ReadStorage, System, Write};
 
 pub struct Sys;
@@ -14,6 +14,7 @@ impl<'a> System<'a> for Sys {
         ReadStorage<'a, Stats>,
         ReadStorage<'a, Inventory>,
         ReadStorage<'a, Loadout>,
+        ReadStorage<'a, AchievementList>,
         ReadExpect<'a, character::CharacterUpdater>,
         Write<'a, SysScheduler<Self>>,
         Write<'a, SysTimer<Self>>,
@@ -26,6 +27,7 @@ impl<'a> System<'a> for Sys {
             player_stats,
             player_inventories,
             player_loadouts,
+            player_achievements,
             updater,
             mut scheduler,
             mut timer,
@@ -33,19 +35,23 @@ impl<'a> System<'a> for Sys {
     ) {
         if scheduler.should_run() {
             timer.start();
+
             updater.batch_update(
                 (
                     &players,
                     &player_stats,
                     &player_inventories,
                     &player_loadouts,
+                    &player_achievements,
                 )
                     .join()
-                    .filter_map(|(player, stats, inventory, loadout)| {
+                    .filter_map(
-                        player
+                        |(player, stats, inventory, loadout, achievements)| {
-                            .character_id
+                            player
-                            .map(|id| (id, stats, inventory, loadout))
+                                .character_id
-                    }),
+                                .map(|id| (id, stats, inventory, loadout, achievements))
+                        },
+                    ),
             );
             timer.end();
         }