diff --git a/pumpkin/src/client/client_packet.rs b/pumpkin/src/client/client_packet.rs index ac21d99f..a6e41944 100644 --- a/pumpkin/src/client/client_packet.rs +++ b/pumpkin/src/client/client_packet.rs @@ -388,6 +388,12 @@ impl Client { client_information: SClientInformationConfig, ) { log::debug!("Handling client settings"); + if client_information.view_distance <= 0 { + self.kick("Cannot have zero or negative view distance!") + .await; + return; + } + if let (Some(main_hand), Some(chat_mode)) = ( Hand::from_i32(client_information.main_hand.into()), ChatMode::from_i32(client_information.chat_mode.into()), diff --git a/pumpkin/src/client/player_packet.rs b/pumpkin/src/client/player_packet.rs index cc54520f..b5dfd447 100644 --- a/pumpkin/src/client/player_packet.rs +++ b/pumpkin/src/client/player_packet.rs @@ -435,28 +435,68 @@ impl Player { ) */ } - pub async fn handle_client_information(&self, client_information: SClientInformationPlay) { + pub async fn handle_client_information( + self: &Arc, + client_information: SClientInformationPlay, + ) { if let (Some(main_hand), Some(chat_mode)) = ( Hand::from_i32(client_information.main_hand.into()), ChatMode::from_i32(client_information.chat_mode.into()), ) { - let mut config = self.config.lock().await; - let update = - config.main_hand != main_hand || config.skin_parts != client_information.skin_parts; - - *config = PlayerConfig { - locale: client_information.locale, - // A Negative view distance would be impossible and make no sense right ?, Mojang: Lets make is signed :D - view_distance: client_information.view_distance as u8, - chat_mode, - chat_colors: client_information.chat_colors, - skin_parts: client_information.skin_parts, - main_hand, - text_filtering: client_information.text_filtering, - server_listing: client_information.server_listing, + if client_information.view_distance <= 0 { + self.kick(TextComponent::text( + "Cannot have zero or negative view distance!", + )) + .await; + return; + } + + let (update_skin, update_watched) = { + let mut config = self.config.lock().await; + let update_skin = config.main_hand != main_hand + || config.skin_parts != client_information.skin_parts; + + let old_view_distance = config.view_distance; + + let update_watched = if old_view_distance == client_information.view_distance as u8 + { + false + } else { + log::debug!( + "Player {} ({}) updated render distance: {} -> {}.", + self.gameprofile.name, + self.client.id, + old_view_distance, + client_information.view_distance + ); + + true + }; + + *config = PlayerConfig { + locale: client_information.locale, + // A Negative view distance would be impossible and make no sense right ?, Mojang: Lets make is signed :D + view_distance: client_information.view_distance as u8, + chat_mode, + chat_colors: client_information.chat_colors, + skin_parts: client_information.skin_parts, + main_hand, + text_filtering: client_information.text_filtering, + server_listing: client_information.server_listing, + }; + (update_skin, update_watched) }; - drop(config); - if update { + + if update_watched { + player_chunker::update_position(self).await; + } + + if update_skin { + log::debug!( + "Player {} ({}) updated their skin.", + self.gameprofile.name, + self.client.id, + ); self.update_client_information().await; } } else { diff --git a/pumpkin/src/entity/player.rs b/pumpkin/src/entity/player.rs index 5e14a0c1..cc8af323 100644 --- a/pumpkin/src/entity/player.rs +++ b/pumpkin/src/entity/player.rs @@ -130,7 +130,7 @@ impl Player { ) -> Self { let gameprofile = client.gameprofile.lock().await.clone().map_or_else( || { - log::error!("No gameprofile?. Impossible"); + log::error!("Client {} has no game profile!", client.id); GameProfile { id: uuid::Uuid::new_v4(), name: String::new(), @@ -141,7 +141,6 @@ impl Player { |profile| profile, ); let config = client.config.lock().await.clone().unwrap_or_default(); - let view_distance = config.view_distance; let bounding_box_size = BoundingBoxSize { width: 0.6, height: 1.8, @@ -172,7 +171,13 @@ impl Player { teleport_id_count: AtomicI32::new(0), abilities: Mutex::new(Abilities::default()), gamemode: AtomicCell::new(gamemode), - watched_section: AtomicCell::new(Cylindrical::new(Vector2::new(0, 0), view_distance)), + // We want this to be an impossible watched section so that `player_chunker::update_position` + // will mark chunks as watched for a new join rather than a respawn + // (We left shift by one so we can search around that chunk) + watched_section: AtomicCell::new(Cylindrical::new( + Vector2::new(i32::MAX >> 1, i32::MAX >> 1), + 0, + )), wait_for_keep_alive: AtomicBool::new(false), keep_alive_id: AtomicI64::new(0), last_keep_alive_time: AtomicCell::new(std::time::Instant::now()), diff --git a/pumpkin/src/world/mod.rs b/pumpkin/src/world/mod.rs index 35d7383e..54c5d105 100644 --- a/pumpkin/src/world/mod.rs +++ b/pumpkin/src/world/mod.rs @@ -410,7 +410,7 @@ impl World { player.send_time(self).await; // Spawn in initial chunks - player_chunker::player_join(self, player.clone()).await; + player_chunker::player_join(&player).await; // if let Some(bossbars) = self..lock().await.get_player_bars(&player.gameprofile.id) { // for bossbar in bossbars { @@ -512,7 +512,7 @@ impl World { ) .await; - player_chunker::player_join(self, player.clone()).await; + player_chunker::player_join(player).await; self.broadcast_packet_all(&entity_metadata_packet).await; // update commands @@ -565,7 +565,6 @@ impl World { rel_x * rel_x + rel_z * rel_z }); - player.world().mark_chunks_as_watched(&chunks); let mut receiver = self.receive_chunks(chunks); let level = self.level.clone(); diff --git a/pumpkin/src/world/player_chunker.rs b/pumpkin/src/world/player_chunker.rs index 73f5c324..a7c6f8a5 100644 --- a/pumpkin/src/world/player_chunker.rs +++ b/pumpkin/src/world/player_chunker.rs @@ -10,8 +10,6 @@ use pumpkin_world::cylindrical_chunk_iterator::Cylindrical; use crate::entity::player::Player; -use super::World; - pub async fn get_view_distance(player: &Player) -> u8 { player .config @@ -21,18 +19,9 @@ pub async fn get_view_distance(player: &Player) -> u8 { .clamp(2, BASIC_CONFIG.view_distance) } -pub async fn player_join(world: &World, player: Arc) { - let new_watched = chunk_section_from_pos(&player.living_entity.entity.block_pos.load()); - - let mut cylindrical = player.watched_section.load(); - cylindrical.center = new_watched.into(); - player.watched_section.store(cylindrical); - +pub async fn player_join(player: &Arc) { let chunk_pos = player.living_entity.entity.chunk_pos.load(); - assert_eq!(new_watched.x, chunk_pos.x); - assert_eq!(new_watched.z, chunk_pos.z); - log::debug!("Sending center chunk to {}", player.gameprofile.name); player .client @@ -41,20 +30,15 @@ pub async fn player_join(world: &World, player: Arc) { chunk_z: chunk_pos.z.into(), }) .await; - let view_distance = get_view_distance(&player).await; + let view_distance = get_view_distance(player).await; log::debug!( "Player {} ({}) joined with view distance: {}", player.gameprofile.name, - player.gameprofile.name, + player.client.id, view_distance ); - let new_cylindrical = Cylindrical::new(chunk_pos, view_distance); - let loading_chunks = new_cylindrical.all_chunks_within(); - - if !loading_chunks.is_empty() { - world.spawn_world_chunks(player, loading_chunks, chunk_pos); - } + update_position(player).await; } pub async fn update_position(player: &Arc) { @@ -74,8 +58,6 @@ pub async fn update_position(player: &Arc) { let new_cylindrical = Cylindrical::new(new_chunk_center, view_distance); if old_cylindrical != new_cylindrical { - player.watched_section.store(new_cylindrical); - player .client .send_packet(&CCenterChunk { @@ -97,14 +79,15 @@ pub async fn update_position(player: &Arc) { }, ); - if !unloading_chunks.is_empty() { - //let inst = std::time::Instant::now(); + // Make sure the watched section and the chunk watcher updates are async atomic. We want to + // ensure what we unload when the player disconnects is correct + entity.world.mark_chunks_as_watched(&loading_chunks); + let chunks_to_clean = entity.world.mark_chunks_as_not_watched(&unloading_chunks); + player.watched_section.store(new_cylindrical); - //log::debug!("Unloading chunks took {:?} (1)", inst.elapsed()); - let chunks_to_clean = entity.world.mark_chunks_as_not_watched(&unloading_chunks); + if !chunks_to_clean.is_empty() { entity.world.clean_chunks(&chunks_to_clean); - //log::debug!("Unloading chunks took {:?} (2)", inst.elapsed()); // This can take a little if we are sending a bunch of packets, queue it up :p let client = player.client.clone(); tokio::spawn(async move { @@ -118,22 +101,12 @@ pub async fn update_position(player: &Arc) { .await; } }); - //log::debug!("Unloading chunks took {:?} (3)", inst.elapsed()); } if !loading_chunks.is_empty() { - //let inst = std::time::Instant::now(); - - // loading_chunks.sort_by(|a, b| { - // let distance_a_squared = a.sub(a).length_squared(); - // let distance_b_squared = b.sub(a).length_squared(); - // distance_a_squared.cmp(&distance_b_squared) - // }); - entity .world .spawn_world_chunks(player.clone(), loading_chunks, new_chunk_center); - //log::debug!("Loading chunks took {:?}", inst.elapsed()); } } }