Clean up dimension/biome code (#239)

crates/valence/src/biome.rs

@@ -54,7 +54,7 @@ pub struct Biome {
 
 impl Biome {
     pub(crate) fn to_biome_registry_item(&self, id: i32) -> Compound {
-        let mut reg = compound! {
+        compound! {
             "name" => self.name.clone(),
             "id" => id,
             "element" => compound! {
@@ -68,7 +68,6 @@ impl Biome {
                 "scale" => 0.05_f32,
                 "downfall" => 0.4_f32,
                 "category" => "none",
-                // "temperature_modifier" =>
                 "effects" => {
                     let mut eff = compound! {
                         "sky_color" => self.sky_color as i32,
@@ -120,24 +119,22 @@ impl Biome {
                         });
                     }
 
+                    if let Some(p) = &self.particle {
+                        eff.insert(
+                            "particle",
+                            compound! {
+                                "probability" => p.probability,
+                                "options" => compound! {
+                                    "type" => p.kind.clone(),
+                                }
+                            },
+                        );
+                    }
+
                     eff
                 },
             }
-        };
-
-        if let Some(p) = &self.particle {
-            reg.insert(
-                "particle",
-                compound! {
-                    "probability" => p.probability,
-                    "options" => compound! {
-                        "type" => p.kind.clone(),
-                    }
-                },
-            );
         }
-
-        reg
     }
 }
 

crates/valence/src/client.rs

@@ -1,3 +1,4 @@
+use std::borrow::Cow;
 use std::net::IpAddr;
 use std::num::Wrapping;
 
@@ -10,12 +11,14 @@ use uuid::Uuid;
 use valence_protocol::packets::s2c::particle::Particle;
 use valence_protocol::packets::s2c::play::{
     AcknowledgeBlockChange, CombatDeath, DisconnectPlay, EntityEvent, GameEvent, KeepAliveS2c,
-    LoginPlayOwned, ParticleS2c, PluginMessageS2c, RemoveEntitiesEncode, ResourcePackS2c,
+    LoginPlay, ParticleS2c, PluginMessageS2c, RemoveEntitiesEncode, ResourcePackS2c, Respawn,
-    RespawnOwned, SetActionBarText, SetCenterChunk, SetDefaultSpawnPosition, SetEntityMetadata,
+    SetActionBarText, SetCenterChunk, SetDefaultSpawnPosition, SetEntityMetadata,
     SetEntityVelocity, SetRenderDistance, SetSubtitleText, SetTitleAnimationTimes, SetTitleText,
     SynchronizePlayerPosition, SystemChatMessage, UnloadChunk,
 };
-use valence_protocol::types::{GameEventKind, GameMode, Property, SyncPlayerPosLookFlags};
+use valence_protocol::types::{
+    GameEventKind, GameMode, GlobalPos, Property, SyncPlayerPosLookFlags,
+};
 use valence_protocol::{
     BlockPos, EncodePacket, Ident, ItemStack, PacketDecoder, PacketEncoder, RawBytes, Text,
     Username, VarInt,
@@ -636,23 +639,30 @@ fn update_one_client(
     if client.is_new {
         client.needs_respawn = false;
 
-        let dimension_names: Vec<_> = server
+        let dimension_names = server
             .dimensions()
-            .map(|(id, _)| id.dimension_name())
+            .map(|(_, dim)| dim.name.as_str_ident())
             .collect();
 
+        let dimension_name = server.dimension(instance.dimension()).name.as_str_ident();
+
+        let last_death_location = client.death_location.map(|(id, pos)| GlobalPos {
+            dimension_name: server.dimension(id).name.as_str_ident(),
+            position: pos,
+        });
+
         // The login packet is prepended so that it is sent before all the other
         // packets. Some packets don't work correctly when sent before the login packet,
         // which is why we're doing this.
-        client.enc.prepend_packet(&LoginPlayOwned {
+        client.enc.prepend_packet(&LoginPlay {
             entity_id: 0, // ID 0 is reserved for clients.
             is_hardcore: client.is_hardcore,
             game_mode: client.game_mode,
             previous_game_mode: -1,
             dimension_names,
-            registry_codec: server.registry_codec().clone(),
+            registry_codec: Cow::Borrowed(server.registry_codec()),
-            dimension_type_name: instance.dimension().dimension_type_name(),
+            dimension_type_name: dimension_name,
-            dimension_name: instance.dimension().dimension_name(),
+            dimension_name,
             hashed_seed: 42,
             max_players: VarInt(0), // Unused
             view_distance: VarInt(client.view_distance() as i32),
@@ -661,9 +671,7 @@ fn update_one_client(
             enable_respawn_screen: client.has_respawn_screen,
             is_debug: false,
             is_flat: client.is_flat,
-            last_death_location: client
+            last_death_location,
-                .death_location
-                .map(|(id, pos)| (id.dimension_name(), pos)),
         })?;
 
         /*
@@ -683,18 +691,23 @@ fn update_one_client(
         if client.needs_respawn {
             client.needs_respawn = false;
 
-            client.enc.append_packet(&RespawnOwned {
+            let dimension_name = server.dimension(instance.dimension()).name.as_str_ident();
-                dimension_type_name: instance.dimension().dimension_type_name(),
+
-                dimension_name: instance.dimension().dimension_name(),
+            let last_death_location = client.death_location.map(|(id, pos)| GlobalPos {
+                dimension_name: server.dimension(id).name.as_str_ident(),
+                position: pos,
+            });
+
+            client.enc.append_packet(&Respawn {
+                dimension_type_name: dimension_name,
+                dimension_name,
                 hashed_seed: 0,
                 game_mode: client.game_mode,
                 previous_game_mode: -1,
                 is_debug: false,
                 is_flat: client.is_flat,
                 copy_metadata: true,
-                last_death_location: client
+                last_death_location,
-                    .death_location
-                    .map(|(id, pos)| (id.dimension_name(), pos)),
             })?;
         }
     }

crates/valence/src/dimension.rs

@@ -1,12 +1,12 @@
 //! Dimension configuration and identification.
 
+use std::collections::HashSet;
+
 use anyhow::ensure;
 use valence_nbt::{compound, Compound};
 use valence_protocol::ident;
 use valence_protocol::ident::Ident;
 
-use crate::LIBRARY_NAMESPACE;
-
 /// Identifies a particular [`Dimension`] on the server.
 ///
 /// The default dimension ID refers to the first dimension added in
@@ -15,20 +15,10 @@ use crate::LIBRARY_NAMESPACE;
 /// To obtain dimension IDs for other dimensions, look at
 /// [`ServerPlugin::dimensions`].
 ///
-/// [`ServerPlugin::dimensions`]: crate::server::SharedServer::dimensions
+/// [`ServerPlugin::dimensions`]: crate::config::ServerPlugin::dimensions
 #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
 pub struct DimensionId(pub(crate) u16);
 
-impl DimensionId {
-    pub(crate) fn dimension_type_name(self) -> Ident<String> {
-        ident!("{LIBRARY_NAMESPACE}:dimension_type_{}", self.0)
-    }
-
-    pub(crate) fn dimension_name(self) -> Ident<String> {
-        ident!("{LIBRARY_NAMESPACE}:dimension_{}", self.0)
-    }
-}
-
 /// The default dimension ID corresponds to the first element in the `Vec`
 /// returned by [`ServerPlugin::dimensions`].
 ///
@@ -53,6 +43,8 @@ impl Default for DimensionId {
 /// [`Instance`]: crate::instance::Instance
 #[derive(Clone, Debug)]
 pub struct Dimension {
+    /// The unique name for this dimension.
+    pub name: Ident<String>,
     /// When false, compasses will spin randomly.
     pub natural: bool,
     /// Must be between 0.0 and 1.0.
@@ -89,36 +81,42 @@ pub struct Dimension {
 }
 
 impl Dimension {
-    pub(crate) fn to_dimension_registry_item(&self) -> Compound {
+    pub(crate) fn to_dimension_registry_item(&self, id: i32) -> Compound {
-        let mut item = compound! {
+        compound! {
-            "piglin_safe" => true,
+            "name" => self.name.clone(),
-            "has_raids" => true,
+            "id" => id,
-            "monster_spawn_light_level" => 0,
+            "element" => {
-            "monster_spawn_block_light_limit" => 0,
+                let mut element = compound! {
-            "natural" => self.natural,
+                    "piglin_safe" => true,
-            "ambient_light" => self.ambient_light,
+                    "has_raids" => true,
-            "infiniburn" => "#minecraft:infiniburn_overworld",
+                    "monster_spawn_light_level" => 0,
-            "respawn_anchor_works" => true,
+                    "monster_spawn_block_light_limit" => 0,
-            "has_skylight" => true,
+                    "natural" => self.natural,
-            "bed_works" => true,
+                    "ambient_light" => self.ambient_light,
-            "effects" => match self.effects {
+                    "infiniburn" => "#minecraft:infiniburn_overworld",
-                DimensionEffects::Overworld => "overworld",
+                    "respawn_anchor_works" => true,
-                DimensionEffects::TheNether => "the_nether",
+                    "has_skylight" => true,
-                DimensionEffects::TheEnd => "the_end",
+                    "bed_works" => true,
+                    "effects" => match self.effects {
+                        DimensionEffects::Overworld => "overworld",
+                        DimensionEffects::TheNether => "the_nether",
+                        DimensionEffects::TheEnd => "the_end",
+                    },
+                    "min_y" => self.min_y,
+                    "height" => self.height,
+                    "logical_height" => self.height,
+                    "coordinate_scale" => 1.0,
+                    "ultrawarm" => false,
+                    "has_ceiling" => false,
+                };
+
+                if let Some(t) = self.fixed_time {
+                    element.insert("fixed_time", t as i64);
+                }
+
+                element
             },
-            "min_y" => self.min_y,
-            "height" => self.height,
-            "logical_height" => self.height,
-            "coordinate_scale" => 1.0,
-            "ultrawarm" => false,
-            "has_ceiling" => false,
-        };
-
-        if let Some(t) = self.fixed_time {
-            item.insert("fixed_time", t as i64);
         }
-
-        item
     }
 }
 
@@ -133,28 +131,37 @@ pub(crate) fn validate_dimensions(dimensions: &[Dimension]) -> anyhow::Result<()
         "more than u16::MAX dimensions present"
     );
 
-    for (i, dim) in dimensions.iter().enumerate() {
+    let mut names = HashSet::new();
+
+    for dim in dimensions {
+        let name = &dim.name;
+
+        ensure!(
+            names.insert(name.clone()),
+            "dimension \"{name}\" already exists",
+        );
+
         ensure!(
             dim.min_y % 16 == 0 && (-2032..=2016).contains(&dim.min_y),
-            "invalid min_y in dimension #{i}",
+            "invalid min_y in dimension {name}",
         );
 
         ensure!(
             dim.height % 16 == 0
                 && (0..=4064).contains(&dim.height)
                 && dim.min_y.saturating_add(dim.height) <= 2032,
-            "invalid height in dimension #{i}",
+            "invalid height in dimension {name}",
         );
 
         ensure!(
             (0.0..=1.0).contains(&dim.ambient_light),
-            "ambient_light is out of range in dimension #{i}",
+            "ambient_light is out of range in dimension {name}",
         );
 
         if let Some(fixed_time) = dim.fixed_time {
             ensure!(
                 (0..=24_000).contains(&fixed_time),
-                "fixed_time is out of range in dimension #{i}",
+                "fixed_time is out of range in dimension {name}",
             );
         }
     }
@@ -165,6 +172,7 @@ pub(crate) fn validate_dimensions(dimensions: &[Dimension]) -> anyhow::Result<()
 impl Default for Dimension {
     fn default() -> Self {
         Self {
+            name: ident!("overworld"),
             natural: true,
             ambient_light: 1.0,
             fixed_time: None,

crates/valence/src/lib.rs

@@ -94,8 +94,6 @@ pub mod prelude {
 #[derive(Copy, Clone, Component)]
 pub struct Despawned;
 
-const LIBRARY_NAMESPACE: &str = "valence";
-
 /// Let's pretend that [`NULL_ENTITY`] was created by spawning an entity,
 /// immediately despawning it, and then stealing its [`Entity`] ID. The user
 /// doesn't need to know about this.

crates/valence/src/server.rs

@@ -1,7 +1,7 @@
 use std::iter::FusedIterator;
 use std::net::{IpAddr, SocketAddr};
 use std::ops::Deref;
-use std::sync::{Arc, Mutex};
+use std::sync::Arc;
 use std::thread;
 use std::time::{Duration, Instant};
 
@@ -106,8 +106,6 @@ struct SharedServerInner {
     /// A semaphore used to limit the number of simultaneous connections to the
     /// server. Closing this semaphore stops new connections.
     connection_sema: Arc<Semaphore>,
-    /// The result that will be returned when the server is shut down.
-    shutdown_result: Mutex<Option<anyhow::Result<()>>>,
     /// The RSA keypair used for encryption with clients.
     rsa_key: RsaPrivateKey,
     /// The public part of `rsa_key` encoded in DER, which is an ASN.1 format.
@@ -166,6 +164,7 @@ impl SharedServer {
     }
 
     /// Obtains a [`Dimension`] by using its corresponding [`DimensionId`].
+    #[track_caller]
     pub fn dimension(&self, id: DimensionId) -> &Dimension {
         self.0
             .dimensions
@@ -184,6 +183,7 @@ impl SharedServer {
     }
 
     /// Obtains a [`Biome`] by using its corresponding [`BiomeId`].
+    #[track_caller]
     pub fn biome(&self, id: BiomeId) -> &Biome {
         self.0.biomes.get(id.0 as usize).expect("invalid biome ID")
     }
@@ -209,19 +209,6 @@ impl SharedServer {
     pub fn start_instant(&self) -> Instant {
         self.0.start_instant
     }
-
-    /// Immediately stops new connections to the server and initiates server
-    /// shutdown.
-    ///
-    /// You may want to disconnect all players with a message prior to calling
-    /// this function.
-    pub fn shutdown<E>(&self, res: Result<(), E>)
-    where
-        E: Into<anyhow::Error>,
-    {
-        self.0.connection_sema.close();
-        *self.0.shutdown_result.lock().unwrap() = Some(res.map_err(|e| e.into()));
-    }
 }
 
 /// Contains information about a new client joining the server.
@@ -296,7 +283,6 @@ pub fn build_plugin(
         new_clients_send,
         new_clients_recv,
         connection_sema: Arc::new(Semaphore::new(plugin.max_connections)),
-        shutdown_result: Mutex::new(None),
         rsa_key,
         public_key_der,
         http_client: Default::default(),
@@ -439,13 +425,7 @@ fn make_registry_codec(dimensions: &[Dimension], biomes: &[Biome]) -> Compound {
     let dimensions = dimensions
         .iter()
         .enumerate()
-        .map(|(id, dim)| {
+        .map(|(id, dim)| dim.to_dimension_registry_item(id as i32))
-            compound! {
-                "name" => DimensionId(id as u16).dimension_type_name(),
-                "id" => id as i32,
-                "element" => dim.to_dimension_registry_item(),
-            }
-        })
         .collect();
 
     let biomes = biomes
@@ -461,9 +441,7 @@ fn make_registry_codec(dimensions: &[Dimension], biomes: &[Biome]) -> Compound {
         },
         ident!("worldgen/biome") => compound! {
             "type" => ident!("worldgen/biome"),
-            "value" => {
+            "value" => List::Compound(biomes),
-                List::Compound(biomes)
-            }
         },
         ident!("chat_type") => compound! {
             "type" => ident!("chat_type"),

crates/valence/src/server/connect.rs

@@ -44,7 +44,7 @@ pub async fn do_accept_loop(shared: SharedServer, callbacks: Arc<impl AsyncCallb
     let listener = match TcpListener::bind(shared.0.address).await {
         Ok(listener) => listener,
         Err(e) => {
-            shared.shutdown(Err(e).context("failed to start TCP listener"));
+            error!("failed to start TCP listener: {e}");
             return;
         }
     };

crates/valence_protocol/src/packets/s2c.rs

@@ -463,7 +463,7 @@ pub mod play {
         /// Same values as `game_mode` but with -1 to indicate no previous.
         pub previous_game_mode: i8,
         pub dimension_names: Vec<Ident<&'a str>>,
-        pub registry_codec: Compound,
+        pub registry_codec: Cow<'a, Compound>,
         pub dimension_type_name: Ident<&'a str>,
         pub dimension_name: Ident<&'a str>,
         pub hashed_seed: i64,
@@ -477,29 +477,6 @@ pub mod play {
         pub last_death_location: Option<GlobalPos<'a>>,
     }
 
-    // TODO: remove this.
-    #[derive(Clone, Debug, Encode, EncodePacket, Decode, DecodePacket)]
-    #[packet_id = 0x24]
-    pub struct LoginPlayOwned {
-        pub entity_id: i32,
-        pub is_hardcore: bool,
-        pub game_mode: GameMode,
-        pub previous_game_mode: i8,
-        pub dimension_names: Vec<Ident<String>>,
-        pub registry_codec: Compound,
-        pub dimension_type_name: Ident<String>,
-        pub dimension_name: Ident<String>,
-        pub hashed_seed: i64,
-        pub max_players: VarInt,
-        pub view_distance: VarInt,
-        pub simulation_distance: VarInt,
-        pub reduced_debug_info: bool,
-        pub enable_respawn_screen: bool,
-        pub is_debug: bool,
-        pub is_flat: bool,
-        pub last_death_location: Option<(Ident<String>, BlockPos)>,
-    }
-
     #[derive(Clone, Debug, Encode, EncodePacket, Decode, DecodePacket)]
     #[packet_id = 0x26]
     pub struct MerchantOffers {
@@ -676,21 +653,6 @@ pub mod play {
         pub last_death_location: Option<GlobalPos<'a>>,
     }
 
-    // TODO: remove
-    #[derive(Clone, PartialEq, Debug, Encode, EncodePacket, Decode, DecodePacket)]
-    #[packet_id = 0x3d]
-    pub struct RespawnOwned {
-        pub dimension_type_name: Ident<String>,
-        pub dimension_name: Ident<String>,
-        pub hashed_seed: u64,
-        pub game_mode: GameMode,
-        pub previous_game_mode: i8,
-        pub is_debug: bool,
-        pub is_flat: bool,
-        pub copy_metadata: bool,
-        pub last_death_location: Option<(Ident<String>, BlockPos)>,
-    }
-
     #[derive(Copy, Clone, Debug, Encode, EncodePacket, Decode, DecodePacket)]
     #[packet_id = 0x3e]
     pub struct SetHeadRotation {