refactor: use BlockId in more places (#3090)

* compare BlockId instead of (str) name

* compare named BlockIds in Block::properties() and Block::from_properties()

* compare named BlockIds instead of u16 literals for `impl BlockProperties`

Author: PigTurtleDev

crates/pumpkin-data/src/generated/block.rs

@@ -325,7 +325,7 @@ impl GeodeFeature {
 
                     // Only place if the target block is replaceable (air/water)
                     let is_air = place_state.is_air();
-                    let is_water = place_raw.to_block().name == "water";
+                    let is_water = place_raw.to_block_id() == BlockId::WATER;
 
                     if is_air || is_water {
                         let mut final_codec = base_codec.clone();

crates/pumpkin-world/src/generation/feature/features/geode.rs

@@ -30,13 +30,12 @@ impl CommandBlock {
     ) -> Option<(BlockPos, CommandBlockLikeProperties)> {
         let target_pos = pos.offset(dir.to_block_direction().to_offset());
         let (block, state_id) = world.get_block_and_state_id(&target_pos);
-
-        let allowed_blocks = [
-            Block::COMMAND_BLOCK.name,
-            Block::CHAIN_COMMAND_BLOCK.name,
-            Block::REPEATING_COMMAND_BLOCK.name,
-        ];
-        if !allowed_blocks.contains(&block.name) {
+        if !matches!(
+            block.id,
+            BlockId::COMMAND_BLOCK
+                | BlockId::CHAIN_COMMAND_BLOCK
+                | BlockId::REPEATING_COMMAND_BLOCK
+        ) {
             return None;
         }
 

crates/pumpkin/src/block/blocks/command.rs

@@ -6,7 +6,7 @@ use pumpkin_data::block_properties::{
 use pumpkin_data::item::Item;
 use pumpkin_data::item_stack::ItemStack;
 use pumpkin_data::sound::{Sound, SoundCategory};
-use pumpkin_data::{Block, BlockStateId};
+use pumpkin_data::{BlockId, BlockStateId};
 use pumpkin_macros::pumpkin_block;
 use pumpkin_world::world::BlockFlags;
 
@@ -17,8 +17,8 @@ use crate::block::{BlockBehaviour, BrokenArgs, OnNeighborUpdateArgs, OnPlaceArgs
 pub struct CreakingHeartBlock;
 
 impl CreakingHeartBlock {
-    fn is_pale_oak_log(block: &Block) -> bool {
-        block.name == "pale_oak_log" || block.name == "stripped_pale_oak_log"
+    const fn is_pale_oak_log(id: BlockId) -> bool {
+        matches!(id, BlockId::PALE_OAK_LOG | BlockId::STRIPPED_PALE_OAK_LOG)
     }
 
     fn check_active_logs(
@@ -32,11 +32,8 @@ impl CreakingHeartBlock {
             Axis::Z => (pos.north(), pos.south()),
         };
 
-        let state_a = world.get_block_state(&pos_a);
-        let state_b = world.get_block_state(&pos_b);
-
-        let block_a = Block::from_state_id(state_a.id);
-        let block_b = Block::from_state_id(state_b.id);
+        let block_a = world.get_block_state_id(&pos_a).to_block_id();
+        let block_b = world.get_block_state_id(&pos_b).to_block_id();
 
         Self::is_pale_oak_log(block_a) && Self::is_pale_oak_log(block_b)
     }

crates/pumpkin/src/block/blocks/creaking_heart.rs

@@ -104,7 +104,7 @@ impl FireBlock {
 
         for dir in BlockDirection::all() {
             let neighbor_block = world.get_block(&pos.offset(dir.to_offset()));
-            if world.get_fluid(&pos.offset(dir.to_offset())).name != Fluid::EMPTY.name {
+            if *world.get_fluid(&pos.offset(dir.to_offset())) != Fluid::EMPTY {
                 continue; // Skip if there is a fluid
             }
             if let Some(flammable) = &neighbor_block.flammable {

crates/pumpkin/src/block/blocks/fire/fire.rs

@@ -2,7 +2,7 @@ use pumpkin_data::block_properties::{BlockProperties, SnifferEggLikeProperties};
 use pumpkin_data::item::Item;
 use pumpkin_data::item_stack::ItemStack;
 use pumpkin_data::sound::{Sound, SoundCategory};
-use pumpkin_data::{Block, BlockStateId};
+use pumpkin_data::{BlockId, BlockStateId};
 use pumpkin_macros::pumpkin_block;
 use pumpkin_world::tick::TickPriority;
 use pumpkin_world::world::BlockFlags;
@@ -19,8 +19,8 @@ impl SnifferEggBlock {
     ) -> bool {
         let below_pos = pos.down();
         let state = world.get_block_state(&below_pos);
-        let block = Block::from_state_id(state.id);
-        block.name == "moss_block"
+        let block = BlockId::from_state_id(state.id);
+        block == BlockId::MOSS_BLOCK
     }
 
     const fn get_hatch_delay(on_moss: bool) -> u8 {

crates/pumpkin/src/block/blocks/sniffer_egg.rs

@@ -22,7 +22,7 @@ impl Ignition {
     where
         F: FnOnce(Arc<World>, BlockPos, BlockStateId),
     {
-        if world.get_fluid(&location).name != Fluid::EMPTY.name {
+        if *world.get_fluid(&location) != Fluid::EMPTY {
             return false;
         }
         let fire_block = FireBlockBase::get_fire_type(world, &fire_pos);

crates/pumpkin/src/item/items/ignite/ignition.rs

@@ -222,12 +222,9 @@ impl ToTokens for BlockPropertyStruct {
             }
         });
 
-        let block_ids = self
-            .data
-            .blocks
-            .iter()
-            .map(|(_, id)| *id)
-            .collect::<Vec<_>>();
+        let block_ids = self.data.blocks.iter().map(|(name, _)| {
+            Ident::new(&const_block_name_from_block_name(name), Span::call_site())
+        });
 
         let to_index_logic = self.data.variant_mappings.iter().rev().map(|entry| {
             let field = Ident::new_raw(&entry.original_name, Span::call_site());
@@ -389,7 +386,7 @@ impl ToTokens for BlockPropertyStruct {
                 #[inline]
                 #[allow(clippy::manual_range_patterns)]
                 fn handles_block_id(block_id: BlockId) -> bool where Self: Sized {
-                    matches!(block_id.as_u16(), #(#block_ids)|*)
+                    matches!(block_id, #(BlockId::#block_ids)|*)
                 }
 
                 fn to_state_id(&self, block: &Block) -> BlockStateId {
@@ -433,7 +430,7 @@ impl ToTokens for BlockPropertyStruct {
                 #[allow(clippy::manual_range_patterns)]
                 fn from_props(props: &[(&str, &str)], block: &Block) -> Self {
                     #[cfg(debug_assertions)]
-                    if !matches!(block.id.as_u16(), #(#block_ids)|*) {
+                    if !Self::handles_block_id(block.id) {
                         panic!("{} is not a valid block for {}", block.name, #struct_name);
                     }
                     let mut block_props = Self::default(block);
@@ -1072,21 +1069,18 @@ pub fn build() -> TokenStream {
             Span::call_site(),
         );
 
-        for (block_name, id) in &property_group.blocks {
-            let const_block_name = Ident::new(
-                &const_block_name_from_block_name(block_name),
-                Span::call_site(),
-            );
-            let id_lit = LitInt::new(&id.to_string(), Span::call_site());
-
-            block_properties_from_state_and_block_id_arms.push(quote! {
-                #id_lit => Box::new(#property_name::from_state_id(state_id, &Block::#const_block_name)),
-            });
+        let idents: Box<_> = property_group
+            .blocks
+            .iter()
+            .map(|(name, _)| Ident::new(&const_block_name_from_block_name(name), Span::call_site()))
+            .collect();
 
-            block_properties_from_props_and_name_arms.push(quote! {
-                #id_lit => Box::new(#property_name::from_props(props, &Block::#const_block_name)),
-            });
-        }
+        block_properties_from_state_and_block_id_arms.push(quote! {
+            #(BlockId::#idents)|* => Box::new(#property_name::from_state_id(state_id, self)),
+        });
+        block_properties_from_props_and_name_arms.push(quote! {
+            #(BlockId::#idents)|* => Box::new(#property_name::from_props(props, self)),
+        });
 
         block_properties.push(BlockPropertyStruct {
             data: property_group,
@@ -1351,7 +1345,7 @@ pub fn build() -> TokenStream {
             #[track_caller]
             #[doc = r" Get the properties of the block."]
             pub fn properties(&self, state_id: BlockStateId) -> Option<Box<dyn BlockProperties>> {
-                Some(match self.id.as_u16() {
+                Some(match self.id {
                     #(#block_properties_from_state_and_block_id_arms)*
                     _ => return None,
                 })
@@ -1360,7 +1354,7 @@ pub fn build() -> TokenStream {
             #[track_caller]
             #[doc = r" Get the properties of the block."]
             pub fn from_properties(&self, props: &[(&str, &str)]) -> Box<dyn BlockProperties> {
-                match self.id.as_u16() {
+                match self.id {
                     #(#block_properties_from_props_and_name_arms)*
                     _ => panic!("Invalid props")
                 }