Made skill groups remain locked if skill to unlock them not acquired.

common/src/comp/skillset/mod.rs

@@ -201,8 +201,8 @@ impl SkillGroup {
 /// refunding skills etc.
 #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
 pub struct SkillSet {
-    pub skill_groups: Vec<SkillGroup>,
+    skill_groups: Vec<SkillGroup>,
-    pub skills: HashMap<Skill, Option<u16>>,
+    skills: HashMap<Skill, Option<u16>>,
     pub modify_health: bool,
     pub modify_energy: bool,
 }
@@ -221,7 +221,7 @@ impl Default for SkillSet {
                 SkillGroup::new(SkillGroupKind::General),
                 SkillGroup::new(SkillGroupKind::Weapon(ToolKind::Pick)),
             ],
-            skills: HashMap::new(),
+            skills: SkillSet::initial_skills(),
             modify_health: false,
             modify_energy: false,
         }
@@ -229,24 +229,79 @@ impl Default for SkillSet {
 }
 
 impl SkillSet {
-    /// Checks if the skill set of an entity contains a particular skill group
+    pub fn initial_skills() -> HashMap<Skill, Option<u16>> {
-    /// type
+        let mut skills = HashMap::new();
-    pub fn contains_skill_group(&self, skill_group_kind: SkillGroupKind) -> bool {
+        skills.insert(Skill::UnlockGroup(SkillGroupKind::General), None);
+        skills.insert(
+            Skill::UnlockGroup(SkillGroupKind::Weapon(ToolKind::Pick)),
+            None,
+        );
+        skills
+    }
+
+    pub fn load_from_database(
+        skill_groups: Vec<SkillGroup>,
+        mut all_skills: HashMap<SkillGroupKind, Vec<Skill>>,
+    ) -> Self {
+        let mut skillset = SkillSet {
+            skill_groups,
+            skills: SkillSet::initial_skills(),
+            modify_health: true,
+            modify_energy: true,
+        };
+
+        // Loops while checking the all_skills hashmap. For as long as it can find an
+        // entry where the skill group kind is unlocked, insert the skills corresponding
+        // to that skill group kind. When no more skill group kinds can be found, break
+        // the loop.
+        while let Some(skill_group_kind) = all_skills
+            .keys()
+            .find(|kind| skillset.has_skill(Skill::UnlockGroup(**kind)))
+            .copied()
+        {
+            // Remove valid skill group kind from the hash map so that loop eventually
+            // terminates.
+            if let Some(skills) = all_skills.remove(&skill_group_kind) {
+                let backup_skillset = skillset.clone();
+                // Iterate over all skills and make sure that unlocking them is successful. If
+                // any fail, fall back to skillset before unlocking any to allow a full respec
+                if !skills
+                    .iter()
+                    .all(|skill| skillset.unlock_skill(*skill).is_ok())
+                {
+                    skillset = backup_skillset;
+                }
+            }
+        }
+
+        skillset
+    }
+
+    /// Checks if a particular skill group is accessible for an entity
+    pub fn skill_group_accessible(&self, skill_group_kind: SkillGroupKind) -> bool {
         self.skill_groups
             .iter()
             .any(|x| x.skill_group_kind == skill_group_kind)
+            && self.has_skill(Skill::UnlockGroup(skill_group_kind))
     }
 
     ///  Unlocks a skill group for a player. It starts with 0 exp and 0 skill
     ///  points.
     pub fn unlock_skill_group(&mut self, skill_group_kind: SkillGroupKind) {
-        if !self.contains_skill_group(skill_group_kind) {
+        if !self
+            .skill_groups
+            .iter()
+            .any(|x| x.skill_group_kind == skill_group_kind)
+        {
             self.skill_groups.push(SkillGroup::new(skill_group_kind));
         } else {
             warn!("Tried to unlock already known skill group");
         }
     }
 
+    /// Returns an iterator over skill groups
+    pub fn skill_groups(&self) -> &Vec<SkillGroup> { &self.skill_groups }
+
     /// Returns a reference to a particular skill group in a skillset
     fn skill_group(&self, skill_group: SkillGroupKind) -> Option<&SkillGroup> {
         self.skill_groups
@@ -376,32 +431,39 @@ impl SkillSet {
             let prerequisites_met = self.prerequisites_met(skill);
             // Check that skill is not yet at max level
             if !matches!(self.skills.get(&skill), Some(level) if *level == skill.max_level()) {
-                if let Some(mut skill_group) = self.skill_group_mut(skill_group_kind) {
+                if self.has_skill(Skill::UnlockGroup(skill_group_kind)) {
-                    if prerequisites_met {
+                    if let Some(mut skill_group) = self.skill_group_mut(skill_group_kind) {
-                        if skill_group.available_sp >= skill.skill_cost(next_level) {
+                        if prerequisites_met {
-                            skill_group.available_sp -= skill.skill_cost(next_level);
+                            if skill_group.available_sp >= skill.skill_cost(next_level) {
-                            skill_group.ordered_skills.push(skill);
+                                skill_group.available_sp -= skill.skill_cost(next_level);
-                            match skill {
+                                skill_group.ordered_skills.push(skill);
-                                Skill::UnlockGroup(group) => {
+                                match skill {
-                                    self.unlock_skill_group(group);
+                                    Skill::UnlockGroup(group) => {
-                                },
+                                        self.unlock_skill_group(group);
-                                Skill::General(GeneralSkill::HealthIncrease) => {
+                                    },
-                                    self.modify_health = true;
+                                    Skill::General(GeneralSkill::HealthIncrease) => {
-                                },
+                                        self.modify_health = true;
-                                Skill::General(GeneralSkill::EnergyIncrease) => {
+                                    },
-                                    self.modify_energy = true;
+                                    Skill::General(GeneralSkill::EnergyIncrease) => {
-                                },
+                                        self.modify_energy = true;
-                                _ => {},
+                                    },
+                                    _ => {},
+                                }
+                                self.skills.insert(skill, next_level);
+                                Ok(())
+                            } else {
+                                trace!(
+                                    "Tried to unlock skill for skill group with insufficient SP"
+                                );
+                                Err(SkillUnlockError::InsufficientSP)
                             }
-                            self.skills.insert(skill, next_level);
-                            Ok(())
                         } else {
-                            trace!("Tried to unlock skill for skill group with insufficient SP");
+                            trace!("Tried to unlock skill without meeting prerequisite skills");
-                            Err(SkillUnlockError::InsufficientSP)
+                            Err(SkillUnlockError::MissingPrerequisites)
                         }
                     } else {
-                        trace!("Tried to unlock skill without meeting prerequisite skills");
+                        trace!("Tried to unlock skill for a skill group that player does not have");
-                        Err(SkillUnlockError::MissingPrerequisites)
+                        Err(SkillUnlockError::UnavailableSkillGroup)
                     }
                 } else {
                     trace!("Tried to unlock skill for a skill group that player does not have");

common/systems/src/stats.rs

@@ -130,7 +130,7 @@ impl<'a> System<'a> for Sys {
             }
 
             let skills_to_level = skill_set
-                .skill_groups
+                .skill_groups()
                 .iter()
                 .filter_map(|s_g| {
                     (s_g.available_experience() >= skill_set.skill_point_cost(s_g.skill_group_kind))

server/src/events/entity_manipulation.rs

@@ -303,7 +303,7 @@ pub fn handle_destroy(server: &mut Server, entity: EcsEntity, last_change: Healt
             let contributor_exp = exp_reward * damage_percent;
             match damage_contributor {
                 DamageContrib::Solo(attacker) => {
-                    // No exp for self kills or PvP 
+                    // No exp for self kills or PvP
                     if *attacker == entity || is_pvp_kill(*attacker) { return None; }
 
                     // Only give EXP to the attacker if they are within EXP range of the killed entity
@@ -1105,7 +1105,7 @@ fn handle_exp_gain(
             });
         if let Some(weapon) = tool_kind {
             // Only adds to xp pools if entity has that skill group available
-            if skill_set.contains_skill_group(SkillGroupKind::Weapon(weapon)) {
+            if skill_set.skill_group_accessible(SkillGroupKind::Weapon(weapon)) {
                 xp_pools.insert(SkillGroupKind::Weapon(weapon));
             }
         }

server/src/persistence/character.rs

@@ -419,7 +419,8 @@ pub fn create_character(
     ])?;
     drop(stmt);
 
-    let db_skill_groups = convert_skill_groups_to_database(character_id, skill_set.skill_groups);
+    let db_skill_groups =
+        convert_skill_groups_to_database(character_id, skill_set.skill_groups().to_vec());
 
     let mut stmt = transactionn.prepare_cached(
         "
@@ -559,16 +560,6 @@ pub fn delete_character(
             "Requested character to delete does not belong to the requesting player".to_string(),
         ));
     }
-    // Delete skills
-    let mut stmt = transaction.prepare_cached(
-        "
-        DELETE
-        FROM    skill
-        WHERE   entity_id = ?1",
-    )?;
-
-    stmt.execute(&[&char_id])?;
-    drop(stmt);
 
     // Delete skill groups
     let mut stmt = transaction.prepare_cached(
@@ -1001,7 +992,8 @@ pub fn update(
         }
     }
 
-    let db_skill_groups = convert_skill_groups_to_database(char_id, char_skill_set.skill_groups);
+    let db_skill_groups =
+        convert_skill_groups_to_database(char_id, char_skill_set.skill_groups().to_vec());
 
     let mut stmt = transaction.prepare_cached(
         "

server/src/persistence/character/conversions.rs

@@ -511,28 +511,17 @@ pub fn convert_stats_from_database(alias: String) -> common::comp::Stats {
 
 pub fn convert_skill_set_from_database(skill_groups: &[SkillGroup]) -> common::comp::SkillSet {
     let (skillless_skill_groups, skills) = convert_skill_groups_from_database(skill_groups);
-    let unskilled_skillset = skillset::SkillSet {
+    common::comp::SkillSet::load_from_database(skillless_skill_groups, skills)
-        skill_groups: skillless_skill_groups,
-        skills: HashMap::new(),
-        modify_health: true,
-        modify_energy: true,
-    };
-    let mut skillset = unskilled_skillset.clone();
-    if skills
-        .iter()
-        .all(|skill| skillset.unlock_skill(*skill).is_ok())
-    {
-        skillset
-    } else {
-        unskilled_skillset
-    }
 }
 
 fn convert_skill_groups_from_database(
     skill_groups: &[SkillGroup],
-) -> (Vec<skillset::SkillGroup>, Vec<skills::Skill>) {
+) -> (
+    Vec<skillset::SkillGroup>,
+    HashMap<skillset::SkillGroupKind, Vec<skills::Skill>>,
+) {
     let mut new_skill_groups = Vec::new();
-    let mut skills = Vec::new();
+    let mut all_skills = HashMap::new();
     for skill_group in skill_groups.iter() {
         let skill_group_kind = json_models::db_string_to_skill_group(&skill_group.skill_group_kind);
         let mut new_skill_group = skillset::SkillGroup {
@@ -551,17 +540,19 @@ fn convert_skill_groups_from_database(
         // points, and the hash stored of the skill group is the same as the current
         // hash of the skill group, don't invalidate skills; otherwise invalidate the
         // skills in this skill_group.
-        if skill_group.spent_exp as u32 == new_skill_group.spent_exp
+        let skills = if skill_group.spent_exp as u32 == new_skill_group.spent_exp
             && Some(&skill_group.hash_val) == 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()
-                serde_json::from_str::<Vec<skills::Skill>>(&skill_group.skills).unwrap_or_default();
+        } else {
-            skills.append(&mut new_skills);
+            Vec::new()
-        }
+        };
+
+        all_skills.insert(skill_group_kind, skills);
 
         new_skill_groups.push(new_skill_group);
     }
-    (new_skill_groups, skills)
+    (new_skill_groups, all_skills)
 }
 
 pub fn convert_skill_groups_to_database(

voxygen/src/hud/diary.rs

@@ -370,7 +370,7 @@ impl<'a> Widget for Diary<'a> {
             };
 
             // Check if we have this skill tree unlocked
-            let locked = !self.skill_set.contains_skill_group(skill_group);
+            let locked = !self.skill_set.skill_group_accessible(skill_group);
 
             // Weapon button image
             let btn_img = {