From 259db56ca66a47a2201227fa40c16cfe1a3a5e1e Mon Sep 17 00:00:00 2001 From: Sam Date: Sat, 20 Nov 2021 22:19:20 -0500 Subject: [PATCH] Addressed more comments in MR review. --- common/src/comp/inventory/item/mod.rs | 17 +++++++---------- common/src/comp/inventory/item/tool.rs | 1 - common/src/recipe.rs | 7 ++----- server/src/persistence/character/conversions.rs | 14 +++++++++++--- voxygen/src/hud/util.rs | 2 +- 5 files changed, 21 insertions(+), 20 deletions(-) diff --git a/common/src/comp/inventory/item/mod.rs b/common/src/comp/inventory/item/mod.rs index 99e325da6f..21ae3d25c6 100644 --- a/common/src/comp/inventory/item/mod.rs +++ b/common/src/comp/inventory/item/mod.rs @@ -806,7 +806,7 @@ impl Item { } } - pub fn add_component( + pub fn persistence_access_add_component( &mut self, component: Item, ability_map: &AbilityMap, @@ -820,7 +820,7 @@ impl Item { self.update_item_config(ability_map, msm); } - pub fn component_mut(&mut self, index: usize) -> Option<&mut Self> { + pub fn persistence_access_mutable_component(&mut self, index: usize) -> Option<&mut Self> { self.components.get_mut(index) } @@ -998,11 +998,10 @@ pub trait ItemDesc { fn num_slots(&self) -> u16; fn item_definition_id(&self) -> &str; fn tags(&self) -> &[ItemTag]; - fn concrete_item(&self) -> Option<&Item>; fn is_modular(&self) -> bool; - fn components(&self) -> &[Item] { self.concrete_item().map_or(&[], |i| i.components()) } + fn components(&self) -> &[Item]; fn tool_info(&self) -> Option { if let ItemKind::Tool(tool) = &*self.kind() { @@ -1028,9 +1027,9 @@ impl ItemDesc for Item { fn tags(&self) -> &[ItemTag] { self.tags() } - fn concrete_item(&self) -> Option<&Item> { Some(self) } - fn is_modular(&self) -> bool { self.is_modular() } + + fn components(&self) -> &[Item] { self.components() } } impl ItemDesc for ItemDef { @@ -1048,9 +1047,9 @@ impl ItemDesc for ItemDef { fn tags(&self) -> &[ItemTag] { &self.tags } - fn concrete_item(&self) -> Option<&Item> { None } - fn is_modular(&self) -> bool { false } + + fn components(&self) -> &[Item] { &[] } } impl Component for Item { @@ -1081,8 +1080,6 @@ impl<'a, T: ItemDesc + ?Sized> ItemDesc for &'a T { fn tags(&self) -> &[ItemTag] { (*self).tags() } - fn concrete_item(&self) -> Option<&Item> { None } - fn is_modular(&self) -> bool { (*self).is_modular() } } diff --git a/common/src/comp/inventory/item/tool.rs b/common/src/comp/inventory/item/tool.rs index b9c1018a85..1f845881eb 100644 --- a/common/src/comp/inventory/item/tool.rs +++ b/common/src/comp/inventory/item/tool.rs @@ -39,7 +39,6 @@ pub enum ToolKind { } impl ToolKind { - // Changing this will break persistence of modular weapons pub fn identifier_name(&self) -> &'static str { match self { ToolKind::Sword => "sword", diff --git a/common/src/recipe.rs b/common/src/recipe.rs index efe5412e35..48fba595ef 100644 --- a/common/src/recipe.rs +++ b/common/src/recipe.rs @@ -125,15 +125,12 @@ impl Recipe { } let (item_def, quantity) = &self.output; - let mut crafted_item = Item::new_from_item_base( + let crafted_item = Item::new_from_item_base( ItemBase::Raw(Arc::clone(item_def)), - &[], + &components, ability_map, msm, ); - for component in components { - crafted_item.add_component(component, ability_map, msm); - } let mut crafted_items = Vec::with_capacity(*quantity as usize); for _ in 0..*quantity { crafted_items.push(crafted_item.duplicate(ability_map, msm)); diff --git a/server/src/persistence/character/conversions.rs b/server/src/persistence/character/conversions.rs index 02452a79a5..14b6824a44 100644 --- a/server/src/persistence/character/conversions.rs +++ b/server/src/persistence/character/conversions.rs @@ -296,7 +296,7 @@ where // Returns mutable reference to parent item of original item, by grabbing // the component representing the parent item from the parent item's parent // item - component_index.and_then(move |i| parent.component_mut(i)) + component_index.and_then(move |i| parent.persistence_access_mutable_component(i)) } else { None } @@ -400,7 +400,11 @@ pub fn convert_inventory_from_database_items( &mut inventory, &|inv, s| inv.slot_mut(slot(s).ok()?).and_then(|a| a.as_mut()), ) { - parent.add_component(item, &ABILITY_MAP, &MATERIAL_STATS_MANIFEST); + parent.persistence_access_add_component( + item, + &ABILITY_MAP, + &MATERIAL_STATS_MANIFEST, + ); } else { return Err(PersistenceError::ConversionError(format!( "Parent slot {} for component {} was empty even though it occurred earlier in \ @@ -459,7 +463,11 @@ pub fn convert_loadout_from_database_items( l.get_mut_item_at_slot_using_persistence_key(s).ok() }) { - parent.add_component(item, &ABILITY_MAP, &MATERIAL_STATS_MANIFEST); + parent.persistence_access_add_component( + item, + &ABILITY_MAP, + &MATERIAL_STATS_MANIFEST, + ); } else { return Err(PersistenceError::ConversionError(format!( "Parent slot {} for component {} was empty even though it occurred earlier in \ diff --git a/voxygen/src/hud/util.rs b/voxygen/src/hud/util.rs index 34e25c2883..11fcb7b6f9 100644 --- a/voxygen/src/hud/util.rs +++ b/voxygen/src/hud/util.rs @@ -70,7 +70,7 @@ pub fn price_desc( } } -pub fn kind_text<'a>(item: &dyn ItemDesc, i18n: &'a Localization) -> Cow<'a, str> { +pub fn kind_text<'a, I: ItemDesc + ?Sized>(item: &I, i18n: &'a Localization) -> Cow<'a, str> { match &*item.kind() { ItemKind::Armor(armor) => Cow::Borrowed(armor_kind(armor, i18n)), ItemKind::Tool(tool) => Cow::Owned(format!(