From d3d13323b258c77097804fb0addf782534233649 Mon Sep 17 00:00:00 2001 From: Alexander Medvedev Date: Sun, 17 May 2026 13:47:34 +0200 Subject: [PATCH] fix: don't store empty chunks should be fixed --- pumpkin-data/src/data_component_impl.rs | 18 ++-- pumpkin-nbt/benches/nbt.rs | 6 +- pumpkin-nbt/src/compound.rs | 36 +++++-- pumpkin-nbt/src/deserializer.rs | 2 +- pumpkin-nbt/src/lib.rs | 4 +- pumpkin-nbt/src/nbt_compress.rs | 6 +- pumpkin-nbt/src/nbt_ops.rs | 8 +- pumpkin-nbt/src/tag.rs | 8 +- pumpkin-util/src/math/position.rs | 2 +- pumpkin-world/src/chunk/mod.rs | 3 + pumpkin-world/src/chunk/palette.rs | 5 + .../src/chunk_system/chunk_listener.rs | 23 ++--- pumpkin-world/src/chunk_system/schedule.rs | 97 ++++++++++++++++--- .../src/generation/structure/template/mod.rs | 6 +- .../structure/template/structure_template.rs | 2 +- pumpkin-world/src/level.rs | 26 ++--- pumpkin-world/src/tick/scheduler.rs | 3 + .../entities/furnace_like_block_entity.rs | 2 +- pumpkin/src/block/entities/sign.rs | 2 +- pumpkin/src/command/commands/data.rs | 4 +- pumpkin/src/command/snbt/rules.rs | 6 +- pumpkin/src/command/snbt/tests.rs | 69 +++++++------ pumpkin/src/entity/player.rs | 15 +-- pumpkin/src/world/chunker.rs | 2 +- pumpkin/src/world/mod.rs | 62 ++++++++---- 25 files changed, 273 insertions(+), 144 deletions(-) diff --git a/pumpkin-data/src/data_component_impl.rs b/pumpkin-data/src/data_component_impl.rs index b571b58cf..9db5e9c6f 100644 --- a/pumpkin-data/src/data_component_impl.rs +++ b/pumpkin-data/src/data_component_impl.rs @@ -198,7 +198,7 @@ pub struct CustomNameImpl { } impl DataComponentImpl for CustomNameImpl { fn write_data(&self) -> NbtTag { - NbtTag::String(self.name.clone()) + NbtTag::String(self.name.clone().into()) } fn get_hash(&self) -> i32 { @@ -228,7 +228,7 @@ impl ItemModelImpl { } impl DataComponentImpl for ItemModelImpl { fn write_data(&self) -> NbtTag { - NbtTag::String(self.id.clone()) + NbtTag::String(self.id.clone().into()) } fn get_hash(&self) -> i32 { @@ -250,7 +250,7 @@ impl EnchantmentsImpl { let data = &data.extract_compound()?.child_tags; let mut enc = Vec::with_capacity(data.len()); for (name, level) in data { - enc.push((Enchantment::from_name(name.as_str())?, level.extract_int()?)); + enc.push((Enchantment::from_name(name.as_ref())?, level.extract_int()?)); } Some(Self { enchantment: Cow::from(enc), @@ -987,14 +987,14 @@ impl IDSet { if tag.starts_with("#") { Some(Self::Tag(Cow::Owned(tag.strip_prefix("#")?.to_string()))) } else { - Some(Self::IDs(Cow::Owned([T::from_str(tag.as_str())?].to_vec()))) + Some(Self::IDs(Cow::Owned([T::from_str(tag.as_ref())?].to_vec()))) } } NbtTag::List(nbt_tags) => { let mut ids = Vec::<&T>::new(); for nbt in nbt_tags { if let NbtTag::String(id) = nbt - && let Some(instance) = T::from_str(id.as_str()) + && let Some(instance) = T::from_str(id.as_ref()) { ids.push(instance); } @@ -1013,8 +1013,10 @@ impl IDSet { compound.put_string(key, tag); } Self::IDs(arr) => { - let id_vec: Vec = - arr.iter().map(|x| NbtTag::String(x.to_string())).collect(); + let id_vec: Vec = arr + .iter() + .map(|x| NbtTag::String(x.to_string().into())) + .collect(); compound.put_list(key, id_vec); } } @@ -1466,7 +1468,7 @@ impl StoredEnchantmentsImpl { let data = &data.extract_compound()?.child_tags; let mut enc = Vec::with_capacity(data.len()); for (name, level) in data { - enc.push((Enchantment::from_name(name.as_str())?, level.extract_int()?)); + enc.push((Enchantment::from_name(name.as_ref())?, level.extract_int()?)); } Some(Self { enchantment: Cow::from(enc), diff --git a/pumpkin-nbt/benches/nbt.rs b/pumpkin-nbt/benches/nbt.rs index ae0d636fc..9a656fe80 100644 --- a/pumpkin-nbt/benches/nbt.rs +++ b/pumpkin-nbt/benches/nbt.rs @@ -59,9 +59,9 @@ fn create_large_compound(depth: usize) -> NbtCompound { ); let list = vec![ - NbtTag::String("one".to_string()), - NbtTag::String("two".to_string()), - NbtTag::String("three".to_string()), + NbtTag::String("one".into()), + NbtTag::String("two".into()), + NbtTag::String("three".into()), ]; compound.put_list("list_string", list); diff --git a/pumpkin-nbt/src/compound.rs b/pumpkin-nbt/src/compound.rs index 874e0cfe9..6d2320b08 100644 --- a/pumpkin-nbt/src/compound.rs +++ b/pumpkin-nbt/src/compound.rs @@ -31,7 +31,7 @@ macro_rules! nbt_compound_tag { /// #[derive(Clone, Debug, Default, PartialEq)] pub struct NbtCompound { - pub child_tags: HashMap, + pub child_tags: HashMap, NbtTag>, } impl NbtCompound { @@ -83,7 +83,7 @@ impl NbtCompound { let name = get_nbt_string(reader)?; let tag = NbtTag::deserialize_data(reader, tag_id)?; - compound.child_tags.insert(name, tag); + compound.child_tags.insert(name.into(), tag); } Ok(compound) @@ -105,13 +105,13 @@ impl NbtCompound { } pub fn put(&mut self, name: &str, value: impl Into) { - if !self.child_tags.iter().any(|(key, _)| key == name) { - self.child_tags.insert(name.to_string(), value.into()); + if !self.child_tags.contains_key(name) { + self.child_tags.insert(name.into(), value.into()); } } pub fn put_string(&mut self, name: &str, value: String) { - self.put(name, NbtTag::String(value)); + self.put(name, NbtTag::String(value.into())); } pub fn put_list(&mut self, name: &str, value: Vec) { @@ -238,9 +238,19 @@ impl FromIterator<(String, NbtTag)> for NbtCompound { } } +impl FromIterator<(Box, NbtTag)> for NbtCompound { + fn from_iter, NbtTag)>>(iter: T) -> Self { + let mut compound = Self::new(); + for (key, value) in iter { + compound.child_tags.insert(key, value); + } + compound + } +} + impl IntoIterator for NbtCompound { - type Item = (String, NbtTag); - type IntoIter = IntoIter; + type Item = (Box, NbtTag); + type IntoIter = IntoIter, NbtTag>; fn into_iter(self) -> Self::IntoIter { self.child_tags.into_iter() @@ -249,6 +259,14 @@ impl IntoIterator for NbtCompound { impl Extend<(String, NbtTag)> for NbtCompound { fn extend>(&mut self, iter: T) { + for (key, value) in iter { + self.put(&key, value); + } + } +} + +impl Extend<(Box, NbtTag)> for NbtCompound { + fn extend, NbtTag)>>(&mut self, iter: T) { self.child_tags.extend(iter); } } @@ -287,8 +305,8 @@ impl<'de> Deserialize<'de> for NbtCompound { mut map: A, ) -> Result { let mut compound = NbtCompound::new(); - while let Some((key, value)) = map.next_entry::()? { - compound.put(&key, value); + while let Some((key, value)) = map.next_entry::, NbtTag>()? { + compound.child_tags.insert(key, value); } Ok(compound) } diff --git a/pumpkin-nbt/src/deserializer.rs b/pumpkin-nbt/src/deserializer.rs index df30a3a2e..4f5822ec7 100644 --- a/pumpkin-nbt/src/deserializer.rs +++ b/pumpkin-nbt/src/deserializer.rs @@ -174,7 +174,7 @@ impl<'de, R: Read + Seek> de::Deserializer<'de> for &mut Deserializer { NbtTag::Long(value) => visitor.visit_i64::(value)?, NbtTag::Float(value) => visitor.visit_f32::(value)?, NbtTag::Double(value) => visitor.visit_f64::(value)?, - NbtTag::String(value) => visitor.visit_string::(value)?, + NbtTag::String(value) => visitor.visit_string::(value.into())?, _ => unreachable!(), }; Ok(result) diff --git a/pumpkin-nbt/src/lib.rs b/pumpkin-nbt/src/lib.rs index 63d43446d..0a7fbb577 100644 --- a/pumpkin-nbt/src/lib.rs +++ b/pumpkin-nbt/src/lib.rs @@ -119,7 +119,7 @@ impl Nbt { let mut bytes = Vec::new(); let mut writer = WriteAdaptor::new(&mut bytes); writer.write_u8_be(COMPOUND_ID).unwrap(); - NbtTag::String(self.name) + NbtTag::String(self.name.into()) .serialize_data(&mut writer) .unwrap(); self.root_tag.serialize_content(&mut writer).unwrap(); @@ -418,7 +418,7 @@ mod test { // These tags will be wrapped during serialization. vec.push(NbtTag::Int(-1823)); vec.push(NbtTag::Int(123)); - vec.push(NbtTag::String("Not an int".to_string())); + vec.push(NbtTag::String("Not an int".into())); vec.push(NbtTag::Byte(2)); // This compound will not, since the list is already a list of compound tags. diff --git a/pumpkin-nbt/src/nbt_compress.rs b/pumpkin-nbt/src/nbt_compress.rs index a2ab71735..e5ec070ab 100644 --- a/pumpkin-nbt/src/nbt_compress.rs +++ b/pumpkin-nbt/src/nbt_compress.rs @@ -125,8 +125,8 @@ mod tests { compound.put_long("long_value", 123456789); compound.put_float("float_value", 123.456); compound.put_double("double_value", 123456.789); - compound.put_bool("bool_value", true); - compound.put("string_value", NbtTag::String("test string".to_string())); + compound.put("bool_value", true); + compound.put("string_value", NbtTag::String("test string".into())); // Create a nested compound let mut nested = NbtCompound::new(); @@ -268,7 +268,7 @@ mod tests { // Create a compound with repetitive data (should compress well) for _i in 0..1000 { - compound.put("repeated_key", NbtTag::String("this is a test string that will be repeated many times to demonstrate compression".to_string())); + compound.put("repeated_key", NbtTag::String("this is a test string that will be repeated many times to demonstrate compression".into())); } let uncompressed = compound.child_tags.len() * 100; // rough estimate diff --git a/pumpkin-nbt/src/nbt_ops.rs b/pumpkin-nbt/src/nbt_ops.rs index cf2b9af73..1be9b00e5 100644 --- a/pumpkin-nbt/src/nbt_ops.rs +++ b/pumpkin-nbt/src/nbt_ops.rs @@ -55,7 +55,7 @@ impl DynamicOps for NbtOps { } fn create_string(&self, data: &str) -> Self::Value { - NbtTag::String(data.to_string()) + NbtTag::String(data.into()) } fn create_list(&self, values: I) -> Self::Value @@ -284,7 +284,7 @@ impl DynamicOps for NbtOps { compound .child_tags .into_iter() - .filter(|s| s.0 != key) + .filter(|s| s.0.as_ref() != key) .collect(), ) } else { @@ -705,14 +705,14 @@ mod test { let mut collector = ListCollector::new_collector(); collector = collector.accept(NbtTag::Byte(99)); - collector = collector.accept(NbtTag::String("99".to_string())); + collector = collector.accept(NbtTag::String("99".into())); collector = collector.accept(NbtTag::LongArray(vec![1, 2, 3])); assert_eq!( collector.result(), NbtTag::List(vec![ NbtTag::Byte(99), - NbtTag::String("99".to_string()), + NbtTag::String("99".into()), NbtTag::LongArray(vec![1, 2, 3]) ]) ); diff --git a/pumpkin-nbt/src/tag.rs b/pumpkin-nbt/src/tag.rs index 80348a12d..6ec3098cb 100644 --- a/pumpkin-nbt/src/tag.rs +++ b/pumpkin-nbt/src/tag.rs @@ -21,7 +21,7 @@ pub enum NbtTag { Float(f32) = FLOAT_ID, Double(f64) = DOUBLE_ID, ByteArray(Vec) = BYTE_ARRAY_ID, - String(String) = STRING_ID, + String(Box) = STRING_ID, List(Vec) = LIST_ID, Compound(NbtCompound) = COMPOUND_ID, IntArray(Vec) = INT_ARRAY_ID, @@ -296,7 +296,7 @@ impl NbtTag { } Ok(Self::ByteArray(byte_array)) } - STRING_ID => Ok(Self::String(get_nbt_string(reader)?)), + STRING_ID => Ok(Self::String(get_nbt_string(reader)?.into())), LIST_ID => { let tag_type_id = reader.get_u8_be()?; let len = reader.get_i32_be()?; @@ -464,7 +464,7 @@ impl NbtTag { impl From<&str> for NbtTag { fn from(value: &str) -> Self { - Self::String(value.to_string()) + Self::String(value.into()) } } @@ -559,7 +559,7 @@ impl<'de> Deserialize<'de> for NbtTag { } fn visit_str(self, v: &str) -> Result { - Ok(NbtTag::String(v.to_string())) + Ok(NbtTag::String(v.into())) } fn visit_seq>( diff --git a/pumpkin-util/src/math/position.rs b/pumpkin-util/src/math/position.rs index ee28cf23e..dcc06a4f2 100644 --- a/pumpkin-util/src/math/position.rs +++ b/pumpkin-util/src/math/position.rs @@ -745,7 +745,7 @@ mod test { NbtTag::List(vec![ NbtTag::Int(1), NbtTag::Float(2.0), - NbtTag::String("69".to_string()) + NbtTag::String("69".into()) ]), NbtOps, is_error diff --git a/pumpkin-world/src/chunk/mod.rs b/pumpkin-world/src/chunk/mod.rs index 769401f46..377aecca3 100644 --- a/pumpkin-world/src/chunk/mod.rs +++ b/pumpkin-world/src/chunk/mod.rs @@ -517,6 +517,9 @@ impl ChunkSections { .store(mask, std::sync::atomic::Ordering::Relaxed); // If no more ticking sections, we could potentially deallocate, but that might be overkill and cause jitter. + if mask == 0 { + *random_tick_sections_guard = None; + } } return replaced_block_state_id; diff --git a/pumpkin-world/src/chunk/palette.rs b/pumpkin-world/src/chunk/palette.rs index fe1192860..95a02b032 100644 --- a/pumpkin-world/src/chunk/palette.rs +++ b/pumpkin-world/src/chunk/palette.rs @@ -106,6 +106,11 @@ impl HeterogeneousPaletteData 16 && self.palette.len() < self.palette.capacity() / 2 { + self.palette.shrink_to_fit(); + self.counts.shrink_to_fit(); + } + // If we are indexed, we need to update all indices because swap_remove changed indices if !upgraded && let PaletteStorage::Indexed(indices) = &mut self.storage { for row in indices.iter_mut() { diff --git a/pumpkin-world/src/chunk_system/chunk_listener.rs b/pumpkin-world/src/chunk_system/chunk_listener.rs index cb2bf707f..0f1a4ee66 100644 --- a/pumpkin-world/src/chunk_system/chunk_listener.rs +++ b/pumpkin-world/src/chunk_system/chunk_listener.rs @@ -1,18 +1,22 @@ use super::ChunkPos; use crate::level::SyncChunk; use crossbeam::channel::{Receiver, Sender}; -use std::sync::Mutex; +use std::sync::Arc; +use std::sync::{Mutex, Weak}; use tokio::sync::oneshot; +#[expect(clippy::type_complexity)] pub struct ChunkListener { single: Mutex)>>, - global: Mutex>>, + global: Mutex)>>>, } + impl Default for ChunkListener { fn default() -> Self { Self::new() } } + impl ChunkListener { #[must_use] pub const fn new() -> Self { @@ -21,16 +25,19 @@ impl ChunkListener { global: Mutex::new(Vec::new()), } } + pub fn add_single_chunk_listener(&self, pos: ChunkPos) -> oneshot::Receiver { let (tx, rx) = oneshot::channel(); self.single.lock().unwrap().push((pos, tx)); rx } - pub fn add_global_chunk_listener(&self) -> Receiver<(ChunkPos, SyncChunk)> { + + pub fn add_global_chunk_listener(&self) -> Receiver<(ChunkPos, Weak)> { let (tx, rx) = crossbeam::channel::unbounded(); self.global.lock().unwrap().push(tx); rx } + pub fn process_new_chunk(&self, pos: ChunkPos, chunk: &SyncChunk) { { let mut single = self.single.lock().unwrap(); @@ -44,22 +51,16 @@ impl ChunkListener { len -= 1; continue; } - if single[i].1.is_closed() { - // let listener_pos = single[i].0; - single.remove(i); - // log::debug!("single listener dropped {listener_pos:?}"); - len -= 1; - continue; - } i += 1; } } { + let weak = Arc::downgrade(chunk); let mut global = self.global.lock().unwrap(); let mut i = 0; let mut len = global.len(); while i < len { - if matches!(global[i].send((pos, chunk.clone())), Ok(())) { + if matches!(global[i].send((pos, weak.clone())), Ok(())) { // log::debug!("global listener {i} send {pos:?}"); } else { // log::debug!("one global listener dropped"); diff --git a/pumpkin-world/src/chunk_system/schedule.rs b/pumpkin-world/src/chunk_system/schedule.rs index b6da05290..defb56f81 100644 --- a/pumpkin-world/src/chunk_system/schedule.rs +++ b/pumpkin-world/src/chunk_system/schedule.rs @@ -528,7 +528,61 @@ impl GenerationSchedule { true } - fn unload_chunk(&mut self) { + fn garbage_collect_dependencies(&mut self) { + // Garbage collect stranded dependencies + let mut stranded = Vec::new(); + for (pos, holder) in &self.chunk_map { + if holder.target_stage == StagedChunkEnum::None + && holder.dependency_stage != StagedChunkEnum::None + { + stranded.push(*pos); + } + } + + for pos in stranded { + let holder = self.chunk_map.get_mut(&pos).unwrap(); + if !holder.occupied.is_null() && self.graph.nodes.contains_key(holder.occupied) { + continue; + } + + let mut cur_edge = holder.occupied_by; + let mut prev_edge = EdgeKey::null(); + let mut change_head = None; + let mut has_valid_task = false; + + while !cur_edge.is_null() { + let edge = self.graph.edges.get(cur_edge).unwrap(); + if self.graph.nodes.contains_key(edge.to) { + prev_edge = cur_edge; + cur_edge = edge.next; + has_valid_task = true; + } else { + let next = edge.next; + self.graph.edges.remove(cur_edge); + cur_edge = next; + if prev_edge.is_null() { + change_head = Some(next); + } else { + self.graph.edges.get_mut(prev_edge).unwrap().next = next; + } + } + } + if let Some(next) = change_head { + holder.occupied_by = next; + } + + if !has_valid_task { + holder.dependency_stage = StagedChunkEnum::None; + self.unload_chunks.insert(pos); + } + } + } + + fn process_unload_queue(&mut self) { + if self.unload_chunks.is_empty() { + return; + } + let mut unload_chunks = HashSetType::default(); swap(&mut unload_chunks, &mut self.unload_chunks); let mut chunks = Vec::with_capacity(unload_chunks.len()); @@ -568,6 +622,8 @@ impl GenerationSchedule { self.chunk_map.remove(&pos); } } + } else { + self.unload_chunks.insert(pos); } } if chunks.is_empty() { @@ -593,7 +649,14 @@ impl GenerationSchedule { if let Some(chunk) = holder.chunk.take() { let should_save = match &chunk { Chunk::Level(sync_chunk) => sync_chunk.is_dirty(), - Chunk::Proto(_) => save_proto_chunk, + Chunk::Proto(proto) => { + save_proto_chunk + && !matches!( + proto.stage, + crate::chunk_system::chunk_state::StagedChunkEnum::Empty + | crate::chunk_system::chunk_state::StagedChunkEnum::None + ) + } }; if should_save { @@ -773,6 +836,7 @@ impl GenerationSchedule { } } } else { + holder.current_stage = StagedChunkEnum::Full; holder.chunk = Some(Chunk::Level(chunk)); } @@ -783,10 +847,9 @@ impl GenerationSchedule { } holder.occupied = NodeKey::null(); - // If this neighbor chunk was only loaded for a dependency and - // is no longer needed, clear dependency_stage and queue unload. + // If this chunk was only loaded for a dependency or cancelled + // and is no longer needed, clear dependency_stage and queue unload. if holder.target_stage == StagedChunkEnum::None - && new_pos != pos && holder.current_stage >= holder.dependency_stage { holder.dependency_stage = StagedChunkEnum::None; @@ -822,14 +885,14 @@ impl GenerationSchedule { { self.drop_node(holder.occupied); } + } - // Clear dependency_stage and queue unload if no longer needed - if holder.target_stage == StagedChunkEnum::None - && holder.current_stage >= holder.dependency_stage - { - holder.dependency_stage = StagedChunkEnum::None; - self.unload_chunks.insert(new_pos); - } + // Clear dependency_stage and queue unload if no longer needed + if holder.target_stage == StagedChunkEnum::None + && holder.current_stage >= holder.dependency_stage + { + holder.dependency_stage = StagedChunkEnum::None; + self.unload_chunks.insert(new_pos); } holder.occupied = NodeKey::null(); @@ -927,13 +990,16 @@ impl GenerationSchedule { ); loop { if level.should_unload.swap(false, Relaxed) { - self.unload_chunk(); + self.garbage_collect_dependencies(); + self.process_unload_queue(); } if level.should_save.swap(false, Relaxed) { self.save_all_chunk(false); } if level.shut_down_chunk_system.load(Relaxed) { info!("Saving chunks before shutdown..."); + self.garbage_collect_dependencies(); + self.process_unload_queue(); self.save_all_chunk(true); break; } @@ -941,6 +1007,11 @@ impl GenerationSchedule { // 1. Get latest world state (player moves, etc) self.resort_work(self.send_level.get()); + // Process unload queue continuously if there are chunks to unload + if !self.unload_chunks.is_empty() { + self.process_unload_queue(); + } + // 2. Process all pending chunk results from workers while let Ok((pos, data)) = self.recv_chunk.try_recv() { self.receive_chunk(pos, data); diff --git a/pumpkin-world/src/generation/structure/template/mod.rs b/pumpkin-world/src/generation/structure/template/mod.rs index b06842367..b90987e5a 100644 --- a/pumpkin-world/src/generation/structure/template/mod.rs +++ b/pumpkin-world/src/generation/structure/template/mod.rs @@ -103,7 +103,11 @@ pub fn place_template( if let Some(template_nbt) = &block.nbt { for (key, value) in &template_nbt.child_tags { - if key != "x" && key != "y" && key != "z" && key != "id" { + if key.as_ref() != "x" + && key.as_ref() != "y" + && key.as_ref() != "z" + && key.as_ref() != "id" + { block_entity_nbt .child_tags .insert(key.clone(), value.clone()); diff --git a/pumpkin-world/src/generation/structure/template/structure_template.rs b/pumpkin-world/src/generation/structure/template/structure_template.rs index 1cffcaec6..828d736aa 100644 --- a/pumpkin-world/src/generation/structure/template/structure_template.rs +++ b/pumpkin-world/src/generation/structure/template/structure_template.rs @@ -178,7 +178,7 @@ impl StructureTemplate { .iter() .filter_map(|(key, value)| { if let NbtTag::String(v) = value { - Some((key.clone(), v.clone())) + Some((key.to_string(), v.to_string())) } else { None } diff --git a/pumpkin-world/src/level.rs b/pumpkin-world/src/level.rs index e8680a6c9..e9b6d4271 100644 --- a/pumpkin-world/src/level.rs +++ b/pumpkin-world/src/level.rs @@ -25,7 +25,7 @@ use pumpkin_data::{Block, block_properties::has_random_ticks, fluid::Fluid}; use pumpkin_util::math::{position::BlockPos, vector2::Vector2}; use pumpkin_util::world_seed::Seed; use rustc_hash::{FxHashMap, FxHashSet}; -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Mutex, Weak}; use std::time::Duration; use std::{ path::PathBuf, @@ -39,7 +39,7 @@ use tracing::{debug, error, info, trace, warn}; use tokio::{ select, sync::{ - mpsc::{self, UnboundedReceiver}, + mpsc::{self, Receiver}, oneshot, }, task::JoinHandle, @@ -226,7 +226,7 @@ impl Level { x: pos.x, z: pos.y, data: tokio::sync::Mutex::new(FxHashMap::default()), - dirty: AtomicBool::new(true), + dirty: AtomicBool::new(false), }); level.loaded_entity_chunks.insert(pos, arc_chunk.clone()); @@ -247,7 +247,7 @@ impl Level { x: pos.x, z: pos.y, data: tokio::sync::Mutex::new(FxHashMap::default()), - dirty: AtomicBool::new(true), + dirty: AtomicBool::new(false), }); level_clone @@ -511,7 +511,7 @@ impl Level { self.chunk_watchers.get(chunk).is_some() } - pub fn clean_memory(&self) { + pub fn clean_memory(self: &Arc) -> Vec> { self.chunk_watchers.retain(|_, watcher| *watcher != 0); let entity_chunks_to_remove: Vec<_> = self @@ -521,8 +521,8 @@ impl Level { .map(|entry| *entry.key()) .collect(); - for pos in entity_chunks_to_remove { - self.loaded_entity_chunks.remove(&pos); + if !entity_chunks_to_remove.is_empty() { + self.clean_entity_chunks(&entity_chunks_to_remove); } // if the difference is too big, we can shrink the loaded chunks @@ -534,6 +534,7 @@ impl Level { if self.loaded_entity_chunks.capacity() - self.loaded_entity_chunks.len() >= 4096 { self.loaded_entity_chunks.shrink_to_fit(); } + entity_chunks_to_remove } pub async fn get_or_fetch_chunk R>( @@ -590,8 +591,8 @@ impl Level { pub fn receive_entity_chunks( self: &Arc, chunks: Vec>, - ) -> UnboundedReceiver<(SyncEntityChunk, bool)> { - let (sender, receiver) = mpsc::unbounded_channel(); + ) -> Receiver<(Weak, bool)> { + let (sender, receiver) = mpsc::channel(64); let level = self.clone(); self.spawn_task(async move { @@ -602,7 +603,7 @@ impl Level { .iter() .filter(|pos| { level.loaded_entity_chunks.get(pos).is_none_or(|chunk| { - let _ = sender.send((chunk.clone(), false)); + let _ = sender.try_send((Arc::downgrade(chunk.value()), false)); false // Don't fetch }) }) @@ -624,7 +625,7 @@ impl Level { LoadedData::Loaded(chunk) => { let pos = Vector2::new(chunk.x, chunk.z); level.loaded_entity_chunks.insert(pos, chunk.clone()); - let _ = sender.send((chunk, false)); + let _ = sender.send((Arc::downgrade(&chunk), true)).await; } LoadedData::Missing(pos) | LoadedData::Error((pos, _)) => { let sender_clone = sender.clone(); @@ -642,7 +643,8 @@ impl Level { } } if let Ok(chunk) = rx.await { - let _ = sender_clone.send((chunk, true)); + let _ = + sender_clone.send((Arc::downgrade(&chunk), true)).await; } }); } diff --git a/pumpkin-world/src/tick/scheduler.rs b/pumpkin-world/src/tick/scheduler.rs index 4299ac074..69c85ae48 100644 --- a/pumpkin-world/src/tick/scheduler.rs +++ b/pumpkin-world/src/tick/scheduler.rs @@ -41,6 +41,9 @@ impl<'a, T: std::hash::Hash + Eq> ChunkTickScheduler<&'a T> { .queued_ticks .remove(&(next_tick.position, next_tick.value)); } + if inner.queued_ticks.is_empty() { + *inner_guard = None; + } } res } diff --git a/pumpkin/src/block/entities/furnace_like_block_entity.rs b/pumpkin/src/block/entities/furnace_like_block_entity.rs index 79183f4c7..c7b0eaf5d 100644 --- a/pumpkin/src/block/entities/furnace_like_block_entity.rs +++ b/pumpkin/src/block/entities/furnace_like_block_entity.rs @@ -524,7 +524,7 @@ macro_rules! impl_block_entity_for_cooking { if let Some(recipes_compound) = nbt.get_compound("RecipesUsed") { for (recipe_id, tag) in &recipes_compound.child_tags { if let pumpkin_nbt::tag::NbtTag::Int(count) = tag { - recipes_used_map.insert(recipe_id.clone(), *count as u32); + recipes_used_map.insert(recipe_id.to_string(), *count as u32); } } } diff --git a/pumpkin/src/block/entities/sign.rs b/pumpkin/src/block/entities/sign.rs index d543dd4ff..d5fac373c 100644 --- a/pumpkin/src/block/entities/sign.rs +++ b/pumpkin/src/block/entities/sign.rs @@ -160,7 +160,7 @@ impl From for NbtTag { .lock() .unwrap() .iter() - .map(|s| Self::String(s.clone())) + .map(|s| Self::String(s.clone().into())) .collect(), ); Self::Compound(nbt) diff --git a/pumpkin/src/command/commands/data.rs b/pumpkin/src/command/commands/data.rs index 7b6b3bf48..e6b00bc79 100644 --- a/pumpkin/src/command/commands/data.rs +++ b/pumpkin/src/command/commands/data.rs @@ -156,7 +156,9 @@ pub fn snbt_colorful_display(tag: &NbtTag, depth: usize) -> Result { } else if literal.eq_ignore_ascii_case("false") { Some(NbtTag::Byte(0)) } else { - Some(NbtTag::String(literal)) + Some(NbtTag::String(literal.into())) } } @@ -716,7 +716,7 @@ impl SnbtParser<'_, '_> { })?; Some(NbtTag::Compound(NbtCompound { - child_tags: entries, + child_tags: entries.into_iter().map(|(k, v)| (k.into(), v)).collect(), })) } @@ -844,7 +844,7 @@ impl SnbtParser<'_, '_> { ), } } - Literal::String(string) => NbtTag::String(string), + Literal::String(string) => NbtTag::String(string.into()), }) } diff --git a/pumpkin/src/command/snbt/tests.rs b/pumpkin/src/command/snbt/tests.rs index eeea9b7f4..c10840a9f 100644 --- a/pumpkin/src/command/snbt/tests.rs +++ b/pumpkin/src/command/snbt/tests.rs @@ -222,28 +222,25 @@ fn floats() { #[test] fn quoted_string_literals() { - assert_parse_ok!("''", NbtTag::String(String::new())); - assert_parse_ok!("\"\"", NbtTag::String(String::new())); + assert_parse_ok!("''", NbtTag::String("".into())); + assert_parse_ok!("\"\"", NbtTag::String("".into())); - assert_parse_ok!("\"'hello'\"", NbtTag::String("'hello'".to_string())); - assert_parse_ok!("'\"hello\"'", NbtTag::String("\"hello\"".to_string())); - assert_parse_ok!("'\\\\'", NbtTag::String("\\".to_string())); + assert_parse_ok!("\"'hello'\"", NbtTag::String("'hello'".into())); + assert_parse_ok!("'\"hello\"'", NbtTag::String("\"hello\"".into())); + assert_parse_ok!("'\\\\'", NbtTag::String("\\".into())); assert_parse_ok_but_trailing!("'\"'\"", "\""); assert_parse_err!("'\\'", "Invalid string contents", 3, ["\"", "'", "\\"]); - assert_parse_ok!("'\\b'", NbtTag::String("\u{8}".to_string())); - assert_parse_ok!("'hello\\sword'", NbtTag::String("hello word".to_string())); - assert_parse_ok!( - "'hello\\tword\n'", - NbtTag::String("hello\tword\n".to_string()) - ); - assert_parse_ok!("'\\f\\r'", NbtTag::String("\u{c}\r".to_string())); + assert_parse_ok!("'\\b'", NbtTag::String("\u{8}".into())); + assert_parse_ok!("'hello\\sword'", NbtTag::String("hello word".into())); + assert_parse_ok!("'hello\\tword\n'", NbtTag::String("hello\tword\n".into())); + assert_parse_ok!("'\\f\\r'", NbtTag::String("\u{c}\r".into())); - assert_parse_ok!("'hello \\x65!'", NbtTag::String("hello e!".to_string())); + assert_parse_ok!("'hello \\x65!'", NbtTag::String("hello e!".into())); assert_parse_ok!( "'\\x53\\x65\\u0063\\U00000072\\x65\\x74\\x21'", - NbtTag::String("Secret!".to_string()) + NbtTag::String("Secret!".into()) ); assert_parse_err!( @@ -255,7 +252,7 @@ fn quoted_string_literals() { assert_parse_ok!( "'\\uD83C\\uDF83 or \\U0001F383'", - NbtTag::String("🎃 or 🎃".to_string()) + NbtTag::String("🎃 or 🎃".into()) ); // TODO: make tests for when \N is implemented @@ -263,19 +260,19 @@ fn quoted_string_literals() { #[test] fn unquoted_string_literals() { - assert_parse_ok!("abc", NbtTag::String("abc".to_string())); + assert_parse_ok!("abc", NbtTag::String("abc".into())); assert_parse_ok!( "abc-def_ghi+jkl.mno", - NbtTag::String("abc-def_ghi+jkl.mno".to_string()) + NbtTag::String("abc-def_ghi+jkl.mno".into()) ); - assert_parse_ok!("_1234", NbtTag::String("_1234".to_string())); - assert_parse_ok!("x+1", NbtTag::String("x+1".to_string())); + assert_parse_ok!("_1234", NbtTag::String("_1234".into())); + assert_parse_ok!("x+1", NbtTag::String("x+1".into())); assert_parse_ok_but_trailing!("x*1", "*1"); assert_parse_ok!("true", NbtTag::Byte(1)); assert_parse_ok!("false", NbtTag::Byte(0)); - assert_parse_ok!("maybe", NbtTag::String("maybe".to_string())); - assert_parse_ok!("bool", NbtTag::String("bool".to_string())); + assert_parse_ok!("maybe", NbtTag::String("maybe".into())); + assert_parse_ok!("bool", NbtTag::String("bool".into())); } #[test] @@ -340,36 +337,36 @@ fn maps() { assert_parse_ok!( "{x:5}", NbtTag::Compound(NbtCompound { - child_tags: HashMap::from([("x".to_string(), NbtTag::Int(5))]) + child_tags: HashMap::from([("x".into(), NbtTag::Int(5))]) }) ); assert_parse_ok!( "{ a:1b,B :2uS , c:3L }", NbtTag::Compound(NbtCompound { child_tags: HashMap::from([ - ("a".to_string(), NbtTag::Byte(1)), - ("B".to_string(), NbtTag::Short(2)), - ("c".to_string(), NbtTag::Long(3)) + ("a".into(), NbtTag::Byte(1)), + ("B".into(), NbtTag::Short(2)), + ("c".into(), NbtTag::Long(3)) ]) }) ); assert_parse_ok!( "{ a:1b, \"a\":2b, 'a':\"hi\" }", nbt_compound_tag! { - "a": NbtTag::String("hi".to_string()) + "a": NbtTag::String("hi".into()) } ); assert_parse_ok!( "{ elem: 'this', next: { elem: 'is', next: { elem: 'a', next: { elem: 'linked', next: 'list' } } } }", nbt_compound_tag! { - "elem": NbtTag::String("this".to_string()), + "elem": NbtTag::String("this".into()), "next": nbt_compound_tag! { - "elem": NbtTag::String("is".to_string()), + "elem": NbtTag::String("is".into()), "next": nbt_compound_tag! { - "elem": NbtTag::String("a".to_string()), + "elem": NbtTag::String("a".into()), "next": nbt_compound_tag! { - "elem": NbtTag::String("linked".to_string()), - "next": NbtTag::String("list".to_string()), + "elem": NbtTag::String("linked".into()), + "next": NbtTag::String("list".into()), } } } @@ -418,22 +415,22 @@ fn lists() { assert_parse_ok!("[ ]", NbtTag::List(Vec::new())); assert_parse_ok!( "[5, _]", - NbtTag::List(vec![NbtTag::Int(5), NbtTag::String("_".to_string())]) + NbtTag::List(vec![NbtTag::Int(5), NbtTag::String("_".into())]) ); assert_parse_ok!( "[a, [true, c], [4s, uuid(f-0-0-0-0), f, [[], 7f, [\"\\n\", [200Ub, {x: 1e1}]]]]]", NbtTag::List(vec![ - NbtTag::String("a".to_string()), - NbtTag::List(vec![NbtTag::Byte(1), NbtTag::String("c".to_string())]), + NbtTag::String("a".into()), + NbtTag::List(vec![NbtTag::Byte(1), NbtTag::String("c".into())]), NbtTag::List(vec![ NbtTag::Short(4), NbtTag::IntArray(vec![15, 0, 0, 0]), - NbtTag::String("f".to_string()), + NbtTag::String("f".into()), NbtTag::List(vec![ NbtTag::List(Vec::new()), NbtTag::Float(7.0), NbtTag::List(vec![ - NbtTag::String("\n".to_string()), + NbtTag::String("\n".into()), NbtTag::List(vec![ NbtTag::Byte(-56), nbt_compound_tag! { diff --git a/pumpkin/src/entity/player.rs b/pumpkin/src/entity/player.rs index cffb66458..816967298 100644 --- a/pumpkin/src/entity/player.rs +++ b/pumpkin/src/entity/player.rs @@ -142,7 +142,7 @@ pub struct ChunkManager { chunks_per_tick: usize, center: Vector2, view_distance: u8, - chunk_listener: Receiver<(Vector2, SyncChunk)>, + chunk_listener: Receiver<(Vector2, Weak)>, chunk_sent: HashMap, Weak>, chunk_queue: BinaryHeap, entity_chunk_queue: VecDeque<(Vector2, Weak)>, @@ -159,7 +159,7 @@ impl ChunkManager { #[must_use] pub fn new( chunks_per_tick: usize, - chunk_listener: Receiver<(Vector2, SyncChunk)>, + chunk_listener: Receiver<(Vector2, Weak)>, world: Arc, ) -> Self { Self { @@ -201,14 +201,15 @@ impl ChunkManager { } pub fn pull_new_chunks(&mut self) { - while let Ok((pos, chunk)) = self.chunk_listener.try_recv() { + while let Ok((pos, chunk_weak)) = self.chunk_listener.try_recv() { let dst = Self::chebyshev(pos, self.center); if dst > i32::from(self.view_distance) { continue; } - if self.should_enqueue_chunk(pos, &chunk) { - self.chunk_queue - .push(HeapNode(dst, pos, Arc::downgrade(&chunk))); + if let Some(chunk) = chunk_weak.upgrade() + && self.should_enqueue_chunk(pos, &chunk) + { + self.chunk_queue.push(HeapNode(dst, pos, chunk_weak)); } } } @@ -801,7 +802,7 @@ impl Player { // Remove chunks with no watchers from the cache if !chunks_to_clean.is_empty() { level.clean_entity_chunks(&chunks_to_clean); - world.remove_entities_in_chunks(&chunks_to_clean).await; + world.remove_entities_in_chunks(&chunks_to_clean); } // Remove left over entries from all possiblily loaded chunks level.clean_memory(); diff --git a/pumpkin/src/world/chunker.rs b/pumpkin/src/world/chunker.rs index 57bb148f0..c3fee855a 100644 --- a/pumpkin/src/world/chunker.rs +++ b/pumpkin/src/world/chunker.rs @@ -115,7 +115,7 @@ pub async fn update_position(player: &Arc) { if !chunks_to_clean.is_empty() { world.level.clean_entity_chunks(&chunks_to_clean); - world.remove_entities_in_chunks(&chunks_to_clean).await; + world.remove_entities_in_chunks(&chunks_to_clean); } if !loading_chunks.is_empty() { diff --git a/pumpkin/src/world/mod.rs b/pumpkin/src/world/mod.rs index 0e42dc896..5b5da8970 100644 --- a/pumpkin/src/world/mod.rs +++ b/pumpkin/src/world/mod.rs @@ -848,9 +848,11 @@ impl World { let entity_elapsed = entity_start.elapsed(); let block_entity_start = tokio::time::Instant::now(); + let active_chunks = self.active_chunks.load(); let block_entities: Vec> = self .block_entities .iter() + .filter(|e| active_chunks.contains(&e.key().chunk_position())) .map(|e| e.value().clone()) .collect(); let block_entity_count = block_entities.len(); @@ -943,6 +945,10 @@ impl World { // Auto-save logic if level_time.world_age % 100 == 0 { self.level.should_unload.store(true, Relaxed); + let cleaned_chunks = self.level.clean_memory(); + if !cleaned_chunks.is_empty() { + self.remove_entities_in_chunks(&cleaned_chunks); + } // If autosave is configured and this tick will trigger an autosave, don't double notify if self.level.autosave_ticks == 0 { self.level.level_channel.notify(); @@ -2661,9 +2667,14 @@ impl World { } }; - let Some((chunk, _first_load)) = recv_result else { + let Some((chunk_weak, first_load)) = recv_result else { break; }; + + let Some(chunk) = chunk_weak.upgrade() else { + continue; + }; + let position = Vector2::new(chunk.x, chunk.z); if !level.is_chunk_watched(&position) { @@ -2692,24 +2703,27 @@ impl World { ids_to_remove.push(VarInt(base_entity.entity_id)); - let mut nbt = NbtCompound::new(); - entity.write_nbt(&mut nbt).await; - if let Some(old_chunk) = base_entity.first_loaded_chunk_position.load() { - let old_chunk = old_chunk.to_vec2_i32(); - let chunk = world.level.get_entity_chunk(old_chunk).await; - chunk.mark_dirty(true); - let base_entity = entity.get_entity(); - let current_chunk_coordinate = - base_entity.block_pos.load().chunk_position(); + if first_load { + let mut nbt = NbtCompound::new(); + entity.write_nbt(&mut nbt).await; + if let Some(old_chunk) = base_entity.first_loaded_chunk_position.load() + { + let old_chunk = old_chunk.to_vec2_i32(); + let chunk = world.level.get_entity_chunk(old_chunk).await; + chunk.mark_dirty(true); + let base_entity = entity.get_entity(); + let current_chunk_coordinate = + base_entity.block_pos.load().chunk_position(); - let mut data = chunk.data.lock().await; - if old_chunk == current_chunk_coordinate { - data.insert(*uuid, nbt); - continue; + let mut data = chunk.data.lock().await; + if old_chunk == current_chunk_coordinate { + data.insert(*uuid, nbt); + continue; + } + + // The chunk has changed, lets remove the entity from the old chunk + data.remove(uuid); } - - // The chunk has changed, lets remove the entity from the old chunk - data.remove(uuid); } } if !ids_to_remove.is_empty() { @@ -2744,9 +2758,11 @@ impl World { .await; entity.init_data_tracker().await; - entities_to_add.push(entity); + if first_load { + entities_to_add.push(entity); + } } - if !entities_to_add.is_empty() { + if first_load && !entities_to_add.is_empty() { world.entities.rcu(|current_entities| { let mut new_entities = (**current_entities).clone(); new_entities.extend(entities_to_add.iter().cloned()); @@ -3215,7 +3231,7 @@ impl World { self.remove_entity_data(base_entity).await; } - pub async fn remove_entities_in_chunks(&self, chunks: &[Vector2]) { + pub fn remove_entities_in_chunks(&self, chunks: &[Vector2]) { let chunks_set: FxHashSet<_> = chunks.iter().copied().collect(); let mut entities_to_remove = Vec::new(); @@ -3236,8 +3252,12 @@ impl World { for entity in entities_to_remove { self.spawn_state.load().remove_entity(self, entity.as_ref()); - self.remove_entity_data(entity.get_entity()).await; + // Important: We do NOT call remove_entity_data here because we want the entities + // to persist in the chunk data on disk. We only remove them from the active world (RAM). } + + self.block_entities + .retain(|pos, _| !chunks_set.contains(&pos.chunk_position())); } pub async fn set_block_breaking(&self, from: &Entity, location: BlockPos, progress: i32) {