fix: inventory saving/loading (#1582)

* fix: inventory saving

* fix: potion NBT schema

* fix: firework NBT schema

* fix: different fireworks won't stack now

* chore: formatting

* fix: restore hopper TransferCooldown saving

* fix: use filter_map

* fix: firework duration default

* fix: proper is_dirty support

* fix: add clear_dirty overrides and remove double furnace mark_dirty

pumpkin-data/src/data_component_impl.rs

@@ -49,6 +49,9 @@ pub fn read_data(id: DataComponent, data: &NbtTag) -> Option<Box<dyn DataCompone
         Enchantments => Some(EnchantmentsImpl::read_data(data)?.to_dyn()),
         Damage => Some(DamageImpl::read_data(data)?.to_dyn()),
         Unbreakable => Some(UnbreakableImpl::read_data(data)?.to_dyn()),
+        PotionContents => Some(PotionContentsImpl::read_data(data)?.to_dyn()),
+        Fireworks => Some(FireworksImpl::read_data(data)?.to_dyn()),
+        FireworkExplosion => Some(FireworkExplosionImpl::read_data(data)?.to_dyn()),
         _ => None,
     }
 }
@@ -648,6 +651,69 @@ pub struct PotionContentsImpl {
     pub custom_name: Option<String>,
 }
 
+impl PotionContentsImpl {
+    pub fn read_data(tag: &NbtTag) -> Option<Self> {
+        let compound = tag.extract_compound()?;
+        let potion_id = if let Some(id) = compound.get_int("potion") {
+            Some(id)
+        } else if let Some(name) = compound.get_string("potion") {
+            // Handle "minecraft:swiftness" -> "swiftness"
+            let name = name.strip_prefix("minecraft:").unwrap_or(name);
+            crate::potion::Potion::from_name(name).map(|p| p.id as i32)
+        } else {
+            None
+        };
+
+        let custom_color = compound.get_int("custom_color");
+        let custom_name = compound.get_string("custom_name").map(|s| s.to_string());
+
+        let custom_effects = compound
+            .get_list("custom_effects")
+            .map(|list| {
+                list.iter()
+                    .filter_map(|item| {
+                        // Try to get the compound for this specific effect
+                        let effect_tag = item.extract_compound()?;
+
+                        // Try to get the ID
+                        let id = effect_tag.get_int("id")?;
+
+                        // Fallback values for optional fields
+                        let amplifier = effect_tag
+                            .get_int("amplifier")
+                            .or_else(|| effect_tag.get_byte("amplifier").map(i32::from))
+                            .unwrap_or(0);
+                        let duration = effect_tag
+                            .get_int("duration")
+                            .or_else(|| effect_tag.get_byte("duration").map(i32::from))
+                            .unwrap_or(0);
+                        let ambient = effect_tag.get_bool("ambient").unwrap_or(false);
+                        let show_particles = effect_tag.get_bool("show_particles").unwrap_or(true);
+                        let show_icon = effect_tag.get_bool("show_icon").unwrap_or(true);
+
+                        // Create the StatusEffectInstance
+                        Some(StatusEffectInstance {
+                            effect_id: id,
+                            amplifier,
+                            duration,
+                            ambient,
+                            show_particles,
+                            show_icon,
+                        })
+                    })
+                    .collect::<Vec<_>>()
+            })
+            .unwrap_or_default();
+
+        Some(Self {
+            potion_id,
+            custom_color,
+            custom_effects,
+            custom_name,
+        })
+    }
+}
+
 impl DataComponentImpl for PotionContentsImpl {
     fn write_data(&self) -> NbtTag {
         let mut compound = NbtCompound::new();
@@ -682,6 +748,39 @@ impl DataComponentImpl for PotionContentsImpl {
         NbtTag::Compound(compound)
     }
 
+    fn get_hash(&self) -> i32 {
+        let mut digest = Digest::new(Crc32Iscsi);
+
+        if let Some(id) = self.potion_id {
+            digest.update(&[1u8]);
+            digest.update(&get_i32_hash(id).to_le_bytes());
+        }
+
+        if let Some(color) = self.custom_color {
+            digest.update(&[2u8]);
+            digest.update(&get_i32_hash(color).to_le_bytes());
+        }
+
+        if let Some(name) = &self.custom_name {
+            digest.update(&[3u8]);
+            digest.update(&get_str_hash(name).to_le_bytes());
+        }
+
+        if !self.custom_effects.is_empty() {
+            digest.update(&[4u8]);
+            for effect in &self.custom_effects {
+                digest.update(&get_i32_hash(effect.effect_id).to_le_bytes());
+                digest.update(&get_i32_hash(effect.amplifier).to_le_bytes());
+                digest.update(&get_i32_hash(effect.duration).to_le_bytes());
+                digest.update(&[effect.ambient as u8]);
+                digest.update(&[effect.show_particles as u8]);
+                digest.update(&[effect.show_icon as u8]);
+            }
+        }
+
+        digest.finalize() as i32
+    }
+
     default_impl!(PotionContents);
 }
 #[derive(Clone, Debug, Hash, PartialEq, Eq)]
@@ -794,6 +893,29 @@ impl FireworkExplosionImpl {
             has_twinkle,
         }
     }
