BlockId refactor.

Moved `BlockId` to it's own file.
Prefixed all of the `block_registry` types with Registry to avoid confusion.
Made stuff that should not be public private.

pumpkin-protocol/src/client/play/c_chunk_data.rs

@@ -3,7 +3,7 @@ use std::collections::HashMap;
 use crate::{bytebuf::ByteBuffer, BitSet, ClientPacket, VarInt};
 use itertools::Itertools;
 use pumpkin_macros::packet;
-use pumpkin_world::{block::block_registry::BlockId, chunk::ChunkData, DIRECT_PALETTE_BITS};
+use pumpkin_world::{chunk::ChunkData, DIRECT_PALETTE_BITS};
 
 #[packet(0x27)]
 pub struct CChunkData<'a>(pub &'a ChunkData);
@@ -23,15 +23,7 @@ impl<'a> ClientPacket for CChunkData<'a> {
 
         let mut data_buf = ByteBuffer::empty();
         self.0.blocks.iter_subchunks().for_each(|chunk| {
-            let block_count = chunk
-                .iter()
-                .dedup()
-                .filter(|block| {
-                    !block.is_air()
-                        && **block != BlockId::from_id(12959)
-                        && **block != BlockId::from_id(12958)
-                })
-                .count() as i16;
+            let block_count = chunk.iter().dedup().filter(|block| !block.is_air()).count() as i16;
             // Block count
             data_buf.put_i16(block_count);
             //// Block states

pumpkin-world/src/block/block_id.rs

@@ -0,0 +1,58 @@
+use std::collections::HashMap;
+
+use serde::Deserialize;
+
+use super::block_registry::BLOCKS;
+use crate::level::WorldError;
+
+// 0 is air -> reasonable default
+#[derive(Default, Deserialize, Debug, Hash, Clone, Copy, PartialEq, Eq)]
+#[serde(transparent)]
+pub struct BlockId {
+    data: u16,
+}
+
+impl BlockId {
+    pub const AIR: Self = Self::from_id(0);
+
+    pub fn new(
+        text_id: &str,
+        properties: Option<&HashMap<String, String>>,
+    ) -> Result<Self, WorldError> {
+        let mut block_states = BLOCKS
+            .get(text_id)
+            .ok_or(WorldError::BlockIdentifierNotFound)?
+            .states
+            .iter();
+
+        let block_state = match properties {
+            Some(properties) => match block_states.find(|state| &state.properties == properties) {
+                Some(state) => state,
+                None => return Err(WorldError::BlockStateIdNotFound),
+            },
+            None => block_states
+                .find(|state| state.is_default)
+                .expect("Every Block should have at least 1 default state"),
+        };
+
+        Ok(block_state.id)
+    }
+
+    pub const fn from_id(id: u16) -> Self {
+        // TODO: add check if the id is actually valid
+        Self { data: id }
+    }
+
+    pub fn is_air(&self) -> bool {
+        self.data == 0 || self.data == 12959 || self.data == 12958
+    }
+
+    pub fn get_id(&self) -> u16 {
+        self.data
+    }
+
+    /// An i32 is the way mojang internally represents their Blocks
+    pub fn get_id_mojang_repr(&self) -> i32 {
+        self.data as i32
+    }
+}

pumpkin-world/src/block/block_registry.rs

@@ -3,101 +3,50 @@ use std::collections::HashMap;
 use lazy_static::lazy_static;
 use serde::Deserialize;
 
-use crate::level::WorldError;
+use super::block_id::BlockId;
 
-const BLOCKS_JSON: &str = include_str!("../../assets/blocks.json");
-
-// 0 is air -> reasonable default
-#[derive(Default, Deserialize, Debug, Hash, Clone, Copy, PartialEq, Eq)]
-#[serde(transparent)]
-pub struct BlockId {
-    data: u16,
-}
-
-impl BlockId {
-    pub fn new(
-        block_id: &str,
-        properties: Option<&HashMap<String, String>>,
-    ) -> Result<Self, WorldError> {
-        let mut block_states = BLOCKS
-            .get(block_id)
-            .ok_or(WorldError::BlockIdentifierNotFound)?
-            .states
-            .iter();
-
-        let block_state = match properties {
-            Some(properties) => match block_states.find(|state| &state.properties == properties) {
-                Some(state) => state,
-                None => return Err(WorldError::BlockStateIdNotFound),
-            },
-            None => block_states
-                .find(|state| state.is_default)
-                .expect("Every Block should have at least 1 default state"),
-        };
-
-        Ok(block_state.id)
-    }
-
-    pub fn from_id(id: u16) -> Self {
-        // TODO: add check if the id is actually valid
-        Self { data: id }
-    }
-
-    pub fn is_air(&self) -> bool {
-        self.data == 0
-    }
-
-    pub fn get_id(&self) -> u16 {
-        self.data
-    }
-
-    /// An i32 is the way mojang internally represents their Blocks
-    pub fn get_id_mojang_repr(&self) -> i32 {
-        self.data as i32
-    }
+lazy_static! {
+    pub static ref BLOCKS: HashMap<String, RegistryBlockType> =
+        serde_json::from_str(include_str!("../../assets/blocks.json"))
+            .expect("Could not parse block.json registry.");
 }
 
 #[derive(Deserialize, Debug, Clone, PartialEq, Eq)]
-pub struct BlockDefinition {
+pub struct RegistryBlockDefinition {
     /// e.g. minecraft:door or minecraft:button
     #[serde(rename = "type")]
-    category: String,
+    pub category: String,
 
     /// Specifies the variant of the blocks category.
     /// e.g. minecraft:iron_door has the variant iron
     #[serde(rename = "block_set_type")]
-    variant: Option<String>,
+    pub variant: Option<String>,
 }
 
 /// One possible state of a Block.
 /// This could e.g. be an extended piston facing left.
 #[derive(Deserialize, Debug, Clone, PartialEq, Eq)]
-pub struct BlockState {
-    id: BlockId,
+pub struct RegistryBlockState {
+    pub id: BlockId,
 
     /// Whether this is the default state of the Block
     #[serde(default, rename = "default")]
-    is_default: bool,
+    pub is_default: bool,
 
     /// The propertise active for this `BlockState`.
     #[serde(default)]
-    properties: HashMap<String, String>,
+    pub properties: HashMap<String, String>,
 }
 
 /// A fully-fledged block definition.
 /// Stores the category, variant, all of the possible states and all of the possible properties.
 #[derive(Deserialize, Debug, Clone, PartialEq, Eq)]
-pub struct BlockType {
-    definition: BlockDefinition,
-    states: Vec<BlockState>,
+pub struct RegistryBlockType {
+    pub definition: RegistryBlockDefinition,
+    pub states: Vec<RegistryBlockState>,
 
     // TODO is this safe to remove? It's currently not used in the Project. @lukas0008 @Snowiiii
     /// A list of valid property keys/values for a block.
     #[serde(default, rename = "properties")]
     valid_properties: HashMap<String, Vec<String>>,
 }
-
-lazy_static! {
-    pub static ref BLOCKS: HashMap<String, BlockType> =
-        serde_json::from_str(BLOCKS_JSON).expect("Could not parse block.json registry.");
-}

pumpkin-world/src/block/mod.rs

@@ -1,9 +1,11 @@
+use crate::vector3::Vector3;
 use num_derive::FromPrimitive;
 
-use crate::vector3::Vector3;
+pub mod block_id;
+mod block_registry;
+
+pub use block_id::BlockId;
 
-pub mod block_registry;
-pub use block_registry::BLOCKS;
 #[derive(FromPrimitive)]
 pub enum BlockFace {
     Bottom = 0,

pumpkin-world/src/chunk.rs

@@ -6,7 +6,7 @@ use fastnbt::LongArray;
 use serde::{Deserialize, Serialize};
 
 use crate::{
-    block::block_registry::BlockId,
+    block::BlockId,
     coordinates::{ChunkCoordinates, ChunkRelativeBlockCoordinates, Height},
     level::WorldError,
     WORLD_HEIGHT,

pumpkin/src/client/player_packet.rs

@@ -25,7 +25,7 @@ use pumpkin_protocol::{
         SUseItemOn, Status,
     },
 };
-use pumpkin_world::block::{block_registry::BlockId, BlockFace};
+use pumpkin_world::block::{BlockFace, BlockId};
 use pumpkin_world::global_registry;
 
 use super::PlayerConfig;