Fix: https://github.com/Snowiiii/Pumpkin/issues/270

extractor/gradle.properties

@@ -4,13 +4,11 @@ org.gradle.parallel=true
 # Fabric Properties
 # check these on https://modmuss50.me/fabric.html
 minecraft_version=1.21.4
-yarn_mappings=1.21.4+build.1
+yarn_mappings=1.21.4+build.2
 loader_version=0.16.9
 kotlin_loader_version=1.12.3+kotlin.2.0.21
 # Mod Properties
 mod_version=1.0-SNAPSHOT
 maven_group=de.snowii
 archives_base_name=extractor
-# Dependencies
+fabric_version=0.112.1+1.21.4
-# check this on https://modmuss50.me/fabric.html
-fabric_version=0.110.5+1.21.4

pumpkin-inventory/Cargo.toml

@@ -4,7 +4,8 @@ version.workspace = true
 edition.workspace = true
 
 [dependencies]
-# For items
+pumpkin-protocol = { path = "../pumpkin-protocol" }
+
 pumpkin-world = { path = "../pumpkin-world" }
 pumpkin-registry = {path = "../pumpkin-registry"}
 pumpkin-macros = { path = "../pumpkin-macros" }

pumpkin-inventory/src/container_click.rs

@@ -1,4 +1,5 @@
 use crate::InventoryError;