+
+    pub fn read_data(tag: &NbtTag) -> Option<Self> {
+        let compound = tag.extract_compound()?;
+        let shape = FireworkExplosionShape::from_name(compound.get_string("shape")?)?;
+        let colors = compound
+            .get_int_array("colors")
+            .map(|v| v.to_vec())
+            .unwrap_or_default();
+        let fade_colors = compound
+            .get_int_array("fade_colors")
+            .map(|v| v.to_vec())
+            .unwrap_or_default();
+        let has_trail = compound.get_bool("has_trail").unwrap_or(false);
+        let has_twinkle = compound.get_bool("has_twinkle").unwrap_or(false);
+
+        Some(Self {
+            shape,
+            colors,
+            fade_colors,
+            has_trail,
+            has_twinkle,
+        })
+    }
 }
 
 impl DataComponentImpl for FireworkExplosionImpl {
@@ -840,6 +962,29 @@ impl FireworksImpl {
             explosions,
         }
     }
+
+    pub fn read_data(tag: &NbtTag) -> Option<Self> {
+        let compound = tag.extract_compound()?;
+        let flight_duration = compound
+            .get_byte("flight_duration")
+            .map(i32::from)
+            .or_else(|| compound.get_int("flight_duration"))
+            .unwrap_or(1);
+
+        let mut explosions = Vec::new();
+        if let Some(list) = compound.get_list("explosions") {
+            for item in list {
+                if let Some(explosion) = FireworkExplosionImpl::read_data(item) {
+                    explosions.push(explosion);
+                }
+            }
+        }
+
+        Some(Self {
+            flight_duration,
+            explosions,
+        })
+    }
 }
 
 impl DataComponentImpl for FireworksImpl {

pumpkin-world/src/block/entities/barrel.rs

@@ -67,11 +67,7 @@ impl BlockEntity for BarrelBlockEntity {
         &'a self,
         nbt: &'a mut NbtCompound,
     ) -> Pin<Box<dyn Future<Output = ()> + Send + 'a>> {
-        Box::pin(async move {
-            self.write_data(nbt, &self.items, true).await;
-        })
-        // Safety precaution
-        //self.clear().await;
+        self.write_inventory_nbt(nbt, true)
     }
 
     fn tick<'a>(
@@ -93,6 +89,10 @@ impl BlockEntity for BarrelBlockEntity {
         self.dirty.load(Ordering::Relaxed)
     }
 
+    fn clear_dirty(&self) {
+        self.dirty.store(false, Ordering::Relaxed);
+    }
+
     fn as_any(&self) -> &dyn Any {
         self
     }
@@ -201,17 +201,23 @@ impl Inventory for BarrelBlockEntity {
             let mut removed = ItemStack::EMPTY.clone();
             let mut guard = self.items[slot].lock().await;
             std::mem::swap(&mut removed, &mut *guard);
+            self.mark_dirty();
             removed
         })
     }
 
     fn remove_stack_specific(&self, slot: usize, amount: u8) -> InventoryFuture<'_, ItemStack> {
