Persitence now tracks spent_exp as an additional point to potentially invalidate skills.

common/src/comp/skillset/mod.rs

@@ -476,71 +476,3 @@ pub enum SpRewardError {
     InsufficientExp,
     UnavailableSkillGroup,
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    // Code reviewers: Open a comment here, I want to refactor these tests
-
-    #[test]
-    fn test_refund_skill() {
-        let mut skillset = SkillSet::default();
-        skillset.unlock_skill_group(SkillGroupKind::Weapon(ToolKind::Axe));
-        skillset.add_skill_points(SkillGroupKind::Weapon(ToolKind::Axe), 1);
-        skillset.unlock_skill(Skill::Axe(AxeSkill::UnlockLeap));
-
-        assert_eq!(skillset.skill_groups[2].available_sp, 0);
-        assert_eq!(skillset.skills.len(), 1);
-        assert!(skillset.has_skill(Skill::Axe(AxeSkill::UnlockLeap)));
-
-        skillset.refund_skill(Skill::Axe(AxeSkill::UnlockLeap));
-
-        assert_eq!(skillset.skill_groups[2].available_sp, 1);
-        assert_eq!(skillset.skills.get(&Skill::Axe(AxeSkill::UnlockLeap)), None);
-    }
-
-    #[test]
-    fn test_unlock_skillgroup() {
-        let mut skillset = SkillSet::default();
-        skillset.unlock_skill_group(SkillGroupKind::Weapon(ToolKind::Axe));
-
-        assert_eq!(skillset.skill_groups.len(), 3);
-        assert_eq!(
-            skillset.skill_groups[2],
-            SkillGroup::new(SkillGroupKind::Weapon(ToolKind::Axe))
-        );
-    }
-
-    #[test]
-    fn test_unlock_skill() {
-        let mut skillset = SkillSet::default();
-
-        skillset.unlock_skill_group(SkillGroupKind::Weapon(ToolKind::Axe));
-        skillset.add_skill_points(SkillGroupKind::Weapon(ToolKind::Axe), 1);
-
-        assert_eq!(skillset.skill_groups[2].available_sp, 1);
-        assert_eq!(skillset.skills.len(), 0);
-
-        // Try unlocking a skill with enough skill points
-        skillset.unlock_skill(Skill::Axe(AxeSkill::UnlockLeap));
-
-        assert_eq!(skillset.skill_groups[2].available_sp, 0);
-        assert_eq!(skillset.skills.len(), 1);
-        assert!(skillset.has_skill(Skill::Axe(AxeSkill::UnlockLeap)));
-
-        // Try unlocking a skill without enough skill points
-        skillset.unlock_skill(Skill::Axe(AxeSkill::DsCombo));
-
-        assert_eq!(skillset.skills.len(), 1);
-        assert_eq!(skillset.skills.get(&Skill::Axe(AxeSkill::DsCombo)), None);
-    }
-
-    #[test]
-    fn test_add_skill_points() {
-        let mut skillset = SkillSet::default();
-        skillset.unlock_skill_group(SkillGroupKind::Weapon(ToolKind::Axe));
-        skillset.add_skill_points(SkillGroupKind::Weapon(ToolKind::Axe), 1);
-
-        assert_eq!(skillset.skill_groups[2].available_sp, 1);
-    }
-}

server/src/migrations/V46__skill_persistence.sql

@@ -40,6 +40,7 @@ CREATE TABLE _skill_group
 	entity_id	        INTEGER NOT NULL,
 	skill_group_kind	TEXT NOT NULL,
     earned_exp          INTEGER NOT NULL,
+    spent_exp           INTEGER NOT NULL,
     skills              TEXT NOT NULL,
     hash_val            TEXT NOT NULL,
 	FOREIGN KEY(entity_id) REFERENCES entity(entity_id),
@@ -47,7 +48,7 @@ CREATE TABLE _skill_group
 );
 
 INSERT INTO _skill_group
-SELECT sg.entity_id, sg.skill_group_kind, sg.exp, "", ""
+SELECT sg.entity_id, sg.skill_group_kind, sg.exp, 0, "", ""
 FROM skill_group sg;
 
 -- Skills now tracked in skill_group table, can ust drop

server/src/persistence/character.rs

