Skill points now attempt to be earned as experience is added.

common/src/comp/skillset/mod.rs

@@ -190,7 +190,7 @@ impl SkillGroup {
     pub fn spent_exp(&self) -> u32 { self.earned_exp - self.available_exp }
 
     /// Adds a skill point while subtracting the necessary amount of experience
-    pub fn earn_skill_point(&mut self) -> Result<(), SpRewardError> {
+    fn earn_skill_point(&mut self) -> Result<(), SpRewardError> {
         let sp_cost = self.skill_group_kind.skill_point_cost(self.earned_sp);
         // If there is insufficient available exp, checked sub will fail as the result
         // would be less than 0
@@ -212,9 +212,19 @@ impl SkillGroup {
         Ok(())
     }
 
-    pub fn add_experience(&mut self, amount: u32) {
+    /// Also attempts to earn a skill point after adding experience. If a skill
+    /// point was earned, returns how many skill points the skill group now has
+    /// earned in total.
+    pub fn add_experience(&mut self, amount: u32) -> Option<u16> {
         self.earned_exp = self.earned_exp.saturating_add(amount);
         self.available_exp = self.available_exp.saturating_add(amount);
+
+        let mut return_val = None;
+        // Attempt to earn skill point
+        while self.earn_skill_point().is_ok() {
+            return_val = Some(self.earned_sp);
+        }
+        return_val
     }
 }
 
@@ -342,12 +352,15 @@ impl SkillSet {
         }
     }
 
-    /// Adds experience to the skill group within an entity's skill set
+    /// Adds experience to the skill group within an entity's skill set, will
-    pub fn add_experience(&mut self, skill_group_kind: SkillGroupKind, amount: u32) {
+    /// attempt to earn a skill point while doing so. If a skill point was
+    /// earned, returns the number of earned skill points in the skill group.
+    pub fn add_experience(&mut self, skill_group_kind: SkillGroupKind, amount: u32) -> Option<u16> {
         if let Some(skill_group) = self.skill_group_mut(skill_group_kind) {
-            skill_group.add_experience(amount);
+            skill_group.add_experience(amount)
         } else {
             warn!("Tried to add experience to a skill group that player does not have");
+            None
         }
     }
 
@@ -376,22 +389,6 @@ impl SkillSet {
         for _ in 0..number_of_skill_points {
             let exp_needed = self.skill_point_cost(skill_group_kind);
             self.add_experience(skill_group_kind, exp_needed);
-            if self.earn_skill_point(skill_group_kind).is_err() {
-                warn!("Failed to add skill point");
-                break;
-            }
-        }
-    }
-
-    /// Adds a skill point while subtracting the necessary amount of experience
-    pub fn earn_skill_point(
-        &mut self,
-        skill_group_kind: SkillGroupKind,
-    ) -> Result<(), SpRewardError> {
-        if let Some(skill_group) = self.skill_group_mut(skill_group_kind) {
-            skill_group.earn_skill_point()
-        } else {
-            Err(SpRewardError::UnavailableSkillGroup)
         }
     }
 

common/systems/src/stats.rs

@@ -7,16 +7,12 @@ use common::{
         StatsModifier,
     },
     event::{EventBus, ServerEvent},
-    outcome::Outcome,
     resources::{DeltaTime, EntitiesDiedLastTick, Time},
-    uid::Uid,
 };
 use common_ecs::{Job, Origin, Phase, System};
-use hashbrown::HashSet;
 use specs::{
     shred::ResourceId, Entities, Join, Read, ReadStorage, SystemData, World, Write, WriteStorage,
 };
-use tracing::warn;
 use vek::Vec3;
 
 const ENERGY_REGEN_ACCEL: f32 = 1.0;
@@ -29,7 +25,6 @@ pub struct ReadData<'a> {
     time: Read<'a, Time>,
     server_bus: Read<'a, EventBus<ServerEvent>>,
     positions: ReadStorage<'a, Pos>,
-    uids: ReadStorage<'a, Uid>,
     bodies: ReadStorage<'a, Body>,
     char_states: ReadStorage<'a, CharacterState>,
     inventories: ReadStorage<'a, Inventory>,
@@ -49,7 +44,6 @@ impl<'a> System<'a> for Sys {
         WriteStorage<'a, Energy>,
         WriteStorage<'a, Combo>,
         Write<'a, EntitiesDiedLastTick>,
-        Write<'a, Vec<Outcome>>,
     );
 
     const NAME: &'static str = "stats";
@@ -67,7 +61,6 @@ impl<'a> System<'a> for Sys {
             mut energies,
             mut combos,
             mut entities_died_last_tick,
-            mut outcomes,
         ): Self::SystemData,
     ) {
         entities_died_last_tick.0.clear();
@@ -75,11 +68,9 @@ impl<'a> System<'a> for Sys {
         let dt = read_data.dt.0;
 
         // Update stats
-        for (entity, uid, stats, mut skill_set, mut health, pos, mut energy, inventory) in (
+        for (entity, stats, mut health, pos, mut energy, inventory) in (
             &read_data.entities,
-            &read_data.uids,
             &stats,
-            &mut skill_sets,
             &mut healths,
             &read_data.positions,
             &mut energies,
@@ -128,30 +119,6 @@ impl<'a> System<'a> for Sys {
             if change_energy {
                 energy.update_maximum(energy_mods);
             }
-
-            let skills_to_level = skill_set
-                .skill_groups()
-                .filter_map(|s_g| {
-                    (s_g.available_exp >= skill_set.skill_point_cost(s_g.skill_group_kind))
-                        .then(|| s_g.skill_group_kind)
-                })
-                .collect::<HashSet<_>>();
-
-            if !skills_to_level.is_empty() {
-                for skill_group in skills_to_level {
-                    match skill_set.earn_skill_point(skill_group) {
-                        Ok(_) => outcomes.push(Outcome::SkillPointGain {
-                            uid: *uid,
-                            skill_tree: skill_group,
-                            total_points: skill_set.earned_sp(skill_group),
-                            pos: pos.0,
-                        }),
-                        Err(_) => warn!(
-                            "Attempted to add skill point to group which is inelgible to earn one"
-                        ),
-                    }
-                }
-            }
         }
 
         // Apply effects from leveling skills

server/src/cmd.rs

@@ -3477,9 +3477,10 @@ fn set_skills(skill_set: &mut comp::SkillSet, preset: &str) -> CmdResult<()> {
             for _ in 0..*level {
                 let cost = skill_set.skill_cost(*skill);
                 skill_set.add_skill_points(group, cost);
-                skill_set
+                match skill_set.unlock_skill(*skill) {
-                    .unlock_skill(*skill)
+                    Ok(_) | Err(comp::skillset::SkillUnlockError::SkillAlreadyUnlocked) => Ok(()),
-                    .map_err(|err| format!("{:?}", err))?;
+                    Err(err) => Err(format!("{:?}", err)),
+                }?;
             }
         }
         Ok(())

server/src/events/entity_manipulation.rs

@@ -353,16 +353,18 @@ pub fn handle_destroy(server: &mut Server, entity: EcsEntity, last_change: Healt
             }
         }).flatten().for_each(|(attacker, exp_reward)| {
             // Process the calculated EXP rewards
-            if let (Some(mut attacker_skill_set), Some(attacker_uid), Some(attacker_inventory)) = (
+            if let (Some(mut attacker_skill_set), Some(attacker_uid), Some(attacker_inventory), Some(pos)) = (
                 skill_sets.get_mut(attacker),
                 uids.get(attacker),
                 inventories.get(attacker),
+                positions.get(attacker),
             ) {
                 handle_exp_gain(
                     exp_reward,
                     attacker_inventory,
                     &mut attacker_skill_set,
                     attacker_uid,
+                    pos,
                     &mut outcomes,
                 );
             }
@@ -1087,6 +1089,7 @@ fn handle_exp_gain(
     inventory: &Inventory,
     skill_set: &mut SkillSet,
     uid: &Uid,
+    pos: &Pos,
     outcomes: &mut Vec<Outcome>,
 ) {
     use comp::inventory::{item::ItemKind, slot::EquipSlot};
@@ -1117,7 +1120,16 @@ fn handle_exp_gain(
     add_tool_from_slot(EquipSlot::InactiveOffhand);
     let num_pools = xp_pools.len() as f32;
     for pool in xp_pools.iter() {
-        skill_set.add_experience(*pool, (exp_reward / num_pools).ceil() as u32);
+        if let Some(level_outcome) =
+            skill_set.add_experience(*pool, (exp_reward / num_pools).ceil() as u32)
+        {
+            outcomes.push(Outcome::SkillPointGain {
+                uid: *uid,
+                skill_tree: *pool,
+                total_points: level_outcome,
+                pos: pos.0,
+            });
+        }
     }
     outcomes.push(Outcome::ExpChange {
         uid: *uid,

server/src/events/interaction.rs

@@ -339,14 +339,24 @@ pub fn handle_mine_block(
                             .0
                             .get(item.item_definition_id()),
                     ) {
-                        skillset.add_experience(SkillGroupKind::Weapon(tool), *exp_reward);
+                        let skill_group = SkillGroupKind::Weapon(tool);
-                        state
+                        let mut outcomes = state.ecs().write_resource::<Vec<Outcome>>();
-                            .ecs()
+                        let positions = state.ecs().read_component::<comp::Pos>();
-                            .write_resource::<Vec<Outcome>>()
+                        if let (Some(level_outcome), Some(pos)) = (
-                            .push(Outcome::ExpChange {
+                            skillset.add_experience(skill_group, *exp_reward),
+                            positions.get(entity),
+                        ) {
+                            outcomes.push(Outcome::SkillPointGain {
+                                uid,
+                                skill_tree: skill_group,
+                                total_points: level_outcome,
+                                pos: pos.0,
+                            });
+                        }
+                        outcomes.push(Outcome::ExpChange {
                             uid,
                             exp: *exp_reward,
-                                xp_pools: HashSet::from_iter(vec![SkillGroupKind::Weapon(tool)]),
+                            xp_pools: HashSet::from_iter(vec![skill_group]),
                         });
                     }
                     use common::comp::skills::{MiningSkill, Skill, SKILL_MODIFIERS};

server/src/persistence/character/conversions.rs

@@ -543,13 +543,10 @@ fn convert_skill_groups_from_database(
 
         // Add experience to skill group through method to ensure invariant of
         // (earned_exp >= available_exp) are maintained
+        // Adding experience will automatically earn all possible skill points
         let skill_group_exp = skill_group.earned_exp.clamp(0, i64::from(u32::MAX)) as u32;
         new_skill_group.add_experience(skill_group_exp);
 
-        // Convert exp into skill points, earn_skill_point will only return an error
-        // when it is no longer able to spend exp to acquire another skill point
-        while new_skill_group.earn_skill_point().is_ok() {}
-
         use skillset::SkillsPersistenceError;
 
         let skills_result = if skill_group.spent_exp != i64::from(new_skill_group.spent_exp()) {