From 43164ee4f12daad0a8ec77202105361e353d5c90 Mon Sep 17 00:00:00 2001 From: DaniD3v Date: Sun, 25 Aug 2024 09:04:15 +0200 Subject: [PATCH] Added abstraction for `BlockId` `i32` is really ambiguous and it is not immediately obvious what it is supposed to represent. `BlockId` is much more readable. Changed internal type to u16 because we currently have ~30k/64k block states. (and i32 doesn't make a lot of sense for an Id, except when replicating mojang behavior) --- .../src/client/play/c_chunk_data.rs | 12 ++- pumpkin-world/src/block/block_registry.rs | 77 ++++++++++++------- pumpkin-world/src/chunk.rs | 18 ++--- pumpkin/src/client/player_packet.rs | 13 +--- 4 files changed, 70 insertions(+), 50 deletions(-) diff --git a/pumpkin-protocol/src/client/play/c_chunk_data.rs b/pumpkin-protocol/src/client/play/c_chunk_data.rs index 076a3f1a5..87eef9972 100644 --- a/pumpkin-protocol/src/client/play/c_chunk_data.rs +++ b/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::{chunk::ChunkData, DIRECT_PALETTE_BITS}; +use pumpkin_world::{block::block_registry::BlockId, chunk::ChunkData, DIRECT_PALETTE_BITS}; #[packet(0x27)] pub struct CChunkData<'a>(pub &'a ChunkData); @@ -26,7 +26,11 @@ impl<'a> ClientPacket for CChunkData<'a> { let block_count = chunk .iter() .dedup() - .filter(|block| **block != 0 && **block != 12959 && **block != 12958) + .filter(|block| { + !block.is_air() + && **block != BlockId::from_id(12959) + && **block != BlockId::from_id(12958) + }) .count() as i16; // Block count data_buf.put_i16(block_count); @@ -63,7 +67,7 @@ impl<'a> ClientPacket for CChunkData<'a> { palette.iter().enumerate().for_each(|(i, id)| { palette_map.insert(*id, i); // Palette - data_buf.put_var_int(&VarInt(**id)); + data_buf.put_var_int(&VarInt(id.get_id_mojang_repr())); }); for block_clump in chunk.chunks(64 / block_size as usize) { let mut out_long: i64 = 0; @@ -83,7 +87,7 @@ impl<'a> ClientPacket for CChunkData<'a> { let mut out_long: i64 = 0; let mut shift = 0; for block in block_clump { - out_long |= (*block as i64) << shift; + out_long |= (block.get_id() as i64) << shift; shift += DIRECT_PALETTE_BITS; } block_data_array.push(out_long); diff --git a/pumpkin-world/src/block/block_registry.rs b/pumpkin-world/src/block/block_registry.rs index 0d79d6d03..0abdd37fb 100644 --- a/pumpkin-world/src/block/block_registry.rs +++ b/pumpkin-world/src/block/block_registry.rs @@ -7,6 +7,56 @@ use crate::level::WorldError; 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>, + ) -> Result { + let mut block_states = BLOCKS + .get(block_id) + .ok_or(WorldError::BlockStateIdNotFound)? + .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 + } +} + #[derive(Deserialize, Debug, Clone, PartialEq, Eq)] pub struct BlockDefinition { /// e.g. minecraft:door or minecraft:button @@ -23,7 +73,7 @@ pub struct BlockDefinition { /// This could e.g. be an extended piston facing left. #[derive(Deserialize, Debug, Clone, PartialEq, Eq)] pub struct BlockState { - id: i64, + id: BlockId, /// Whether this is the default state of the Block #[serde(default, rename = "default")] @@ -51,28 +101,3 @@ lazy_static! { pub static ref BLOCKS: HashMap = serde_json::from_str(BLOCKS_JSON).expect("Could not parse block.json registry."); } - -pub fn block_id_and_properties_to_block_state_id( - block_id: &str, - properties: Option<&HashMap>, -) -> Result { - let block = match BLOCKS.get(block_id) { - Some(block) => block, - None => return Err(WorldError::BlockStateIdNotFound), - }; - let block_state_id = match properties { - None => Ok(block - .states - .iter() - .find(|state| state.is_default) - .expect("Each block should have at least one default state") - .id), - Some(properties) => block - .states - .iter() - .find(|state| &state.properties == properties) - .map(|state| state.id) - .ok_or(WorldError::BlockStateIdNotFound), - }; - block_state_id -} diff --git a/pumpkin-world/src/chunk.rs b/pumpkin-world/src/chunk.rs index c8c0a2adc..40f033bcb 100644 --- a/pumpkin-world/src/chunk.rs +++ b/pumpkin-world/src/chunk.rs @@ -4,13 +4,14 @@ use fastnbt::LongArray; use serde::{Deserialize, Serialize}; use crate::{ + block::block_registry::BlockId, coordinates::{ChunkCoordinates, ChunkRelativeBlockCoordinates}, level::WorldError, WORLD_HEIGHT, }; pub struct ChunkData { - pub blocks: Box<[i32; 16 * 16 * WORLD_HEIGHT]>, + pub blocks: Box<[BlockId; 16 * 16 * WORLD_HEIGHT]>, pub position: ChunkCoordinates, pub heightmaps: ChunkHeightmaps, } @@ -61,23 +62,18 @@ impl ChunkData { }; // this needs to be boxed, otherwise it will cause a stack-overflow - let mut blocks = Box::new([0; 16 * 16 * WORLD_HEIGHT]); + let mut blocks = Box::new([BlockId::default(); 16 * 16 * WORLD_HEIGHT]); for (k, section) in chunk_data.sections.into_iter().enumerate() { let block_states = match section.block_states { Some(states) => states, None => continue, // this should instead fill all blocks with the only element of the palette }; + let palette = block_states .palette .iter() - .map(|entry| { - crate::block::block_registry::block_id_and_properties_to_block_state_id( - &entry.name, - entry.properties.as_ref(), - ) - .map(|v| v as i32) - }) + .map(|entry| BlockId::new(&entry.name, entry.properties.as_ref())) .collect::, _>>()?; let block_data = match block_states.data { None => continue, @@ -119,8 +115,8 @@ impl ChunkData { pub fn set_block( &mut self, at: ChunkRelativeBlockCoordinates, - block_id: i32, - ) -> Result { + block_id: BlockId, + ) -> Result { Ok(std::mem::replace( &mut self.blocks [(at.y.get_absolute() * 16 * 16 + *at.z as u16 * 16 + *at.x as u16) as usize], diff --git a/pumpkin/src/client/player_packet.rs b/pumpkin/src/client/player_packet.rs index 5cee5a1ae..3ce92017d 100644 --- a/pumpkin/src/client/player_packet.rs +++ b/pumpkin/src/client/player_packet.rs @@ -25,7 +25,7 @@ use pumpkin_protocol::{ SUseItemOn, Status, }, }; -use pumpkin_world::block::BlockFace; +use pumpkin_world::block::{block_registry::BlockId, BlockFace}; use pumpkin_world::global_registry; use super::PlayerConfig; @@ -400,21 +400,16 @@ impl Player { let minecraft_id = global_registry::find_minecraft_id(global_registry::ITEM_REGISTRY, item.item_id) .expect("All item ids are in the global registry"); - if let Ok(block_state_id) = - pumpkin_world::block::block_registry::block_id_and_properties_to_block_state_id( - minecraft_id, - None, - ) - { + if let Ok(block_state_id) = BlockId::new(minecraft_id, None) { server.broadcast_packet( self, - &CBlockUpdate::new(&location, (block_state_id as i32).into()), + &CBlockUpdate::new(&location, block_state_id.get_id_mojang_repr().into()), ); server.broadcast_packet( self, &CBlockUpdate::new( &WorldPosition(location.0 + face.to_offset()), - (block_state_id as i32).into(), + block_state_id.get_id_mojang_repr().into(), ), ); }