Fixed tests. Addressed comments.

common/src/comp/ability.rs

@@ -314,7 +314,7 @@ impl CharacterAbility {
         }
     }
 
-    pub fn adjust_stats(mut self, power: f32, speed: f32) -> Self {
+    pub fn adjusted_by_stats(mut self, power: f32, speed: f32) -> Self {
         use CharacterAbility::*;
         match self {
             BasicMelee {
@@ -337,7 +337,7 @@ impl CharacterAbility {
             } => {
                 *buildup_duration = (*buildup_duration as f32 / speed) as u64;
                 *recover_duration = (*recover_duration as f32 / speed) as u64;
-                *projectile = projectile.modify_projectile(power);
+                *projectile = projectile.modified_projectile(power);
             },
             RepeaterRanged {
                 ref mut movement_duration,
@@ -351,7 +351,7 @@ impl CharacterAbility {
                 *buildup_duration = (*buildup_duration as f32 / speed) as u64;
                 *shoot_duration = (*shoot_duration as f32 / speed) as u64;
                 *recover_duration = (*recover_duration as f32 / speed) as u64;
-                *projectile = projectile.modify_projectile(power);
+                *projectile = projectile.modified_projectile(power);
             },
             Boost {
                 ref mut movement_duration,
@@ -389,7 +389,7 @@ impl CharacterAbility {
             } => {
                 *stage_data = stage_data
                     .iter_mut()
-                    .map(|s| s.adjust_stats(power, speed))
+                    .map(|s| s.adjusted_by_stats(power, speed))
                     .collect();
             },
             LeapMelee {
@@ -701,11 +701,7 @@ impl From<(&CharacterAbility, AbilityKey)> for CharacterState {
             } => CharacterState::ComboMelee(combo_melee::Data {
                 static_data: combo_melee::StaticData {
                     num_stages: stage_data.len() as u32,
-                    stage_data: stage_data
+                    stage_data: stage_data.iter().map(|stage| stage.to_duration()).collect(),
-                        .clone()
-                        .into_iter()
-                        .map(|stage| stage.to_duration())
-                        .collect(),
                     initial_energy_gain: *initial_energy_gain,
                     max_energy_gain: *max_energy_gain,
                     energy_increase: *energy_increase,

common/src/comp/inventory/item/tool.rs

@@ -87,7 +87,7 @@ impl Tool {
 
     pub fn get_abilities(&self, map: &AbilityMap) -> AbilitySet<CharacterAbility> {
         if let Some(set) = map.0.get(&self.kind).cloned() {
-            set.modify_from_tool(&self)
+            set.modified_by_tool(&self)
         } else {
             error!(
                 "ToolKind: {:?} has no AbilitySet in the ability map falling back to default",
@@ -106,29 +106,17 @@ pub struct AbilitySet<T> {
 }
 
 impl AbilitySet<CharacterAbility> {
-    pub fn modify_from_tool(self, tool: &Tool) -> Self {
+    pub fn modified_by_tool(self, tool: &Tool) -> Self {
-        Self {
+        self.map(|a| a.adjusted_by_stats(tool.base_power(), tool.base_speed()))
-            primary: self
-                .primary
-                .adjust_stats(tool.base_power(), tool.base_speed()),
-            secondary: self
-                .secondary
-                .adjust_stats(tool.base_power(), tool.base_speed()),
-            skills: self
-                .skills
-                .into_iter()
-                .map(|a| a.adjust_stats(tool.base_power(), tool.base_speed()))
-                .collect(),
-        }
     }
 }
 
-impl<T: Clone> AbilitySet<T> {
+impl<T> AbilitySet<T> {
     pub fn map<U, F: FnMut(T) -> U>(self, mut f: F) -> AbilitySet<U> {
         AbilitySet {
             primary: f(self.primary),
             secondary: f(self.secondary),
-            skills: self.skills.iter().map(|x| f(x.clone())).collect(),
+            skills: self.skills.into_iter().map(|x| f(x)).collect(),
         }
     }
 }
@@ -145,6 +133,7 @@ impl Default for AbilitySet<CharacterAbility> {
 
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct AbilityMap<T = CharacterAbility>(HashMap<ToolKind, AbilitySet<T>>);
+
 impl Asset for AbilityMap {
     const ENDINGS: &'static [&'static str] = &["ron"];
 

common/src/comp/inventory/slot.rs

@@ -139,7 +139,9 @@ fn loadout_insert(
 ///
 /// ```
 /// use veloren_common::{
+///     assets::Asset,
 ///     comp::{
+///         item::tool::AbilityMap,
 ///         slot::{loadout_remove, EquipSlot},
 ///         Inventory,
 ///     },
@@ -148,16 +150,19 @@ fn loadout_insert(
 ///
 /// let mut inv = Inventory::new_empty();
 ///
+/// let map = AbilityMap::load_expect_cloned("common.abilities.weapon_ability_manifest");
+///
 /// let mut loadout = LoadoutBuilder::new()
 ///     .defaults()
 ///     .active_item(Some(LoadoutBuilder::default_item_config_from_str(
 ///         "common.items.weapons.sword.zweihander_sword_0",
+///         &map,
 ///     )))
 ///     .build();
 ///
 /// let slot = EquipSlot::Mainhand;
 ///
-/// loadout_remove(slot, &mut loadout);
+/// loadout_remove(slot, &mut loadout, &map);
 /// assert_eq!(None, loadout.active_item);
 /// ```
 pub fn loadout_remove(
@@ -260,6 +265,7 @@ pub fn swap(
 /// use veloren_common::{
 ///     assets::Asset,
 ///     comp::{
+///         item::tool::AbilityMap,
 ///         slot::{equip, EquipSlot},
 ///         Inventory, Item,
 ///     },
@@ -273,7 +279,9 @@ pub fn swap(
 ///
 /// let mut loadout = LoadoutBuilder::new().defaults().build();
 ///
-/// equip(0, &mut inv, &mut loadout);
+/// let map = AbilityMap::load_expect_cloned("common.abilities.weapon_ability_manifest");
+///
+/// equip(0, &mut inv, &mut loadout, &map);
 /// assert_eq!(Some(boots), loadout.foot);
 /// ```
 pub fn equip(slot: usize, inventory: &mut Inventory, loadout: &mut Loadout, map: &AbilityMap) {
@@ -316,7 +324,9 @@ pub fn equip(slot: usize, inventory: &mut Inventory, loadout: &mut Loadout, map:
 ///
 /// ```
 /// use veloren_common::{
+///     assets::Asset,
 ///     comp::{
+///         item::tool::AbilityMap,
 ///         slot::{unequip, EquipSlot},
 ///         Inventory,
 ///     },
@@ -325,16 +335,19 @@ pub fn equip(slot: usize, inventory: &mut Inventory, loadout: &mut Loadout, map:
 ///
 /// let mut inv = Inventory::new_empty();
 ///
+/// let map = AbilityMap::load_expect_cloned("common.abilities.weapon_ability_manifest");
+///
 /// let mut loadout = LoadoutBuilder::new()
 ///     .defaults()
 ///     .active_item(Some(LoadoutBuilder::default_item_config_from_str(
 ///         "common.items.weapons.sword.zweihander_sword_0",
+///         &map,
 ///     )))
 ///     .build();
 ///
 /// let slot = EquipSlot::Mainhand;
 ///
-/// unequip(slot, &mut inv, &mut loadout);
+/// unequip(slot, &mut inv, &mut loadout, &map);
 /// assert_eq!(None, loadout.active_item);
 /// ```
 pub fn unequip(
@@ -361,8 +374,12 @@ mod tests {
             amount: 0,
         };
 
+        use crate::assets::Asset;
+        let map = AbilityMap::load_expect_cloned("common.abilities.weapon_ability_manifest");
+
         let sword = LoadoutBuilder::default_item_config_from_str(
             "common.items.weapons.sword.zweihander_sword_0",
+            &map,
         );
 
         let mut loadout = LoadoutBuilder::new()
@@ -372,11 +389,11 @@ mod tests {
             .build();
 
         assert_eq!(Some(sword.clone()), loadout.active_item);
-        unequip(EquipSlot::Mainhand, &mut inv, &mut loadout);
+        unequip(EquipSlot::Mainhand, &mut inv, &mut loadout, &map);
         // We have space in the inventory, so this should have unequipped
         assert_eq!(None, loadout.active_item);
 
-        unequip(EquipSlot::Offhand, &mut inv, &mut loadout);
+        unequip(EquipSlot::Offhand, &mut inv, &mut loadout, &map);
         // There is no more space in the inventory, so this should still be equipped
         assert_eq!(Some(sword.clone()), loadout.second_item);
 
@@ -402,9 +419,12 @@ mod tests {
 
         let mut loadout = LoadoutBuilder::new().defaults().build();
 
+        use crate::assets::Asset;
+        let map = AbilityMap::load_expect_cloned("common.abilities.weapon_ability_manifest");
+
         // We should start with the starting sandles
         assert_eq!(starting_sandles, loadout.foot);
-        equip(0, &mut inv, &mut loadout);
+        equip(0, &mut inv, &mut loadout, &map);
 
         // We should now have the testing boots equiped
         assert_eq!(boots, loadout.foot);
@@ -424,6 +444,9 @@ mod tests {
             "common.items.armor.starter.sandals_0",
         ));
 
+        use crate::assets::Asset;
+        let map = AbilityMap::load_expect_cloned("common.abilities.weapon_ability_manifest");
+
         let mut loadout = LoadoutBuilder::new().defaults().build();
 
         // We should start with the starting sandles
@@ -436,6 +459,7 @@ mod tests {
                 EquipSlot::Armor(ArmorSlot::Feet),
                 boots.clone(),
                 &mut loadout,
+                &map,
             )
         );
 
@@ -445,8 +469,12 @@ mod tests {
 
     #[test]
     fn test_loadout_remove() {
+        use crate::assets::Asset;
+        let map = AbilityMap::load_expect_cloned("common.abilities.weapon_ability_manifest");
+
         let sword = LoadoutBuilder::default_item_config_from_str(
             "common.items.weapons.sword.zweihander_sword_0",
+            &map,
         );
 
         let mut loadout = LoadoutBuilder::new()
@@ -457,7 +485,7 @@ mod tests {
         // The swap should return the sword
         assert_eq!(
             Some(sword.item),
-            loadout_remove(EquipSlot::Mainhand, &mut loadout,)
+            loadout_remove(EquipSlot::Mainhand, &mut loadout, &map)
         );
 
         // We should now have nothing equiped

common/src/comp/projectile.rs

@@ -1,6 +1,6 @@
 use crate::{
     comp::buff::{BuffCategory, BuffData, BuffKind},
-    effect::{BuffEffect, Effect as EffectB},
+    effect::{self, BuffEffect},
     sync::Uid,
     Damage, DamageSource, Explosion, GroupTarget, Knockback, RadiusEffect,
 };
@@ -107,7 +107,7 @@ impl ProjectileConstructor {
                     Effect::Explode(Explosion {
                         effects: vec![RadiusEffect::Entity(
                             Some(GroupTarget::OutOfGroup),
-                            EffectB::Damage(Damage {
+                            effect::Effect::Damage(Damage {
                                 source: DamageSource::Explosion,
                                 value: damage,
                             }),
@@ -121,7 +121,7 @@ impl ProjectileConstructor {
                     Effect::Explode(Explosion {
                         effects: vec![RadiusEffect::Entity(
                             Some(GroupTarget::OutOfGroup),
-                            EffectB::Damage(Damage {
+                            effect::Effect::Damage(Damage {
                                 source: DamageSource::Explosion,
                                 value: damage,
                             }),
@@ -145,14 +145,14 @@ impl ProjectileConstructor {
                         effects: vec![
                             RadiusEffect::Entity(
                                 Some(GroupTarget::OutOfGroup),
-                                EffectB::Damage(Damage {
+                                effect::Effect::Damage(Damage {
                                     source: DamageSource::Explosion,
                                     value: damage,
                                 }),
                             ),
                             RadiusEffect::Entity(
                                 Some(GroupTarget::InGroup),
-                                EffectB::Damage(Damage {
+                                effect::Effect::Damage(Damage {
                                     source: DamageSource::Healing,
                                     value: heal,
                                 }),
@@ -168,14 +168,14 @@ impl ProjectileConstructor {
                         effects: vec![
                             RadiusEffect::Entity(
                                 Some(GroupTarget::OutOfGroup),
-                                EffectB::Damage(Damage {
+                                effect::Effect::Damage(Damage {
                                     source: DamageSource::Explosion,
                                     value: damage,
                                 }),
                             ),
                             RadiusEffect::Entity(
                                 Some(GroupTarget::InGroup),
-                                EffectB::Damage(Damage {
+                                effect::Effect::Damage(Damage {
                                     source: DamageSource::Healing,
                                     value: heal,
                                 }),
@@ -200,7 +200,7 @@ impl ProjectileConstructor {
         }
     }
 
-    pub fn modify_projectile(mut self, power: f32) -> Self {
+    pub fn modified_projectile(mut self, power: f32) -> Self {
         use ProjectileConstructor::*;
         match self {
             Arrow { ref mut damage, .. } => {

common/src/loadout_builder.rs

@@ -10,13 +10,19 @@ use rand::Rng;
 /// ItemConfig
 ///
 /// ```
-/// use veloren_common::LoadoutBuilder;
+/// use veloren_common::{
+///     assets::Asset,
+///     comp::item::tool::AbilityMap,
+///     LoadoutBuilder,
+/// };
+///
+/// let map = AbilityMap::load_expect_cloned("common.abilities.weapon_ability_manifest");
 ///
 /// // Build a loadout with character starter defaults and a specific sword with default sword abilities
 /// let loadout = LoadoutBuilder::new()
 ///     .defaults()
 ///     .active_item(Some(LoadoutBuilder::default_item_config_from_str(
-///         "common.items.weapons.sword.zweihander_sword_0"
+///         "common.items.weapons.sword.zweihander_sword_0", &map
 ///     )))
 ///     .build();
 /// ```

common/src/states/combo_melee.rs

@@ -51,7 +51,7 @@ impl Stage<u64> {
         }
     }
 
-    pub fn adjust_stats(mut self, power: f32, speed: f32) -> Self {
+    pub fn adjusted_by_stats(mut self, power: f32, speed: f32) -> Self {
         self.base_damage = (self.base_damage as f32 * power) as u32;
         self.max_damage = (self.max_damage as f32 * power) as u32;
         self.damage_increase = (self.damage_increase as f32 * power) as u32;

server/src/events/interaction.rs

@@ -108,7 +108,7 @@ pub fn handle_unmount(server: &mut Server, mounter: EcsEntity) {
 #[allow(clippy::nonminimal_bool)] // TODO: Pending review in #587
 pub fn handle_possess(server: &Server, possessor_uid: Uid, possesse_uid: Uid) {
     let ecs = &server.state.ecs();
-    let map = ecs.fetch::<AbilityMap>();
+    let ability_map = ecs.fetch::<AbilityMap>();
     if let (Some(possessor), Some(possesse)) = (
         ecs.entity_from_uid(possessor_uid.into()),
         ecs.entity_from_uid(possesse_uid.into()),
@@ -181,7 +181,7 @@ pub fn handle_possess(server: &Server, possessor_uid: Uid, possesse_uid: Uid) {
 
         let item = comp::Item::new_from_asset_expect("common.items.debug.possess");
         if let item::ItemKind::Tool(_) = item.kind() {
-            let debug_item = comp::ItemConfig::from((item, &*map));
+            let debug_item = comp::ItemConfig::from((item, &*ability_map));
             std::mem::swap(&mut loadout.active_item, &mut loadout.second_item);
             loadout.active_item = Some(debug_item);
         }

voxygen/src/menu/char_selection/mod.rs

@@ -10,12 +10,7 @@ use crate::{
     Direction, GlobalState, PlayState, PlayStateResult,
 };
 use client::{self, Client};
-use common::{
+use common::{assets::Asset, comp, span, state::DeltaTime};
-    assets::Asset,
-    comp::{self},
-    span,
-    state::DeltaTime,
-};
 use specs::WorldExt;
 use std::{cell::RefCell, rc::Rc};
 use tracing::error;

voxygen/src/menu/main/mod.rs

@@ -12,11 +12,7 @@ use crate::{
     Direction, GlobalState, PlayState, PlayStateResult,
 };
 use client_init::{ClientInit, Error as InitError, Msg as InitMsg};
-use common::{
+use common::{assets::Asset, comp, span};
-    assets::Asset,
-    comp::{self},
-    span,
-};
 use tracing::error;
 use ui::{Event as MainMenuEvent, MainMenuUi};