Split skillset and skills into separate files.

common/src/combat.rs

@@ -10,7 +10,7 @@ use crate::{
             },
             slot::EquipSlot,
         },
-        skills::SkillGroupKind,
+        skillset::SkillGroupKind,
         Alignment, Body, CharacterState, Combo, Energy, Health, HealthChange, Inventory, Ori,
         Player, Poise, SkillSet, Stats,
     },

common/src/comp/ability.rs

@@ -12,7 +12,10 @@ use crate::{
             Inventory,
         },
         projectile::ProjectileConstructor,
-        skills::{self, Skill, SkillSet, SKILL_MODIFIERS},
+        skillset::{
+            skills::{self, Skill, SKILL_MODIFIERS},
+            SkillSet,
+        },
         Body, CharacterState, LightEmitter, StateUpdate,
     },
     states::{
@@ -1158,11 +1161,7 @@ impl CharacterAbility {
 
     #[must_use = "method returns new ability and doesn't mutate the original value"]
     #[warn(clippy::pedantic)]
-    pub fn adjusted_by_skills(
+    pub fn adjusted_by_skills(mut self, skillset: &SkillSet, tool: Option<ToolKind>) -> Self {
-        mut self,
-        skillset: &skills::SkillSet,
-        tool: Option<ToolKind>,
-    ) -> Self {
         match tool {
             Some(ToolKind::Sword) => self.adjusted_by_sword_skills(skillset),
             Some(ToolKind::Axe) => self.adjusted_by_axe_skills(skillset),
@@ -1178,7 +1177,7 @@ impl CharacterAbility {
     }
 
     #[warn(clippy::pedantic)]
-    fn adjusted_by_mining_skills(&mut self, skillset: &skills::SkillSet) {
+    fn adjusted_by_mining_skills(&mut self, skillset: &SkillSet) {
         use skills::MiningSkill::Speed;
 
         if let CharacterAbility::BasicMelee {
@@ -1200,7 +1199,7 @@ impl CharacterAbility {
     }
 
     #[warn(clippy::pedantic)]
-    fn adjusted_by_general_skills(&mut self, skillset: &skills::SkillSet) {
+    fn adjusted_by_general_skills(&mut self, skillset: &SkillSet) {
         if let CharacterAbility::Roll {
             ref mut energy_cost,
             ref mut roll_strength,
@@ -1225,7 +1224,7 @@ impl CharacterAbility {
     }
 
     #[warn(clippy::pedantic)]
-    fn adjusted_by_sword_skills(&mut self, skillset: &skills::SkillSet) {
+    fn adjusted_by_sword_skills(&mut self, skillset: &SkillSet) {
         #![allow(clippy::enum_glob_use)]
         use skills::{Skill::Sword, SwordSkill::*};
 
@@ -1319,7 +1318,7 @@ impl CharacterAbility {
     }
 
     #[warn(clippy::pedantic)]
-    fn adjusted_by_axe_skills(&mut self, skillset: &skills::SkillSet) {
+    fn adjusted_by_axe_skills(&mut self, skillset: &SkillSet) {
         #![allow(clippy::enum_glob_use)]
         use skills::{AxeSkill::*, Skill::Axe};
 
@@ -1405,7 +1404,7 @@ impl CharacterAbility {
     }
 
     #[warn(clippy::pedantic)]
-    fn adjusted_by_hammer_skills(&mut self, skillset: &skills::SkillSet) {
+    fn adjusted_by_hammer_skills(&mut self, skillset: &SkillSet) {
         #![allow(clippy::enum_glob_use)]
         use skills::{HammerSkill::*, Skill::Hammer};
 
@@ -1497,7 +1496,7 @@ impl CharacterAbility {
     }
 
     #[warn(clippy::pedantic)]
-    fn adjusted_by_bow_skills(&mut self, skillset: &skills::SkillSet) {
+    fn adjusted_by_bow_skills(&mut self, skillset: &SkillSet) {
         #![allow(clippy::enum_glob_use)]
         use skills::{BowSkill::*, Skill::Bow};
 
@@ -1598,7 +1597,7 @@ impl CharacterAbility {
     }
 
     #[warn(clippy::pedantic)]
-    fn adjusted_by_staff_skills(&mut self, skillset: &skills::SkillSet) {
+    fn adjusted_by_staff_skills(&mut self, skillset: &SkillSet) {
         #![allow(clippy::enum_glob_use)]
         use skills::{Skill::Staff, StaffSkill::*};
 
@@ -1668,7 +1667,7 @@ impl CharacterAbility {
     }
 
     #[warn(clippy::pedantic)]
-    fn adjusted_by_sceptre_skills(&mut self, skillset: &skills::SkillSet) {
+    fn adjusted_by_sceptre_skills(&mut self, skillset: &SkillSet) {
         #![allow(clippy::enum_glob_use)]
         use skills::{SceptreSkill::*, Skill::Sceptre};
 

common/src/comp/mod.rs

@@ -40,7 +40,7 @@ pub mod projectile;
 #[cfg(not(target_arch = "wasm32"))]
 pub mod shockwave;
 #[cfg(not(target_arch = "wasm32"))]
-pub mod skills;
+pub mod skillset;
 #[cfg(not(target_arch = "wasm32"))] mod stats;
 #[cfg(not(target_arch = "wasm32"))]
 pub mod visual;
@@ -99,7 +99,10 @@ pub use self::{
     poise::{Poise, PoiseState},
     projectile::{Projectile, ProjectileConstructor},
     shockwave::{Shockwave, ShockwaveHitEntities},
-    skills::{Skill, SkillGroup, SkillGroupKind, SkillSet},
+    skillset::{
+        skills::{self, Skill},
+        SkillGroup, SkillGroupKind, SkillSet,
+    },
     stats::{Stats, StatsModifier},
     visual::{LightAnimation, LightEmitter},
 };

common/src/comp/skillset/mod.rs

@@ -1,6 +1,9 @@
 use crate::{
     assets::{self, Asset, AssetExt},
-    comp::item::tool::ToolKind,
+    comp::{
+        item::tool::ToolKind,
+        skills::{GeneralSkill, Skill},
+    },
 };
 use hashbrown::{HashMap, HashSet};
 use lazy_static::lazy_static;
@@ -10,6 +13,8 @@ use specs_idvs::IdvStorage;
 use std::hash::Hash;
 use tracing::{trace, warn};
 
+pub mod skills;
+
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct SkillTreeMap(HashMap<SkillGroupKind, HashSet<Skill>>);
 
@@ -91,566 +96,6 @@ lazy_static! {
     };
 }
 
-/// Represents a skill that a player can unlock, that either grants them some
-/// kind of active ability, or a passive effect etc. Obviously because this is
-/// an enum it doesn't describe what the skill actually -does-, this will be
-/// handled by dedicated ECS systems.
-// NOTE: if skill does use some constant, add it to corresponding
-// SkillTree Modifiers below.
-#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
-pub enum Skill {
-    General(GeneralSkill),
-    Sword(SwordSkill),
-    Axe(AxeSkill),
-    Hammer(HammerSkill),
-    Bow(BowSkill),
-    Staff(StaffSkill),
-    Sceptre(SceptreSkill),
-    UnlockGroup(SkillGroupKind),
-    Roll(RollSkill),
-    Climb(ClimbSkill),
-    Swim(SwimSkill),
-    Pick(MiningSkill),
-}
-
-/// Tree of modifiers that represent how stats are
-/// changed per each skill level.
-///
-/// It's used as bridge between ECS systems
-/// and voxygen Diary for skill descriptions and helps to sync them.
-///
-/// NOTE: Just adding constant does nothing, you need to use it in both
-/// ECS systems and Diary.
-// TODO: make it lazy_static and move to .ron?
-pub const SKILL_MODIFIERS: SkillTreeModifiers = SkillTreeModifiers::get();
-
-pub struct SkillTreeModifiers {
-    pub sword_tree: SwordTreeModifiers,
-    pub axe_tree: AxeTreeModifiers,
-    pub hammer_tree: HammerTreeModifiers,
-    pub bow_tree: BowTreeModifiers,
-    pub staff_tree: StaffTreeModifiers,
-    pub sceptre_tree: SceptreTreeModifiers,
-    pub mining_tree: MiningTreeModifiers,
-    pub general_tree: GeneralTreeModifiers,
-}
-
-impl SkillTreeModifiers {
-    const fn get() -> Self {
-        Self {
-            sword_tree: SwordTreeModifiers::get(),
-            axe_tree: AxeTreeModifiers::get(),
-            hammer_tree: HammerTreeModifiers::get(),
-            bow_tree: BowTreeModifiers::get(),
-            staff_tree: StaffTreeModifiers::get(),
-            sceptre_tree: SceptreTreeModifiers::get(),
-            mining_tree: MiningTreeModifiers::get(),
-            general_tree: GeneralTreeModifiers::get(),
-        }
-    }
-}
-
-pub struct SwordTreeModifiers {
-    pub dash: SwordDashModifiers,
-    pub spin: SwordSpinModifiers,
-}
-
-pub struct SwordDashModifiers {
-    pub energy_cost: f32,
-    pub energy_drain: f32,
-    pub base_damage: f32,
-    pub scaled_damage: f32,
-    pub forward_speed: f32,
-}
-
-pub struct SwordSpinModifiers {
-    pub base_damage: f32,
-    pub swing_duration: f32,
-    pub energy_cost: f32,
-    pub num: u32,
-}
-
-impl SwordTreeModifiers {
-    const fn get() -> Self {
-        Self {
-            dash: SwordDashModifiers {
-                energy_cost: 0.9,
-                energy_drain: 0.9,
-                base_damage: 1.1,
-                scaled_damage: 1.1,
-                forward_speed: 1.05,
-            },
-            spin: SwordSpinModifiers {
-                base_damage: 1.2,
-                swing_duration: 0.9,
-                energy_cost: 0.9,
-                num: 1,
-            },
-        }
-    }
-}
-
-pub struct AxeTreeModifiers {
-    pub spin: AxeSpinModifiers,
-    pub leap: AxeLeapModifiers,
-}
-
-pub struct AxeSpinModifiers {
-    pub base_damage: f32,
-    pub swing_duration: f32,
-    pub energy_cost: f32,
-}
-
-pub struct AxeLeapModifiers {
-    pub base_damage: f32,
-    pub knockback: f32,
-    pub energy_cost: f32,
-    // TODO: split to forward and vertical?
-    pub leap_strength: f32,
-}
-
-impl AxeTreeModifiers {
-    const fn get() -> Self {
-        Self {
-            spin: AxeSpinModifiers {
-                base_damage: 1.2,
-                swing_duration: 0.85,
-                energy_cost: 0.85,
-            },
-            leap: AxeLeapModifiers {
-                base_damage: 1.2,
-                knockback: 1.2,
-                energy_cost: 0.75,
-                leap_strength: 1.1,
-            },
-        }
-    }
-}
-
-pub struct HammerTreeModifiers {
-    pub single_strike: HammerStrikeModifiers,
-    pub charged: HammerChargedModifers,
-    pub leap: HammerLeapModifiers,
-}
-
-pub struct HammerStrikeModifiers {
-    pub knockback: f32,
-}
-
-pub struct HammerChargedModifers {
-    pub scaled_damage: f32,
-    pub scaled_knockback: f32,
-    pub energy_drain: f32,
-    pub charge_rate: f32,
-}
-
-pub struct HammerLeapModifiers {
-    pub base_damage: f32,
-    pub knockback: f32,
-    pub energy_cost: f32,
-    pub leap_strength: f32,
-    pub range: f32,
-}
-
-impl HammerTreeModifiers {
-    const fn get() -> Self {
-        Self {
-            single_strike: HammerStrikeModifiers { knockback: 1.25 },
-            charged: HammerChargedModifers {
-                scaled_damage: 1.2,
-                scaled_knockback: 1.3,
-                energy_drain: 0.85,
-                charge_rate: 1.15,
-            },
-            leap: HammerLeapModifiers {
-                base_damage: 1.25,
-                knockback: 1.3,
-                energy_cost: 0.75,
-                leap_strength: 1.1,
-                range: 0.5,
-            },
-        }
-    }
-}
-
-pub struct BowTreeModifiers {
-    pub universal: BowUniversalModifiers,
-    pub charged: BowChargedModifiers,
-    pub repeater: BowRepeaterModifiers,
-    pub shotgun: BowShotgunModifiers,
-}
-
-pub struct BowUniversalModifiers {
-    // TODO: split per abilities?
-    pub projectile_speed: f32,
-}
-
-pub struct BowChargedModifiers {
-    pub damage_scaling: f32,
-    pub regen_scaling: f32,
-    pub knockback_scaling: f32,
-    pub charge_rate: f32,
-    pub move_speed: f32,
-}
-
-pub struct BowRepeaterModifiers {
-    pub power: f32,
-    pub energy_cost: f32,
-    pub max_speed: f32,
-}
-
-pub struct BowShotgunModifiers {
-    pub power: f32,
-    pub energy_cost: f32,
-    pub num_projectiles: u32,
-    pub spread: f32,
-}
-
-impl BowTreeModifiers {
-    const fn get() -> Self {
-        Self {
-            universal: BowUniversalModifiers {
-                projectile_speed: 1.1,
-            },
-            charged: BowChargedModifiers {
-                damage_scaling: 1.1,
-                regen_scaling: 1.1,
-                knockback_scaling: 1.1,
-                charge_rate: 1.1,
-                move_speed: 1.1,
-            },
-            repeater: BowRepeaterModifiers {
-                power: 1.1,
-                energy_cost: 0.9,
-                max_speed: 1.2,
-            },
-            shotgun: BowShotgunModifiers {
-                power: 1.1,
-                energy_cost: 0.9,
-                num_projectiles: 1,
-                spread: 0.9,
-            },
-        }
-    }
-}
-
-pub struct StaffTreeModifiers {
-    pub fireball: StaffFireballModifiers,
-    pub flamethrower: StaffFlamethrowerModifiers,
-    pub shockwave: StaffShockwaveModifiers,
-}
-
-pub struct StaffFireballModifiers {
-    pub power: f32,
-    pub regen: f32,
-    pub range: f32,
-}
-
-pub struct StaffFlamethrowerModifiers {
-    pub damage: f32,
-    pub range: f32,
-    pub energy_drain: f32,
-    pub velocity: f32,
-}
-
-pub struct StaffShockwaveModifiers {
-    pub damage: f32,
-    pub knockback: f32,
-    pub duration: f32,
-    pub energy_cost: f32,
-}
-
-impl StaffTreeModifiers {
-    const fn get() -> Self {
-        Self {
-            fireball: StaffFireballModifiers {
-                power: 1.1,
-                regen: 1.1,
-                range: 1.1,
-            },
-            flamethrower: StaffFlamethrowerModifiers {
-                damage: 1.2,
-                range: 1.1,
-                energy_drain: 0.9,
-                velocity: 1.1,
-            },
-            shockwave: StaffShockwaveModifiers {
-                damage: 1.15,
-                knockback: 1.15,
-                duration: 1.1,
-                energy_cost: 0.9,
-            },
-        }
-    }
-}
-
-pub struct SceptreTreeModifiers {
-    pub beam: SceptreBeamModifiers,
-    pub healing_aura: SceptreHealingAuraModifiers,
-    pub warding_aura: SceptreWardingAuraModifiers,
-}
-
-pub struct SceptreBeamModifiers {
-    pub damage: f32,
-    pub range: f32,
-    pub energy_regen: f32,
-    pub lifesteal: f32,
-}
-
-pub struct SceptreHealingAuraModifiers {
-    pub strength: f32,
-    pub duration: f32,
-    pub range: f32,
-    pub energy_cost: f32,
-}
-
-pub struct SceptreWardingAuraModifiers {
-    pub strength: f32,
-    pub duration: f32,
-    pub range: f32,
-    pub energy_cost: f32,
-}
-
-impl SceptreTreeModifiers {
-    const fn get() -> Self {
-        Self {
-            beam: SceptreBeamModifiers {
-                damage: 1.1,
-                range: 1.1,
-                energy_regen: 1.1,
-                lifesteal: 1.05,
-            },
-            healing_aura: SceptreHealingAuraModifiers {
-                strength: 1.05,
-                duration: 1.1,
-                range: 1.1,
-                energy_cost: 0.90,
-            },
-            warding_aura: SceptreWardingAuraModifiers {
-                strength: 1.05,
-                duration: 1.1,
-                range: 1.1,
-                energy_cost: 0.95,
-            },
-        }
-    }
-}
-
-pub struct MiningTreeModifiers {
-    pub speed: f32,
-    pub gem_gain: f32,
-    pub ore_gain: f32,
-}
-
-impl MiningTreeModifiers {
-    const fn get() -> Self {
-        Self {
-            speed: 1.1,
-            gem_gain: 0.05,
-            ore_gain: 0.05,
-        }
-    }
-}
-
-pub struct GeneralTreeModifiers {
-    pub roll: RollTreeModifiers,
-    pub swim: SwimTreeModifiers,
-    pub climb: ClimbTreeModifiers,
-}
-
-pub struct RollTreeModifiers {
-    pub energy_cost: f32,
-    pub strength: f32,
-    pub duration: f32,
-}
-
-pub struct SwimTreeModifiers {
-    pub speed: f32,
-}
-
-pub struct ClimbTreeModifiers {
-    pub energy_cost: f32,
-    pub speed: f32,
-}
-
-impl GeneralTreeModifiers {
-    const fn get() -> Self {
-        Self {
-            roll: RollTreeModifiers {
-                energy_cost: 0.95,
-                strength: 1.05,
-                duration: 1.05,
-            },
-            swim: SwimTreeModifiers { speed: 1.25 },
-            climb: ClimbTreeModifiers {
-                energy_cost: 0.8,
-                speed: 1.2,
-            },
-        }
-    }
-}
-pub enum SkillError {
-    MissingSkill,
-}
-
-#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
-pub enum SwordSkill {
-    // Sword passives
-    InterruptingAttacks,
-    // Triple strike upgrades
-    TsCombo,
-    TsDamage,
-    TsRegen,
-    TsSpeed,
-    // Dash upgrades
-    DCost,
-    DDrain,
-    DDamage,
-    DScaling,
-    DSpeed,
-    DInfinite, // Represents charge through, not migrated because laziness
-    // Spin upgrades
-    UnlockSpin,
-    SDamage,
-    SSpeed,
-    SCost,
-    SSpins,
-}
-
-#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
-pub enum AxeSkill {
-    // Double strike upgrades
-    DsCombo,
-    DsDamage,
-    DsSpeed,
-    DsRegen,
-    // Spin upgrades
-    SInfinite,
-    SHelicopter,
-    SDamage,
-    SSpeed,
-    SCost,
-    // Leap upgrades
-    UnlockLeap,
-    LDamage,
-    LKnockback,
-    LCost,
-    LDistance,
-}
-
-#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
-pub enum HammerSkill {
-    // Single strike upgrades
-    SsKnockback,
-    SsDamage,
-    SsSpeed,
-    SsRegen,
-    // Charged melee upgrades
-    CDamage,
-    CKnockback,
-    CDrain,
-    CSpeed,
-    // Leap upgrades
-    UnlockLeap,
-    LDamage,
-    LCost,
-    LDistance,
-    LKnockback,
-    LRange,
-}
-
-#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
-pub enum BowSkill {
-    // Passives
-    ProjSpeed,
-    // Charged upgrades
-    CDamage,
-    CRegen,
-    CKnockback,
-    CSpeed,
-    CMove,
-    // Repeater upgrades
-    RDamage,
-    RCost,
-    RSpeed,
-    // Shotgun upgrades
-    UnlockShotgun,
-    SDamage,
-    SCost,
-    SArrows,
-    SSpread,
-}
-
-#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
-pub enum StaffSkill {
-    // Basic ranged upgrades
-    BDamage,
-    BRegen,
-    BRadius,
-    // Flamethrower upgrades
-    FDamage,
-    FRange,
-    FDrain,
-    FVelocity,
-    // Shockwave upgrades
-    UnlockShockwave,
-    SDamage,
-    SKnockback,
-    SRange,
-    SCost,
-}
-
-#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
-pub enum SceptreSkill {
-    // Lifesteal beam upgrades
-    LDamage,
-    LRange,
-    LLifesteal,
-    LRegen,
-    // Healing aura upgrades
-    HHeal,
-    HRange,
-    HDuration,
-    HCost,
-    // Warding aura upgrades
-    UnlockAura,
-    AStrength,
-    ADuration,
-    ARange,
-    ACost,
-}
-
-#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
-pub enum GeneralSkill {
-    HealthIncrease,
-    EnergyIncrease,
-}
-
-#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
-pub enum RollSkill {
-    Cost,
-    Strength,
-    Duration,
-}
-
-#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
-pub enum ClimbSkill {
-    Cost,
-    Speed,
-}
-
-#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
-pub enum SwimSkill {
-    Speed,
-}
-
-#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
-pub enum MiningSkill {
-    Speed,
-    OreGain,
-    GemGain,
-}
-
 #[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
 pub enum SkillGroupKind {
     General,
@@ -738,6 +183,7 @@ impl SkillGroup {
 pub struct SkillSet {
     pub skill_groups: Vec<SkillGroup>,
     pub skills: HashMap<Skill, Option<u16>>,
+    pub ordered_skills: Vec<Skill>,
     pub modify_health: bool,
     pub modify_energy: bool,
 }
@@ -757,6 +203,7 @@ impl Default for SkillSet {
                 SkillGroup::new(SkillGroupKind::Weapon(ToolKind::Pick)),
             ],
             skills: HashMap::new(),
+            ordered_skills: Vec::new(),
             modify_health: false,
             modify_energy: false,
         }
@@ -764,20 +211,16 @@ impl Default for SkillSet {
 }
 
 impl SkillSet {
+    /// Checks if the skill set of an entity contains a particular skill group
+    /// type
+    pub fn contains_skill_group(&self, skill_group_kind: SkillGroupKind) -> bool {
+        self.skill_groups
+            .iter()
+            .any(|x| x.skill_group_kind == skill_group_kind)
+    }
+
     ///  Unlocks a skill group for a player. It starts with 0 exp and 0 skill
     ///  points.
-    ///
-    /// ```
-    /// use veloren_common::comp::{
-    ///     item::tool::ToolKind,
-    ///     skills::{SkillGroupKind, SkillSet},
-    /// };
-    ///
-    /// let mut skillset = SkillSet::default();
-    /// skillset.unlock_skill_group(SkillGroupKind::Weapon(ToolKind::Sword));
-    ///
-    /// assert_eq!(skillset.skill_groups.len(), 3);
-    /// ```
     pub fn unlock_skill_group(&mut self, skill_group_kind: SkillGroupKind) {
         if !self.contains_skill_group(skill_group_kind) {
             self.skill_groups.push(SkillGroup::new(skill_group_kind));
@@ -788,17 +231,6 @@ impl SkillSet {
 
     /// Unlocks a skill for a player, assuming they have the relevant skill
     /// group unlocked and available SP in that skill group.
-    ///
-    /// ```
-    /// use veloren_common::comp::skills::{GeneralSkill, Skill, SkillGroupKind, SkillSet};
-    ///
-    /// let mut skillset = SkillSet::default();
-    /// skillset.add_skill_points(SkillGroupKind::General, 1);
-    ///
-    /// skillset.unlock_skill(Skill::General(GeneralSkill::HealthIncrease));
-    ///
-    /// assert_eq!(skillset.skills.len(), 1);
-    /// ```
     pub fn unlock_skill(&mut self, skill: Skill) {
         if let Some(skill_group_kind) = skill.skill_group_kind() {
             let next_level = self.next_skill_level(skill);
@@ -840,18 +272,6 @@ impl SkillSet {
 
     /// Removes a skill from a player and refunds 1 skill point in the relevant
     /// skill group.
-    ///
-    /// ```
-    /// use veloren_common::comp::skills::{GeneralSkill, Skill, SkillGroupKind, SkillSet};
-    ///
-    /// let mut skillset = SkillSet::default();
-    /// skillset.add_skill_points(SkillGroupKind::General, 1);
-    /// skillset.unlock_skill(Skill::General(GeneralSkill::HealthIncrease));
-    ///
-    /// skillset.refund_skill(Skill::General(GeneralSkill::HealthIncrease));
-    ///
-    /// assert_eq!(skillset.skills.len(), 0);
-    /// ```
     pub fn refund_skill(&mut self, skill: Skill) {
         if let Ok(level) = self.skill_level(skill) {
             if let Some(skill_group_kind) = skill.skill_group_kind() {
@@ -878,15 +298,6 @@ impl SkillSet {
 
     /// Adds skill points to a skill group as long as the player has that skill
     /// group type.
-    ///
-    /// ```
-    /// use veloren_common::comp::skills::{SkillGroupKind, SkillSet};
-    ///
-    /// let mut skillset = SkillSet::default();
-    /// skillset.add_skill_points(SkillGroupKind::General, 1);
-    ///
-    /// assert_eq!(skillset.skill_groups[0].available_sp, 1);
-    /// ```
     pub fn add_skill_points(
         &mut self,
         skill_group_kind: SkillGroupKind,
@@ -914,14 +325,6 @@ impl SkillSet {
         }
     }
 
-    /// Checks if the skill set of an entity contains a particular skill group
-    /// type
-    pub fn contains_skill_group(&self, skill_group_kind: SkillGroupKind) -> bool {
-        self.skill_groups
-            .iter()
-            .any(|x| x.skill_group_kind == skill_group_kind)
-    }
-
     /// Adds/subtracts experience to the skill group within an entity's skill
     /// set
     pub fn add_experience(&mut self, skill_group_kind: SkillGroupKind, amount: u32) {
@@ -1052,32 +455,8 @@ impl SkillSet {
     }
 }
 
-impl Skill {
+pub enum SkillError {
-    /// Returns a vec of prerequisite skills (it should only be necessary to
+    MissingSkill,
-    /// note direct prerequisites)
-    pub fn prerequisite_skills(&self) -> impl Iterator<Item = (Skill, Option<u16>)> {
-        SKILL_PREREQUISITES
-            .get(self)
-            .into_iter()
-            .flatten()
-            .map(|(skill, level)| (*skill, *level))
-    }
-
-    /// Returns the cost in skill points of unlocking a particular skill
-    pub fn skill_cost(&self, level: Option<u16>) -> u16 {
-        // TODO: Better balance the costs later
-        level.unwrap_or(1)
-    }
-
-    /// Returns the maximum level a skill can reach, returns None if the skill
-    /// doesn't level
-    pub fn max_level(&self) -> Option<u16> { SKILL_MAX_LEVEL.get(self).copied().flatten() }
-
-    /// Returns the skill group type for a skill from the static skill group
-    /// definitions.
-    pub fn skill_group_kind(&self) -> Option<SkillGroupKind> {
-        SKILL_GROUP_LOOKUP.get(self).copied()
-    }
 }
 
 pub enum SpRewardError {
@@ -1088,6 +467,7 @@ pub enum SpRewardError {
 #[cfg(test)]
 mod tests {
     use super::*;
+    // Code reviewers: Open a comment here, I want to refactor these tests
 
     #[test]
     fn test_refund_skill() {

common/src/comp/skillset/skills.rs

@@ -0,0 +1,589 @@
+use crate::comp::skillset::{
+    SkillGroupKind, SKILL_GROUP_LOOKUP, SKILL_MAX_LEVEL, SKILL_PREREQUISITES,
+};
+use serde::{Deserialize, Serialize};
+
+/// Represents a skill that a player can unlock, that either grants them some
+/// kind of active ability, or a passive effect etc. Obviously because this is
+/// an enum it doesn't describe what the skill actually -does-, this will be
+/// handled by dedicated ECS systems.
+// NOTE: if skill does use some constant, add it to corresponding
+// SkillTree Modifiers below.
+#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
+pub enum Skill {
+    General(GeneralSkill),
+    Sword(SwordSkill),
+    Axe(AxeSkill),
+    Hammer(HammerSkill),
+    Bow(BowSkill),
+    Staff(StaffSkill),
+    Sceptre(SceptreSkill),
+    UnlockGroup(SkillGroupKind),
+    Roll(RollSkill),
+    Climb(ClimbSkill),
+    Swim(SwimSkill),
+    Pick(MiningSkill),
+}
+
+#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
+pub enum SwordSkill {
+    // Sword passives
+    InterruptingAttacks,
+    // Triple strike upgrades
+    TsCombo,
+    TsDamage,
+    TsRegen,
+    TsSpeed,
+    // Dash upgrades
+    DCost,
+    DDrain,
+    DDamage,
+    DScaling,
+    DSpeed,
+    DInfinite, // Represents charge through, not migrated because laziness
+    // Spin upgrades
+    UnlockSpin,
+    SDamage,
+    SSpeed,
+    SCost,
+    SSpins,
+}
+
+#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
+pub enum AxeSkill {
+    // Double strike upgrades
+    DsCombo,
+    DsDamage,
+    DsSpeed,
+    DsRegen,
+    // Spin upgrades
+    SInfinite,
+    SHelicopter,
+    SDamage,
+    SSpeed,
+    SCost,
+    // Leap upgrades
+    UnlockLeap,
+    LDamage,
+    LKnockback,
+    LCost,
+    LDistance,
+}
+
+#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
+pub enum HammerSkill {
+    // Single strike upgrades
+    SsKnockback,
+    SsDamage,
+    SsSpeed,
+    SsRegen,
+    // Charged melee upgrades
+    CDamage,
+    CKnockback,
+    CDrain,
+    CSpeed,
+    // Leap upgrades
+    UnlockLeap,
+    LDamage,
+    LCost,
+    LDistance,
+    LKnockback,
+    LRange,
+}
+
+#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
+pub enum BowSkill {
+    // Passives
+    ProjSpeed,
+    // Charged upgrades
+    CDamage,
+    CRegen,
+    CKnockback,
+    CSpeed,
+    CMove,
+    // Repeater upgrades
+    RDamage,
+    RCost,
+    RSpeed,
+    // Shotgun upgrades
+    UnlockShotgun,
+    SDamage,
+    SCost,
+    SArrows,
+    SSpread,
+}
+
+#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
+pub enum StaffSkill {
+    // Basic ranged upgrades
+    BDamage,
+    BRegen,
+    BRadius,
+    // Flamethrower upgrades
+    FDamage,
+    FRange,
+    FDrain,
+    FVelocity,
+    // Shockwave upgrades
+    UnlockShockwave,
+    SDamage,
+    SKnockback,
+    SRange,
+    SCost,
+}
+
+#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
+pub enum SceptreSkill {
+    // Lifesteal beam upgrades
+    LDamage,
+    LRange,
+    LLifesteal,
+    LRegen,
+    // Healing aura upgrades
+    HHeal,
+    HRange,
+    HDuration,
+    HCost,
+    // Warding aura upgrades
+    UnlockAura,
+    AStrength,
+    ADuration,
+    ARange,
+    ACost,
+}
+
+#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
+pub enum GeneralSkill {
+    HealthIncrease,
+    EnergyIncrease,
+}
+
+#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
+pub enum RollSkill {
+    Cost,
+    Strength,
+    Duration,
+}
+
+#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
+pub enum ClimbSkill {
+    Cost,
+    Speed,
+}
+
+#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
+pub enum SwimSkill {
+    Speed,
+}
+
+#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)]
+pub enum MiningSkill {
+    Speed,
+    OreGain,
+    GemGain,
+}
+
+impl Skill {
+    /// Returns a vec of prerequisite skills (it should only be necessary to
+    /// note direct prerequisites)
+    pub fn prerequisite_skills(&self) -> impl Iterator<Item = (Skill, Option<u16>)> {
+        SKILL_PREREQUISITES
+            .get(self)
+            .into_iter()
+            .flatten()
+            .map(|(skill, level)| (*skill, *level))
+    }
+
+    /// Returns the cost in skill points of unlocking a particular skill
+    pub fn skill_cost(&self, level: Option<u16>) -> u16 {
+        // TODO: Better balance the costs later
+        level.unwrap_or(1)
+    }
+
+    /// Returns the maximum level a skill can reach, returns None if the skill
+    /// doesn't level
+    pub fn max_level(&self) -> Option<u16> { SKILL_MAX_LEVEL.get(self).copied().flatten() }
+
+    /// Returns the skill group type for a skill from the static skill group
+    /// definitions.
+    pub fn skill_group_kind(&self) -> Option<SkillGroupKind> {
+        SKILL_GROUP_LOOKUP.get(self).copied()
+    }
+}
+
+/// Tree of modifiers that represent how stats are
+/// changed per each skill level.
+///
+/// It's used as bridge between ECS systems
+/// and voxygen Diary for skill descriptions and helps to sync them.
+///
+/// NOTE: Just adding constant does nothing, you need to use it in both
+/// ECS systems and Diary.
+// TODO: make it lazy_static and move to .ron?
+pub const SKILL_MODIFIERS: SkillTreeModifiers = SkillTreeModifiers::get();
+
+pub struct SkillTreeModifiers {
+    pub sword_tree: SwordTreeModifiers,
+    pub axe_tree: AxeTreeModifiers,
+    pub hammer_tree: HammerTreeModifiers,
+    pub bow_tree: BowTreeModifiers,
+    pub staff_tree: StaffTreeModifiers,
+    pub sceptre_tree: SceptreTreeModifiers,
+    pub mining_tree: MiningTreeModifiers,
+    pub general_tree: GeneralTreeModifiers,
+}
+
+impl SkillTreeModifiers {
+    const fn get() -> Self {
+        Self {
+            sword_tree: SwordTreeModifiers::get(),
+            axe_tree: AxeTreeModifiers::get(),
+            hammer_tree: HammerTreeModifiers::get(),
+            bow_tree: BowTreeModifiers::get(),
+            staff_tree: StaffTreeModifiers::get(),
+            sceptre_tree: SceptreTreeModifiers::get(),
+            mining_tree: MiningTreeModifiers::get(),
+            general_tree: GeneralTreeModifiers::get(),
+        }
+    }
+}
+
+pub struct SwordTreeModifiers {
+    pub dash: SwordDashModifiers,
+    pub spin: SwordSpinModifiers,
+}
+
+pub struct SwordDashModifiers {
+    pub energy_cost: f32,
+    pub energy_drain: f32,
+    pub base_damage: f32,
+    pub scaled_damage: f32,
+    pub forward_speed: f32,
+}
+
+pub struct SwordSpinModifiers {
+    pub base_damage: f32,
+    pub swing_duration: f32,
+    pub energy_cost: f32,
+    pub num: u32,
+}
+
+impl SwordTreeModifiers {
+    const fn get() -> Self {
+        Self {
+            dash: SwordDashModifiers {
+                energy_cost: 0.9,
+                energy_drain: 0.9,
+                base_damage: 1.1,
+                scaled_damage: 1.1,
+                forward_speed: 1.05,
+            },
+            spin: SwordSpinModifiers {
+                base_damage: 1.2,
+                swing_duration: 0.9,
+                energy_cost: 0.9,
+                num: 1,
+            },
+        }
+    }
+}
+
+pub struct AxeTreeModifiers {
+    pub spin: AxeSpinModifiers,
+    pub leap: AxeLeapModifiers,
+}
+
+pub struct AxeSpinModifiers {
+    pub base_damage: f32,
+    pub swing_duration: f32,
+    pub energy_cost: f32,
+}
+
+pub struct AxeLeapModifiers {
+    pub base_damage: f32,
+    pub knockback: f32,
+    pub energy_cost: f32,
+    // TODO: split to forward and vertical?
+    pub leap_strength: f32,
+}
+
+impl AxeTreeModifiers {
+    const fn get() -> Self {
+        Self {
+            spin: AxeSpinModifiers {
+                base_damage: 1.2,
+                swing_duration: 0.85,
+                energy_cost: 0.85,
+            },
+            leap: AxeLeapModifiers {
+                base_damage: 1.2,
+                knockback: 1.2,
+                energy_cost: 0.75,
+                leap_strength: 1.1,
+            },
+        }
+    }
+}
+
+pub struct HammerTreeModifiers {
+    pub single_strike: HammerStrikeModifiers,
+    pub charged: HammerChargedModifers,
+    pub leap: HammerLeapModifiers,
+}
+
+pub struct HammerStrikeModifiers {
+    pub knockback: f32,
+}
+
+pub struct HammerChargedModifers {
+    pub scaled_damage: f32,
+    pub scaled_knockback: f32,
+    pub energy_drain: f32,
+    pub charge_rate: f32,
+}
+
+pub struct HammerLeapModifiers {
+    pub base_damage: f32,
+    pub knockback: f32,
+    pub energy_cost: f32,
+    pub leap_strength: f32,
+    pub range: f32,
+}
+
+impl HammerTreeModifiers {
+    const fn get() -> Self {
+        Self {
+            single_strike: HammerStrikeModifiers { knockback: 1.25 },
+            charged: HammerChargedModifers {
+                scaled_damage: 1.2,
+                scaled_knockback: 1.3,
+                energy_drain: 0.85,
+                charge_rate: 1.15,
+            },
+            leap: HammerLeapModifiers {
+                base_damage: 1.25,
+                knockback: 1.3,
+                energy_cost: 0.75,
+                leap_strength: 1.1,
+                range: 0.5,
+            },
+        }
+    }
+}
+
+pub struct BowTreeModifiers {
+    pub universal: BowUniversalModifiers,
+    pub charged: BowChargedModifiers,
+    pub repeater: BowRepeaterModifiers,
+    pub shotgun: BowShotgunModifiers,
+}
+
+pub struct BowUniversalModifiers {
+    // TODO: split per abilities?
+    pub projectile_speed: f32,
+}
+
+pub struct BowChargedModifiers {
+    pub damage_scaling: f32,
+    pub regen_scaling: f32,
+    pub knockback_scaling: f32,
+    pub charge_rate: f32,
+    pub move_speed: f32,
+}
+
+pub struct BowRepeaterModifiers {
+    pub power: f32,
+    pub energy_cost: f32,
+    pub max_speed: f32,
+}
+
+pub struct BowShotgunModifiers {
+    pub power: f32,
+    pub energy_cost: f32,
+    pub num_projectiles: u32,
+    pub spread: f32,
+}
+
+impl BowTreeModifiers {
+    const fn get() -> Self {
+        Self {
+            universal: BowUniversalModifiers {
+                projectile_speed: 1.1,
+            },
+            charged: BowChargedModifiers {
+                damage_scaling: 1.1,
+                regen_scaling: 1.1,
+                knockback_scaling: 1.1,
+                charge_rate: 1.1,
+                move_speed: 1.1,
+            },
+            repeater: BowRepeaterModifiers {
+                power: 1.1,
+                energy_cost: 0.9,
+                max_speed: 1.2,
+            },
+            shotgun: BowShotgunModifiers {
+                power: 1.1,
+                energy_cost: 0.9,
+                num_projectiles: 1,
+                spread: 0.9,
+            },
+        }
+    }
+}
+
+pub struct StaffTreeModifiers {
+    pub fireball: StaffFireballModifiers,
+    pub flamethrower: StaffFlamethrowerModifiers,
+    pub shockwave: StaffShockwaveModifiers,
+}
+
+pub struct StaffFireballModifiers {
+    pub power: f32,
+    pub regen: f32,
+    pub range: f32,
+}
+
+pub struct StaffFlamethrowerModifiers {
+    pub damage: f32,
+    pub range: f32,
+    pub energy_drain: f32,
+    pub velocity: f32,
+}
+
+pub struct StaffShockwaveModifiers {
+    pub damage: f32,
+    pub knockback: f32,
+    pub duration: f32,
+    pub energy_cost: f32,
+}
+
+impl StaffTreeModifiers {
+    const fn get() -> Self {
+        Self {
+            fireball: StaffFireballModifiers {
+                power: 1.1,
+                regen: 1.1,
+                range: 1.1,
+            },
+            flamethrower: StaffFlamethrowerModifiers {
+                damage: 1.2,
+                range: 1.1,
+                energy_drain: 0.9,
+                velocity: 1.1,
+            },
+            shockwave: StaffShockwaveModifiers {
+                damage: 1.15,
+                knockback: 1.15,
+                duration: 1.1,
+                energy_cost: 0.9,
+            },
+        }
+    }
+}
+
+pub struct SceptreTreeModifiers {
+    pub beam: SceptreBeamModifiers,
+    pub healing_aura: SceptreHealingAuraModifiers,
+    pub warding_aura: SceptreWardingAuraModifiers,
+}
+
+pub struct SceptreBeamModifiers {
+    pub damage: f32,
+    pub range: f32,
+    pub energy_regen: f32,
+    pub lifesteal: f32,
+}
+
+pub struct SceptreHealingAuraModifiers {
+    pub strength: f32,
+    pub duration: f32,
+    pub range: f32,
+    pub energy_cost: f32,
+}
+
+pub struct SceptreWardingAuraModifiers {
+    pub strength: f32,
+    pub duration: f32,
+    pub range: f32,
+    pub energy_cost: f32,
+}
+
+impl SceptreTreeModifiers {
+    const fn get() -> Self {
+        Self {
+            beam: SceptreBeamModifiers {
+                damage: 1.1,
+                range: 1.1,
+                energy_regen: 1.1,
+                lifesteal: 1.05,
+            },
+            healing_aura: SceptreHealingAuraModifiers {
+                strength: 1.05,
+                duration: 1.1,
+                range: 1.1,
+                energy_cost: 0.90,
+            },
+            warding_aura: SceptreWardingAuraModifiers {
+                strength: 1.05,
+                duration: 1.1,
+                range: 1.1,
+                energy_cost: 0.95,
+            },
+        }
+    }
+}
+
+pub struct MiningTreeModifiers {
+    pub speed: f32,
+    pub gem_gain: f32,
+    pub ore_gain: f32,
+}
+
+impl MiningTreeModifiers {
+    const fn get() -> Self {
+        Self {
+            speed: 1.1,
+            gem_gain: 0.05,
+            ore_gain: 0.05,
+        }
+    }
+}
+
+pub struct GeneralTreeModifiers {
+    pub roll: RollTreeModifiers,
+    pub swim: SwimTreeModifiers,
+    pub climb: ClimbTreeModifiers,
+}
+
+pub struct RollTreeModifiers {
+    pub energy_cost: f32,
+    pub strength: f32,
+    pub duration: f32,
+}
+
+pub struct SwimTreeModifiers {
+    pub speed: f32,
+}
+
+pub struct ClimbTreeModifiers {
+    pub energy_cost: f32,
+    pub speed: f32,
+}
+
+impl GeneralTreeModifiers {
+    const fn get() -> Self {
+        Self {
+            roll: RollTreeModifiers {
+                energy_cost: 0.95,
+                strength: 1.05,
+                duration: 1.05,
+            },
+            swim: SwimTreeModifiers { speed: 1.25 },
+            climb: ClimbTreeModifiers {
+                energy_cost: 0.8,
+                speed: 1.2,
+            },
+        }
+    }
+}

common/src/outcome.rs

@@ -1,5 +1,5 @@
 use crate::{comp, uid::Uid};
-use comp::{beam, item::Reagent, poise::PoiseState, skills::SkillGroupKind, UtteranceKind};
+use comp::{beam, item::Reagent, poise::PoiseState, skillset::SkillGroupKind, UtteranceKind};
 use hashbrown::HashSet;
 use serde::{Deserialize, Serialize};
 use vek::*;
@@ -41,7 +41,7 @@ pub enum Outcome {
     },
     SkillPointGain {
         uid: Uid,
-        skill_tree: comp::skills::SkillGroupKind,
+        skill_tree: comp::skillset::SkillGroupKind,
         total_points: u16,
         // TODO: Access ECS to get position from Uid to conserve bandwidth
         pos: Vec3<f32>,

common/src/skillset_builder.rs

@@ -1,6 +1,6 @@
 #![warn(clippy::pedantic)]
 //#![warn(clippy::nursery)]
-use crate::comp::skills::{Skill, SkillGroupKind, SkillSet};
+use crate::comp::skillset::{skills::Skill, SkillGroupKind, SkillSet};
 
 use crate::assets::{self, AssetExt};
 use serde::{Deserialize, Serialize};

server/src/cmd.rs

@@ -2854,8 +2854,8 @@ fn handle_skill_point(
     }
 }
 
-fn parse_skill_tree(skill_tree: &str) -> CmdResult<comp::skills::SkillGroupKind> {
+fn parse_skill_tree(skill_tree: &str) -> CmdResult<comp::skillset::SkillGroupKind> {
-    use comp::{item::tool::ToolKind, skills::SkillGroupKind};
+    use comp::{item::tool::ToolKind, skillset::SkillGroupKind};
     match skill_tree {
         "general" => Ok(SkillGroupKind::General),
         "sword" => Ok(SkillGroupKind::Weapon(ToolKind::Sword)),

server/src/events/entity_manipulation.rs

@@ -2,7 +2,7 @@ use crate::{
     client::Client,
     comp::{
         agent::{Agent, AgentEvent, Sound, SoundKind},
-        skills::SkillGroupKind,
+        skillset::SkillGroupKind,
         BuffKind, BuffSource, PhysicsState,
     },
     rtsim::RtSim,

server/src/persistence/character/conversions.rs

@@ -16,7 +16,7 @@ use common::{
             loadout_builder::LoadoutBuilder,
             slot::InvSlotId,
         },
-        skills, Body as CompBody, Waypoint, *,
+        skillset, Body as CompBody, Waypoint, *,
     },
     resources::Time,
 };
@@ -504,9 +504,10 @@ pub fn convert_skill_set_from_database(
     skills: &[Skill],
     skill_groups: &[SkillGroup],
 ) -> common::comp::SkillSet {
-    skills::SkillSet {
+    skillset::SkillSet {
         skill_groups: convert_skill_groups_from_database(skill_groups),
         skills: convert_skills_from_database(skills),
+        ordered_skills: Vec::new(),
         modify_health: true,
         modify_energy: true,
     }
@@ -521,11 +522,11 @@ fn get_item_from_asset(item_definition_id: &str) -> Result<common::comp::Item, P
     })
 }
 
-fn convert_skill_groups_from_database(skill_groups: &[SkillGroup]) -> Vec<skills::SkillGroup> {
+fn convert_skill_groups_from_database(skill_groups: &[SkillGroup]) -> Vec<skillset::SkillGroup> {
     let mut new_skill_groups = Vec::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 = skills::SkillGroup {
+        let mut new_skill_group = skillset::SkillGroup {
             skill_group_kind,
             earned_exp: skill_group.earned_exp as u32,
             spent_exp: 0,
@@ -543,15 +544,16 @@ fn convert_skill_groups_from_database(skill_groups: &[SkillGroup]) -> Vec<skills
 fn convert_skills_from_database(skills: &[Skill]) -> HashMap<skills::Skill, Option<u16>> {
     let mut new_skills = HashMap::new();
     for skill in skills.iter() {
-        let new_skill = json_models::db_string_to_skill(&skill.skill);
+        if let Some(new_skill) = json_models::db_string_to_skill(&skill.skill) {
-        new_skills.insert(new_skill, skill.level.map(|l| l as u16));
+            new_skills.insert(new_skill, skill.level.map(|l| l as u16));
+        }
     }
     new_skills
 }
 
 pub fn convert_skill_groups_to_database(
     entity_id: CharacterId,
-    skill_groups: Vec<skills::SkillGroup>,
+    skill_groups: Vec<skillset::SkillGroup>,
 ) -> Vec<SkillGroup> {
     skill_groups
         .into_iter()

server/src/persistence/json_models.rs

@@ -1,6 +1,7 @@
 use common::comp;
 use serde::{Deserialize, Serialize};
 use std::string::ToString;
+use tracing::error;
 use vek::Vec3;
 
 #[derive(Serialize, Deserialize)]
@@ -68,8 +69,9 @@ pub fn skill_to_db_string(skill: comp::skills::Skill) -> String {
         item::tool::ToolKind,
         skills::{
             AxeSkill, BowSkill, ClimbSkill, GeneralSkill, HammerSkill, MiningSkill, RollSkill,
-            SceptreSkill, Skill::*, SkillGroupKind, StaffSkill, SwimSkill, SwordSkill,
+            SceptreSkill, Skill::*, StaffSkill, SwimSkill, SwordSkill,
         },
+        skillset::SkillGroupKind,
     };
     let skill_string = match skill {
         General(GeneralSkill::HealthIncrease) => "General HealthIncrease",
@@ -181,132 +183,135 @@ pub fn skill_to_db_string(skill: comp::skills::Skill) -> String {
         | UnlockGroup(SkillGroupKind::Weapon(ToolKind::Empty))
         | UnlockGroup(SkillGroupKind::Weapon(ToolKind::Natural))
         | UnlockGroup(SkillGroupKind::General) => {
-            panic!("Tried to add unsupported skill to database: {:?}", skill)
+            error!("Tried to add unsupported skill to database: {:?}", skill);
+            "Invalid Skill"
         },
     };
     skill_string.to_string()
 }
 
-pub fn db_string_to_skill(skill_string: &str) -> comp::skills::Skill {
+pub fn db_string_to_skill(skill_string: &str) -> Option<comp::skills::Skill> {
     use comp::{
         item::tool::ToolKind,
         skills::{
             AxeSkill, BowSkill, ClimbSkill, GeneralSkill, HammerSkill, MiningSkill, RollSkill,
-            SceptreSkill, Skill::*, SkillGroupKind, StaffSkill, SwimSkill, SwordSkill,
+            SceptreSkill, Skill::*, StaffSkill, SwimSkill, SwordSkill,
         },
+        skillset::SkillGroupKind,
     };
     match skill_string {
-        "General HealthIncrease" => General(GeneralSkill::HealthIncrease),
+        "General HealthIncrease" => Some(General(GeneralSkill::HealthIncrease)),
-        "General EnergyIncrease" => General(GeneralSkill::EnergyIncrease),
+        "General EnergyIncrease" => Some(General(GeneralSkill::EnergyIncrease)),
-        "Sword InterruptingAttacks" => Sword(SwordSkill::InterruptingAttacks),
+        "Sword InterruptingAttacks" => Some(Sword(SwordSkill::InterruptingAttacks)),
-        "Sword TsCombo" => Sword(SwordSkill::TsCombo),
+        "Sword TsCombo" => Some(Sword(SwordSkill::TsCombo)),
-        "Sword TsDamage" => Sword(SwordSkill::TsDamage),
+        "Sword TsDamage" => Some(Sword(SwordSkill::TsDamage)),
-        "Sword TsRegen" => Sword(SwordSkill::TsRegen),
+        "Sword TsRegen" => Some(Sword(SwordSkill::TsRegen)),
-        "Sword TsSpeed" => Sword(SwordSkill::TsSpeed),
+        "Sword TsSpeed" => Some(Sword(SwordSkill::TsSpeed)),
-        "Sword DCost" => Sword(SwordSkill::DCost),
+        "Sword DCost" => Some(Sword(SwordSkill::DCost)),
-        "Sword DDrain" => Sword(SwordSkill::DDrain),
+        "Sword DDrain" => Some(Sword(SwordSkill::DDrain)),
-        "Sword DDamage" => Sword(SwordSkill::DDamage),
+        "Sword DDamage" => Some(Sword(SwordSkill::DDamage)),
-        "Sword DScaling" => Sword(SwordSkill::DScaling),
+        "Sword DScaling" => Some(Sword(SwordSkill::DScaling)),
-        "Sword DSpeed" => Sword(SwordSkill::DSpeed),
+        "Sword DSpeed" => Some(Sword(SwordSkill::DSpeed)),
-        "Sword DInfinite" => Sword(SwordSkill::DInfinite),
+        "Sword DInfinite" => Some(Sword(SwordSkill::DInfinite)),
-        "Sword UnlockSpin" => Sword(SwordSkill::UnlockSpin),
+        "Sword UnlockSpin" => Some(Sword(SwordSkill::UnlockSpin)),
-        "Sword SDamage" => Sword(SwordSkill::SDamage),
+        "Sword SDamage" => Some(Sword(SwordSkill::SDamage)),
-        "Sword SSpeed" => Sword(SwordSkill::SSpeed),
+        "Sword SSpeed" => Some(Sword(SwordSkill::SSpeed)),
-        "Sword SCost" => Sword(SwordSkill::SCost),
+        "Sword SCost" => Some(Sword(SwordSkill::SCost)),
-        "Sword SSpins" => Sword(SwordSkill::SSpins),
+        "Sword SSpins" => Some(Sword(SwordSkill::SSpins)),
-        "Axe DsCombo" => Axe(AxeSkill::DsCombo),
+        "Axe DsCombo" => Some(Axe(AxeSkill::DsCombo)),
-        "Axe DsDamage" => Axe(AxeSkill::DsDamage),
+        "Axe DsDamage" => Some(Axe(AxeSkill::DsDamage)),
-        "Axe DsSpeed" => Axe(AxeSkill::DsSpeed),
+        "Axe DsSpeed" => Some(Axe(AxeSkill::DsSpeed)),
-        "Axe DsRegen" => Axe(AxeSkill::DsRegen),
+        "Axe DsRegen" => Some(Axe(AxeSkill::DsRegen)),
-        "Axe SInfinite" => Axe(AxeSkill::SInfinite),
+        "Axe SInfinite" => Some(Axe(AxeSkill::SInfinite)),
-        "Axe SHelicopter" => Axe(AxeSkill::SHelicopter),
+        "Axe SHelicopter" => Some(Axe(AxeSkill::SHelicopter)),
-        "Axe SDamage" => Axe(AxeSkill::SDamage),
+        "Axe SDamage" => Some(Axe(AxeSkill::SDamage)),
-        "Axe SSpeed" => Axe(AxeSkill::SSpeed),
+        "Axe SSpeed" => Some(Axe(AxeSkill::SSpeed)),
-        "Axe SCost" => Axe(AxeSkill::SCost),
+        "Axe SCost" => Some(Axe(AxeSkill::SCost)),
-        "Axe UnlockLeap" => Axe(AxeSkill::UnlockLeap),
+        "Axe UnlockLeap" => Some(Axe(AxeSkill::UnlockLeap)),
-        "Axe LDamage" => Axe(AxeSkill::LDamage),
+        "Axe LDamage" => Some(Axe(AxeSkill::LDamage)),
-        "Axe LKnockback" => Axe(AxeSkill::LKnockback),
+        "Axe LKnockback" => Some(Axe(AxeSkill::LKnockback)),
-        "Axe LCost" => Axe(AxeSkill::LCost),
+        "Axe LCost" => Some(Axe(AxeSkill::LCost)),
-        "Axe LDistance" => Axe(AxeSkill::LDistance),
+        "Axe LDistance" => Some(Axe(AxeSkill::LDistance)),
-        "Hammer SsKnockback" => Hammer(HammerSkill::SsKnockback),
+        "Hammer SsKnockback" => Some(Hammer(HammerSkill::SsKnockback)),
-        "Hammer SsDamage" => Hammer(HammerSkill::SsDamage),
+        "Hammer SsDamage" => Some(Hammer(HammerSkill::SsDamage)),
-        "Hammer SsSpeed" => Hammer(HammerSkill::SsSpeed),
+        "Hammer SsSpeed" => Some(Hammer(HammerSkill::SsSpeed)),
-        "Hammer SsRegen" => Hammer(HammerSkill::SsRegen),
+        "Hammer SsRegen" => Some(Hammer(HammerSkill::SsRegen)),
-        "Hammer CDamage" => Hammer(HammerSkill::CDamage),
+        "Hammer CDamage" => Some(Hammer(HammerSkill::CDamage)),
-        "Hammer CKnockback" => Hammer(HammerSkill::CKnockback),
+        "Hammer CKnockback" => Some(Hammer(HammerSkill::CKnockback)),
-        "Hammer CDrain" => Hammer(HammerSkill::CDrain),
+        "Hammer CDrain" => Some(Hammer(HammerSkill::CDrain)),
-        "Hammer CSpeed" => Hammer(HammerSkill::CSpeed),
+        "Hammer CSpeed" => Some(Hammer(HammerSkill::CSpeed)),
-        "Hammer UnlockLeap" => Hammer(HammerSkill::UnlockLeap),
+        "Hammer UnlockLeap" => Some(Hammer(HammerSkill::UnlockLeap)),
-        "Hammer LDamage" => Hammer(HammerSkill::LDamage),
+        "Hammer LDamage" => Some(Hammer(HammerSkill::LDamage)),
-        "Hammer LCost" => Hammer(HammerSkill::LCost),
+        "Hammer LCost" => Some(Hammer(HammerSkill::LCost)),
-        "Hammer LDistance" => Hammer(HammerSkill::LDistance),
+        "Hammer LDistance" => Some(Hammer(HammerSkill::LDistance)),
-        "Hammer LKnockback" => Hammer(HammerSkill::LKnockback),
+        "Hammer LKnockback" => Some(Hammer(HammerSkill::LKnockback)),
-        "Hammer LRange" => Hammer(HammerSkill::LRange),
+        "Hammer LRange" => Some(Hammer(HammerSkill::LRange)),
-        "Bow ProjSpeed" => Bow(BowSkill::ProjSpeed),
+        "Bow ProjSpeed" => Some(Bow(BowSkill::ProjSpeed)),
-        "Bow CDamage" => Bow(BowSkill::CDamage),
+        "Bow CDamage" => Some(Bow(BowSkill::CDamage)),
-        "Bow CRegen" => Bow(BowSkill::CRegen),
+        "Bow CRegen" => Some(Bow(BowSkill::CRegen)),
-        "Bow CKnockback" => Bow(BowSkill::CKnockback),
+        "Bow CKnockback" => Some(Bow(BowSkill::CKnockback)),
-        "Bow CSpeed" => Bow(BowSkill::CSpeed),
+        "Bow CSpeed" => Some(Bow(BowSkill::CSpeed)),
-        "Bow CMove" => Bow(BowSkill::CMove),
+        "Bow CMove" => Some(Bow(BowSkill::CMove)),
-        "Bow RDamage" => Bow(BowSkill::RDamage),
+        "Bow RDamage" => Some(Bow(BowSkill::RDamage)),
-        "Bow RCost" => Bow(BowSkill::RCost),
+        "Bow RCost" => Some(Bow(BowSkill::RCost)),
-        "Bow RSpeed" => Bow(BowSkill::RSpeed),
+        "Bow RSpeed" => Some(Bow(BowSkill::RSpeed)),
-        "Bow UnlockShotgun" => Bow(BowSkill::UnlockShotgun),
+        "Bow UnlockShotgun" => Some(Bow(BowSkill::UnlockShotgun)),
-        "Bow SDamage" => Bow(BowSkill::SDamage),
+        "Bow SDamage" => Some(Bow(BowSkill::SDamage)),
-        "Bow SCost" => Bow(BowSkill::SCost),
+        "Bow SCost" => Some(Bow(BowSkill::SCost)),
-        "Bow SArrows" => Bow(BowSkill::SArrows),
+        "Bow SArrows" => Some(Bow(BowSkill::SArrows)),
-        "Bow SSpread" => Bow(BowSkill::SSpread),
+        "Bow SSpread" => Some(Bow(BowSkill::SSpread)),
-        "Staff BDamage" => Staff(StaffSkill::BDamage),
+        "Staff BDamage" => Some(Staff(StaffSkill::BDamage)),
-        "Staff BRegen" => Staff(StaffSkill::BRegen),
+        "Staff BRegen" => Some(Staff(StaffSkill::BRegen)),
-        "Staff BRadius" => Staff(StaffSkill::BRadius),
+        "Staff BRadius" => Some(Staff(StaffSkill::BRadius)),
-        "Staff FDamage" => Staff(StaffSkill::FDamage),
+        "Staff FDamage" => Some(Staff(StaffSkill::FDamage)),
-        "Staff FRange" => Staff(StaffSkill::FRange),
+        "Staff FRange" => Some(Staff(StaffSkill::FRange)),
-        "Staff FDrain" => Staff(StaffSkill::FDrain),
+        "Staff FDrain" => Some(Staff(StaffSkill::FDrain)),
-        "Staff FVelocity" => Staff(StaffSkill::FVelocity),
+        "Staff FVelocity" => Some(Staff(StaffSkill::FVelocity)),
-        "Staff UnlockShockwave" => Staff(StaffSkill::UnlockShockwave),
+        "Staff UnlockShockwave" => Some(Staff(StaffSkill::UnlockShockwave)),
-        "Staff SDamage" => Staff(StaffSkill::SDamage),
+        "Staff SDamage" => Some(Staff(StaffSkill::SDamage)),
-        "Staff SKnockback" => Staff(StaffSkill::SKnockback),
+        "Staff SKnockback" => Some(Staff(StaffSkill::SKnockback)),
-        "Staff SRange" => Staff(StaffSkill::SRange),
+        "Staff SRange" => Some(Staff(StaffSkill::SRange)),
-        "Staff SCost" => Staff(StaffSkill::SCost),
+        "Staff SCost" => Some(Staff(StaffSkill::SCost)),
-        "Sceptre LDamage" => Sceptre(SceptreSkill::LDamage),
+        "Sceptre LDamage" => Some(Sceptre(SceptreSkill::LDamage)),
-        "Sceptre LRange" => Sceptre(SceptreSkill::LRange),
+        "Sceptre LRange" => Some(Sceptre(SceptreSkill::LRange)),
-        "Sceptre LLifesteal" => Sceptre(SceptreSkill::LLifesteal),
+        "Sceptre LLifesteal" => Some(Sceptre(SceptreSkill::LLifesteal)),
-        "Sceptre LRegen" => Sceptre(SceptreSkill::LRegen),
+        "Sceptre LRegen" => Some(Sceptre(SceptreSkill::LRegen)),
-        "Sceptre HHeal" => Sceptre(SceptreSkill::HHeal),
+        "Sceptre HHeal" => Some(Sceptre(SceptreSkill::HHeal)),
-        "Sceptre HDuration" => Sceptre(SceptreSkill::HDuration),
+        "Sceptre HDuration" => Some(Sceptre(SceptreSkill::HDuration)),
-        "Sceptre HRange" => Sceptre(SceptreSkill::HRange),
+        "Sceptre HRange" => Some(Sceptre(SceptreSkill::HRange)),
-        "Sceptre HCost" => Sceptre(SceptreSkill::HCost),
+        "Sceptre HCost" => Some(Sceptre(SceptreSkill::HCost)),
-        "Sceptre UnlockAura" => Sceptre(SceptreSkill::UnlockAura),
+        "Sceptre UnlockAura" => Some(Sceptre(SceptreSkill::UnlockAura)),
-        "Sceptre AStrength" => Sceptre(SceptreSkill::AStrength),
+        "Sceptre AStrength" => Some(Sceptre(SceptreSkill::AStrength)),
-        "Sceptre ADuration" => Sceptre(SceptreSkill::ADuration),
+        "Sceptre ADuration" => Some(Sceptre(SceptreSkill::ADuration)),
-        "Sceptre ARange" => Sceptre(SceptreSkill::ARange),
+        "Sceptre ARange" => Some(Sceptre(SceptreSkill::ARange)),
-        "Sceptre ACost" => Sceptre(SceptreSkill::ACost),
+        "Sceptre ACost" => Some(Sceptre(SceptreSkill::ACost)),
-        "Roll Cost" => Roll(RollSkill::Cost),
+        "Roll Cost" => Some(Roll(RollSkill::Cost)),
-        "Roll Strength" => Roll(RollSkill::Strength),
+        "Roll Strength" => Some(Roll(RollSkill::Strength)),
-        "Roll Duration" => Roll(RollSkill::Duration),
+        "Roll Duration" => Some(Roll(RollSkill::Duration)),
-        "Climb Cost" => Climb(ClimbSkill::Cost),
+        "Climb Cost" => Some(Climb(ClimbSkill::Cost)),
-        "Climb Speed" => Climb(ClimbSkill::Speed),
+        "Climb Speed" => Some(Climb(ClimbSkill::Speed)),
-        "Swim Speed" => Swim(SwimSkill::Speed),
+        "Swim Speed" => Some(Swim(SwimSkill::Speed)),
-        "Pick Speed" => Pick(MiningSkill::Speed),
+        "Pick Speed" => Some(Pick(MiningSkill::Speed)),
-        "Pick GemGain" => Pick(MiningSkill::GemGain),
+        "Pick GemGain" => Some(Pick(MiningSkill::GemGain)),
-        "Pick OreGain" => Pick(MiningSkill::OreGain),
+        "Pick OreGain" => Some(Pick(MiningSkill::OreGain)),
-        "Unlock Weapon Sword" => UnlockGroup(SkillGroupKind::Weapon(ToolKind::Sword)),
+        "Unlock Weapon Sword" => Some(UnlockGroup(SkillGroupKind::Weapon(ToolKind::Sword))),
-        "Unlock Weapon Axe" => UnlockGroup(SkillGroupKind::Weapon(ToolKind::Axe)),
+        "Unlock Weapon Axe" => Some(UnlockGroup(SkillGroupKind::Weapon(ToolKind::Axe))),
-        "Unlock Weapon Hammer" => UnlockGroup(SkillGroupKind::Weapon(ToolKind::Hammer)),
+        "Unlock Weapon Hammer" => Some(UnlockGroup(SkillGroupKind::Weapon(ToolKind::Hammer))),
-        "Unlock Weapon Bow" => UnlockGroup(SkillGroupKind::Weapon(ToolKind::Bow)),
+        "Unlock Weapon Bow" => Some(UnlockGroup(SkillGroupKind::Weapon(ToolKind::Bow))),
-        "Unlock Weapon Staff" => UnlockGroup(SkillGroupKind::Weapon(ToolKind::Staff)),
+        "Unlock Weapon Staff" => Some(UnlockGroup(SkillGroupKind::Weapon(ToolKind::Staff))),
-        "Unlock Weapon Sceptre" => UnlockGroup(SkillGroupKind::Weapon(ToolKind::Sceptre)),
+        "Unlock Weapon Sceptre" => Some(UnlockGroup(SkillGroupKind::Weapon(ToolKind::Sceptre))),
         _ => {
-            panic!(
+            error!(
                 "Tried to convert an unsupported string from the database: {}",
                 skill_string
-            )
+            );
+            None
         },
     }
 }
 
-pub fn skill_group_to_db_string(skill_group: comp::skills::SkillGroupKind) -> String {
+pub fn skill_group_to_db_string(skill_group: comp::skillset::SkillGroupKind) -> String {
-    use comp::{item::tool::ToolKind, skills::SkillGroupKind::*};
+    use comp::{item::tool::ToolKind, skillset::SkillGroupKind::*};
     let skill_group_string = match skill_group {
         General => "General",
         Weapon(ToolKind::Sword) => "Weapon Sword",
@@ -330,8 +335,8 @@ pub fn skill_group_to_db_string(skill_group: comp::skills::SkillGroupKind) -> St
     skill_group_string.to_string()
 }
 
-pub fn db_string_to_skill_group(skill_group_string: &str) -> comp::skills::SkillGroupKind {
+pub fn db_string_to_skill_group(skill_group_string: &str) -> comp::skillset::SkillGroupKind {
-    use comp::{item::tool::ToolKind, skills::SkillGroupKind::*};
+    use comp::{item::tool::ToolKind, skillset::SkillGroupKind::*};
     match skill_group_string {
         "General" => General,
         "Weapon Sword" => Weapon(ToolKind::Sword),

voxygen/src/hud/diary.rs

@@ -21,10 +21,9 @@ use common::{
         item::tool::ToolKind,
         skills::{
             self, AxeSkill, BowSkill, ClimbSkill, GeneralSkill, HammerSkill, MiningSkill,
-            RollSkill, SceptreSkill, Skill, SkillGroupKind, StaffSkill, SwimSkill, SwordSkill,
+            RollSkill, SceptreSkill, Skill, StaffSkill, SwimSkill, SwordSkill, SKILL_MODIFIERS,
-            SKILL_MODIFIERS,
         },
-        SkillSet,
+        skillset::{SkillGroupKind, SkillSet},
     },
     consts::{ENERGY_PER_LEVEL, HUMANOID_HP_PER_LEVEL},
 };
@@ -242,7 +241,7 @@ impl<'a> Diary<'a> {
     }
 }
 
-pub type SelectedSkillTree = skills::SkillGroupKind;
+pub type SelectedSkillTree = SkillGroupKind;
 
 // TODO: make it enum?
 const TREES: [&str; 8] = [
@@ -642,7 +641,8 @@ impl<'a> Diary<'a> {
             skills_bot_l,
             skills_bot_r,
         );
-        use skills::{GeneralSkill::*, RollSkill::*, SkillGroupKind::*};
+        use skills::{GeneralSkill::*, RollSkill::*};
+        use SkillGroupKind::*;
         use ToolKind::*;
         // General Combat
         Image::new(animate_by_pulse(

voxygen/src/hud/mod.rs

@@ -77,7 +77,7 @@ use common::{
         self, fluid_dynamics,
         inventory::{slot::InvSlotId, trade_pricing::TradePricing},
         item::{tool::ToolKind, ItemDesc, MaterialStatManifest, Quality},
-        skills::{Skill, SkillGroupKind},
+        skillset::{skills::Skill, SkillGroupKind},
         BuffData, BuffKind, Item,
     },
     consts::MAX_PICKUP_RANGE,