-        Box::pin(async move { split_stack(&self.items, slot, amount).await })
+        Box::pin(async move {
+            let res = split_stack(&self.items, slot, amount).await;
+            self.mark_dirty();
+            res
+        })
     }
 
     fn set_stack(&self, slot: usize, stack: ItemStack) -> InventoryFuture<'_, ()> {
         Box::pin(async move {
             *self.items[slot].lock().await = stack;
+            self.mark_dirty();
         })
     }
 
@@ -242,6 +248,7 @@ impl Clearable for BarrelBlockEntity {
             for slot in &self.items {
                 *slot.lock().await = ItemStack::EMPTY.clone();
             }
+            self.mark_dirty();
         })
     }
 }

pumpkin-world/src/block/entities/chest_like_block_entity.rs

@@ -38,9 +38,8 @@ macro_rules! impl_block_entity_for_chest {
             ) -> std::pin::Pin<Box<dyn std::future::Future<Output = ()> + Send + 'a>> {
                 use $crate::inventory::Inventory;
 
-                Box::pin(async move {
-                    self.write_data(nbt, &self.items, true).await;
-                })
+                // Write inventory data to NBT
+                self.write_inventory_nbt(nbt, true)
             }
 
             fn tick<'a>(
@@ -62,6 +61,11 @@ macro_rules! impl_block_entity_for_chest {
                 self.dirty.load(std::sync::atomic::Ordering::Relaxed)
             }
 
+            fn clear_dirty(&self) {
+                self.dirty
+                    .store(false, std::sync::atomic::Ordering::Relaxed);
+            }
+
             fn as_any(&self) -> &dyn std::any::Any {
                 self
             }
@@ -105,6 +109,7 @@ macro_rules! impl_inventory_for_chest {
                     let mut removed = ItemStack::EMPTY.clone();
                     let mut guard = self.items[slot].lock().await;
                     std::mem::swap(&mut removed, &mut *guard);
+                    self.mark_dirty();
                     removed
                 })
             }
@@ -114,9 +119,11 @@ macro_rules! impl_inventory_for_chest {
                 slot: usize,
                 amount: u8,
             ) -> $crate::inventory::InventoryFuture<'_, ItemStack> {
-                Box::pin(
-                    async move { $crate::inventory::split_stack(&self.items, slot, amount).await },
-                )
+                Box::pin(async move {
+                    let res = $crate::inventory::split_stack(&self.items, slot, amount).await;
+                    self.mark_dirty();
+                    res
+                })
             }
 
             fn set_stack(
@@ -126,6 +133,7 @@ macro_rules! impl_inventory_for_chest {
             ) -> $crate::inventory::InventoryFuture<'_, ()> {
                 Box::pin(async move {
                     *self.items[slot].lock().await = stack;
+                    self.mark_dirty();
                 })
             }
 
@@ -164,6 +172,7 @@ macro_rules! impl_clearable_for_chest {
                     for slot in &self.items {
                         *slot.lock().await = ItemStack::EMPTY.clone();
                     }
+                    <$struct_name as $crate::inventory::Inventory>::mark_dirty(self);
                 })
             }
         }

pumpkin-world/src/block/entities/chiseled_bookshelf.rs

