refactored state_id to PlayerInventory

pumpkin-inventory/src/lib.rs

@@ -92,11 +92,6 @@ pub trait Container: Sync + Send {
         self.all_slots()
     }
 
-    fn advance_state_id(&mut self) -> i32;
-
-    fn reset_state_id(&mut self);
-    fn state_id(&self) -> i32;
-
     fn print_all_contents(&self) {
         self.all_slots_ref()
             .into_iter()
@@ -211,6 +206,10 @@ impl<'a, 'b> OptionallyCombinedContainer<'a, 'b> {
             container,
         }
     }
+    /// Returns None if the slot is in the players inventory, Returns Some(Option<&ItemStack>) if it's inside of the container
+    pub fn get_slot_excluding_inventory(&self, slot: usize) -> Option<Option<&ItemStack>> {
+        self.container.as_ref()?.all_slots_ref().get(slot).copied()
+    }
 }
 
 impl<'a> Container for OptionallyCombinedContainer<'a, 'a> {
@@ -244,16 +243,4 @@ impl<'a> Container for OptionallyCombinedContainer<'a, 'a> {
             None => self.inventory.all_slots_ref(),
         }
     }
-
-    fn advance_state_id(&mut self) -> i32 {
-        self.inventory.advance_state_id()
-    }
-
-    fn reset_state_id(&mut self) {
-        self.inventory.reset_state_id()
-    }
-
-    fn state_id(&self) -> i32 {
-        self.inventory.state_id()
-    }
 }

pumpkin-inventory/src/open_container.rs

@@ -47,17 +47,11 @@ impl OpenContainer {
     }
 }
 
-struct Chest {
+struct Chest([Option<ItemStack>; 27]);
-    slots: [Option<ItemStack>; 27],
-    state_id: i32,
-}
 
 impl Chest {
     pub fn new() -> Self {
-        Self {
+        Self([None; 27])
-            slots: [None; 27],
-            state_id: 0,
-        }
     }
 }
 impl Container for Chest {
@@ -66,23 +60,10 @@ impl Container for Chest {
     }
 
     fn all_slots(&mut self) -> Vec<&mut Option<ItemStack>> {
-        self.slots.iter_mut().collect()
+        self.0.iter_mut().collect()
     }
 
     fn all_slots_ref(&self) -> Vec<Option<&ItemStack>> {
-        self.slots.iter().map(|slot| slot.as_ref()).collect()
+        self.0.iter().map(|slot| slot.as_ref()).collect()
-    }
-
-    fn advance_state_id(&mut self) -> i32 {
-        self.state_id += 1;
-        self.state_id - 1
-    }
-
-    fn reset_state_id(&mut self) {
-        self.state_id = 0;
-    }
-
-    fn state_id(&self) -> i32 {
-        self.state_id
     }
 }

pumpkin-inventory/src/player.rs

@@ -13,7 +13,8 @@ pub struct PlayerInventory {
     offhand: Option<ItemStack>,
     // current selected slot in hotbar
     selected: usize,
-    state_id: i32,
+    pub state_id: u32,
+    // Notchian server wraps this value at 100, we can just keep it as a u8 that automatically wraps
     pub total_opened_containers: u8,
 }
 
@@ -166,16 +167,4 @@ impl Container for PlayerInventory {
     fn all_combinable_slots_mut(&mut self) -> Vec<&mut Option<ItemStack>> {
         self.items.iter_mut().collect()
     }
-
-    fn advance_state_id(&mut self) -> i32 {
-        self.state_id += 1;
-        self.state_id
-    }
-
-    fn reset_state_id(&mut self) {
-        self.state_id = 0;
-    }
-    fn state_id(&self) -> i32 {
-        self.state_id
-    }
 }

pumpkin-protocol/src/client/play/c_set_container_slot.rs

@@ -4,15 +4,15 @@ use pumpkin_macros::packet;
 use serde::Serialize;
 #[derive(Serialize)]
 #[packet(0x15)]
-pub struct CSetContainerSlot {
+pub struct CSetContainerSlot<'a> {
     window_id: i8,
     state_id: VarInt,
     slot: i16,
-    slot_data: Slot,
+    slot_data: &'a Slot,
 }
 
