diff --git a/common/src/comp/inventory/mod.rs b/common/src/comp/inventory/mod.rs index c8bcdaf27c..ad1d7ab78b 100644 --- a/common/src/comp/inventory/mod.rs +++ b/common/src/comp/inventory/mod.rs @@ -1,5 +1,5 @@ use core::ops::Not; -use std::{collections::HashMap, convert::TryFrom, iter::once, mem, ops::Range}; +use std::{collections::HashMap, convert::TryFrom, mem, ops::Range}; use serde::{Deserialize, Serialize}; use specs::{Component, DerefFlaggedStorage}; @@ -392,25 +392,19 @@ impl Inventory { /// go into inventory. If the item is going to mainhand, put mainhand in /// offhand and place offhand into inventory. #[must_use = "Returned items will be lost if not used"] - pub fn equip(&mut self, inv_slot: InvSlotId) -> Option> { - let mut leftover_items = None; + pub fn equip(&mut self, inv_slot: InvSlotId) -> Vec { self.get(inv_slot) - .map(|x| x.kind().clone()) - .map(|item_kind| { - self.loadout - .get_slot_to_equip_into(&item_kind) - .map(|equip_slot| { - // Special case when equipping into main hand - swap with offhand first - if equip_slot == EquipSlot::Mainhand { - self.loadout - .swap_slots(EquipSlot::Mainhand, EquipSlot::Offhand); - } + .and_then(|item| self.loadout.get_slot_to_equip_into(item.kind())) + .map(|equip_slot| { + // Special case when equipping into main hand - swap with offhand first + if equip_slot == EquipSlot::Mainhand { + self.loadout + .swap_slots(EquipSlot::Mainhand, EquipSlot::Offhand); + } - leftover_items = self.swap_inventory_loadout(inv_slot, equip_slot); - }) - }); - - leftover_items + self.swap_inventory_loadout(inv_slot, equip_slot) + }) + .unwrap_or_else(Vec::new) } /// Determines how many free inventory slots will be left after equipping an @@ -419,8 +413,7 @@ impl Inventory { pub fn free_after_equip(&self, inv_slot: InvSlotId) -> i32 { let (inv_slot_for_equipped, slots_from_equipped) = self .get(inv_slot) - .map(|x| x.kind().clone()) - .and_then(|item_kind| self.loadout.get_slot_to_equip_into(&item_kind)) + .and_then(|item| self.loadout.get_slot_to_equip_into(item.kind())) .and_then(|equip_slot| self.equipped(equip_slot)) .map_or((1, 0), |item| (0, item.slots().len())); @@ -454,13 +447,10 @@ impl Inventory { // Unload any items contained within the item, and push those items and the item // itself into the inventory. We already know that there are enough free slots // so push will never give us an item back. - item.drain() - .collect::>() - .into_iter() - .chain(once(item)) - .for_each(|item| { - self.push(item).unwrap_none(); - }); + item.drain().for_each(|item| { + self.push(item).unwrap_none(); + }); + self.push(item).unwrap_none(); Ok(()) } @@ -486,7 +476,7 @@ impl Inventory { // any that don't fit to be to be dropped on the floor by the caller match self.push_all(unloaded_items.into_iter()) { Err(Error::Full(leftovers)) => Some(leftovers), - Ok(_) => None, + Ok(()) => None, } })) } @@ -509,11 +499,11 @@ impl Inventory { /// Swaps items from two slots, regardless of if either is inventory or /// loadout. #[must_use = "Returned items will be lost if not used"] - pub fn swap(&mut self, slot_a: Slot, slot_b: Slot) -> Option> { + pub fn swap(&mut self, slot_a: Slot, slot_b: Slot) -> Vec { match (slot_a, slot_b) { (Slot::Inventory(slot_a), Slot::Inventory(slot_b)) => { self.swap_slots(slot_a, slot_b); - None + Vec::new() }, (Slot::Inventory(inv_slot), Slot::Equip(equip_slot)) | (Slot::Equip(equip_slot), Slot::Inventory(inv_slot)) => { @@ -521,7 +511,7 @@ impl Inventory { }, (Slot::Equip(slot_a), Slot::Equip(slot_b)) => { self.loadout.swap_slots(slot_a, slot_b); - None + Vec::new() }, } } @@ -556,47 +546,42 @@ impl Inventory { &mut self, inv_slot_id: InvSlotId, equip_slot: EquipSlot, - ) -> Option> { + ) -> Vec { if !self.can_swap(inv_slot_id, equip_slot) { - return None; + return Vec::new(); } - let mut unloaded_items = None; - // Take the item from the inventory let from_inv = self.remove(inv_slot_id); // Swap the equipped item for the item from the inventory let from_equip = self.loadout.swap(equip_slot, from_inv); - if let Some(mut from_equip) = from_equip { - // Unload any items held inside the previously equipped item - let items: Vec = from_equip.drain().collect(); - if items.iter().len() > 0 { - unloaded_items = Some(items); - } - // Attempt to put the unequipped item in the same slot that the inventory item - // was in - if that slot no longer exists (because a large container was - // swapped for a smaller one) then push the item to the first free - // inventory slot instead. - if let Err(returned) = self.insert_at(inv_slot_id, from_equip) { - self.push(returned) - .expect_none("Unable to push to inventory, no slots (bug in can_swap()?)"); - } - } + let unloaded_items = from_equip + .map(|mut from_equip| { + // Unload any items held inside the previously equipped item + let items: Vec = from_equip.drain().collect(); + + // Attempt to put the unequipped item in the same slot that the inventory item + // was in - if that slot no longer exists (because a large container was + // swapped for a smaller one) then push the item to the first free + // inventory slot instead. + if let Err(returned) = self.insert_at(inv_slot_id, from_equip) { + self.push(returned) + .expect_none("Unable to push to inventory, no slots (bug in can_swap()?)"); + } + + items + }) + .unwrap_or_default(); // Attempt to put any items unloaded from the unequipped item into empty // inventory slots and return any that don't fit to the caller where they // will be dropped on the ground - if let Some(unloaded_items) = unloaded_items { - let leftovers = match self.push_all(unloaded_items.into_iter()) { - Err(Error::Full(leftovers)) => leftovers, - Ok(_) => vec![], - }; - return Some(leftovers); + match self.push_all(unloaded_items.into_iter()) { + Err(Error::Full(leftovers)) => leftovers, + Ok(()) => Vec::new(), } - - None } /// Determines if an inventory and loadout slot can be swapped, taking into diff --git a/common/src/comp/inventory/test.rs b/common/src/comp/inventory/test.rs index 26f9d9abce..45de613774 100644 --- a/common/src/comp/inventory/test.rs +++ b/common/src/comp/inventory/test.rs @@ -167,7 +167,7 @@ fn can_swap_equipped_bag_into_only_empty_slot_provided_by_itself_should_return_f let result = inv.can_swap(InvSlotId::new(15, 17), EquipSlot::Armor(ArmorSlot::Bag1)); - assert_eq!(false, result); + assert!(!result); } #[test] @@ -211,7 +211,7 @@ fn equip_replace_already_equipped_item() { inv.push(boots.clone()); inv.replace_loadout_item(EquipSlot::Armor(ArmorSlot::Feet), starting_sandles.clone()); - inv.equip(InvSlotId::new(0, 0)).unwrap_none(); + let _ = inv.equip(InvSlotId::new(0, 0)); // We should now have the testing boots equipped assert_eq!( @@ -253,7 +253,7 @@ fn equip_equipping_smaller_bag_from_last_slot_of_big_bag() { inv.get(InvSlotId::new(0, 0)).unwrap().item_definition_id(), LARGE_BAG_ID ); - assert_eq!(result, None); + assert!(result.is_empty()); } #[test] @@ -268,8 +268,9 @@ fn unequip_unequipping_bag_into_its_own_slot_with_no_other_free_slots() { // Fill all inventory built-in slots fill_inv_slots(&mut inv, 18); - inv.swap_inventory_loadout(InvSlotId::new(15, 0), EquipSlot::Armor(ArmorSlot::Bag1)) - .unwrap_none(); + let result = + inv.swap_inventory_loadout(InvSlotId::new(15, 0), EquipSlot::Armor(ArmorSlot::Bag1)); + assert!(result.is_empty()) } #[test] @@ -283,12 +284,9 @@ fn equip_one_bag_equipped_equip_second_bag() { inv.push(bag); - inv.equip(InvSlotId::new(0, 0)).unwrap_none(); + let _ = inv.equip(InvSlotId::new(0, 0)); - assert_eq!( - true, - inv.equipped(EquipSlot::Armor(ArmorSlot::Bag2)).is_some() - ); + assert!(inv.equipped(EquipSlot::Armor(ArmorSlot::Bag2)).is_some()); } #[test] diff --git a/common/sys/src/character_behavior.rs b/common/sys/src/character_behavior.rs index 60eae9dde0..b019e10ba9 100644 --- a/common/sys/src/character_behavior.rs +++ b/common/sys/src/character_behavior.rs @@ -37,6 +37,7 @@ fn incorporate_update(tuple: &mut JoinTuple, state_update: StateUpdate) { Slot::Equip(EquipSlot::Mainhand), Slot::Equip(EquipSlot::Offhand), ) + .first() .unwrap_none(); // Swapping main and offhand never results in leftover items } } diff --git a/server-cli/src/logging.rs b/server-cli/src/logging.rs index 72909db2dd..6a738ed2c5 100644 --- a/server-cli/src/logging.rs +++ b/server-cli/src/logging.rs @@ -16,11 +16,6 @@ pub fn init(basic: bool) { let base_exceptions = |env: EnvFilter| { env.add_directive("veloren_world::sim=info".parse().unwrap()) .add_directive("veloren_world::civ=info".parse().unwrap()) - .add_directive( - "veloren_common::comp::inventory::slot=info" - .parse() - .unwrap(), - ) .add_directive("uvth=warn".parse().unwrap()) .add_directive("tiny_http=warn".parse().unwrap()) .add_directive("mio::sys::windows=debug".parse().unwrap()) diff --git a/server/src/events/interaction.rs b/server/src/events/interaction.rs index 8d2e85f337..4090e9be0a 100644 --- a/server/src/events/interaction.rs +++ b/server/src/events/interaction.rs @@ -183,6 +183,7 @@ pub fn handle_possess(server: &Server, possessor_uid: Uid, possesse_uid: Uid) { Slot::Equip(EquipSlot::Mainhand), Slot::Equip(EquipSlot::Offhand), ) + .first() .unwrap_none(); // Swapping main and offhand never results in leftover items inventory.replace_loadout_item(EquipSlot::Mainhand, Some(debug_item)); diff --git a/server/src/events/inventory_manip.rs b/server/src/events/inventory_manip.rs index 95a521b478..5e63197e58 100644 --- a/server/src/events/inventory_manip.rs +++ b/server/src/events/inventory_manip.rs @@ -234,17 +234,15 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv swap_lantern(&mut state.ecs().write_storage(), entity, &lantern); } if let Some(pos) = state.ecs().read_storage::().get(entity) { - if let Some(leftover_items) = inventory.equip(slot) { - dropped_items.extend(leftover_items.into_iter().map(|x| { - ( - *pos, - state - .read_component_copied::(entity) - .unwrap_or_default(), - x, - ) - })); - } + dropped_items.extend(inventory.equip(slot).into_iter().map(|x| { + ( + *pos, + state + .read_component_copied::(entity) + .unwrap_or_default(), + x, + ) + })); } Some(comp::InventoryUpdateEvent::Used) } else if let Some(item) = inventory.take(slot) { @@ -413,17 +411,15 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv if let Some(pos) = ecs.read_storage::().get(entity) { if let Some(mut inventory) = ecs.write_storage::().get_mut(entity) { - if let Some(leftover_items) = inventory.swap(a, b) { - dropped_items.extend(leftover_items.into_iter().map(|x| { - ( - *pos, - state - .read_component_copied::(entity) - .unwrap_or_default(), - x, - ) - })); - } + dropped_items.extend(inventory.swap(a, b).into_iter().map(|x| { + ( + *pos, + state + .read_component_copied::(entity) + .unwrap_or_default(), + x, + ) + })); } } diff --git a/voxygen/src/logging.rs b/voxygen/src/logging.rs index e7ee26dac7..e0b4a962df 100644 --- a/voxygen/src/logging.rs +++ b/voxygen/src/logging.rs @@ -45,11 +45,6 @@ pub fn init(settings: &Settings) -> Vec { .add_directive("uvth=warn".parse().unwrap()) .add_directive("tiny_http=warn".parse().unwrap()) .add_directive("mio::sys::windows=debug".parse().unwrap()) - .add_directive( - "veloren_common::comp::inventory::slot=info" - .parse() - .unwrap(), - ) .add_directive( "veloren_server::persistence::character=info" .parse()