@@ -63,7 +63,10 @@ impl BlockEntity for ChiseledBookshelfBlockEntity {
         nbt: &'a mut NbtCompound,
     ) -> Pin<Box<dyn Future<Output = ()> + Send + 'a>> {
         Box::pin(async move {
-            self.write_data(nbt, &self.items, true).await;
+            // Write inventory data to NBT
+            self.write_inventory_nbt(nbt, true).await;
+
+            // Save last interacted slot
             nbt.put_int(
                 LAST_INTERACTED_SLOT,
                 self.last_interacted_slot.load(Ordering::Relaxed).into(),
@@ -79,6 +82,10 @@ impl BlockEntity for ChiseledBookshelfBlockEntity {
         self.dirty.load(Ordering::Relaxed)
     }
 
+    fn clear_dirty(&self) {
+        self.dirty.store(false, Ordering::Relaxed);
+    }
+
     fn as_any(&self) -> &dyn std::any::Any {
         self
     }
@@ -156,17 +163,23 @@ impl Inventory for ChiseledBookshelfBlockEntity {
             let mut removed = ItemStack::EMPTY.clone();
             let mut guard = self.items[slot].lock().await;
             std::mem::swap(&mut removed, &mut *guard);
+            self.mark_dirty();
             removed
         })
     }
 
     fn remove_stack_specific(&self, slot: usize, amount: u8) -> InventoryFuture<'_, ItemStack> {
-        Box::pin(async move { split_stack(&self.items, slot, amount).await })
+        Box::pin(async move {
+            let res = split_stack(&self.items, slot, amount).await;
+            self.mark_dirty();
+            res
+        })
     }
 
     fn set_stack(&self, slot: usize, stack: ItemStack) -> InventoryFuture<'_, ()> {
         Box::pin(async move {
             *self.items[slot].lock().await = stack;
+            self.mark_dirty();
         })
     }
 
@@ -185,6 +198,7 @@ impl Clearable for ChiseledBookshelfBlockEntity {
             for slot in &self.items {
                 *slot.lock().await = ItemStack::EMPTY.clone();
             }
+            self.mark_dirty();
         })
     }
 }

pumpkin-world/src/block/entities/dropper.rs

@@ -22,11 +22,7 @@ impl BlockEntity for DropperBlockEntity {
         &'a self,
         nbt: &'a mut NbtCompound,
     ) -> Pin<Box<dyn Future<Output = ()> + Send + 'a>> {
-        Box::pin(async move {
-            self.write_data(nbt, &self.items, true).await;
-        })
-        // Safety precaution
-        //self.clear().await;
+        self.write_inventory_nbt(nbt, true)
     }
 
     fn from_nbt(nbt: &pumpkin_nbt::compound::NbtCompound, position: BlockPos) -> Self
@@ -60,6 +56,10 @@ impl BlockEntity for DropperBlockEntity {
         self.dirty.load(Ordering::Relaxed)
     }
 
+    fn clear_dirty(&self) {
+        self.dirty.store(false, Ordering::Relaxed);
+    }
+
     fn as_any(&self) -> &dyn std::any::Any {
         self
     }
@@ -120,17 +120,23 @@ impl Inventory for DropperBlockEntity {
             let mut removed = ItemStack::EMPTY.clone();
             let mut guard = self.items[slot].lock().await;
             std::mem::swap(&mut removed, &mut *guard);
+            self.mark_dirty();
             removed
         })
     }
 
     fn remove_stack_specific(&self, slot: usize, amount: u8) -> InventoryFuture<'_, ItemStack> {
-        Box::pin(async move { split_stack(&self.items, slot, amount).await })
+        Box::pin(async move {
+            let res = split_stack(&self.items, slot, amount).await;
+            self.mark_dirty();
+            res
+        })
     }
 
     fn set_stack(&self, slot: usize, stack: ItemStack) -> InventoryFuture<'_, ()> {
         Box::pin(async move {
             *self.items[slot].lock().await = stack;
+            self.mark_dirty();
         })
     }
 
@@ -149,6 +155,7 @@ impl Clearable for DropperBlockEntity {
             for slot in &self.items {
                 *slot.lock().await = ItemStack::EMPTY.clone();
             }
+            self.mark_dirty();
         })
     }
 }

pumpkin-world/src/block/entities/furnace_like_block_entity.rs

@@ -250,6 +250,7 @@ macro_rules! impl_clearable_for_cooking {
                     for slot in self.items.iter() {
                         *slot.lock().await = ItemStack::EMPTY.clone();
                     }
+                    self.mark_dirty();
                 })
             }
         }
@@ -304,6 +305,7 @@ macro_rules! impl_inventory_for_cooking {
                     let mut removed = ItemStack::EMPTY.clone();
                     let mut guard = self.items[slot].lock().await;
                     std::mem::swap(&mut removed, &mut *guard);
+                    self.mark_dirty();
                     removed
                 })
             }
