Addressed review issues

common/benches/chonk_benchmark.rs

@@ -16,7 +16,7 @@ fn criterion_benchmark(c: &mut Criterion) {
     // Setup: Create chunk and fill it (dense) for z in [140, 220).
     let mut chunk = TerrainChunk::new(
         MIN_Z,
-        Block::new(BlockKind::Rock, Default::default()),
+        Block::new(BlockKind::Rock, Rgb::zero()),
         Block::empty(),
         TerrainChunkMeta::void(),
     );
@@ -29,7 +29,7 @@ fn criterion_benchmark(c: &mut Criterion) {
         ),
     ) {
         chunk
-            .set(pos, Block::new(BlockKind::Rock, Default::default()))
+            .set(pos, Block::new(BlockKind::Rock, Rgb::zero()))
             .unwrap();
     }
 
@@ -118,7 +118,7 @@ fn criterion_benchmark(c: &mut Criterion) {
                     MAX_Z,
                 ),
             ) {
-                let _ = chunk.set(pos, Block::new(BlockKind::Rock, Default::default()));
+                let _ = chunk.set(pos, Block::new(BlockKind::Rock, Rgb::zero()));
             }
         })
     });

common/src/terrain/block.rs

@@ -45,18 +45,32 @@ make_case_elim!(
         // Covers all other cases (we sometimes have bizarrely coloured misc blocks, and also we
         // often want to experiment with new kinds of block without allocating them a
         // dedicated block kind.
-        Misc = 0xFF,
+        Misc = 0xFE,
     }
 );
 
 impl BlockKind {
+    #[inline]
     pub const fn is_air(&self) -> bool { matches!(self, BlockKind::Air) }
 
+    /// Determine whether the block kind is a gas or a liquid. This does not
+    /// consider any sprites that may occupy the block (the definition of
+    /// fluid is 'a substance that deforms to fit containers')
+    #[inline]
     pub const fn is_fluid(&self) -> bool { *self as u8 & 0xF0 == 0x00 }
 
+    #[inline]
     pub const fn is_liquid(&self) -> bool { self.is_fluid() && !self.is_air() }
 
+    /// Determine whether the block is filled (i.e: fully solid). Right now,
+    /// this is the opposite of being a fluid.
+    #[inline]
     pub const fn is_filled(&self) -> bool { !self.is_fluid() }
+
+    /// Determine whether the block has an RGB color storaged in the attribute
+    /// fields.
+    #[inline]
+    pub const fn has_color(&self) -> bool { self.is_filled() }
 }
 
 impl fmt::Display for BlockKind {
@@ -76,7 +90,6 @@ impl<'a> TryFrom<&'a str> for BlockKind {
 }
 
 #[derive(Copy, Clone, Debug, PartialEq, Serialize, Deserialize)]
-#[repr(packed)]
 pub struct Block {
     kind: BlockKind,
     attr: [u8; 3],
@@ -114,14 +127,23 @@ impl Block {
         }
     }
 
+    pub const fn air(sprite: SpriteKind) -> Self {
+        Self {
+            kind: BlockKind::Air,
+            attr: [sprite as u8, 0, 0],
+        }
+    }
+
+    #[inline]
     pub fn get_color(&self) -> Option<Rgb<u8>> {
-        if self.is_filled() {
+        if self.has_color() {
             Some(self.attr.into())
         } else {
             None
         }
     }
 
+    #[inline]
     pub fn get_sprite(&self) -> Option<SpriteKind> {
         if !self.is_filled() {
             SpriteKind::from_u8(self.attr[0])
@@ -130,6 +152,7 @@ impl Block {
         }
     }
 
+    #[inline]
     pub fn get_ori(&self) -> Option<u8> {
         if self.get_sprite()?.has_ori() {
             // TODO: Formalise this a bit better
@@ -139,6 +162,7 @@ impl Block {
         }
     }
 
+    #[inline]
     pub fn get_glow(&self) -> Option<u8> {
         // TODO: When we have proper volumetric lighting
         // match self.get_sprite()? {
@@ -149,39 +173,46 @@ impl Block {
         None
     }
 
+    #[inline]
     pub fn is_solid(&self) -> bool {
         self.get_sprite()
             .map(|s| s.solid_height().is_some())
             .unwrap_or(true)
     }
 
+    #[inline]
     pub fn is_explodable(&self) -> bool {
         match self.kind() {
             BlockKind::Leaves | BlockKind::Grass | BlockKind::WeakRock => true,
-            // Explodable means that the terrain sprite will get removed anyway, so is good for
+            // Explodable means that the terrain sprite will get removed anyway, so all is good for
-            // empty fluids TODO: Handle the case of terrain sprites we don't want to
+            // empty fluids.
-            // have explode
+            // TODO: Handle the case of terrain sprites we don't want to have explode
             _ => true,
         }
     }
 
+    #[inline]
     pub fn is_collectible(&self) -> bool {
         self.get_sprite()
             .map(|s| s.is_collectible())
             .unwrap_or(false)
     }
 
+    #[inline]
     pub fn is_opaque(&self) -> bool { self.kind().is_filled() }
 
+    #[inline]
     pub fn solid_height(&self) -> f32 {
         self.get_sprite()
             .map(|s| s.solid_height().unwrap_or(0.0))
             .unwrap_or(1.0)
     }
 
+    #[inline]
     pub fn kind(&self) -> BlockKind { self.kind }
 
     /// If this block is a fluid, replace its sprite.
+    #[inline]
     pub fn with_sprite(mut self, sprite: SpriteKind) -> Self {
         if !self.is_filled() {
             self.attr[0] = sprite as u8;
@@ -190,14 +221,18 @@ impl Block {
     }
 
     /// If this block can have orientation, give it a new orientation.
-    pub fn with_ori(mut self, ori: u8) -> Self {
+    #[inline]
+    pub fn with_ori(mut self, ori: u8) -> Option<Self> {
         if self.get_sprite().map(|s| s.has_ori()).unwrap_or(false) {
             self.attr[1] = (self.attr[1] & !0b111) | (ori & 0b111);
+            Some(self)
+        } else {
+            None
         }
-        self
     }
 
     /// Remove the terrain sprite or solid aspects of a block
+    #[inline]
     pub fn into_vacant(self) -> Self {
         if self.is_fluid() {
             Block::new(self.kind(), Rgb::zero())

server/src/events/entity_manipulation.rs

@@ -14,7 +14,7 @@ use common::{
     sync::{Uid, UidAllocator, WorldSyncExt},
     sys::combat::BLOCK_ANGLE,
     terrain::{Block, TerrainGrid},
-    vol::{ReadVol, Vox},
+    vol::ReadVol,
 };
 use comp::item::Reagent;
 use rand::prelude::*;
@@ -572,7 +572,7 @@ pub fn handle_explosion(
             .until(|block| block.is_liquid() || rand::random::<f32>() < 0.05)
             .for_each(|block: &Block, pos| {
                 if block.is_explodable() {
-                    block_change.set(pos, Block::empty());
+                    block_change.set(pos, block.into_vacant());
                 }
             })
             .cast();

world/src/block/mod.rs

@@ -506,23 +506,23 @@ pub fn block_from_structure(
         )),
         // None of these BlockKinds has an orientation, so we just use zero for the other color
         // bits.
-        StructureBlock::Liana => Some(Block::empty().with_sprite(SpriteKind::Liana)),
+        StructureBlock::Liana => Some(Block::air(SpriteKind::Liana)),
         StructureBlock::Fruit => Some(if field.get(pos + structure_pos) % 24 == 0 {
-            Block::empty().with_sprite(SpriteKind::Beehive)
+            Block::air(SpriteKind::Beehive)
         } else if field.get(pos + structure_pos + 1) % 3 == 0 {
-            Block::empty().with_sprite(SpriteKind::Apple)
+            Block::air(SpriteKind::Apple)
         } else {
             Block::empty()
         }),
         StructureBlock::Coconut => Some(if field.get(pos + structure_pos) % 3 > 0 {
             Block::empty()
         } else {
-            Block::empty().with_sprite(SpriteKind::Coconut)
+            Block::air(SpriteKind::Coconut)
         }),
         StructureBlock::Chest => Some(if structure_seed % 10 < 7 {
             Block::empty()
         } else {
-            Block::empty().with_sprite(SpriteKind::Chest)
+            Block::air(SpriteKind::Chest)
         }),
         // We interpolate all these BlockKinds as needed.
         StructureBlock::TemperateLeaves

world/src/site/dungeon/mod.rs

@@ -593,7 +593,7 @@ impl Floor {
 
         let floor_sprite = if RandomField::new(7331).chance(Vec3::from(pos), 0.00005) {
             BlockMask::new(
-                Block::empty().with_sprite(
+                Block::air(
                     match (RandomField::new(1337).get(Vec3::from(pos)) / 2) % 20 {
                         0 => SpriteKind::Apple,
                         1 => SpriteKind::VeloriteFrag,
@@ -609,7 +609,7 @@ impl Floor {
         {
             let room = &self.rooms[*room];
             if RandomField::new(room.seed).chance(Vec3::from(pos), room.loot_density * 0.5) {
-                BlockMask::new(Block::empty().with_sprite(SpriteKind::Chest), 1)
+                BlockMask::new(Block::air(SpriteKind::Chest), 1)
             } else {
                 empty
             }

world/src/site/settlement/building/archetype/house.rs

@@ -296,16 +296,15 @@ impl Archetype for House {
         let empty = BlockMask::nothing();
         let internal = BlockMask::new(Block::empty(), internal_layer);
         let end_window = BlockMask::new(
-            Block::empty().with_sprite(attr.window).with_ori(match ori {
+            Block::air(attr.window)
-                Ori::East => 2,
+                .with_ori(match ori {
-                Ori::North => 0,
+                    Ori::East => 2,
-            }),
+                    Ori::North => 0,
+                })
+                .unwrap(),
             structural_layer,
         );
-        let fire = BlockMask::new(
+        let fire = BlockMask::new(Block::air(SpriteKind::Ember), foundation_layer);
-            Block::empty().with_sprite(SpriteKind::Ember),
-            foundation_layer,
-        );
 
         let storey_height = 6;
         let storey = ((z - 1) / storey_height).min(attr.levels - 1);
@@ -436,16 +435,18 @@ impl Archetype for House {
                                 // Doors on first floor only
                                 if profile.y == foundation_height + 1 {
                                     BlockMask::new(
-                                        Block::empty().with_sprite(SpriteKind::Door).with_ori(
+                                        Block::air(SpriteKind::Door)
-                                            match ori {
+                                            .with_ori(
-                                                Ori::East => 2,
+                                                match ori {
-                                                Ori::North => 0,
+                                                    Ori::East => 2,
-                                            } + if bound_offset.x == (width - 1) / 2 {
+                                                    Ori::North => 0,
-                                                0
+                                                } + if bound_offset.x == (width - 1) / 2 {
-                                            } else {
+                                                    0
-                                                4
+                                                } else {
-                                            },
+                                                    4
-                                        ),
+                                                },
+                                            )
+                                            .unwrap(),
                                         structural_layer,
                                     )
                                 } else {
@@ -558,7 +559,7 @@ impl Archetype for House {
                         };
 
                         return Some(BlockMask::new(
-                            Block::empty().with_sprite(furniture).with_ori(edge_ori),
+                            Block::air(furniture).with_ori(edge_ori).unwrap(),
                             internal_layer,
                         ));
                     } else {
@@ -587,9 +588,7 @@ impl Archetype for House {
                         };
 
                     Some(BlockMask::new(
-                        Block::empty()
+                        Block::air(ornament).with_ori((edge_ori + 4) % 8).unwrap(),
-                            .with_sprite(ornament)
-                            .with_ori((edge_ori + 4) % 8),
                         internal_layer,
                     ))
                 } else {

world/src/site/settlement/building/archetype/keep.rs

@@ -150,12 +150,12 @@ impl Archetype for Keep {
             stone_color.2 + brick_tex,
         );
         let window = BlockMask::new(
-            Block::empty()
+            Block::air(SpriteKind::Window1)
-                .with_sprite(SpriteKind::Window1)
                 .with_ori(match ori {
                     Ori::East => 2,
                     Ori::North => 0,
-                }),
+                })
+                .unwrap(),
             normal_layer,
         );
         let floor = make_block(