Refactor Slot (#87)

* create ItemStack type

* move slot back into protocol

* change DropItemStack to use ItemStack, because it's imposible to drop nothing

examples/inventory_piano.rs

@@ -12,8 +12,8 @@ use valence::entity::{Entity, EntityId, EntityKind};
 use valence::player_list::PlayerListId;
 use valence::protocol::packets::c2s::play::ClickContainerMode;
 use valence::protocol::packets::s2c::play::SoundCategory;
+use valence::protocol::SlotId;
 use valence::server::{Server, SharedServer, ShutdownResult};
-use valence::slot::SlotId;
 use valence::text::{Color, TextFormat};
 use valence::{async_trait, ident};
 

src/client.rs

@@ -35,10 +35,9 @@ use crate::protocol::packets::s2c::play::{
     TeleportEntity, UnloadChunk, UpdateAttributes, UpdateEntityPosition,
     UpdateEntityPositionAndRotation, UpdateEntityRotation, UpdateTime,
 };
-use crate::protocol::{BoundedInt, BoundedString, ByteAngle, RawBytes, VarInt};
+use crate::protocol::{BoundedInt, BoundedString, ByteAngle, RawBytes, Slot, VarInt};
 use crate::server::{C2sPacketChannels, NewClientData, S2cPlayMessage, SharedServer};
 use crate::slab_versioned::{Key, VersionedSlab};
-use crate::slot::Slot;
 use crate::text::Text;
 use crate::util::{chunks_in_view_distance, is_chunk_in_view_distance};
 use crate::world::{WorldId, Worlds};
@@ -789,15 +788,9 @@ impl<C: Config> Client<C> {
                     let held = std::mem::replace(&mut self.cursor_held_item, Slot::Empty);
                     match held {
                         Slot::Empty => {}
-                        Slot::Present {
-                            item_id,
-                            item_count,
-                            nbt,
-                        } => self.events.push_back(ClientEvent::DropItemStack {
-                            item_id,
-                            item_count,
-                            nbt,
-                        }),
+                        Slot::Present(stack) => {
+                            self.events.push_back(ClientEvent::DropItemStack { stack })
+                        }
                     }
                 } else {
                     self.cursor_held_item = p.carried_item.clone();
@@ -981,15 +974,9 @@ impl<C: Config> Client<C> {
                         Slot::Empty => log::warn!(
                             "Invalid packet, creative client tried to drop a stack of nothing."
                         ),
-                        Slot::Present {
-                            item_id,
-                            item_count,
-                            nbt,
-                        } => self.events.push_back(ClientEvent::DropItemStack {
-                            item_id,
-                            item_count,
-                            nbt,
-                        }),
+                        Slot::Present(stack) => {
+                            self.events.push_back(ClientEvent::DropItemStack { stack })
+                        }
                     }
                 } else {
                     self.events.push_back(ClientEvent::SetSlotCreative {

src/client/event.rs

@@ -1,6 +1,5 @@
 use std::time::Duration;
 
-use valence_nbt::Compound;
 use vek::Vec3;
 
 use super::Client;
@@ -8,13 +7,13 @@ use crate::block_pos::BlockPos;
 use crate::config::Config;
 use crate::entity::types::Pose;
 use crate::entity::{Entity, EntityEvent, EntityId, TrackedData};
+use crate::itemstack::ItemStack;
 use crate::protocol::packets::c2s::play::ClickContainerMode;
 pub use crate::protocol::packets::c2s::play::{
     BlockFace, ChatMode, DisplayedSkinParts, Hand, MainHand, ResourcePackC2s as ResourcePackStatus,
 };
 pub use crate::protocol::packets::s2c::play::GameMode;
-use crate::protocol::VarInt;
-use crate::slot::{Slot, SlotId};
+use crate::protocol::{Slot, SlotId, VarInt};
 
 /// Represents an action performed by a client.
 ///
@@ -147,9 +146,7 @@ pub enum ClientEvent {
     /// user's inventory.
     DropItemStack {
         // TODO: maybe we could add `from_slot_id` to make validation easier
-        item_id: VarInt,
-        item_count: u8,
-        nbt: Option<Compound>,
+        stack: ItemStack,
     },
     /// The client is in creative mode, and is trying to set it's inventory slot
     /// to a value.

src/itemstack.rs

@@ -0,0 +1,9 @@
+use crate::nbt::Compound;
+use crate::protocol::VarInt;
+
+#[derive(Debug, Clone, PartialEq)]
+pub struct ItemStack {
+    pub item_id: VarInt,
+    pub item_count: u8,
+    pub nbt: Option<Compound>,
+}

src/lib.rs

@@ -100,6 +100,7 @@ pub mod config;
 pub mod dimension;
 pub mod entity;
 pub mod ident;
+pub mod itemstack;
 pub mod player_list;
 pub mod player_textures;
 #[allow(dead_code)]
@@ -109,7 +110,6 @@ pub mod server;
 mod slab;
 mod slab_rc;
 mod slab_versioned;
-pub mod slot;
 pub mod spatial_index;
 pub mod text;
 pub mod util;

src/protocol.rs

@@ -10,6 +10,7 @@ use arrayvec::ArrayVec;
 use bitvec::prelude::*;
 pub use byte_angle::ByteAngle;
 use byteorder::{BigEndian, ReadBytesExt, WriteBytesExt};
+pub use slot::{Slot, SlotId};
 use uuid::Uuid;
 use valence_nbt::Compound;
 pub use var_int::VarInt;
@@ -22,6 +23,7 @@ use crate::nbt;
 mod byte_angle;
 pub mod codec;
 pub mod packets;
+mod slot;
 mod var_int;
 mod var_long;
 

src/protocol/packets.rs

@@ -20,9 +20,9 @@ use crate::block_pos::BlockPos;
 use crate::ident::Ident;
 use crate::nbt::Compound;
 use crate::protocol::{
-    BoundedArray, BoundedInt, BoundedString, ByteAngle, Decode, Encode, RawBytes, VarInt, VarLong,
+    BoundedArray, BoundedInt, BoundedString, ByteAngle, Decode, Encode, RawBytes, Slot, VarInt,
+    VarLong,
 };
-use crate::slot::Slot;
 use crate::text::Text;
 
 /// Provides the name of a packet for debugging purposes.

src/protocol/slot.rs

@@ -2,6 +2,7 @@ use std::io::Write;
 
 use byteorder::ReadBytesExt;
 
+use crate::itemstack::ItemStack;
 use crate::nbt::Compound;
 use crate::protocol::{Decode, Encode, VarInt};
 
@@ -12,26 +13,18 @@ pub type SlotId = i16;
 pub enum Slot {
     #[default]
     Empty,
-    Present {
-        item_id: VarInt,
-        item_count: u8,
-        nbt: Option<Compound>,
-    },
+    Present(ItemStack),
 }
 
 impl Encode for Slot {
     fn encode(&self, w: &mut impl Write) -> anyhow::Result<()> {
         match self {
             Slot::Empty => false.encode(w),
-            Slot::Present {
-                item_id,
-                item_count,
-                nbt,
-            } => {
+            Slot::Present(s) => {
                 true.encode(w)?;
-                item_id.encode(w)?;
-                item_count.encode(w)?;
-                match &nbt {
+                s.item_id.encode(w)?;
+                s.item_count.encode(w)?;
+                match &s.nbt {
                     Some(n) => n.encode(w),
                     None => 0u8.encode(w),
                 }
@@ -46,7 +39,7 @@ impl Decode for Slot {
         if !present {
             return Ok(Slot::Empty);
         }
-        Ok(Slot::Present {
+        Ok(Slot::Present(ItemStack {
             item_id: VarInt::decode(r)?,
             item_count: u8::decode(r)?,
             nbt: if r.first() == Some(&0) {
@@ -55,6 +48,26 @@ impl Decode for Slot {
             } else {
                 Some(Compound::decode(r)?)
             },
-        })
+        }))
+    }
+}
+
+impl From<Option<ItemStack>> for Slot {
+    fn from(s: Option<ItemStack>) -> Self {
+        if let Some(s) = s {
+            Slot::Present(s)
+        } else {
+            Slot::Empty
+        }
+    }
+}
+
+impl From<Slot> for Option<ItemStack> {
+    fn from(s: Slot) -> Self {
+        if let Slot::Present(s) = s {
+            Some(s)
+        } else {
+            None
+        }
     }
 }