Correctly encode biomes (#112)

examples/biomes.rs

@@ -1,3 +1,4 @@
+use std::iter;
 use std::net::SocketAddr;
 use std::sync::atomic::{AtomicUsize, Ordering};
 
@@ -43,7 +44,9 @@ struct ClientState {
 
 const MAX_PLAYERS: usize = 10;
 
-const BIOME_COUNT: usize = 4 * 4;
+const BIOME_COUNT: usize = 10;
+
+const MIN_Y: i32 = -64;
 
 #[async_trait]
 impl Config for Game {
@@ -67,7 +70,7 @@ impl Config for Game {
     }
 
     fn biomes(&self) -> Vec<Biome> {
-        (0..BIOME_COUNT)
+        (1..BIOME_COUNT)
             .map(|i| {
                 let color = (0xffffff / BIOME_COUNT * i) as u32;
                 Biome {
@@ -81,6 +84,10 @@ impl Config for Game {
                     ..Default::default()
                 }
             })
+            .chain(iter::once(Biome {
+                name: ident!("plains"),
+                ..Default::default()
+            }))
             .collect()
     }
 
@@ -102,7 +109,9 @@ impl Config for Game {
     fn init(&self, server: &mut Server<Self>) {
         let world = server.worlds.insert(DimensionId::default(), ()).1;
         server.state.player_list = Some(server.player_lists.insert(()).0);
-        let height = server.shared.dimensions().next().unwrap().1.height as usize;
+
+        let height = world.chunks.height();
+        assert_eq!(world.chunks.min_y(), MIN_Y);
 
         for chunk_z in 0..3 {
             for chunk_x in 0..3 {
@@ -112,16 +121,21 @@ impl Config for Game {
                     // Set chunk blocks
                     for z in 0..16 {
                         for x in 0..16 {
-                            chunk.set_block_state(x, 50, z, BlockState::GRASS_BLOCK);
+                            chunk.set_block_state(x, 1, z, BlockState::GRASS_BLOCK);
                         }
                     }
 
                     // Set chunk biomes
                     for z in 0..4 {
                         for x in 0..4 {
-                            let biome_id = server.shared.biomes().nth(x + z * 4).unwrap().0;
-
                             for y in 0..height / 4 {
+                                let biome_id = server
+                                    .shared
+                                    .biomes()
+                                    .nth((x + z * 4 + y * 4 * 4) % BIOME_COUNT)
+                                    .unwrap()
+                                    .0;
+
                                 chunk.set_biome(x, y, z, biome_id);
                             }
                         }
@@ -194,7 +208,7 @@ impl Config for Game {
                 return false;
             }
 
-            if client.position().y <= -20.0 {
+            if client.position().y < MIN_Y as _ {
                 client.teleport(spawn_pos, client.yaw(), client.pitch());
             }
 

examples/building.rs

@@ -3,7 +3,7 @@ use std::sync::atomic::{AtomicUsize, Ordering};
 
 use log::LevelFilter;
 use num::Integer;
-use valence::biome::Biome;
+use valence::async_trait;
 use valence::block::BlockState;
 use valence::chunk::{Chunk, UnloadedChunk};
 use valence::client::{handle_event_default, ClientEvent, DiggingStatus, GameMode, Hand};
@@ -13,7 +13,6 @@ use valence::entity::{EntityId, EntityKind};
 use valence::player_list::PlayerListId;
 use valence::server::{Server, SharedServer, ShutdownResult};
 use valence::text::{Color, TextFormat};
-use valence::{async_trait, ident};
 
 pub fn main() -> ShutdownResult {
     env_logger::Builder::new()
@@ -68,14 +67,6 @@ impl Config for Game {
         }]
     }
 
-    fn biomes(&self) -> Vec<Biome> {
-        vec![Biome {
-            name: ident!("valence:default_biome"),
-            grass_color: Some(0x00ff00),
-            ..Biome::default()
-        }]
-    }
-
     async fn server_list_ping(
         &self,
         _server: &SharedServer<Self>,

examples/conway.rs

@@ -84,7 +84,7 @@ impl Config for Game {
 
     fn biomes(&self) -> Vec<Biome> {
         vec![Biome {
-            name: ident!("valence:default_biome"),
+            name: ident!("plains"),
             grass_color: Some(0x00ff00),
             ..Biome::default()
         }]

src/chunk.rs

@@ -12,7 +12,7 @@ use std::io::Write;
 use std::iter::FusedIterator;
 
 use bitvec::vec::BitVec;
-use paletted_container::{PalettedContainer, PalettedContainerElement};
+use paletted_container::PalettedContainer;
 use rayon::iter::{IntoParallelRefIterator, IntoParallelRefMutIterator, ParallelIterator};
 use valence_nbt::compound;
 
@@ -505,26 +505,6 @@ impl Default for ChunkSection {
 const SECTION_BLOCK_COUNT: usize = 4096;
 const USIZE_BITS: usize = usize::BITS as _;
 
-impl PalettedContainerElement for BlockState {
-    const DIRECT_BITS: usize = log2_ceil(BlockState::max_raw() as _);
-    const MAX_INDIRECT_BITS: usize = 8;
-    const MIN_INDIRECT_BITS: usize = 4;
-
-    fn to_bits(self) -> u64 {
-        self.to_raw() as _
-    }
-}
-
-impl PalettedContainerElement for BiomeId {
-    const DIRECT_BITS: usize = 6;
-    const MAX_INDIRECT_BITS: usize = 4;
-    const MIN_INDIRECT_BITS: usize = 0;
-
-    fn to_bits(self) -> u64 {
-        self.0 as _
-    }
-}
-
 impl ChunkSection {
     fn mark_block_as_modified(&mut self, idx: usize) {
         if !self.is_block_modified(idx) {
@@ -560,13 +540,35 @@ impl<C: Config> LoadedChunk<C> {
     }
 
     /// Gets the chunk data packet for this chunk with the given position.
-    pub(crate) fn chunk_data_packet(&self, pos: ChunkPos) -> ChunkDataAndUpdateLight {
+    pub(crate) fn chunk_data_packet(
+        &self,
+        pos: ChunkPos,
+        biome_registry_len: usize,
+    ) -> ChunkDataAndUpdateLight {
         let mut blocks_and_biomes = Vec::new();
 
         for sect in self.sections.iter() {
             sect.non_air_count.encode(&mut blocks_and_biomes).unwrap();
-            sect.block_states.encode(&mut blocks_and_biomes).unwrap();
+
-            sect.biomes.encode(&mut blocks_and_biomes).unwrap();
+            sect.block_states
+                .encode_mc_format(
+                    &mut blocks_and_biomes,
+                    |b| b.to_raw().into(),
+                    4,
+                    8,
+                    log2_ceil(BlockState::max_raw().into()),
+                )
+                .unwrap();
+
+            sect.biomes
+                .encode_mc_format(
+                    &mut blocks_and_biomes,
+                    |b| b.0.into(),
+                    0,
+                    3,
+                    log2_ceil(biome_registry_len),
+                )
+                .unwrap();
         }
 
         ChunkDataAndUpdateLight {

src/chunk/paletted_container.rs

@@ -9,39 +9,21 @@ use crate::util::log2_ceil;
 
 /// `HALF_LEN` must be equal to `ceil(LEN / 2)`.
 #[derive(Clone, Debug)]
-pub enum PalettedContainer<T: PalettedContainerElement, const LEN: usize, const HALF_LEN: usize> {
+pub enum PalettedContainer<T, const LEN: usize, const HALF_LEN: usize> {
     Single(T),
     Indirect(Box<Indirect<T, LEN, HALF_LEN>>),
     Direct(Box<[T; LEN]>),
 }
 
-pub trait PalettedContainerElement: Copy + Eq + Default {
-    /// The minimum number of bits required to represent all instances of the
-    /// element type. If `N` is the total number of possible values, then
-    /// `DIRECT_BITS` is `ceil(log2(N))`.
-    const DIRECT_BITS: usize;
-    /// The maximum number of bits per element allowed in the indirect
-    /// representation while encoding. Any higher than this will force
-    /// conversion to the direct representation while encoding.
-    const MAX_INDIRECT_BITS: usize;
-    /// The minimum number of bits used to represent the element type in the
-    /// indirect representation while encoding. If the bits per index is lower,
-    /// it will be rounded up to this.
-    const MIN_INDIRECT_BITS: usize;
-    /// Converts the element type to bits. The output must be less than two to
-    /// the power of `DIRECT_BITS`.
-    fn to_bits(self) -> u64;
-}
-
 #[derive(Clone, Debug)]
-pub struct Indirect<T: PalettedContainerElement, const LEN: usize, const HALF_LEN: usize> {
+pub struct Indirect<T, const LEN: usize, const HALF_LEN: usize> {
     /// Each element is a unique instance of `T`.
     palette: ArrayVec<T, 16>,
     /// Each half-byte is an index into `palette`.
     indices: [u8; HALF_LEN],
 }
 
-impl<T: PalettedContainerElement, const LEN: usize, const HALF_LEN: usize>
+impl<T: Copy + Eq + Default, const LEN: usize, const HALF_LEN: usize>
     PalettedContainer<T, LEN, HALF_LEN>
 {
     pub fn new() -> Self {
@@ -150,9 +132,108 @@ impl<T: PalettedContainerElement, const LEN: usize, const HALF_LEN: usize>
             "index {idx} is out of bounds in paletted container of length {LEN}"
         );
     }
+
+    /// Encodes the paletted container in the format that Minecraft expects.
+    ///
+    /// - **`writer`**: The [`Write`] instance to write the paletted container
+    ///   to.
+    /// - **`to_bits`**: A function to convert the element type to bits. The
+    ///   output must be less than two to the power of `direct_bits`.
+    /// - **`min_indirect_bits`**: The minimum number of bits used to represent
+    ///   the element type in the indirect representation. If the bits per index
+    ///   is lower, it will be rounded up to this.
+    /// - **`max_indirect_bits`**: The maximum number of bits per element
+    ///   allowed in the indirect representation. Any higher than this will
+    ///   force conversion to the direct representation while encoding.
+    /// - **`direct_bits`**: The minimum number of bits required to represent
+    ///   all instances of the element type. If `N` is the total number of
+    ///   possible values, then `DIRECT_BITS` is `ceil(log2(N))`.
+    pub fn encode_mc_format<W, F>(
+        &self,
+        mut writer: W,
+        mut to_bits: F,
+        min_indirect_bits: usize,
+        max_indirect_bits: usize,
+        direct_bits: usize,
+    ) -> anyhow::Result<()>
+    where
+        W: Write,
+        F: FnMut(T) -> u64,
+    {
+        debug_assert!(min_indirect_bits <= 4);
+        debug_assert!(min_indirect_bits <= max_indirect_bits);
+        debug_assert!(max_indirect_bits <= 64);
+        debug_assert!(direct_bits <= 64);
+
+        match self {
+            Self::Single(val) => {
+                // Bits per entry
+                0_u8.encode(&mut writer)?;
+
+                // Palette
+                VarInt(to_bits(*val) as i32).encode(&mut writer)?;
+
+                // Number of longs
+                VarInt(0).encode(&mut writer)?;
+            }
+            Self::Indirect(ind) => {
+                let bits_per_entry = min_indirect_bits.max(log2_ceil(ind.palette.len()));
+
+                // Encode as direct if necessary.
+                if bits_per_entry > max_indirect_bits {
+                    // Bits per entry
+                    (direct_bits as u8).encode(&mut writer)?;
+
+                    // Number of longs in data array.
+                    VarInt(compact_u64s_len(LEN, direct_bits) as _).encode(&mut writer)?;
+                    // Data array
+                    encode_compact_u64s(
+                        &mut writer,
+                        (0..LEN).map(|i| to_bits(ind.get(i))),
+                        direct_bits,
+                    )?;
+                } else {
+                    // Bits per entry
+                    (bits_per_entry as u8).encode(&mut writer)?;
+
+                    // Palette len
+                    VarInt(ind.palette.len() as i32).encode(&mut writer)?;
+                    // Palette
+                    for val in &ind.palette {
+                        VarInt(to_bits(*val) as i32).encode(&mut writer)?;
                     }
 
-impl<T: PalettedContainerElement, const LEN: usize, const HALF_LEN: usize> Default
+                    // Number of longs in data array.
+                    VarInt(compact_u64s_len(LEN, bits_per_entry) as _).encode(&mut writer)?;
+                    // Data array
+                    encode_compact_u64s(
+                        &mut writer,
+                        ind.indices
+                            .iter()
+                            .cloned()
+                            .flat_map(|byte| [byte & 0b1111, byte >> 4])
+                            .map(u64::from)
+                            .take(LEN),
+                        bits_per_entry,
+                    )?;
+                }
+            }
+            Self::Direct(dir) => {
+                // Bits per entry
+                (direct_bits as u8).encode(&mut writer)?;
+
+                // Number of longs in data array.
+                VarInt(compact_u64s_len(LEN, direct_bits) as _).encode(&mut writer)?;
+                // Data array
+                encode_compact_u64s(&mut writer, dir.iter().cloned().map(to_bits), direct_bits)?;
+            }
+        }
+
+        Ok(())
+    }
+}
+
+impl<T: Copy + Eq + Default, const LEN: usize, const HALF_LEN: usize> Default
     for PalettedContainer<T, LEN, HALF_LEN>
 {
     fn default() -> Self {
@@ -160,9 +241,7 @@ impl<T: PalettedContainerElement, const LEN: usize, const HALF_LEN: usize> Defau
     }
 }
 
-impl<T: PalettedContainerElement, const LEN: usize, const HALF_LEN: usize>
+impl<T: Copy + Eq + Default, const LEN: usize, const HALF_LEN: usize> Indirect<T, LEN, HALF_LEN> {
-    Indirect<T, LEN, HALF_LEN>
-{
     pub fn get(&self, idx: usize) -> T {
         let palette_idx = self.indices[idx / 2] >> (idx % 2 * 4) & 0b1111;
         self.palette[palette_idx as usize]
@@ -184,79 +263,13 @@ impl<T: PalettedContainerElement, const LEN: usize, const HALF_LEN: usize>
     }
 }
 
-/// Encodes the paletted container in the format that Minecraft expects.
-impl<T: PalettedContainerElement, const LEN: usize, const HALF_LEN: usize> Encode
-    for PalettedContainer<T, LEN, HALF_LEN>
-{
-    fn encode(&self, w: &mut impl Write) -> anyhow::Result<()> {
-        assert!(T::DIRECT_BITS <= 64);
-        assert!(T::MAX_INDIRECT_BITS <= 64);
-        assert!(T::MIN_INDIRECT_BITS <= T::MAX_INDIRECT_BITS);
-        assert!(T::MIN_INDIRECT_BITS <= 4);
-
-        match self {
-            Self::Single(val) => {
-                // Bits per entry
-                0_u8.encode(w)?;
-
-                // Palette
-                VarInt(val.to_bits() as i32).encode(w)?;
-
-                // Number of longs
-                VarInt(0).encode(w)?;
-            }
-            Self::Indirect(ind) => {
-                let bits_per_entry = T::MIN_INDIRECT_BITS.max(log2_ceil(ind.palette.len()));
-
-                // TODO: if bits_per_entry > MAX_INDIRECT_BITS, encode as direct.
-                debug_assert!(bits_per_entry <= T::MAX_INDIRECT_BITS);
-
-                // Bits per entry
-                (bits_per_entry as u8).encode(w)?;
-
-                // Palette len
-                VarInt(ind.palette.len() as i32).encode(w)?;
-                // Palette
-                for val in &ind.palette {
-                    VarInt(val.to_bits() as i32).encode(w)?;
-                }
-
-                // Number of longs in data array.
-                VarInt(compact_u64s_len(LEN, bits_per_entry) as _).encode(w)?;
-                // Data array
-                encode_compact_u64s(
-                    w,
-                    ind.indices
-                        .iter()
-                        .cloned()
-                        .flat_map(|byte| [byte & 0b1111, byte >> 4])
-                        .map(u64::from)
-                        .take(LEN),
-                    bits_per_entry,
-                )?;
-            }
-            Self::Direct(dir) => {
-                // Bits per entry
-                (T::DIRECT_BITS as u8).encode(w)?;
-
-                // Number of longs in data array.
-                VarInt(compact_u64s_len(LEN, T::DIRECT_BITS) as _).encode(w)?;
-                // Data array
-                encode_compact_u64s(w, dir.iter().map(|v| v.to_bits()), T::DIRECT_BITS)?;
-            }
-        }
-
-        Ok(())
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use rand::Rng;
 
     use super::*;
 
-    fn check<T: PalettedContainerElement, const LEN: usize, const HALF_LEN: usize>(
+    fn check<T: Copy + Eq + Default, const LEN: usize, const HALF_LEN: usize>(
         p: &PalettedContainer<T, LEN, HALF_LEN>,
         s: &[T],
     ) -> bool {
@@ -264,16 +277,6 @@ mod tests {
         (0..LEN).all(|i| p.get(i) == s[i])
     }
 
-    impl PalettedContainerElement for u32 {
-        const DIRECT_BITS: usize = 0;
-        const MAX_INDIRECT_BITS: usize = 0;
-        const MIN_INDIRECT_BITS: usize = 0;
-
-        fn to_bits(self) -> u64 {
-            self.into()
-        }
-    }
-
     #[test]
     fn random_assignments() {
         const LEN: usize = 100;

src/client.rs

@@ -1138,7 +1138,7 @@ impl<C: Config> Client<C> {
                 registry_codec: shared.registry_codec().clone(),
                 dimension_type_name: world.meta.dimension().dimension_type_name(),
                 dimension_name: world.meta.dimension().dimension_name(),
-                hashed_seed: 0,
+                hashed_seed: 10,
                 max_players: VarInt(0),
                 view_distance: BoundedInt(VarInt(self.view_distance() as i32)),
                 simulation_distance: VarInt(16),
@@ -1332,7 +1332,7 @@ impl<C: Config> Client<C> {
         for pos in chunks_in_view_distance(center, self.view_distance) {
             if let Some(chunk) = world.chunks.get(pos) {
                 if self.loaded_chunks.insert(pos) {
-                    self.send_packet(chunk.chunk_data_packet(pos));
+                    self.send_packet(chunk.chunk_data_packet(pos, shared.biomes().len()));
                 }
             }
         }

src/config.rs

@@ -176,6 +176,10 @@ pub trait Config: Sized + Send + Sync + 'static {
     /// Additionally, the documented requirements on the fields of [`Biome`]
     /// must be met.
     ///
+    /// **NOTE**: As of 1.19.2, there is a bug in the client which prevents
+    /// joining the game when a biome named "minecraft:plains" is not present.
+    /// Ensure there is a biome named "plains".
+    ///
     /// # Default Implementation
     ///
     /// Returns `vec![Biome::default()]`.

src/server.rs

@@ -393,24 +393,13 @@ fn make_registry_codec(dimensions: &[Dimension], biomes: &[Biome]) -> Compound {
         ident!("worldgen/biome") => compound! {
             "type" => ident!("worldgen/biome"),
             "value" => {
-                let mut biomes: Vec<_> = biomes
+                List::Compound(biomes
                     .iter()
                     .enumerate()
                     .map(|(id, biome)| biome.to_biome_registry_item(id as i32))
-                    .collect();
+                    .collect())
-
-                // The client needs a biome named "minecraft:plains" in the registry to
-                // connect. This is probably a bug in the client.
-                //
-                // If the issue is resolved, remove this if.
-                if !biomes.iter().any(|b| b["name"] == "plains".into()) {
-                    let biome = Biome::default();
-                    assert_eq!(biome.name, ident!("plains"));
-                    biomes.push(biome.to_biome_registry_item(biomes.len() as i32));
             }
 
-                List::Compound(biomes)
-            }
         },
         ident!("chat_type_registry") => compound! {
             "type" => ident!("chat_type"),