+use pumpkin_protocol::server::play::SlotActionType;
 use pumpkin_world::item::ItemStack;
 
 pub struct Click {
@@ -7,23 +8,22 @@ pub struct Click {
 }
 
 impl Click {
-    pub fn new(mode: u8, button: i8, slot: i16) -> Result<Self, InventoryError> {
+    pub fn new(mode: SlotActionType, button: i8, slot: i16) -> Result<Self, InventoryError> {
         match mode {
-            0 => Self::new_normal_click(button, slot),
+            SlotActionType::Pickup => Self::new_normal_click(button, slot),
             // Both buttons do the same here, so we omit it
-            1 => Self::new_shift_click(slot),
+            SlotActionType::QuickMove => Self::new_shift_click(slot),
-            2 => Self::new_key_click(button, slot),
+            SlotActionType::Swap => Self::new_key_click(button, slot),
-            3 => Ok(Self {
+            SlotActionType::Clone => Ok(Self {
                 click_type: ClickType::CreativePickItem,
                 slot: Slot::Normal(slot.try_into().or(Err(InventoryError::InvalidSlot))?),
             }),
-            4 => Self::new_drop_item(button),
+            SlotActionType::Throw => Self::new_drop_item(button),
-            5 => Self::new_drag_item(button, slot),
+            SlotActionType::QuickCraft => Self::new_drag_item(button, slot),
-            6 => Ok(Self {
+            SlotActionType::PickupAll => Ok(Self {
                 click_type: ClickType::DoubleClick,
                 slot: Slot::Normal(slot.try_into().or(Err(InventoryError::InvalidSlot))?),
             }),
-            _ => Err(InventoryError::InvalidPacket),
         }
     }
 
@@ -31,7 +31,7 @@ impl Click {
         let slot = match slot {
             -999 => Slot::OutsideInventory,
             _ => {
-                let slot = slot.try_into().or(Err(InventoryError::InvalidSlot))?;
+                let slot = slot.try_into().unwrap_or(0);
                 Slot::Normal(slot)
             }
         };

pumpkin-protocol/src/packet_decoder.rs

@@ -120,17 +120,12 @@ impl PacketDecoder {
     pub fn set_encryption(&mut self, key: Option<&[u8; 16]>) {
         if let Some(key) = key {
             assert!(self.cipher.is_none(), "encryption is already enabled");
-
             let mut cipher = Cipher::new_from_slices(key, key).expect("invalid key");
-
             // Don't forget to decrypt the data we already have.
-
             Self::decrypt_bytes(&mut cipher, &mut self.buf);
-
             self.cipher = Some(cipher);
         } else {
             assert!(self.cipher.is_some(), "encryption is already disabled");
-
             self.cipher = None;
         }
     }

pumpkin-protocol/src/packet_encoder.rs

@@ -9,13 +9,13 @@ use crate::{ClientPacket, VarInt, MAX_PACKET_SIZE};
 
 type Cipher = cfb8::Encryptor<aes::Aes128>;
 
-// Encoder: Server -> Client
+/// Encoder: Server -> Client
-// Supports ZLib endecoding/compression
+/// Supports ZLib endecoding/compression
-// Supports Aes128 Encryption
+/// Supports Aes128 Encryption
 pub struct PacketEncoder {
     buf: BytesMut,
     compress_buf: Vec<u8>,
-    compression: Option<CompressionInfo>,
+    compression_threshold: Option<u32>,
     cipher: Option<Cipher>,
     compressor: Compressor, // Reuse the compressor for all packets
 }
@@ -27,25 +27,58 @@ impl Default for PacketEncoder {
         Self {
             buf: BytesMut::with_capacity(1024),
             compress_buf: Vec::with_capacity(1024),
-            compression: None,
+            compression_threshold: None,
             cipher: None,
-            compressor: Compressor::new(CompressionLvl::fastest()), // init compressor with no compression level
+            compressor: Compressor::new(CompressionLvl::fastest()), // init compressor with fastest compression level
         }
     }
 }
 
 impl PacketEncoder {
+    /// Appends a Clientbound `ClientPacket` to the internal buffer and applies compression when needed.
+    ///
+    /// If compression is enabled and the packet size exceeds the threshold, the packet is compressed.
+    /// The packet is prefixed with its length and, if compressed, the uncompressed data length.
+    /// The packet format is as follows:
+    ///
+    /// **Uncompressed:**
+    /// ```
+    /// +-----------------------+
+    /// | Packet Length (VarInt)|
+    /// +-----------------------+
+    /// | Packet ID (VarInt)    |
+    /// +-----------------------+
+    /// | Data (Byte Array)     |
+    /// +-----------------------+
+    /// ```
+    ///
+    /// **Compressed:**
+    /// ```
+    /// +-----------------------+
+    /// | Packet Length (VarInt) |
+    /// +-----------------------+
+    /// | Data Length (VarInt)   |
+    /// +-----------------------+
+    /// | Packet ID (VarInt)     |
+    /// +-----------------------+
+    /// | Data (Byte Array)      |
+    /// +-----------------------+
+    ///
+    /// *   `Packet Length`: The total length of the packet *excluding* the `Packet Length` field itself.
+    /// *   `Data Length`: (Only present in compressed packets) The length of the uncompressed `Packet ID` and `Data`.
+    /// *   `Packet ID`: The ID of the packet.
+    /// *   `Data`: The packet's data.
+    /// ```
     pub fn append_packet<P: ClientPacket>(&mut self, packet: &P) -> Result<(), PacketEncodeError> {
         let start_len = self.buf.len();
         // Write the Packet ID first
         VarInt(P::PACKET_ID).encode(&mut self.buf);
         // Now write the packet into an empty buffer
         packet.write(&mut self.buf);
-
         let data_len = self.buf.len() - start_len;
 
-        if let Some(compression) = &self.compression {
+        if let Some(compression_threshold) = self.compression_threshold {
-            if data_len > compression.threshold as usize {
+            if data_len > compression_threshold as usize {
                 // Get the data to compress
                 let data_to_compress = &self.buf[start_len..];
 
@@ -73,7 +106,7 @@ impl PacketEncoder {
                 let packet_len = data_len_size + compressed_size;
 
                 if packet_len >= MAX_PACKET_SIZE as usize {
-                    return Err(PacketEncodeError::TooLong);
+                    return Err(PacketEncodeError::TooLong(packet_len));
                 }
 
                 self.buf.truncate(start_len);
@@ -86,7 +119,7 @@ impl PacketEncoder {
                 let packet_len = data_len_size + data_len;
 
                 if packet_len >= MAX_PACKET_SIZE as usize {
-                    Err(PacketEncodeError::TooLong)?
+                    Err(PacketEncodeError::TooLong(packet_len))?
                 }
 
                 let packet_len_size = VarInt(packet_len as i32).written_size();
@@ -110,7 +143,7 @@ impl PacketEncoder {
         let packet_len = data_len;
 
         if packet_len >= MAX_PACKET_SIZE as usize {
-            Err(PacketEncodeError::TooLong)?
+            Err(PacketEncodeError::TooLong(packet_len))?
         }
 
         let packet_len_size = VarInt(packet_len as i32).written_size();
@@ -124,6 +157,7 @@ impl PacketEncoder {
         Ok(())
     }
 
+    /// Enable encryption for taking all packets buffer `
     pub fn set_encryption(&mut self, key: Option<&[u8; 16]>) {
         if let Some(key) = key {
             assert!(self.cipher.is_none(), "encryption is already enabled");
@@ -136,23 +170,37 @@ impl PacketEncoder {
         }
     }
 
-    /// Enables ZLib Compression
+    /// Enables or disables Zlib compression with the given options.
+    ///
+    /// If `compression` is `Some`, compression is enabled with the provided
+    /// options. If `compression` is `None`, compression is disabled.
     pub fn set_compression(&mut self, compression: Option<CompressionInfo>) {
-        self.compression = compression;
-
         // Reset the compressor with the new compression level
-        if let Some(compression) = &self.compression {
+        if let Some(compression) = &compression {
+            self.compression_threshold = Some(compression.threshold);
             let compression_level = compression.level as i32;
-
             let level = match CompressionLvl::new(compression_level) {
                 Ok(level) => level,
-                Err(_) => return,
+                Err(error) => {
+                    log::error!("Invalid compression level {:?}", error);
+                    return;
+                }
             };
-
             self.compressor = Compressor::new(level);
+        } else {
+            self.compression_threshold = None;
         }
     }
 
+    /// Encrypts the data in the internal buffer and returns it as a `BytesMut`.
+    ///
+    /// If a cipher is set, the data is encrypted in-place using block cipher encryption.
+    /// The buffer is processed in chunks of the cipher's block size. If the buffer's
+    /// length is not a multiple of the block size, the last partial block is *not* encrypted.
+    /// It's important to ensure that the data being encrypted is padded appropriately
+    /// beforehand if necessary.
+    ///
+    /// If no cipher is set, the buffer is returned as is.
     pub fn take(&mut self) -> BytesMut {
         if let Some(cipher) = &mut self.cipher {
             for chunk in self.buf.chunks_mut(Cipher::block_size()) {
@@ -165,11 +213,12 @@ impl PacketEncoder {
     }
 }
 
+/// Errors that can occur during packet encoding.
 #[derive(Error, Debug)]
 pub enum PacketEncodeError {
-    #[error("packet exceeds maximum length")]
+    #[error("Packet exceeds maximum length: {0}")]
-    TooLong,
+    TooLong(usize),
-    #[error("compression failed {0}")]
+    #[error("Compression failed {0}")]
     CompressionFailed(String),
 }
 

pumpkin-protocol/src/server/play/s_click_container.rs

@@ -1,17 +1,18 @@
 use crate::slot::Slot;
 use crate::VarInt;
+use num_derive::FromPrimitive;
+use num_traits::FromPrimitive;
 use pumpkin_macros::server_packet;
 use serde::de::SeqAccess;
 use serde::{de, Deserialize};
 
-#[derive(Debug)]
 #[server_packet("play:container_click")]
 pub struct SClickContainer {
     pub window_id: VarInt,
     pub state_id: VarInt,
     pub slot: i16,
     pub button: i8,
-    pub mode: VarInt,
+    pub mode: SlotActionType,
     pub length_of_array: VarInt,
     pub array_of_changed_slots: Vec<(i16, Slot)>,
     pub carried_item: Slot,
@@ -73,7 +74,8 @@ impl<'de> Deserialize<'de> for SClickContainer {
                     state_id,
                     slot,
                     button,
-                    mode,
+                    mode: SlotActionType::from_i32(mode.0)
+                        .expect("Invalid Slot action, TODO better error handling ;D"),
                     length_of_array,
                     array_of_changed_slots,
                     carried_item,
@@ -84,3 +86,24 @@ impl<'de> Deserialize<'de> for SClickContainer {
         deserializer.deserialize_seq(Visitor)
     }
 }
+
+#[derive(Deserialize, FromPrimitive)]
+pub enum SlotActionType {
+    /// Performs a normal slot click. This can pickup or place items in the slot, possibly merging the cursor stack into the slot, or swapping the slot stack with the cursor stack if they can't be merged.
+    Pickup,
+    /// Performs a shift-click. This usually quickly moves items between the player's inventory and the open screen handler.
+    QuickMove,
+    /// Exchanges items between a slot and a hotbar slot. This is usually triggered by the player pressing a 1-9 number key while hovering over a slot.
+    /// When the action type is swap, the click data is the hotbar slot to swap with (0-8).
+    Swap,
+    /// Clones the item in the slot. Usually triggered by middle clicking an item in creative mode.
+    Clone,
+    /// Throws the item out of the inventory. This is usually triggered by the player pressing Q while hovering over a slot, or clicking outside the window.
+    /// When the action type is throw, the click data determines whether to throw a whole stack (1) or a single item from that stack (0).
+    Throw,
+    /// Drags items between multiple slots. This is usually triggered by the player clicking and dragging between slots.
+    /// This action happens in 3 stages. Stage 0 signals that the drag has begun, and stage 2 signals that the drag has ended. In between multiple stage 1s signal which slots were dragged on.
+    QuickCraft,
+    /// Replenishes the cursor stack with items from the screen handler. This is usually triggered by the player double clicking
+    PickupAll,
+}

pumpkin-protocol/src/var_int.rs

@@ -14,7 +14,7 @@ pub type VarIntType = i32;
 pub struct VarInt(pub VarIntType);
 
 impl VarInt {
-    /// The maximum number of bytes a `VarInt`
+    /// The maximum number of bytes a `VarInt` can occupy.
     pub const MAX_SIZE: usize = 5;
 
     /// Returns the exact number of bytes this varint will write when
@@ -86,7 +86,7 @@ impl From<VarInt> for i32 {
 
 #[derive(Copy, Clone, PartialEq, Eq, Debug, Error)]
 pub enum VarIntDecodeError {
-    #[error("incomplete VarInt decode")]
+    #[error("Incomplete VarInt decode")]
     Incomplete,
     #[error("VarInt is too large")]
     TooLarge,

pumpkin/src/client/container.rs

@@ -134,15 +134,7 @@ impl Player {
             return Err(InventoryError::ClosedContainerInteract(self.entity_id()));
         }
 
-        let click = Click::new(
+        let click = Click::new(packet.mode, packet.button, packet.slot)?;
-            packet
-                .mode
-                .0
-                .try_into()
-                .expect("Mode can only be between 0-6"),
-            packet.button,
-            packet.slot,
-        )?;
         let (crafted_item, crafted_item_slot) = {
             let mut inventory = self.inventory().lock().await;
             let combined =