Added abstraction for coordinates.

These ensure that there aren't any out of bound values
and make the code significantly more readable

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

@@ -11,9 +11,9 @@ pub struct CChunkData<'a>(pub &'a ChunkData);
 impl<'a> ClientPacket for CChunkData<'a> {
     fn write(&self, buf: &mut crate::bytebuf::ByteBuffer) {
         // Chunk X
-        buf.put_i32(self.0.position.0);
+        buf.put_i32(self.0.position.x);
         // Chunk Z
-        buf.put_i32(self.0.position.1);
+        buf.put_i32(self.0.position.z);
 
         let heightmap_nbt =
             fastnbt::to_bytes_with_opts(&self.0.heightmaps, fastnbt::SerOpts::network_nbt())

pumpkin-world/src/chunk.rs

@@ -2,11 +2,15 @@ use std::collections::HashMap;
 
 use fastnbt::LongArray;
 
-use crate::{level::WorldError, vector3::Vector3, WORLD_HEIGHT, WORLD_Y_START_AT};
+use crate::{
+    coordinates::{ChunkCoordinates, ChunkRelativeBlockCoordinates},
+    level::WorldError,
+    WORLD_HEIGHT,
+};
 
 pub struct ChunkData {
     pub blocks: Box<[i32; 16 * 16 * WORLD_HEIGHT]>,
-    pub position: (i32, i32),
+    pub position: ChunkCoordinates,
     pub heightmaps: ChunkHeightmaps,
 }
 
@@ -49,7 +53,7 @@ struct ChunkNbt {
 }
 
 impl ChunkData {
-    pub fn from_bytes(chunk_data: Vec<u8>, at: (i32, i32)) -> Result<Self, WorldError> {
+    pub fn from_bytes(chunk_data: Vec<u8>, at: ChunkCoordinates) -> Result<Self, WorldError> {
         let chunk_data = match fastnbt::from_bytes::<ChunkNbt>(chunk_data.as_slice()) {
             Ok(v) => v,
             Err(err) => return Err(WorldError::ErrorDeserializingChunk(err.to_string())),
@@ -111,19 +115,14 @@ impl ChunkData {
         })
     }
     /// Sets the given block in the chunk, returning the old block
-    pub fn set_block(&mut self, at: Vector3<i32>, block_id: i32) -> Result<i32, WorldError> {
-        let x = at.x - self.position.0 * 16;
-        let z = at.z - self.position.1 * 16;
-        let y = at.y - WORLD_Y_START_AT;
-        if !(0..16).contains(&x)
-            || !(0..16).contains(&z)
-            || !(0..(WORLD_HEIGHT as i32)).contains(&y)
-        {
-            return Err(WorldError::BlockOutsideChunk);
-        }
-
+    pub fn set_block(
+        &mut self,
+        at: ChunkRelativeBlockCoordinates,
+        block_id: i32,
+    ) -> Result<i32, WorldError> {
         Ok(std::mem::replace(
-            &mut self.blocks[(y * 16 * 16 + z * 16 + x) as usize],
+            &mut self.blocks
+                [(at.y.get_absolute() * 16 * 16 + *at.z as u16 * 16 + *at.x as u16) as usize],
             block_id,
         ))
     }

pumpkin-world/src/coordinates.rs

@@ -0,0 +1,133 @@
+use std::ops::Deref;
+
+use crate::{WORLD_HEIGHT, WORLD_Y_START_AT};
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct Height {
+    height: i16,
+}
+
+impl Height {
+    pub fn new(height: i16) -> Self {
+        assert!(height <= (WORLD_HEIGHT as i16 - WORLD_Y_START_AT.abs() as i16));
+        assert!(height >= WORLD_Y_START_AT as i16);
+
+        Self { height }
+    }
+
+    pub fn from_absolute(height: u16) -> Self {
+        Self::new(height as i16 - WORLD_Y_START_AT.abs() as i16)
+    }
+
+    /// Absolute height ranges from `0..WORLD_HEIGHT`
+    /// instead of `WORLD_Y_START_AT..(WORLD_HEIGHT-WORLD_Y_START_AT)`
+    pub fn get_absolute(&self) -> u16 {
+        (self.height + WORLD_Y_START_AT.abs() as i16) as u16
+    }
+}
+
+impl Deref for Height {
+    type Target = i16;
+
+    fn deref(&self) -> &Self::Target {
+        &self.height
+    }
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct ChunkRelativeScalar {
+    scalar: u8,
+}
+
+macro_rules! derive_chunk_relative_scalar_from_int_impl {
+    ($integer:ty) => {
+        impl From<$integer> for ChunkRelativeScalar {
+            fn from(scalar: $integer) -> Self {
+                assert!(scalar < 16);
+                Self {
+                    scalar: scalar as u8,
+                }
+            }
+        }
+    };
+}
+
+derive_chunk_relative_scalar_from_int_impl! {u8}
+derive_chunk_relative_scalar_from_int_impl! {u16}
+derive_chunk_relative_scalar_from_int_impl! {u32}
+derive_chunk_relative_scalar_from_int_impl! {u64}
+derive_chunk_relative_scalar_from_int_impl! {u128}
+derive_chunk_relative_scalar_from_int_impl! {usize}
+
+impl Deref for ChunkRelativeScalar {
+    type Target = u8;
+
+    fn deref(&self) -> &Self::Target {
+        &self.scalar
+    }
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct BlockCoordinates {
+    pub x: i32,
+    pub y: Height,
+    pub z: i32,
+}
+
+/// BlockCoordinates that do not specify a height.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct XZBlockCoordinates {
+    pub x: i32,
+    pub z: i32,
+}
+
+/// Coordinates of a block relative to a chunk
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct ChunkRelativeBlockCoordinates {
+    pub x: ChunkRelativeScalar,
+    pub y: Height,
+    pub z: ChunkRelativeScalar,
+}
+
+impl ChunkRelativeBlockCoordinates {
+    pub fn with_chunk_coordinates(&self, chunk_coordinates: ChunkCoordinates) -> BlockCoordinates {
+        BlockCoordinates {
+            x: *self.x as i32 + chunk_coordinates.x * 16,
+            y: self.y,
+            z: *self.z as i32 + chunk_coordinates.z * 16,
+        }
+    }
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct ChunkRelativeXZBlockCoordinates {
+    pub x: ChunkRelativeScalar,
+    pub z: ChunkRelativeScalar,
+}
+
+impl ChunkRelativeXZBlockCoordinates {
+    pub fn with_chunk_coordinates(
+        &self,
+        chunk_coordinates: ChunkCoordinates,
+    ) -> XZBlockCoordinates {
+        XZBlockCoordinates {
+            x: *self.x as i32 + chunk_coordinates.x * 16,
+            z: *self.z as i32 + chunk_coordinates.z * 16,
+        }
+    }
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct ChunkCoordinates {
+    pub x: i32,
+    pub z: i32,
+}
+
+macro_rules! impl_get_absolute_height {
+    ($struct_name:ident) => {
+        impl $struct_name {}
+    };
+}
+
+impl_get_absolute_height! {BlockCoordinates}
+impl_get_absolute_height! {ChunkRelativeBlockCoordinates}

pumpkin-world/src/level.rs

@@ -10,7 +10,7 @@ use rayon::prelude::*;
 use thiserror::Error;
 use tokio::sync::mpsc;
 
-use crate::chunk::ChunkData;
+use crate::{chunk::ChunkData, coordinates::ChunkCoordinates};
 
 #[allow(dead_code)]
 /// The Level represents a
@@ -102,15 +102,15 @@ impl Level {
     /// Note: The order of the output chunks will almost never be in the same order as the order of input chunks
     pub async fn read_chunks(
         &self,
-        chunks: Vec<(i32, i32)>,
-        channel: mpsc::Sender<((i32, i32), Result<ChunkData, WorldError>)>,
+        chunks: Vec<ChunkCoordinates>,
+        channel: mpsc::Sender<(ChunkCoordinates, Result<ChunkData, WorldError>)>,
     ) {
         chunks
             .into_par_iter()
             .map(|chunk| {
                 let region = (
-                    ((chunk.0 as f32) / 32.0).floor() as i32,
-                    ((chunk.1 as f32) / 32.0).floor() as i32,
+                    ((chunk.x as f32) / 32.0).floor() as i32,
+                    ((chunk.z as f32) / 32.0).floor() as i32,
                 );
                 let channel = channel.clone();
 
@@ -157,8 +157,8 @@ impl Level {
                 }
 
                 let modulus = |a: i32, b: i32| ((a % b) + b) % b;
-                let chunk_x = modulus(chunk.0, 32) as u32;
-                let chunk_z = modulus(chunk.1, 32) as u32;
+                let chunk_x = modulus(chunk.x, 32) as u32;
+                let chunk_z = modulus(chunk.z, 32) as u32;
                 let channel = channel.clone();
                 let table_entry = (chunk_x + chunk_z * 32) * 4;
 
@@ -170,23 +170,20 @@ impl Level {
                 let size = location_table[table_entry as usize + 3] as usize * 4096;
 
                 if offset == 0 && size == 0 {
-                    let _ =
-                        channel.blocking_send(((chunk.0, chunk.1), Err(WorldError::ChunkNotFound)));
+                    let _ = channel.blocking_send((chunk, Err(WorldError::ChunkNotFound)));
                     return;
                 }
                 // Read the file using the offset and size
                 let mut file_buf = {
                     let seek_result = region_file.seek(std::io::SeekFrom::Start(offset));
                     if seek_result.is_err() {
-                        let _ = channel
-                            .blocking_send(((chunk.0, chunk.1), Err(WorldError::RegionIsInvalid)));
+                        let _ = channel.blocking_send((chunk, Err(WorldError::RegionIsInvalid)));
                         return;
                     }
                     let mut out = vec![0; size];
                     let read_result = region_file.read_exact(&mut out);
                     if read_result.is_err() {
-                        let _ = channel
-                            .blocking_send(((chunk.0, chunk.1), Err(WorldError::RegionIsInvalid)));
+                        let _ = channel.blocking_send((chunk, Err(WorldError::RegionIsInvalid)));
                         return;
                     }
                     out
@@ -199,7 +196,7 @@ impl Level {
                     Some(c) => c,
                     None => {
                         let _ = channel.blocking_send((
-                            (chunk.0, chunk.1),
+                            chunk,
                             Err(WorldError::Compression(
                                 CompressionError::UnknownCompression,
                             )),
@@ -216,7 +213,7 @@ impl Level {
                     Ok(data) => data,
                     Err(e) => {
                         channel
-                            .blocking_send(((chunk.0, chunk.1), Err(WorldError::Compression(e))))
+                            .blocking_send((chunk, Err(WorldError::Compression(e))))
                             .expect("Failed to send Compression error");
                         return;
                     }

pumpkin-world/src/lib.rs

@@ -1,6 +1,7 @@
 use level::Level;
 
 pub mod chunk;
+pub mod coordinates;
 pub mod dimension;
 pub const WORLD_HEIGHT: usize = 384;
 pub const WORLD_Y_START_AT: i32 = -64;

pumpkin-world/src/radial_chunk_iterator.rs

@@ -1,7 +1,9 @@
+use crate::coordinates::ChunkCoordinates;
+
 pub struct RadialIterator {
     radius: u32,
     direction: usize,
-    current: (i32, i32),
+    current: ChunkCoordinates,
     step_size: i32,
     steps_taken: u32,
     steps_in_direction: i32,
@@ -12,7 +14,7 @@ impl RadialIterator {
         RadialIterator {
             radius,
             direction: 0,
-            current: (0, 0),
+            current: ChunkCoordinates { x: 0, z: 0 },
             step_size: 1,
             steps_taken: 0,
             steps_in_direction: 0,
@@ -21,7 +23,7 @@ impl RadialIterator {
 }
 
 impl Iterator for RadialIterator {
-    type Item = (i32, i32);
+    type Item = ChunkCoordinates;
 
     fn next(&mut self) -> Option<Self::Item> {
         if self.steps_taken >= self.radius * self.radius * 4 {
@@ -34,10 +36,10 @@ impl Iterator for RadialIterator {
 
         // Move in the current direction
         match self.direction {
-            0 => self.current.0 += 1, // Right
-            1 => self.current.1 += 1, // Up
-            2 => self.current.0 -= 1, // Left
-            3 => self.current.1 -= 1, // Down
+            0 => self.current.x += 1, // East
+            1 => self.current.z += 1, // North
+            2 => self.current.x -= 1, // West
+            3 => self.current.z -= 1, // South
             _ => {}
         }
 

pumpkin/src/server.rs

@@ -374,7 +374,7 @@ impl Server {
                 Err(_) => continue,
             };
             #[cfg(debug_assertions)]
-            if _chunk_pos == (0, 0) {
+            if _chunk_pos == (pumpkin_world::coordinates::ChunkCoordinates { x: 0, z: 0 }) {
                 use pumpkin_protocol::bytebuf::ByteBuffer;
                 let mut test = ByteBuffer::empty();
                 CChunkData(&chunk_data).write(&mut test);