Merge branch 'aweinstock/sprite-collectible-refactoring' into 'master'

Deduplicate data involved in sprite harvesting, making it harder to make mistakes with.

See merge request veloren/veloren!2542

common/src/bin/csv_export/main.rs

@@ -239,7 +239,7 @@ fn loot_table(loot_table: &str) -> Result<(), Box<dyn Error>> {
 
     let loot_table = "common.loot_tables.".to_owned() + loot_table;
 
-    let loot_table = Lottery::<LootSpec>::load_expect(&loot_table).read();
+    let loot_table = Lottery::<LootSpec<String>>::load_expect(&loot_table).read();
 
     for (i, (chance, item)) in loot_table.iter().enumerate() {
         let chance = if let Some((next_chance, _)) = loot_table.iter().nth(i + 1) {

common/src/bin/csv_import/main.rs

@@ -418,7 +418,7 @@ fn loot_table(loot_table: &str) -> Result<(), Box<dyn Error>> {
         .map(|(i, x)| (x.to_string(), i))
         .collect();
 
-    let mut items = Vec::<(f32, LootSpec)>::new();
+    let mut items = Vec::<(f32, LootSpec<String>)>::new();
 
     for ref record in rdr.records().flatten() {
         let item = match record.get(headers["Kind"]).expect("No loot specifier") {

common/src/comp/inventory/item/mod.rs

@@ -13,9 +13,8 @@ use crate::{
         CharacterAbility,
     },
     effect::Effect,
-    lottery::{LootSpec, Lottery},
     recipe::RecipeInput,
-    terrain::{Block, SpriteKind},
+    terrain::Block,
 };
 use core::{
     convert::TryFrom,
@@ -795,134 +794,7 @@ impl Item {
     pub fn slot_mut(&mut self, slot: usize) -> Option<&mut InvSlot> { self.slots.get_mut(slot) }
 
     pub fn try_reclaim_from_block(block: Block) -> Option<Self> {
-        Some(Item::new_from_asset_expect(match block.get_sprite()? {
+        Some(block.get_sprite()?.collectible_id()?.to_item())
-            SpriteKind::Apple => "common.items.food.apple",
-            SpriteKind::Mushroom => "common.items.food.mushroom",
-            SpriteKind::Velorite => "common.items.mineral.ore.velorite",
-            SpriteKind::VeloriteFrag => "common.items.mineral.ore.veloritefrag",
-            SpriteKind::BlueFlower => "common.items.flowers.blue",
-            SpriteKind::PinkFlower => "common.items.flowers.pink",
-            SpriteKind::PurpleFlower => "common.items.flowers.purple",
-            SpriteKind::RedFlower => "common.items.flowers.red",
-            SpriteKind::WhiteFlower => "common.items.flowers.white",
-            SpriteKind::YellowFlower => "common.items.flowers.yellow",
-            SpriteKind::Sunflower => "common.items.flowers.sunflower",
-            SpriteKind::LongGrass => "common.items.grasses.long",
-            SpriteKind::MediumGrass => "common.items.grasses.medium",
-            SpriteKind::ShortGrass => "common.items.grasses.short",
-            SpriteKind::Coconut => "common.items.food.coconut",
-            SpriteKind::Beehive => "common.items.crafting_ing.honey",
-            SpriteKind::Stones => "common.items.crafting_ing.stones",
-            SpriteKind::Twigs => "common.items.crafting_ing.twigs",
-            SpriteKind::VialEmpty => "common.items.crafting_ing.empty_vial",
-            SpriteKind::Bowl => "common.items.crafting_ing.bowl",
-            SpriteKind::PotionMinor => "common.items.consumable.potion_minor",
-            SpriteKind::Amethyst => "common.items.mineral.gem.amethyst",
-            SpriteKind::Ruby => "common.items.mineral.gem.ruby",
-            SpriteKind::Diamond => "common.items.mineral.gem.diamond",
-            SpriteKind::Sapphire => "common.items.mineral.gem.sapphire",
-            SpriteKind::Topaz => "common.items.mineral.gem.topaz",
-            SpriteKind::Emerald => "common.items.mineral.gem.emerald",
-            SpriteKind::AmethystSmall => "common.items.mineral.gem.amethyst",
-            SpriteKind::TopazSmall => "common.items.mineral.gem.topaz",
-            SpriteKind::DiamondSmall => "common.items.mineral.gem.diamond",
-            SpriteKind::RubySmall => "common.items.mineral.gem.ruby",
-            SpriteKind::EmeraldSmall => "common.items.mineral.gem.emerald",
-            SpriteKind::SapphireSmall => "common.items.mineral.gem.sapphire",
-            SpriteKind::Bloodstone => "common.items.mineral.ore.bloodstone",
-            SpriteKind::Coal => "common.items.mineral.ore.coal",
-            SpriteKind::Cobalt => "common.items.mineral.ore.cobalt",
-            SpriteKind::Copper => "common.items.mineral.ore.copper",
-            SpriteKind::Iron => "common.items.mineral.ore.iron",
-            SpriteKind::Tin => "common.items.mineral.ore.tin",
-            SpriteKind::Silver => "common.items.mineral.ore.silver",
-            SpriteKind::Gold => "common.items.mineral.ore.gold",
-            SpriteKind::Cotton => "common.items.crafting_ing.cotton_boll",
-            SpriteKind::Moonbell => "common.items.flowers.moonbell",
-            SpriteKind::Pyrebloom => "common.items.flowers.pyrebloom",
-            SpriteKind::WildFlax => "common.items.flowers.wild_flax",
-            SpriteKind::Seashells => "common.items.crafting_ing.seashells",
-            SpriteKind::RoundCactus => "common.items.crafting_ing.cactus",
-            SpriteKind::ShortFlatCactus => "common.items.crafting_ing.cactus",
-            SpriteKind::MedFlatCactus => "common.items.crafting_ing.cactus",
-            // Containers
-            // IMPORTANT: Add any new container to `SpriteKind::is_container`
-            container
-            @
-            (SpriteKind::DungeonChest0
-            | SpriteKind::DungeonChest1
-            | SpriteKind::DungeonChest2
-            | SpriteKind::DungeonChest3
-            | SpriteKind::DungeonChest4
-            | SpriteKind::DungeonChest5
-            | SpriteKind::Chest
-            | SpriteKind::Mud
-            | SpriteKind::Crate
-            | SpriteKind::ChestBuried) => {
-                return Item::from_container(container);
-            },
-            _ => return None,
-        }))
-    }
-
-    fn from_container(container: SpriteKind) -> Option<Item> {
-        let chosen;
-        match container {
-            SpriteKind::DungeonChest0 => {
-                chosen =
-                    Lottery::<LootSpec>::load_expect("common.loot_tables.dungeon.tier-0.chest")
-                        .read();
-                return Some(chosen.choose().to_item());
-            },
-            SpriteKind::DungeonChest1 => {
-                chosen =
-                    Lottery::<LootSpec>::load_expect("common.loot_tables.dungeon.tier-1.chest")
-                        .read();
-                return Some(chosen.choose().to_item());
-            },
-            SpriteKind::DungeonChest2 => {
-                chosen =
-                    Lottery::<LootSpec>::load_expect("common.loot_tables.dungeon.tier-2.chest")
-                        .read();
-                return Some(chosen.choose().to_item());
-            },
-            SpriteKind::DungeonChest3 => {
-                chosen =
-                    Lottery::<LootSpec>::load_expect("common.loot_tables.dungeon.tier-3.chest")
-                        .read();
-                return Some(chosen.choose().to_item());
-            },
-            SpriteKind::DungeonChest4 => {
-                chosen =
-                    Lottery::<LootSpec>::load_expect("common.loot_tables.dungeon.tier-4.chest")
-                        .read();
-                return Some(chosen.choose().to_item());
-            },
-            SpriteKind::DungeonChest5 => {
-                chosen =
-                    Lottery::<LootSpec>::load_expect("common.loot_tables.dungeon.tier-5.chest")
-                        .read();
-                return Some(chosen.choose().to_item());
-            },
-            SpriteKind::Chest => {
-                chosen = Lottery::<LootSpec>::load_expect("common.loot_tables.sprite.chest").read();
-                return Some(chosen.choose().to_item());
-            },
-            SpriteKind::ChestBuried => {
-                chosen = Lottery::<LootSpec>::load_expect("common.loot_tables.sprite.chest-buried")
-                    .read();
-                return Some(chosen.choose().to_item());
-            },
-            SpriteKind::Mud => {
-                chosen = Lottery::<LootSpec>::load_expect("common.loot_tables.sprite.mud").read();
-                return Some(chosen.choose().to_item());
-            },
-            SpriteKind::Crate => {
-                chosen = Lottery::<LootSpec>::load_expect("common.loot_tables.sprite.crate").read();
-                return Some(chosen.choose().to_item());
-            },
-            _ => None,
-        }
     }
 
     pub fn ability_spec(&self) -> Option<&AbilitySpec> { self.item_def.ability_spec.as_ref() }

common/src/comp/inventory/trade_pricing.rs

@@ -84,14 +84,14 @@ struct ProbabilityFile {
 }
 
 impl assets::Asset for ProbabilityFile {
-    type Loader = assets::LoadFrom<Vec<(f32, LootSpec)>, assets::RonLoader>;
+    type Loader = assets::LoadFrom<Vec<(f32, LootSpec<String>)>, assets::RonLoader>;
 
     const EXTENSION: &'static str = "ron";
 }
 
-impl From<Vec<(f32, LootSpec)>> for ProbabilityFile {
+impl From<Vec<(f32, LootSpec<String>)>> for ProbabilityFile {
     #[allow(clippy::cast_precision_loss)]
-    fn from(content: Vec<(f32, LootSpec)>) -> Self {
+    fn from(content: Vec<(f32, LootSpec<String>)>) -> Self {
         Self {
             content: content
                 .into_iter()
@@ -101,7 +101,7 @@ impl From<Vec<(f32, LootSpec)>> for ProbabilityFile {
                         vec![(p0 * (a + b) as f32 / 2.0, asset)].into_iter()
                     },
                     LootSpec::LootTable(table_asset) => {
-                        let total = Lottery::<LootSpec>::load_expect(&table_asset)
+                        let total = Lottery::<LootSpec<String>>::load_expect(&table_asset)
                             .read()
                             .total();
                         Self::load_expect_cloned(&table_asset)

common/src/generation.rs

@@ -131,7 +131,7 @@ impl EntityInfo {
                     self = self.with_loot_drop(Item::new_from_asset_expect(&asset));
                 },
                 LootKind::LootTable(asset) => {
-                    let table = Lottery::<LootSpec>::load_expect(&asset);
+                    let table = Lottery::<LootSpec<String>>::load_expect(&asset);
                     let drop = table.read().choose().to_item();
                     self = self.with_loot_drop(drop);
                 },
@@ -352,7 +352,7 @@ mod tests {
                     LootKind::LootTable(asset) => {
                         // we need to just load it check if it exists,
                         // because all loot tables are tested in Lottery module
-                        let _ = Lottery::<LootSpec>::load_expect(&asset);
+                        let _ = Lottery::<LootSpec<String>>::load_expect(&asset);
                     },
                 }
             }

common/src/lottery.rs

@@ -77,30 +77,30 @@ impl<T> Lottery<T> {
 }
 
 #[derive(Clone, Debug, PartialEq, Deserialize, Serialize)]
-pub enum LootSpec {
+pub enum LootSpec<T: AsRef<str>> {
     /// Asset specifier
-    Item(String),
+    Item(T),
     /// Asset specifier, lower range, upper range
-    ItemQuantity(String, u32, u32),
+    ItemQuantity(T, u32, u32),
     /// Loot table
-    LootTable(String),
+    LootTable(T),
 }
 
-impl LootSpec {
+impl<T: AsRef<str>> LootSpec<T> {
     pub fn to_item(&self) -> Item {
         match self {
-            Self::Item(item) => Item::new_from_asset_expect(&item),
+            Self::Item(item) => Item::new_from_asset_expect(item.as_ref()),
             Self::ItemQuantity(item, lower, upper) => {
                 let range = *lower..=*upper;
                 let quantity = thread_rng().gen_range(range);
-                let mut item = Item::new_from_asset_expect(&item);
+                let mut item = Item::new_from_asset_expect(item.as_ref());
                 // TODO: Handle multiple of an item that is unstackable
                 if item.set_amount(quantity).is_err() {
                     warn!("Tried to set quantity on non stackable item");
                 }
                 item
             },
-            Self::LootTable(table) => Lottery::<LootSpec>::load_expect(&table)
+            Self::LootTable(table) => Lottery::<LootSpec<String>>::load_expect(table.as_ref())
                 .read()
                 .choose()
                 .to_item(),
@@ -115,7 +115,7 @@ mod tests {
 
     #[test]
     fn test_loot_tables() {
-        fn validate_table_contents(table: Lottery<LootSpec>) {
+        fn validate_table_contents(table: Lottery<LootSpec<String>>) {
             for (_, item) in table.iter() {
                 match item {
                     LootSpec::Item(item) => {
@@ -137,14 +137,16 @@ mod tests {
                         Item::new_from_asset_expect(&item);
                     },
                     LootSpec::LootTable(loot_table) => {
-                        let loot_table = Lottery::<LootSpec>::load_expect_cloned(&loot_table);
+                        let loot_table =
+                            Lottery::<LootSpec<String>>::load_expect_cloned(&loot_table);
                         validate_table_contents(loot_table);
                     },
                 }
             }
         }
 
-        let loot_tables = assets::load_expect_dir::<Lottery<LootSpec>>("common.loot_tables", true);
+        let loot_tables =
+            assets::load_expect_dir::<Lottery<LootSpec<String>>>("common.loot_tables", true);
         for loot_table in loot_tables.iter() {
             validate_table_contents(loot_table.cloned());
         }

common/src/terrain/sprite.rs

@@ -1,4 +1,4 @@
-use crate::{comp::tool::ToolKind, make_case_elim};
+use crate::{comp::tool::ToolKind, lottery::LootSpec, make_case_elim};
 use enum_iterator::IntoEnumIterator;
 use hashbrown::HashMap;
 use lazy_static::lazy_static;
@@ -262,68 +262,85 @@ impl SpriteKind {
         })
     }
 
-    pub fn is_collectible(&self) -> bool {
+    /// What loot table does collecting this sprite draw from?
-        match self {
+    pub fn collectible_id(&self) -> Option<LootSpec<&'static str>> {
-            SpriteKind::BlueFlower => false,
+        let item = |id: &'static str| LootSpec::Item(id);
-            SpriteKind::PinkFlower => false,
+        let table = |id: &'static str| LootSpec::LootTable(id);
-            SpriteKind::PurpleFlower => false,
+        Some(match self {
-            SpriteKind::RedFlower => true,
+            SpriteKind::Apple => item("common.items.food.apple"),
-            SpriteKind::WhiteFlower => false,
+            SpriteKind::Mushroom => item("common.items.food.mushroom"),
-            SpriteKind::YellowFlower => false,
+            SpriteKind::Velorite => item("common.items.mineral.ore.velorite"),
-            SpriteKind::Sunflower => true,
+            SpriteKind::VeloriteFrag => item("common.items.mineral.ore.veloritefrag"),
-            SpriteKind::LongGrass => false,
+            //SpriteKind::BlueFlower => item("common.items.flowers.blue"),
-            SpriteKind::MediumGrass => false,
+            //SpriteKind::PinkFlower => item("common.items.flowers.pink"),
-            SpriteKind::ShortGrass => false,
+            //SpriteKind::PurpleFlower => item("common.items.flowers.purple"),
-            SpriteKind::Apple => true,
+            SpriteKind::RedFlower => item("common.items.flowers.red"),
-            SpriteKind::Mushroom => true,
+            //SpriteKind::WhiteFlower => item("common.items.flowers.white"),
-            // SpriteKind::Velorite => true,
+            //SpriteKind::YellowFlower => item("common.items.flowers.yellow"),
-            // SpriteKind::VeloriteFrag => true,
+            SpriteKind::Sunflower => item("common.items.flowers.sunflower"),
-            SpriteKind::Chest => true,
+            //SpriteKind::LongGrass => item("common.items.grasses.long"),
-            SpriteKind::DungeonChest0 => true,
+            //SpriteKind::MediumGrass => item("common.items.grasses.medium"),
-            SpriteKind::DungeonChest1 => true,
+            //SpriteKind::ShortGrass => item("common.items.grasses.short"),
-            SpriteKind::DungeonChest2 => true,
+            SpriteKind::Coconut => item("common.items.food.coconut"),
-            SpriteKind::DungeonChest3 => true,
+            SpriteKind::Beehive => item("common.items.crafting_ing.honey"),
-            SpriteKind::DungeonChest4 => true,
+            SpriteKind::Stones => item("common.items.crafting_ing.stones"),
-            SpriteKind::DungeonChest5 => true,
+            SpriteKind::Twigs => item("common.items.crafting_ing.twigs"),
-            SpriteKind::Coconut => true,
+            SpriteKind::VialEmpty => item("common.items.crafting_ing.empty_vial"),
-            SpriteKind::Stones => true,
+            SpriteKind::Bowl => item("common.items.crafting_ing.bowl"),
-            SpriteKind::Twigs => true,
+            SpriteKind::PotionMinor => item("common.items.consumable.potion_minor"),
-            SpriteKind::Crate => true,
+            SpriteKind::Amethyst => item("common.items.mineral.gem.amethyst"),
-            SpriteKind::Beehive => true,
+            SpriteKind::Ruby => item("common.items.mineral.gem.ruby"),
-            SpriteKind::VialEmpty => true,
+            SpriteKind::Diamond => item("common.items.mineral.gem.diamond"),
-            SpriteKind::PotionMinor => true,
+            SpriteKind::Sapphire => item("common.items.mineral.gem.sapphire"),
-            SpriteKind::Bowl => true,
+            SpriteKind::Topaz => item("common.items.mineral.gem.topaz"),
-            SpriteKind::ChestBuried => true,
+            SpriteKind::Emerald => item("common.items.mineral.gem.emerald"),
-            SpriteKind::Mud => true,
+            SpriteKind::AmethystSmall => item("common.items.mineral.gem.amethyst"),
-            SpriteKind::Seashells => true,
+            SpriteKind::TopazSmall => item("common.items.mineral.gem.topaz"),
-            SpriteKind::Cotton => true,
+            SpriteKind::DiamondSmall => item("common.items.mineral.gem.diamond"),
-            SpriteKind::Moonbell => true,
+            SpriteKind::RubySmall => item("common.items.mineral.gem.ruby"),
-            SpriteKind::Pyrebloom => true,
+            SpriteKind::EmeraldSmall => item("common.items.mineral.gem.emerald"),
-            SpriteKind::WildFlax => true,
+            SpriteKind::SapphireSmall => item("common.items.mineral.gem.sapphire"),
-            SpriteKind::RoundCactus => true,
+            SpriteKind::Bloodstone => item("common.items.mineral.ore.bloodstone"),
-            SpriteKind::ShortFlatCactus => true,
+            SpriteKind::Coal => item("common.items.mineral.ore.coal"),
-            SpriteKind::MedFlatCactus => true,
+            SpriteKind::Cobalt => item("common.items.mineral.ore.cobalt"),
-            _ => false,
+            SpriteKind::Copper => item("common.items.mineral.ore.copper"),
+            SpriteKind::Iron => item("common.items.mineral.ore.iron"),
+            SpriteKind::Tin => item("common.items.mineral.ore.tin"),
+            SpriteKind::Silver => item("common.items.mineral.ore.silver"),
+            SpriteKind::Gold => item("common.items.mineral.ore.gold"),
+            SpriteKind::Cotton => item("common.items.crafting_ing.cotton_boll"),
+            SpriteKind::Moonbell => item("common.items.flowers.moonbell"),
+            SpriteKind::Pyrebloom => item("common.items.flowers.pyrebloom"),
+            SpriteKind::WildFlax => item("common.items.flowers.wild_flax"),
+            SpriteKind::Seashells => item("common.items.crafting_ing.seashells"),
+            SpriteKind::RoundCactus => item("common.items.crafting_ing.cactus"),
+            SpriteKind::ShortFlatCactus => item("common.items.crafting_ing.cactus"),
+            SpriteKind::MedFlatCactus => item("common.items.crafting_ing.cactus"),
+            SpriteKind::DungeonChest0 => table("common.loot_tables.dungeon.tier-0.chest"),
+            SpriteKind::DungeonChest1 => table("common.loot_tables.dungeon.tier-1.chest"),
+            SpriteKind::DungeonChest2 => table("common.loot_tables.dungeon.tier-2.chest"),
+            SpriteKind::DungeonChest3 => table("common.loot_tables.dungeon.tier-3.chest"),
+            SpriteKind::DungeonChest4 => table("common.loot_tables.dungeon.tier-4.chest"),
+            SpriteKind::DungeonChest5 => table("common.loot_tables.dungeon.tier-5.chest"),
+            SpriteKind::Chest => table("common.loot_tables.sprite.chest"),
+            SpriteKind::ChestBuried => table("common.loot_tables.sprite.chest-buried"),
+            SpriteKind::Mud => table("common.loot_tables.sprite.mud"),
+            SpriteKind::Crate => table("common.loot_tables.sprite.crate"),
+            _ => return None,
+        })
     }
+
+    /// Can this sprite be picked up to yield an item without a tool?
+    pub fn is_collectible(&self) -> bool {
+        self.collectible_id().is_some() && self.mine_tool().is_none()
     }
 
     /// Is the sprite a container that will emit a mystery item?
     pub fn is_container(&self) -> bool {
-        matches!(
+        matches!(self.collectible_id(), Some(LootSpec::LootTable(_)))
-            self,
-            SpriteKind::DungeonChest0
-                | SpriteKind::DungeonChest1
-                | SpriteKind::DungeonChest2
-                | SpriteKind::DungeonChest3
-                | SpriteKind::DungeonChest4
-                | SpriteKind::DungeonChest5
-                | SpriteKind::Chest
-                | SpriteKind::ChestBuried
-                | SpriteKind::Mud
-                | SpriteKind::Crate,
-        )
     }
 
+    /// Which tool (if any) is needed to collect this sprite?
     pub fn mine_tool(&self) -> Option<ToolKind> {
         match self {
             SpriteKind::Velorite

server/src/events/entity_manipulation.rs

@@ -374,7 +374,7 @@ pub fn handle_destroy(server: &mut Server, entity: EcsEntity, cause: HealthSourc
         // Decide for a loot drop before turning into a lootbag
         let old_body = state.ecs().write_storage::<Body>().remove(entity);
         let lottery = || {
-            Lottery::<LootSpec>::load_expect(match old_body {
+            Lottery::<LootSpec<String>>::load_expect(match old_body {
                 Some(common::comp::Body::Humanoid(_)) => "common.loot_tables.creature.humanoid",
                 Some(common::comp::Body::QuadrupedSmall(quadruped_small)) => {
                     match quadruped_small.species {