From fbd742abdbeb39f7c990d8dd976a57faf69ce624 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 25 Oct 2021 15:34:39 -0400 Subject: [PATCH] Changed crafting to only consume items after checking that the crafting would be successful instead of consuming items first and reinserting on failure. --- client/src/lib.rs | 5 +- common/src/comp/controller.rs | 2 +- common/src/comp/inventory/item/mod.rs | 64 +++++-------- common/src/recipe.rs | 125 +++++++++++++++----------- 4 files changed, 100 insertions(+), 96 deletions(-) diff --git a/client/src/lib.rs b/client/src/lib.rs index 5baf84d9f7..f490e302c2 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -998,7 +998,7 @@ impl Client { pub fn craft_recipe( &mut self, recipe: &str, - slots: Vec, + slots: Vec<(u32, InvSlotId)>, craft_sprite: Option<(Vec3, SpriteKind)>, ) -> bool { let (can_craft, required_sprite) = self.can_craft_recipe(recipe); @@ -1019,6 +1019,7 @@ impl Client { } } + /// Checks if the item in the given slot can be salvaged. pub fn can_salvage_item(&self, slot: InvSlotId) -> bool { self.inventories() .get(self.entity()) @@ -1026,6 +1027,8 @@ impl Client { .map_or(false, |item| item.is_salvageable()) } + /// Salvage the item in the given inventory slot. `salvage_pos` should be + /// the location of a relevant crafting station within range of the player. pub fn salvage_item(&mut self, slot: InvSlotId, salvage_pos: Vec3) -> bool { let is_salvageable = self.can_salvage_item(slot); if is_salvageable { diff --git a/common/src/comp/controller.rs b/common/src/comp/controller.rs index a12fa2b404..97e190d0ac 100644 --- a/common/src/comp/controller.rs +++ b/common/src/comp/controller.rs @@ -94,7 +94,7 @@ impl From for InventoryManip { pub enum CraftEvent { Simple { recipe: String, - slots: Vec, + slots: Vec<(u32, InvSlotId)>, }, Salvage(InvSlotId), } diff --git a/common/src/comp/inventory/item/mod.rs b/common/src/comp/inventory/item/mod.rs index 7919bb4852..a951c6565c 100644 --- a/common/src/comp/inventory/item/mod.rs +++ b/common/src/comp/inventory/item/mod.rs @@ -171,38 +171,38 @@ impl Material { } } - pub fn asset_identifier(&self) -> &'static str { + pub fn asset_identifier(&self) -> Option<&'static str> { match self { - Material::Bronze => "common.items.mineral.ingot.bronze", - Material::Iron => "common.items.mineral.ingot.iron", - Material::Steel => "common.items.mineral.ingot.steel", - Material::Cobalt => "common.items.mineral.ingot.cobalt", - Material::Bloodsteel => "common.items.mineral.ingot.bloodsteel", - Material::Orichalcum => "common.items.mineral.ingot.orichalcum", + Material::Bronze => Some("common.items.mineral.ingot.bronze"), + Material::Iron => Some("common.items.mineral.ingot.iron"), + Material::Steel => Some("common.items.mineral.ingot.steel"), + Material::Cobalt => Some("common.items.mineral.ingot.cobalt"), + Material::Bloodsteel => Some("common.items.mineral.ingot.bloodsteel"), + Material::Orichalcum => Some("common.items.mineral.ingot.orichalcum"), Material::Wood | Material::Bamboo | Material::Hardwood | Material::Ironwood | Material::Frostwood - | Material::Eldwood => unimplemented!(), + | Material::Eldwood => None, Material::Rock | Material::Granite | Material::Bone | Material::Basalt | Material::Obsidian - | Material::Velorite => unimplemented!(), - Material::Linen => "common.items.crafting_ing.cloth.linen", - Material::Wool => "common.items.crafting_ing.cloth.wool", - Material::Silk => "common.items.crafting_ing.cloth.silk", - Material::Lifecloth => "common.items.crafting_ing.cloth.lifecloth", - Material::Moonweave => "common.items.crafting_ing.cloth.moonweave", - Material::Sunsilk => "common.items.crafting_ing.cloth.sunsilk", - Material::Rawhide => "common.items.crafting_ing.leather.simple_leather", - Material::Leather => "common.items.crafting_ing.leather.thick_leather", - Material::Scale => "common.items.crafting_ing.hide.scales", - Material::Carapace => "common.items.crafting_ing.hide.carapace", - Material::Plate => "common.items.crafting_ing.hide.plate", - Material::Dragonscale => "common.items.crafting_ing.hide.dragon_scale", + | Material::Velorite => None, + Material::Linen => Some("common.items.crafting_ing.cloth.linen"), + Material::Wool => Some("common.items.crafting_ing.cloth.wool"), + Material::Silk => Some("common.items.crafting_ing.cloth.silk"), + Material::Lifecloth => Some("common.items.crafting_ing.cloth.lifecloth"), + Material::Moonweave => Some("common.items.crafting_ing.cloth.moonweave"), + Material::Sunsilk => Some("common.items.crafting_ing.cloth.sunsilk"), + Material::Rawhide => Some("common.items.crafting_ing.leather.simple_leather"), + Material::Leather => Some("common.items.crafting_ing.leather.thick_leather"), + Material::Scale => Some("common.items.crafting_ing.hide.scales"), + Material::Carapace => Some("common.items.crafting_ing.hide.carapace"), + Material::Plate => Some("common.items.crafting_ing.hide.plate"), + Material::Dragonscale => Some("common.items.crafting_ing.hide.dragon_scale"), } } } @@ -815,27 +815,7 @@ impl Item { None } }) - .map(|material| material.asset_identifier()) - } - - // Attempts to salvage an item by consuming it, returns the salvaged items if - // salvageable, else the original item - pub fn try_salvage(self) -> Result, Item> { - if !self.is_salvageable() { - return Err(self); - } - - // Creates one item for every salvage tag in the target item - let salvaged_items: Vec<_> = self - .salvage_output() - .map(|asset| Item::new_from_asset_expect(asset)) - .collect(); - - if salvaged_items.is_empty() { - Err(self) - } else { - Ok(salvaged_items) - } + .filter_map(|material| material.asset_identifier()) } pub fn name(&self) -> &str { &self.item_def.name } diff --git a/common/src/recipe.rs b/common/src/recipe.rs index a5398ff034..23309e37f2 100644 --- a/common/src/recipe.rs +++ b/common/src/recipe.rs @@ -30,50 +30,65 @@ impl Recipe { pub fn craft_simple( &self, inv: &mut Inventory, - slots: Vec, + // Vec tying an input to a slot + slots: Vec<(u32, InvSlotId)>, ability_map: &AbilityMap, msm: &MaterialStatManifest, ) -> Result, Vec<(&RecipeInput, u32)>> { - let mut recipe_inputs = Vec::new(); + let mut slot_claims = HashMap::new(); let mut unsatisfied_requirements = Vec::new(); - // Checks each input against a slot in the inventory. If the slot contains an - // item that fulfills the need of the input, takes from the inventory up to the - // quantity needed for the crafting input. If the item either cannot be used, or - // there is insufficient quantity, adds input and number of materials needed to - // unsatisfied requirements. + // Checks each input against slots in the inventory. If the slots contain an + // item that fulfills the need of the input, marks some of the item as claimed + // up to quantity needed for the crafting input. If the item either + // cannot be used, or there is insufficient quantity, adds input and + // number of materials needed to unsatisfied requirements. self.inputs .iter() .enumerate() - .for_each(|(i, (input, amount))| { - let valid_input = if let Some(item) = slots.get(i).and_then(|slot| inv.get(*slot)) { - item.matches_recipe_input(input) - } else { - false - }; - - if let Some(slot) = slots.get(i) { - if !valid_input { - unsatisfied_requirements.push((input, *amount)); - } else { - for taken in 0..*amount { - if let Some(item) = inv.take(*slot, ability_map, msm) { - recipe_inputs.push(item); - } else { - unsatisfied_requirements.push((input, *amount - taken)); - break; - } - } + .for_each(|(i, (input, mut required))| { + // Check used for recipes that have an input that is not consumed, e.g. + // craftsman hammer + let mut contains_any = false; + // Gets all slots provided for this input by the frontend + let input_slots = slots + .iter() + .filter_map(|(j, slot)| if i as u32 == *j { Some(slot) } else { None }); + // Goes through each slot and marks some amount from each slot as claimed + for slot in input_slots { + // Checks that the item in the slot can be used for the input + if let Some(item) = inv + .get(*slot) + .filter(|item| item.matches_recipe_input(input)) + { + // Gets the number of items claimed from the slot, or sets to 0 if slot has + // not been claimed by another input yet + let claimed = slot_claims.entry(*slot).or_insert(0); + let available = item.amount().saturating_sub(*claimed); + let provided = available.min(required); + required -= provided; + *claimed += provided; + contains_any = true; } - } else { - unsatisfied_requirements.push((input, *amount)); + } + // If there were not sufficient items to cover requirement between all provided + // slots, or if non-consumed item was not present, mark input as not satisfied + if required > 0 || !contains_any { + unsatisfied_requirements.push((input, required)); } }); // If there are no unsatisfied requirements, create the items produced by the - // recipe in the necessary quantity, else insert the ingredients back into the - // inventory + // recipe in the necessary quantity and remove the items that the recipe + // consumes if unsatisfied_requirements.is_empty() { + for (slot, to_remove) in slot_claims.iter() { + for _ in 0..*to_remove { + let _ = inv + .take(*slot, ability_map, msm) + .expect("Expected item to exist in the inventory"); + } + } let (item_def, quantity) = &self.output; let crafted_item = Item::new_from_item_def(Arc::clone(item_def), &[], ability_map, msm); let mut crafted_items = Vec::with_capacity(*quantity as usize); @@ -82,10 +97,6 @@ impl Recipe { } Ok(crafted_items) } else { - for item in recipe_inputs { - inv.push(item) - .expect("Item was in inventory before craft attempt"); - } Err(unsatisfied_requirements) } } @@ -104,26 +115,28 @@ impl Recipe { pub fn inventory_contains_ingredients<'a>( &self, inv: &'a Inventory, - ) -> Result, Vec<(&RecipeInput, u32)>> { + ) -> Result, Vec<(&RecipeInput, u32)>> { + // Hashmap tracking the quantity that needs to be removed from each slot (so + // that it doesn't think a slot can provide more items than it contains) let mut slot_claims = HashMap::::new(); // Important to be a vec and to remain separate from slot_claims as it must // remain ordered, unlike the hashmap - let mut slots = Vec::::new(); + let mut slots = Vec::<(u32, InvSlotId)>::new(); + // The inputs to a recipe that have missing items, and the amount missing let mut missing = Vec::<(&RecipeInput, u32)>::new(); - for (input, mut needed) in self.inputs() { + for (i, (input, mut needed)) in self.inputs().enumerate() { let mut contains_any = false; - + // Checks through every slot, filtering to only those that contain items that + // can satisfy the input for (inv_slot_id, slot) in inv.slots_with_id() { if let Some(item) = slot .as_ref() .filter(|item| item.matches_recipe_input(&*input)) { let claim = slot_claims.entry(inv_slot_id).or_insert(0); - slots.push(inv_slot_id); - // FIXME: Fishy, looks like it can underflow before min which can trigger an - // overflow check. - let can_claim = (item.amount() - *claim).min(needed); + slots.push((i as u32, inv_slot_id)); + let can_claim = (item.amount().saturating_sub(*claim)).min(needed); *claim += can_claim; needed -= can_claim; contains_any = true; @@ -154,16 +167,24 @@ pub fn try_salvage( msm: &MaterialStatManifest, ) -> Result, SalvageError> { if inv.get(slot).map_or(false, |item| item.is_salvageable()) { - let salvage_item = inv - .take(slot, ability_map, msm) - .expect("Expected item to exist in inventory"); - match salvage_item.try_salvage() { - Ok(items) => Ok(items), - Err(item) => { - inv.push(item) - .expect("Item taken from inventory just before"); - Err(SalvageError::NotSalvageable) - }, + let salvage_item = inv.get(slot).expect("Expected item to exist in inventory"); + let salvage_output: Vec<_> = salvage_item + .salvage_output() + .map(|asset| Item::new_from_asset_expect(asset)) + .collect(); + if salvage_output.is_empty() { + // If no output items, assume salvaging was a failure + // TODO: If we ever change salvaging to have a percent chance, remove the check + // of outputs being empty (requires assets to exist for rock and wood materials + // so that salvaging doesn't silently fail) + Err(SalvageError::NotSalvageable) + } else { + // Remove item that is being salvaged + let _ = inv + .take(slot, ability_map, msm) + .expect("Expected item to exist in inventory"); + // Return the salvaging output + Ok(salvage_output) } } else { Err(SalvageError::NotSalvageable)