@@ -313,9 +315,11 @@ macro_rules! impl_inventory_for_cooking {
                 slot: usize,
                 amount: u8,
             ) -> $crate::inventory::InventoryFuture<'_, ItemStack> {
-                Box::pin(
-                    async move { $crate::inventory::split_stack(&self.items, slot, amount).await },
-                )
+                Box::pin(async move {
+                    let res = $crate::inventory::split_stack(&self.items, slot, amount).await;
+                    self.mark_dirty();
+                    res
+                })
             }
 
             fn set_stack(
@@ -345,8 +349,10 @@ macro_rules! impl_inventory_for_cooking {
                             self.set_cooking_total_time(0);
                         }
                         self.set_cooking_time_spent(0);
-                        self.mark_dirty();
                     }
+
+                    // Always consider the inventory changed when setting a stack
+                    self.mark_dirty();
                 })
             }
 
@@ -490,7 +496,7 @@ macro_rules! impl_block_entity_for_cooking {
                     }
 
                     if is_dirty {
-                        self.is_dirty();
+                        self.mark_dirty();
                     }
                 })
             }
@@ -557,6 +563,7 @@ macro_rules! impl_block_entity_for_cooking {
                     nbt.put_short("cooking_time_spent", self.get_cooking_time_spent() as i16);
                     nbt.put_short("lit_total_time", self.get_lit_total_time() as i16);
                     nbt.put_short("lit_time_remaining", self.get_lit_time_remaining() as i16);
+
                     // Save RecipesUsed in vanilla format (map of recipe ID -> craft count)
                     // Scope the mutex guard so it's dropped before the await
                     {
@@ -575,7 +582,8 @@ macro_rules! impl_block_entity_for_cooking {
                             );
                         }
                     }
-                    self.write_data(nbt, &self.items, true).await;
+
+                    self.write_inventory_nbt(nbt, true).await;
                 })
             }
 

pumpkin-world/src/block/entities/hopper.rs

@@ -14,6 +14,7 @@ use std::any::Any;
 use std::array::from_fn;
 use std::pin::Pin;
 use std::sync::Arc;
+use std::sync::atomic::Ordering;
 use std::sync::atomic::{AtomicBool, AtomicI32, AtomicI64};
 use tokio::sync::Mutex;
 
@@ -44,17 +45,12 @@ impl BlockEntity for HopperBlockEntity {
         nbt: &'a mut NbtCompound,
     ) -> Pin<Box<dyn Future<Output = ()> + Send + 'a>> {
         Box::pin(async move {
-            self.write_data(nbt, &self.items, true).await;
             nbt.put(
                 "TransferCooldown",
-                NbtTag::Int(
-                    self.cooldown_time
-                        .load(std::sync::atomic::Ordering::Relaxed),
-                ),
+                NbtTag::Int(self.cooldown_time.load(Ordering::Relaxed)),
             );
+            self.write_inventory_nbt(nbt, true).await;
         })
-        // Safety precaution
-        //self.clear().await;
     }
 
     fn from_nbt(nbt: &pumpkin_nbt::compound::NbtCompound, position: BlockPos) -> Self
