Add tests

common/src/comp/ability.rs

@@ -1894,3 +1894,55 @@ impl From<(&CharacterAbility, AbilityInfo)> for CharacterState {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use comp::{
+        inventory::item::{Item, ItemKind},
+        tool,
+    };
+    #[test]
+    // As we have only player skills, test it with starter tools
+    // Load ability_config of each tool and try each CharacterAbility of it
+    fn test_adjusting_by_skills() {
+        let test_tools = [
+            "common.items.weapons.sword.starter",
+            "common.items.weapons.hammer.starter_hammer",
+            "common.items.weapons.bow.starter",
+            "common.items.weapons.axe.starter_axe",
+            "common.items.weapons.staff.starter_staff",
+            "common.items.weapons.sceptre.starter_sceptre",
+            "common.items.tool.pickaxe_stone",
+        ];
+
+        let dummy_skillset = skills::SkillSet::default();
+        // check non-combat abilities
+        let dummy_ability = CharacterAbility::default();
+        dummy_ability.adjusted_by_skills(&dummy_skillset, None);
+        for tool_id in test_tools {
+            let item = Item::new_from_asset_expect(tool_id);
+            let tool::AbilitySet {
+                primary,
+                secondary,
+                abilities,
+            } = &item.item_config_expect().abilities;
+
+            // It should be a tool, I swear
+            if let ItemKind::Tool(tool) = &item.kind {
+                primary
+                    .clone()
+                    .adjusted_by_skills(&dummy_skillset, Some(tool.kind));
+                secondary
+                    .clone()
+                    .adjusted_by_skills(&dummy_skillset, Some(tool.kind));
+                for entry in abilities {
+                    let (_, ability) = entry;
+                    ability
+                        .clone()
+                        .adjusted_by_skills(&dummy_skillset, Some(tool.kind));
+                }
+            }
+        }
+    }
+}

common/src/comp/skills.rs

@@ -150,34 +150,55 @@ impl From<i16> for BoostValue {
 }
 
 pub fn adjust_with_level(skillset: &SkillSet, skill: Skill, effect: impl FnOnce(f32, u16)) {
+    // NOTE: We are unwrapping before checking skill level,
+    // because if it falls we want know it even if we don't have this level
+    let multiplier = match skill.boost().as_mult_maybe() {
+        Some(m) => m,
+        None => return invalid_skill_boost(skill),
+    };
     if let Ok(Some(level)) = skillset.skill_level(skill) {
-        skill
+        effect(multiplier, level);
-            .boost()
-            .as_mult_maybe()
-            .map_or_else(|| invalid_skill_boost(skill), |m| effect(m, level))
     }
 }
 
 pub fn adjust_counter_with_level(skillset: &SkillSet, skill: Skill, effect: impl FnOnce(i16, i16)) {
+    // NOTE: We are unwrapping before checking skill level,
+    // because if it falls we want know it even if we don't have this level
+    let counter = match skill.boost().as_i16_maybe() {
+        Some(c) => c,
+        None => return invalid_skill_boost(skill),
+    };
     if let Ok(Some(level)) = skillset.skill_level(skill) {
-        skill
+        effect(counter, level as i16);
-            .boost()
-            .as_i16_maybe()
-            .map_or_else(|| invalid_skill_boost(skill), |m| effect(m, level as i16))
     }
 }
 
 pub fn set_if_has(skillset: &SkillSet, skill: Skill, effect: impl FnOnce(f32)) {
+    // NOTE: We are unwrapping before checking skill level,
+    // because if it falls we want know it even if we don't have this level
+    let multiplier = match skill.boost().as_mult_maybe() {
+        Some(c) => c,
+        None => return invalid_skill_boost(skill),
+    };
     if skillset.has_skill(skill) {
-        skill
+        effect(multiplier);
-            .boost()
-            .as_mult_maybe()
-            .map_or_else(|| invalid_skill_boost(skill), |m| effect(m))
     }
 }
+
+#[track_caller]
 pub fn invalid_skill_boost(skill: Skill) {
     let err_msg = format!(
-        "{:?} produced unexpected BoostValue: {:?}",
+        r#"
+
+        {:?} produced unexpected BoostValue: {:?}
+
+        Clearly that shouldn't happen and tests should catch this.
+
+        If they didn't, probably because we've added new skills/weapons.
+        In this case, please find `test_adjusting_skills`,
+        fix tests and fix corresponding `impl Boost` for this skill.
+
+        "#,
         skill,
         skill.boost()
     );