From 8d1df956d6fb5e4436d7e517f3839f43235f5041 Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 13 Oct 2023 20:21:23 -0400 Subject: [PATCH] Addressed review comments --- .../common/items/debug/admin_black_hole.ron | 2 +- common/src/comp/inventory/item/mod.rs | 23 ++++++++++++ common/src/comp/inventory/mod.rs | 35 ++++--------------- common/src/comp/inventory/slot.rs | 10 ------ .../src/persistence/character/conversions.rs | 4 +-- 5 files changed, 32 insertions(+), 42 deletions(-) diff --git a/assets/common/items/debug/admin_black_hole.ron b/assets/common/items/debug/admin_black_hole.ron index c565e9aa5b..8cc8cd3f12 100644 --- a/assets/common/items/debug/admin_black_hole.ron +++ b/assets/common/items/debug/admin_black_hole.ron @@ -9,5 +9,5 @@ ItemDef( ), quality: Debug, tags: [], - slots: 1, + slots: 900, ) diff --git a/common/src/comp/inventory/item/mod.rs b/common/src/comp/inventory/item/mod.rs index 83eb5350cc..2678441efe 100644 --- a/common/src/comp/inventory/item/mod.rs +++ b/common/src/comp/inventory/item/mod.rs @@ -1439,6 +1439,29 @@ impl Item { self.update_item_state(ability_map, msm); } + /// If an item is stackable and has an amount greater than 1, creates a new + /// item with half the amount (rounded down), and decreases the amount of + /// the original item by the same quantity. + pub fn take_half( + &mut self, + ability_map: &AbilityMap, + msm: &MaterialStatManifest, + ) -> Option { + if self.is_stackable() && self.amount() > 1 { + let mut return_item = self.duplicate(ability_map, msm); + let returning_amount = self.amount() / 2; + self.decrease_amount(returning_amount).ok()?; + return_item.set_amount(returning_amount).expect( + "return_item.amount() = self.amount() / 2 < self.amount() (since self.amount() ≥ \ + 1) ≤ self.max_amount() = return_item.max_amount(), since return_item is a \ + duplicate of item", + ); + Some(return_item) + } else { + None + } + } + #[cfg(test)] pub fn create_test_item_from_kind(kind: ItemKind) -> Self { let ability_map = &AbilityMap::load().read(); diff --git a/common/src/comp/inventory/mod.rs b/common/src/comp/inventory/mod.rs index 1a02293113..6275aeadb2 100644 --- a/common/src/comp/inventory/mod.rs +++ b/common/src/comp/inventory/mod.rs @@ -562,6 +562,7 @@ impl Inventory { } /// Remove an item from an overflow slot + #[must_use = "Returned items will be lost if not used"] pub fn overflow_remove(&mut self, overflow_slot: usize) -> Option { if overflow_slot < self.overflow_items.len() { Some(self.overflow_items.remove(overflow_slot)) @@ -595,6 +596,7 @@ impl Inventory { } /// Takes half of the items from a slot in the inventory + #[must_use = "Returned items will be lost if not used"] pub fn take_half( &mut self, inv_slot_id: InvSlotId, @@ -602,25 +604,15 @@ impl Inventory { msm: &MaterialStatManifest, ) -> Option { if let Some(Some(item)) = self.slot_mut(inv_slot_id) { - if item.is_stackable() && item.amount() > 1 { - let mut return_item = item.duplicate(ability_map, msm); - let returning_amount = item.amount() / 2; - item.decrease_amount(returning_amount).ok()?; - return_item.set_amount(returning_amount).expect( - "return_item.amount() = item.amount() / 2 < item.amount() (since \ - item.amount() ≥ 1) ≤ item.max_amount() = return_item.max_amount(), since \ - return_item is a duplicate of item", - ); - Some(return_item) - } else { - self.remove(inv_slot_id) - } + item.take_half(ability_map, msm) + .or(self.remove(inv_slot_id)) } else { None } } /// Takes half of the items from an overflow slot + #[must_use = "Returned items will be lost if not used"] pub fn overflow_take_half( &mut self, overflow_slot: usize, @@ -628,21 +620,8 @@ impl Inventory { msm: &MaterialStatManifest, ) -> Option { if let Some(item) = self.overflow_items.get_mut(overflow_slot) { - if item.is_stackable() && item.amount() > 1 { - let mut return_item = item.duplicate(ability_map, msm); - let returning_amount = item.amount() / 2; - item.decrease_amount(returning_amount).ok()?; - return_item.set_amount(returning_amount).expect( - "return_item.amount() = item.amount() / 2 < item.amount() (since \ - item.amount() ≥ 1) ≤ item.max_amount() = return_item.max_amount(), since \ - return_item is a duplicate of item", - ); - Some(return_item) - } else if overflow_slot < self.overflow_items.len() { - Some(self.overflow_items.remove(overflow_slot)) - } else { - None - } + item.take_half(ability_map, msm) + .or(self.overflow_remove(overflow_slot)) } else { None } diff --git a/common/src/comp/inventory/slot.rs b/common/src/comp/inventory/slot.rs index 1fe38fd41b..4ac74e064d 100644 --- a/common/src/comp/inventory/slot.rs +++ b/common/src/comp/inventory/slot.rs @@ -107,16 +107,6 @@ pub enum ArmorSlot { Bag4, } -impl Slot { - pub fn can_hold(self, item_kind: &ItemKind) -> bool { - match (self, item_kind) { - (Self::Inventory(_), _) => true, - (Self::Equip(slot), item_kind) => slot.can_hold(item_kind), - (Self::Overflow(_), _) => true, - } - } -} - impl EquipSlot { pub fn can_hold(self, item_kind: &ItemKind) -> bool { match (self, item_kind) { diff --git a/server/src/persistence/character/conversions.rs b/server/src/persistence/character/conversions.rs index 6e0cb8d0be..9855767636 100644 --- a/server/src/persistence/character/conversions.rs +++ b/server/src/persistence/character/conversions.rs @@ -67,7 +67,7 @@ pub fn convert_items_to_database_items( .slots_with_id() .map(|(pos, item)| { ( - serde_json::to_string(&pos).expect("failed to serialize InventorySlotPos"), + serde_json::to_string(&pos).expect("failed to serialize InvSlotId"), item.as_ref(), inventory_container_id, ) @@ -473,8 +473,6 @@ pub fn convert_inventory_from_database_items( &|(inv, f_i): &mut (&mut Inventory, &mut FailedInserts), s| { // Attempts first to access inventory if that slot exists there. If it does not // it instead attempts to access failed inserts list. - // Question for Sharp/XVar: Should this attempt to look in failed inserts list - // first? slot(s) .ok() .and_then(|slot| inv.slot_mut(slot))