@@ -80,17 +76,10 @@ impl BlockEntity for HopperBlockEntity {
         world: &'a Arc<dyn SimpleWorld>,
     ) -> Pin<Box<dyn Future<Output = ()> + Send + 'a>> {
         Box::pin(async move {
-            self.ticked_game_time.store(
-                world.get_world_age().await,
-                std::sync::atomic::Ordering::Relaxed,
-            );
-            if self
-                .cooldown_time
-                .fetch_sub(1, std::sync::atomic::Ordering::Relaxed)
-                <= 0
-            {
-                self.cooldown_time
-                    .store(0, std::sync::atomic::Ordering::Relaxed);
+            self.ticked_game_time
+                .store(world.get_world_age().await, Ordering::Relaxed);
+            if self.cooldown_time.fetch_sub(1, Ordering::Relaxed) <= 0 {
+                self.cooldown_time.store(0, Ordering::Relaxed);
                 let state = HopperLikeProperties::from_state_id(
                     world.get_block_state(&self.position).await.id,
                     &Block::HOPPER,
@@ -118,7 +107,11 @@ impl BlockEntity for HopperBlockEntity {
     }
 
     fn is_dirty(&self) -> bool {
-        self.dirty.load(std::sync::atomic::Ordering::Relaxed)
+        self.dirty.load(Ordering::Relaxed)
+    }
+
+    fn clear_dirty(&self) {
+        self.dirty.store(false, Ordering::Relaxed);
     }
 
     fn as_any(&self) -> &dyn std::any::Any {
@@ -142,12 +135,7 @@ impl HopperBlockEntity {
         }
     }
     async fn try_move_items(&self, state: &HopperLikeProperties, world: &Arc<dyn SimpleWorld>) {
-        if self
-            .cooldown_time
-            .load(std::sync::atomic::Ordering::Relaxed)
-            <= 0
-            && state.enabled
-        {
+        if self.cooldown_time.load(Ordering::Relaxed) <= 0 && state.enabled {
             let mut success = false;
             if !self.is_empty().await {
                 success = self.eject_items(world).await;
@@ -156,8 +144,7 @@ impl HopperBlockEntity {
                 success |= self.suck_in_items(world).await;
             }
             if success {
-                self.cooldown_time
-                    .store(8, std::sync::atomic::Ordering::Relaxed);
+                self.cooldown_time.store(8, Ordering::Relaxed);
                 self.mark_dirty();
             }
         }
@@ -269,31 +256,18 @@ impl HopperBlockEntity {
                 if success {
                     if to_empty
                         && let Some(hopper) = to.as_any().downcast_ref::<Self>()
-                        && hopper
-                            .cooldown_time
-                            .load(std::sync::atomic::Ordering::Relaxed)
-                            <= 8
+                        && hopper.cooldown_time.load(Ordering::Relaxed) <= 8
                     {
                         if let Some(from_hopper) = from.as_any().downcast_ref::<Self>() {
-                            if from_hopper
-                                .cooldown_time
-                                .load(std::sync::atomic::Ordering::Relaxed)
-                                >= hopper
-                                    .cooldown_time
-                                    .load(std::sync::atomic::Ordering::Relaxed)
+                            if from_hopper.cooldown_time.load(Ordering::Relaxed)
+                                >= hopper.cooldown_time.load(Ordering::Relaxed)
                             {
-                                hopper
-                                    .cooldown_time
-                                    .store(7, std::sync::atomic::Ordering::Relaxed);
+                                hopper.cooldown_time.store(7, Ordering::Relaxed);
                             } else {
-                                hopper
-                                    .cooldown_time
-                                    .store(8, std::sync::atomic::Ordering::Relaxed);
+                                hopper.cooldown_time.store(8, Ordering::Relaxed);
                             }
                         } else {
-                            hopper
-                                .cooldown_time
-                                .store(8, std::sync::atomic::Ordering::Relaxed);
+                            hopper.cooldown_time.store(8, Ordering::Relaxed);
                         }
                     }
                     to.mark_dirty();
@@ -331,22 +305,28 @@ impl Inventory for HopperBlockEntity {
             let mut removed = ItemStack::EMPTY.clone();
             let mut guard = self.items[slot].lock().await;
             std::mem::swap(&mut removed, &mut *guard);
+            self.mark_dirty();
             removed
         })
     }
 
     fn remove_stack_specific(&self, slot: usize, amount: u8) -> InventoryFuture<'_, ItemStack> {
-        Box::pin(async move { split_stack(&self.items, slot, amount).await })
+        Box::pin(async move {
+            let res = split_stack(&self.items, slot, amount).await;
+            self.mark_dirty();
+            res
+        })
     }
 
     fn set_stack(&self, slot: usize, stack: ItemStack) -> InventoryFuture<'_, ()> {
         Box::pin(async move {
             *self.items[slot].lock().await = stack;
+            self.mark_dirty();
         })
     }
 
     fn mark_dirty(&self) {
-        self.dirty.store(true, std::sync::atomic::Ordering::Relaxed);
+        self.dirty.store(true, Ordering::Relaxed);
     }
 
     fn as_any(&self) -> &dyn Any {
@@ -360,6 +340,7 @@ impl Clearable for HopperBlockEntity {
             for slot in &self.items {
                 *slot.lock().await = ItemStack::EMPTY.clone();
             }
+            self.mark_dirty();
         })
     }
 }

pumpkin-world/src/block/entities/mod.rs

@@ -117,6 +117,11 @@ pub trait BlockEntity: Send + Sync {
         false
     }
 
+    fn clear_dirty(&self) {
+        // Default implementation does nothing
+        // Override in implementations that have a dirty flag
+    }
+
     fn as_any(&self) -> &dyn Any;
     fn to_property_delegate(self: Arc<Self>) -> Option<Arc<dyn PropertyDelegate>> {
         None

pumpkin-world/src/block/entities/shulker_box.rs

@@ -62,11 +62,7 @@ impl BlockEntity for ShulkerBoxBlockEntity {
         &'a self,
         nbt: &'a mut NbtCompound,
     ) -> Pin<Box<dyn Future<Output = ()> + Send + 'a>> {
-        Box::pin(async move {
-            self.write_data(nbt, &self.items, true).await;
-        })
-        // Safety precaution
-        //self.clear().await;
+        self.write_inventory_nbt(nbt, true)
     }
 
     fn tick<'a>(
@@ -98,7 +94,11 @@ impl BlockEntity for ShulkerBoxBlockEntity {
     }
 
     fn is_dirty(&self) -> bool {
-        self.dirty.load(std::sync::atomic::Ordering::Relaxed)
+        self.dirty.load(Ordering::Relaxed)
+    }
+
+    fn clear_dirty(&self) {
+        self.dirty.store(false, Ordering::Relaxed);
     }
 
     fn as_any(&self) -> &dyn std::any::Any {
@@ -202,17 +202,23 @@ impl Inventory for ShulkerBoxBlockEntity {
             let mut removed = ItemStack::EMPTY.clone();
             let mut guard = self.items[slot].lock().await;
             std::mem::swap(&mut removed, &mut *guard);
+            self.mark_dirty();
             removed
         })
     }
 
     fn remove_stack_specific(&self, slot: usize, amount: u8) -> InventoryFuture<'_, ItemStack> {
-        Box::pin(async move { split_stack(&self.items, slot, amount).await })
+        Box::pin(async move {
+            let res = split_stack(&self.items, slot, amount).await;
+            self.mark_dirty();
+            res
+        })
     }
 
     fn set_stack(&self, slot: usize, stack: ItemStack) -> InventoryFuture<'_, ()> {
         Box::pin(async move {
             *self.items[slot].lock().await = stack;
+            self.mark_dirty();
         })
     }
 
@@ -243,6 +249,7 @@ impl Clearable for ShulkerBoxBlockEntity {
             for slot in &self.items {
                 *slot.lock().await = ItemStack::EMPTY.clone();
             }
+            self.mark_dirty();
         })
     }
 }