-impl CSetContainerSlot {
+impl<'a> CSetContainerSlot<'a> {
-    pub fn new(window_id: i8, state_id: i32, slot: usize, slot_data: Slot) -> Self {
+    pub fn new(window_id: i8, state_id: i32, slot: usize, slot_data: &'a Slot) -> Self {
         Self {
             window_id,
             state_id: state_id.into(),

pumpkin/src/client/container.rs

@@ -3,7 +3,9 @@ use crate::server::Server;
 use itertools::Itertools;
 use pumpkin_core::text::TextComponent;
 use pumpkin_core::GameMode;
-use pumpkin_inventory::container_click::{KeyClick, MouseClick, MouseDragState, MouseDragType};
+use pumpkin_inventory::container_click::{
+    Click, ClickType, KeyClick, MouseClick, MouseDragState, MouseDragType,
+};
 use pumpkin_inventory::drag_handler::DragHandler;
 use pumpkin_inventory::window_property::{WindowProperty, WindowPropertyTrait};
 use pumpkin_inventory::{container_click, InventoryError, OptionallyCombinedContainer};
@@ -14,7 +16,6 @@ use pumpkin_protocol::client::play::{
 use pumpkin_protocol::server::play::SClickContainer;
 use pumpkin_protocol::slot::Slot;
 use pumpkin_world::item::ItemStack;
-use std::ops::DerefMut;
 use std::sync::Mutex;
 
 impl Player {
@@ -24,7 +25,7 @@ impl Player {
         minecraft_menu_id: &str,
         window_title: Option<&str>,
     ) {
-        self.inventory.reset_state_id();
+        self.inventory.state_id = 0;
         let total_opened_containers = self.inventory.total_opened_containers;
         let mut container = self
             .get_open_container(server)
@@ -70,14 +71,13 @@ impl Player {
                 Slot::empty()
             }
         };
-
+        self.inventory.state_id += 1;
         let packet = CSetContainerContent::new(
             total_opened_containers,
-            self.inventory.state_id().into(),
+            (self.inventory.state_id as i32).into(),
             &slots,
             &carried_item,
         );
-        self.inventory.advance_state_id();
         self.client.send_packet(&packet);
     }
 
@@ -106,20 +106,14 @@ impl Player {
         server: &mut Server,
         packet: SClickContainer,
     ) -> Result<(), InventoryError> {
-        use container_click::*;
         let mut opened_container = self
             .get_open_container(server)
             .map(|container| container.lock().unwrap());
         let drag_handler = &server.drag_handler;
 
-        let current_state_id = if let Some(container) = opened_container.as_ref() {
+        let state_id = self.inventory.state_id;
-            container.state_id()
-        } else {
-            self.inventory.state_id()
-        };
         // This is just checking for regular desync, client hasn't done anything malicious
-        if current_state_id != packet.state_id.0 {
+        if state_id != packet.state_id.0 as u32 {
-            dbg!(current_state_id, packet.state_id.0);
             self.set_container_content(opened_container.as_deref_mut());
             return Ok(());
         }
@@ -174,9 +168,19 @@ impl Player {
             }
             ClickType::DropType(_drop_type) => todo!(),
         }?;
-        std::mem::drop(opened_container);
+        if let Some(mut opened_container) = opened_container {
-        if let container_click::Slot::Normal(slot) = click.slot {
+            if let container_click::Slot::Normal(slot_index) = click.slot {
-            self.send_container_changes(server, slot).await?;
+                let combined_container = OptionallyCombinedContainer::new(
+                    &mut self.inventory,
+                    Some(&mut opened_container),
+                );
+                if let Some(slot) = combined_container.get_slot_excluding_inventory(slot_index) {
+                    let slot = Slot::from(slot);
+                    drop(opened_container);
+                    self.send_container_changes(server, slot_index, slot)
+                        .await?;
+                }
+            }
         }
         Ok(())
     }
@@ -193,7 +197,7 @@ impl Player {
             container_click::Slot::Normal(slot) => {
                 container.handle_item_change(&mut self.carried_item, slot, mouse_click)
             }
-            container_click::Slot::OutsideInventory => todo!(),
+            container_click::Slot::OutsideInventory => Ok(()),
         }
     }
 
@@ -316,11 +320,8 @@ impl Player {
         &mut self,
         server: &Server,
         slot_index: usize,
+        slot: Slot,
     ) -> Result<(), InventoryError> {
-        // If we don't have a container open, then we don't need to share state
-        let Some(container) = self.get_open_container(server) else {
-            return Ok(());
-        };
         let player_ids = server
             .open_containers
             .get(&self.open_container.unwrap())
@@ -329,19 +330,16 @@ impl Player {
             .into_iter()
             .filter(|player_id| *player_id != self.entity_id())
             .collect_vec();
-        dbg!(&player_ids);
         let player_token = self.client.token;
 
         // TODO: Figure out better way to get only the players from player_ids
         // Also refactor out a better method to get individual advanced state ids
 
         let world = self.world.lock().await;
-        dbg!("here");
         let players = world
             .current_players
             .iter()
             .filter_map(|(token, player)| {
-                dbg!(token);
                 if *token != player_token {
                     let entity_id = player.lock().unwrap().entity_id();
                     if player_ids.contains(&entity_id) {
@@ -356,27 +354,17 @@ impl Player {
             .collect_vec();
         drop(world);
         for player in players {
-            dbg!("here");
             let mut player = player.lock().unwrap();
-            dbg!(player.entity_id());
             let total_opened_containers = player.inventory.total_opened_containers;
-            let mut container = container.lock().or(Err(InventoryError::LockError))?;
-            let mut combined_container = OptionallyCombinedContainer::new(
-                &mut player.inventory,
-                Some(container.deref_mut()),
-            );
-            let state_id = combined_container.advance_state_id();
-            let all_slots = combined_container.all_slots_ref();
 
-            let slot = Slot::from(all_slots[slot_index]);
+            player.inventory.state_id += 1;
             let packet = CSetContainerSlot::new(
-                dbg!(total_opened_containers) as i8,
+                total_opened_containers as i8,
-                state_id,
+                player.inventory.state_id as i32,
                 slot_index,
-                slot,
+                &slot,
             );
             player.client.send_packet(&packet);
-            drop(container)
         }
 
         Ok(())

pumpkin/src/client/player_packet.rs

@@ -533,6 +533,7 @@ impl Player {
     // But this is not possible yet
     pub fn handle_close_container(&mut self, server: &mut Server, packet: SCloseContainer) {
         // window_id 0 represents both 9x1 Generic AND inventory here
+        self.inventory.state_id = 0;
         if let Some(id) = self.open_container {
             if let Some(container) = server.open_containers.get_mut(&id) {
                 container.remove_player(self.entity_id())