Enforced certain invariants in how an item was equipped into the loadout:

- a 2h weapon can only be equipped in a mainhand slot if the offhand slot is empty
 - a 1h weapon can only be equipped in an offhand slot if the mainhand slot has a 1h weapon
 - 2h weapons can never be equipped in an offhand slot

Fixed some tests

common/src/comp/inventory/loadout.rs

@@ -1,6 +1,6 @@
 use crate::comp::{
     inventory::{
-        item::ItemKind,
+        item::{tool, ItemKind},
         slot::{ArmorSlot, EquipSlot},
         InvSlot,
     },
@@ -173,10 +173,10 @@ impl Loadout {
         // Check if items can go in the other slots
         if item_a
             .as_ref()
-            .map_or(true, |i| equip_slot_b.can_hold(&i.kind()))
+            .map_or(true, |i| self.slot_can_hold(equip_slot_b, &i.kind()))
             && item_b
                 .as_ref()
-                .map_or(true, |i| equip_slot_a.can_hold(&i.kind()))
+                .map_or(true, |i| self.slot_can_hold(equip_slot_a, &i.kind()))
         {
             // Swap
             self.swap(equip_slot_b, item_a).unwrap_none();
@@ -197,7 +197,7 @@ impl Loadout {
         let mut suitable_slots = self
             .slots
             .iter()
-            .filter(|s| s.equip_slot.can_hold(item_kind));
+            .filter(|s| self.slot_can_hold(s.equip_slot, item_kind));
 
         let first = suitable_slots.next();
 
@@ -217,7 +217,7 @@ impl Loadout {
     ) -> impl Iterator<Item = &Item> {
         self.slots
             .iter()
-            .filter(move |s| s.equip_slot.can_hold(&item_kind))
+            .filter(move |s| self.slot_can_hold(s.equip_slot, &item_kind))
             .filter_map(|s| s.slot.as_ref())
     }
 
@@ -296,21 +296,73 @@ impl Loadout {
     /// If no slot is available the item is returned.
     #[must_use = "Returned item will be lost if not used"]
     pub(super) fn try_equip(&mut self, item: Item) -> Result<(), Item> {
-        if let Some(loadout_slot) = self
+        /* if let Some(loadout_slot) = self
             .slots
             .iter_mut()
-            .find(|s| s.slot.is_none() && s.equip_slot.can_hold(item.kind()))
+            .find(|s| s.slot.is_none() && self.slot_can_hold(s.equip_slot, item.kind()))
         {
             loadout_slot.slot = Some(item);
             Ok(())
         } else {
             Err(item)
+        } */
+        // TODO: Get XVar to see if there better way to handle mutability issues
+        let loadout_slot = self
+            .slots
+            .iter()
+            .find(|s| s.slot.is_none() && self.slot_can_hold(s.equip_slot, item.kind()))
+            .map(|s| s.equip_slot);
+        if let Some(slot) = self
+            .slots
+            .iter_mut()
+            .find(|s| Some(s.equip_slot) == loadout_slot)
+        {
+            slot.slot = Some(item);
+            Ok(())
+        } else {
+            Err(item)
         }
     }
 
     pub(super) fn items(&self) -> impl Iterator<Item = &Item> {
         self.slots.iter().filter_map(|x| x.slot.as_ref())
     }
+
+    /// Checks that a slot can hold a given item
+    pub(super) fn slot_can_hold(&self, equip_slot: EquipSlot, item_kind: &ItemKind) -> bool {
+        // Checks if item can be equipped in a mainhand slot
+        let mainhand_check = |offhand_slot| {
+            // Allows item to be equipped if itemkind is a tool and...
+            matches!(item_kind, ItemKind::Tool(mainhand) if {
+                if let Some(ItemKind::Tool(offhand)) = self.equipped(offhand_slot).map(|i| i.kind()) {
+                    // if offhand is 1 handed, only if mainhand is also 1 handed
+                    matches!(offhand.hands, tool::Hands::One) && matches!(mainhand.hands, tool::Hands::One)
+                } else {
+                    // else there is no tool equipped in offhand, so only if slot can normally hold this item
+                    equip_slot.can_hold(item_kind)
+                }
+            })
+        };
+
+        // Checks if item can be equipped in an offhand slot
+        let offhand_check = |mainhand_slot| {
+            // Allows item to be equipped if itemkind is a tool and...
+            matches!(item_kind, ItemKind::Tool(offhand) if {
+                // if offhand weapon is 1 handed...
+                matches!(offhand.hands, tool::Hands::One)
+                // and if mainhand has a 1 handed weapon...
+                && matches!(self.equipped(mainhand_slot).map(|i| i.kind()), Some(ItemKind::Tool(mainhand)) if matches!(mainhand.hands, tool::Hands::One))
+            })
+        };
+
+        match equip_slot {
+            EquipSlot::ActiveMainhand => mainhand_check(EquipSlot::ActiveOffhand),
+            EquipSlot::ActiveOffhand => offhand_check(EquipSlot::ActiveMainhand),
+            EquipSlot::InactiveMainhand => mainhand_check(EquipSlot::InactiveOffhand),
+            EquipSlot::InactiveOffhand => offhand_check(EquipSlot::InactiveMainhand),
+            _ => equip_slot.can_hold(item_kind),
+        }
+    }
 }
 
 #[cfg(test)]

common/src/comp/inventory/mod.rs

@@ -744,12 +744,12 @@ impl Inventory {
     /// account whether there will be free space in the inventory for the
     /// loadout item once any slots that were provided by it have been
     /// removed.
+    #[allow(clippy::blocks_in_if_conditions)]
     pub fn can_swap(&self, inv_slot_id: InvSlotId, equip_slot: EquipSlot) -> bool {
         // Check if loadout slot can hold item
-        if !self
+        if !self.get(inv_slot_id).map_or(true, |item| {
-            .get(inv_slot_id)
+            self.loadout.slot_can_hold(equip_slot, &item.kind())
-            .map_or(true, |item| equip_slot.can_hold(&item.kind()))
+        }) {
-        {
             trace!("can_swap = false, equip slot can't hold item");
             return false;
         }

common/src/comp/inventory/test.rs

@@ -231,23 +231,29 @@ fn unequip_items_both_hands() {
 
     let sword = Item::new_from_asset_expect("common.items.weapons.sword.steel-8");
 
-    inv.replace_loadout_item(EquipSlot::Mainhand, Some(sword.duplicate(ability_map, msm)));
+    inv.replace_loadout_item(
-    inv.replace_loadout_item(EquipSlot::Offhand, Some(sword.duplicate(ability_map, msm)));
+        EquipSlot::ActiveMainhand,
+        Some(sword.duplicate(ability_map, msm)),
+    );
+    inv.replace_loadout_item(
+        EquipSlot::InactiveMainhand,
+        Some(sword.duplicate(ability_map, msm)),
+    );
 
     // Fill all inventory slots except one
     fill_inv_slots(&mut inv, 17);
 
-    let result = inv.unequip(EquipSlot::Mainhand);
+    let result = inv.unequip(EquipSlot::ActiveMainhand);
     // We have space in the inventory, so this should have unequipped
-    assert_eq!(None, inv.equipped(EquipSlot::Mainhand));
+    assert_eq!(None, inv.equipped(EquipSlot::ActiveMainhand));
     assert_eq!(18, inv.populated_slots());
     assert_eq!(true, result.is_ok());
 
-    let result = inv.unequip(EquipSlot::Offhand).unwrap_err();
+    let result = inv.unequip(EquipSlot::InactiveMainhand).unwrap_err();
     assert_eq!(SlotError::InventoryFull, result);
 
     // There is no more space in the inventory, so this should still be equipped
-    assert_eq!(&sword, inv.equipped(EquipSlot::Offhand).unwrap());
+    assert_eq!(&sword, inv.equipped(EquipSlot::InactiveMainhand).unwrap());
 
     // Verify inventory
     assert_eq!(inv.slots[17], Some(sword));

voxygen/src/audio/sfx/event_mapper/combat/tests.rs

@@ -13,7 +13,7 @@ use std::time::{Duration, Instant};
 #[test]
 fn maps_wield_while_equipping() {
     let loadout = LoadoutBuilder::new()
-        .active_item(Some(Item::new_from_asset_expect(
+        .active_mainhand(Some(Item::new_from_asset_expect(
             "common.items.weapons.axe.starter_axe",
         )))
         .build();
@@ -40,7 +40,7 @@ fn maps_wield_while_equipping() {
 #[test]
 fn maps_unwield() {
     let loadout = LoadoutBuilder::new()
-        .active_item(Some(Item::new_from_asset_expect(
+        .active_mainhand(Some(Item::new_from_asset_expect(
             "common.items.weapons.bow.starter",
         )))
         .build();
@@ -62,7 +62,7 @@ fn maps_unwield() {
 #[test]
 fn maps_basic_melee() {
     let loadout = LoadoutBuilder::new()
-        .active_item(Some(Item::new_from_asset_expect(
+        .active_mainhand(Some(Item::new_from_asset_expect(
             "common.items.weapons.axe.starter_axe",
         )))
         .build();
@@ -104,7 +104,7 @@ fn maps_basic_melee() {
 #[test]
 fn matches_ability_stage() {
     let loadout = LoadoutBuilder::new()
-        .active_item(Some(Item::new_from_asset_expect(
+        .active_mainhand(Some(Item::new_from_asset_expect(
             "common.items.weapons.sword.starter",
         )))
         .build();
@@ -163,7 +163,7 @@ fn matches_ability_stage() {
 #[test]
 fn ignores_different_ability_stage() {
     let loadout = LoadoutBuilder::new()
-        .active_item(Some(Item::new_from_asset_expect(
+        .active_mainhand(Some(Item::new_from_asset_expect(
             "common.items.weapons.axe.starter_axe",
         )))
         .build();