pumpkin-world/src/chunk/format/mod.rs

@@ -70,11 +70,36 @@ impl Dirtiable for ChunkData {
     #[inline]
     fn mark_dirty(&self, flag: bool) {
         self.dirty.store(flag, Ordering::Relaxed);
+
+        if flag {
+            return;
+        }
+
+        // When marking chunk as clean, also clear all block entity dirty flags
+        if let Ok(block_entities) = self.block_entities.lock() {
+            for block_entity in block_entities.values() {
+                block_entity.clear_dirty();
+            }
+        }
     }
 
     #[inline]
     fn is_dirty(&self) -> bool {
-        self.dirty.load(Ordering::Relaxed)
+        // Check if chunk itself is dirty
+        if self.dirty.load(Ordering::Relaxed) {
+            return true;
+        }
+
+        // Also check if any block entities are dirty (e.g., inventory changes)
+        if let Ok(block_entities) = self.block_entities.lock() {
+            for block_entity in block_entities.values() {
+                if block_entity.is_dirty() {
+                    return true;
+                }
+            }
+        }
+
+        false
     }
 }
 

pumpkin-world/src/chunk_system/schedule.rs

@@ -8,6 +8,7 @@ use super::{
     ChunkLevel, ChunkListener, ChunkLoading, ChunkPos, HashMapType, HashSetType, IOLock,
     LevelChannel,
 };