@@ -166,6 +166,7 @@ pub fn load_character_data(
         "
         SELECT  skill_group_kind,
                 earned_exp,
+                spent_exp,
                 skills,
                 hash_val
         FROM    skill_group
@@ -178,8 +179,9 @@ pub fn load_character_data(
                 entity_id: char_id,
                 skill_group_kind: row.get(0)?,
                 earned_exp: row.get(1)?,
-                skills: row.get(2)?,
+                spent_exp: row.get(2)?,
-                hash_val: row.get(3)?,
+                skills: row.get(3)?,
+                hash_val: row.get(4)?,
             })
         })?
         .filter_map(Result::ok)
@@ -424,9 +426,10 @@ pub fn create_character(
         INSERT INTO skill_group (entity_id,
                                  skill_group_kind,
                                  earned_exp,
+                                 spent_exp,
                                  skills,
                                  hash_val)
-        VALUES (?1, ?2, ?3, ?4, ?5)",
+        VALUES (?1, ?2, ?3, ?4, ?5, ?6)",
     )?;
 
     for skill_group in db_skill_groups {
@@ -434,6 +437,7 @@ pub fn create_character(
             &character_id as &dyn ToSql,
             &skill_group.skill_group_kind,
             &skill_group.earned_exp,
+            &skill_group.spent_exp,
             &skill_group.skills,
             &skill_group.hash_val,
         ])?;
@@ -1005,9 +1009,10 @@ pub fn update(
         INTO    skill_group (entity_id,
                              skill_group_kind,
                              earned_exp,
+                             spent_exp,
                              skills,
                              hash_val)
-        VALUES (?1, ?2, ?3, ?4, ?5)",
+        VALUES (?1, ?2, ?3, ?4, ?5, ?6)",
     )?;
 
     for skill_group in db_skill_groups {
@@ -1015,6 +1020,7 @@ pub fn update(
             &skill_group.entity_id as &dyn ToSql,
             &skill_group.skill_group_kind,
             &skill_group.earned_exp,
+            &skill_group.spent_exp,
             &skill_group.skills,
             &skill_group.hash_val,
         ])?;

server/src/persistence/character/conversions.rs

@@ -544,20 +544,25 @@ fn convert_skill_groups_from_database(
             ordered_skills: Vec::new(),
         };
 
+        // Convert exp into skill points
         while new_skill_group.earn_skill_point().is_ok() {}
-        new_skill_groups.push(new_skill_group);
 
-        // If the hash stored of the skill group is the same as the current hash of the
+        // If the stored spent exp is the same as the spent exp after adding skill
-        // skill group, don't invalidate skills
+        // points, and the hash stored of the skill group is the same as the current
-        if serde_json::from_str::<Vec<u8>>(&skill_group.hash_val)
+        // hash of the skill group, don't invalidate skills; otherwise invalidate the
-            .ok()
+        // skills in this skill_group.
-            .as_ref()
+        if skill_group.spent_exp as u32 == new_skill_group.spent_exp
-            == skillset::SKILL_GROUP_HASHES.get(&skill_group_kind)
+            && serde_json::from_str::<Vec<u8>>(&skill_group.hash_val)
+                .ok()
+                .as_ref()
+                == skillset::SKILL_GROUP_HASHES.get(&skill_group_kind)
         {
             let mut new_skills =
                 serde_json::from_str::<Vec<skills::Skill>>(&skill_group.skills).unwrap_or_default();
             skills.append(&mut new_skills);
         }
+
+        new_skill_groups.push(new_skill_group);
     }
     (new_skill_groups, skills)
 }
@@ -572,6 +577,7 @@ pub fn convert_skill_groups_to_database(
             entity_id,
             skill_group_kind: json_models::skill_group_to_db_string(sg.skill_group_kind),
             earned_exp: sg.earned_exp as i32,
+            spent_exp: sg.spent_exp as i32,
             // If fails to convert, just forces a respec on next login
             skills: serde_json::to_string(&sg.ordered_skills).unwrap_or_else(|_| "".to_string()),
             hash_val: serde_json::to_string(

server/src/persistence/models.rs

@@ -24,6 +24,7 @@ pub struct SkillGroup {
     pub entity_id: i64,
     pub skill_group_kind: String,
     pub earned_exp: i32,
+    pub spent_exp: i32,
     pub skills: String,
     pub hash_val: String,
 }