+use crate::chunk::io::Dirtiable;
 use crate::level::{Level, SyncChunk};
 use dashmap::DashMap;
 use pumpkin_config::lighting::LightingEngineConfig;
@@ -431,7 +432,7 @@ impl GenerationSchedule {
                 match chunk {
                     Chunk::Level(sync_chunk) => {
                         // Only save level chunks that are marked dirty
-                        if sync_chunk.dirty.load(Relaxed) {
+                        if sync_chunk.is_dirty() {
                             chunks.push((*pos, Chunk::Level(sync_chunk.clone())));
                         }
                     }

pumpkin-world/src/inventory/inventory.rs

@@ -14,8 +14,6 @@ pub type InventoryFuture<'a, T> = Pin<Box<dyn Future<Output = T> + Send + 'a>>;
 pub trait Inventory: Send + Sync + Clearable {
     fn size(&self) -> usize;
 
-    // --- Asynchronous Methods (Using BlockFuture) ---
-
     fn is_empty(&self) -> InventoryFuture<'_, bool>;
 
     fn get_stack(&self, slot: usize) -> InventoryFuture<'_, Arc<Mutex<ItemStack>>>;
@@ -75,23 +73,19 @@ pub trait Inventory: Send + Sync + Clearable {
         })
     }
 
-    // --- Default Implementation: write_data (Using BlockFuture) ---
-    fn write_data(
-        &self,
-        nbt: &mut NbtCompound,
-        stacks: &[Arc<Mutex<ItemStack>>],
+    fn write_inventory_nbt<'a>(
+        &'a self,
+        nbt: &'a mut NbtCompound,
         include_empty: bool,
-    ) -> InventoryFuture<'_, ()> {
-        // Clone for the move block (requires `nbt` and `stacks` to be cloneable/to_owned)
-        let nbt = nbt.to_owned();
-        let stacks = stacks.to_owned();
-
+    ) -> InventoryFuture<'a, ()> {
         Box::pin(async move {
             let mut slots = Vec::new();
-            let mut nbt = nbt;
+            let size = self.size();
+
+            for i in 0..size {
+                let stack_lock = self.get_stack(i).await;
+                let stack = stack_lock.lock().await;
 
-            for (i, item) in stacks.iter().enumerate() {
-                let stack = item.lock().await;
                 if !stack.is_empty() {
                     let mut item_compound = NbtCompound::new();
                     item_compound.put_byte("Slot", i as i8);
@@ -100,16 +94,12 @@ pub trait Inventory: Send + Sync + Clearable {
                 }
             }
 
-            if !include_empty && slots.is_empty() {
-                return;
+            if include_empty || !slots.is_empty() {
+                nbt.put("Items", NbtTag::List(slots));
             }
-
-            nbt.put("Items", NbtTag::List(slots));
         })
     }
 
-    // --- Synchronous Methods (No Change) ---
-
     fn get_max_count_per_stack(&self) -> u8 {
         99
     }

pumpkin-world/src/item/mod.rs

@@ -332,23 +332,23 @@ impl ItemStack {
             return false;
         }
         for (id, data) in &self.patch {
-            let mut not_find = true;
+            let mut not_found = true;
             'out: for (other_id, other_data) in &other.patch {
                 if id == other_id {
                     if let (Some(data), Some(other_data)) = (data, other_data) {
-                        if data.equal(other_data.as_ref()) {
+                        if !data.equal(other_data.as_ref()) {
                             return false;
                         }
-                        not_find = false;
+                        not_found = false;
                         break 'out;
                     } else if data.is_none() && other_data.is_none() {
-                        not_find = false;
+                        not_found = false;
                         break 'out;
                     }
                     return false;
                 }
             }
-            if not_find {
+            if not_found {
                 return false;
             }
         }