From d518732a76304a75d94020dc71bd13a6e1c122b9 Mon Sep 17 00:00:00 2001 From: kralverde <80051564+kralverde@users.noreply.github.com> Date: Sat, 1 Mar 2025 08:09:14 -1000 Subject: [PATCH] Fix dupe, double click, and shift click (re-PR) (#591) * Fix dupe, doubble click and shift click * Fix inventory desync * Fix clippy * refactor magic numbers * fix clippy * refactor item logic * fix inventory * prefer main inventory over offhand * Set slot stack to None if empty * fix container deadlock * fix linter * fix lint pt 2 --------- Co-authored-by: 4lve <72332750+4lve@users.noreply.github.com> Co-authored-by: Alexander Medvedev --- pumpkin-data/build/item.rs | 18 +- pumpkin-data/src/tag.rs | 19 +- pumpkin-inventory/src/container_click.rs | 52 ++- pumpkin-inventory/src/crafting.rs | 16 +- pumpkin-inventory/src/drag_handler.rs | 68 ++-- pumpkin-inventory/src/lib.rs | 74 ++-- pumpkin-inventory/src/open_container.rs | 67 ++-- pumpkin-inventory/src/player.rs | 231 ++++++------ pumpkin-inventory/src/window_property.rs | 1 + pumpkin-macros/src/lib.rs | 2 +- .../src/client/config/update_tags.rs | 18 +- pumpkin-protocol/src/codec/slot.rs | 49 ++- .../src/server/play/click_container.rs | 3 +- pumpkin-util/src/lib.rs | 56 +++ pumpkin-world/src/item/categories.rs | 79 ++-- pumpkin-world/src/item/mod.rs | 8 +- pumpkin/src/block/mod.rs | 7 +- pumpkin/src/command/args/resource/item.rs | 17 +- pumpkin/src/command/commands/give.rs | 2 +- pumpkin/src/entity/item.rs | 164 +++++--- pumpkin/src/entity/player.rs | 19 +- pumpkin/src/net/container.rs | 351 +++++++++++------- pumpkin/src/net/packet/play.rs | 102 ++--- 23 files changed, 859 insertions(+), 564 deletions(-) diff --git a/pumpkin-data/build/item.rs b/pumpkin-data/build/item.rs index 344034426..e22a3c946 100644 --- a/pumpkin-data/build/item.rs +++ b/pumpkin-data/build/item.rs @@ -225,6 +225,7 @@ pub(crate) fn build() -> TokenStream { constants.extend(quote! { pub const #const_ident: Item = Item { id: #id_lit, + registry_key: #name, components: #components_tokens }; }); @@ -240,10 +241,12 @@ pub(crate) fn build() -> TokenStream { quote! { use pumpkin_util::text::TextComponent; + use crate::tag::{Tagable, RegistryKey}; - #[derive(Clone, Copy, Debug)] + #[derive(Clone, Debug)] pub struct Item { pub id: u16, + pub registry_key: &'static str, pub components: ItemComponents, } @@ -313,7 +316,7 @@ pub(crate) fn build() -> TokenStream { } #[doc = r" Try to parse a Item from a resource location string"] - pub fn from_name(name: &str) -> Option { + pub fn from_registry_key(name: &str) -> Option { match name { #type_from_name _ => None @@ -326,7 +329,18 @@ pub(crate) fn build() -> TokenStream { _ => None } } + } + impl Tagable for Item { + #[inline] + fn tag_key() -> RegistryKey { + RegistryKey::Item + } + + #[inline] + fn registry_key(&self) -> &str { + self.registry_key + } } } } diff --git a/pumpkin-data/src/tag.rs b/pumpkin-data/src/tag.rs index 0cd26b5d6..9ac94551e 100644 --- a/pumpkin-data/src/tag.rs +++ b/pumpkin-data/src/tag.rs @@ -39,14 +39,25 @@ impl RegistryKey { } } +#[allow(dead_code)] +pub trait Tagable { + fn tag_key() -> RegistryKey; + fn registry_key(&self) -> &str; + + /// Returns none if tag does not exist + fn is_tagged_with(&self, tag: &str) -> Option { + let tag = tag.strip_prefix("#").unwrap_or(tag); + let items = get_tag_values(Self::tag_key(), tag)?; + Some(items.iter().any(|elem| elem == self.registry_key())) + } +} + const TAG_JSON: &str = include_str!("../../assets/tags.json"); -#[expect(clippy::type_complexity)] -pub static TAGS: LazyLock>>>> = +pub static TAGS: LazyLock>>> = LazyLock::new(|| serde_json::from_str(TAG_JSON).expect("Valid tag collections")); -#[allow(dead_code)] -pub fn get_tag_values(tag_category: RegistryKey, tag: &str) -> Option<&Vec>> { +pub fn get_tag_values(tag_category: RegistryKey, tag: &str) -> Option<&Vec> { TAGS.get(&tag_category) .expect("Should deserialize all tag categories") .get(tag) diff --git a/pumpkin-inventory/src/container_click.rs b/pumpkin-inventory/src/container_click.rs index ec1d70fea..2bb7568f4 100644 --- a/pumpkin-inventory/src/container_click.rs +++ b/pumpkin-inventory/src/container_click.rs @@ -1,12 +1,20 @@ -use crate::InventoryError; +use crate::{InventoryError, player::SLOT_INDEX_OUTSIDE}; use pumpkin_protocol::server::play::SlotActionType; use pumpkin_world::item::ItemStack; +#[derive(Debug)] pub struct Click { pub slot: Slot, pub click_type: ClickType, } +const BUTTON_CLICK_LEFT: i8 = 0; +const BUTTON_CLICK_RIGHT: i8 = 1; + +const KEY_CLICK_OFFHAND: i8 = 40; +const KEY_CLICK_HOTBAR_START: i8 = 0; +const KEY_CLICK_HOTBAR_END: i8 = 9; + impl Click { pub fn new(mode: SlotActionType, button: i8, slot: i16) -> Result { match mode { @@ -18,7 +26,7 @@ impl Click { click_type: ClickType::CreativePickItem, slot: Slot::Normal(slot.try_into().or(Err(InventoryError::InvalidSlot))?), }), - SlotActionType::Throw => Self::new_drop_item(button), + SlotActionType::Throw => Self::new_drop_item(button, slot), SlotActionType::QuickCraft => Self::new_drag_item(button, slot), SlotActionType::PickupAll => Ok(Self { click_type: ClickType::DoubleClick, @@ -29,15 +37,15 @@ impl Click { fn new_normal_click(button: i8, slot: i16) -> Result { let slot = match slot { - -999 => Slot::OutsideInventory, + SLOT_INDEX_OUTSIDE => Slot::OutsideInventory, _ => { let slot = slot.try_into().unwrap_or(0); Slot::Normal(slot) } }; let button = match button { - 0 => MouseClick::Left, - 1 => MouseClick::Right, + BUTTON_CLICK_LEFT => MouseClick::Left, + BUTTON_CLICK_RIGHT => MouseClick::Right, _ => Err(InventoryError::InvalidPacket)?, }; Ok(Self { @@ -55,8 +63,10 @@ impl Click { fn new_key_click(button: i8, slot: i16) -> Result { let key = match button { - 0..9 => KeyClick::Slot(button.try_into().or(Err(InventoryError::InvalidSlot))?), - 40 => KeyClick::Offhand, + KEY_CLICK_HOTBAR_START..KEY_CLICK_HOTBAR_END => { + KeyClick::Slot(button.try_into().or(Err(InventoryError::InvalidSlot))?) + } + KEY_CLICK_OFFHAND => KeyClick::Offhand, _ => Err(InventoryError::InvalidSlot)?, }; @@ -66,15 +76,18 @@ impl Click { }) } - fn new_drop_item(button: i8) -> Result { - let drop_type = match button { - 0 => DropType::SingleItem, - 1 => DropType::FullStack, - _ => Err(InventoryError::InvalidPacket)?, + fn new_drop_item(button: i8, slot: i16) -> Result { + let drop_type = DropType::from_i8(button)?; + let slot = match slot { + SLOT_INDEX_OUTSIDE => Slot::OutsideInventory, + _ => { + let slot = slot.try_into().unwrap_or(0); + Slot::Normal(slot) + } }; Ok(Self { click_type: ClickType::DropType(drop_type), - slot: Slot::OutsideInventory, + slot, }) } @@ -120,7 +133,7 @@ pub enum KeyClick { Slot(u8), Offhand, } -#[derive(Copy, Clone)] +#[derive(Debug, Copy, Clone)] pub enum Slot { Normal(usize), OutsideInventory, @@ -131,6 +144,17 @@ pub enum DropType { SingleItem, FullStack, } + +impl DropType { + fn from_i8(value: i8) -> Result { + Ok(match value { + 0 => Self::SingleItem, + 1 => Self::FullStack, + _ => return Err(InventoryError::InvalidPacket), + }) + } +} + #[derive(Debug, PartialEq)] pub enum MouseDragType { Left, diff --git a/pumpkin-inventory/src/crafting.rs b/pumpkin-inventory/src/crafting.rs index b5fcd1195..91c5664cf 100644 --- a/pumpkin-inventory/src/crafting.rs +++ b/pumpkin-inventory/src/crafting.rs @@ -7,7 +7,7 @@ use pumpkin_world::item::ItemStack; use rayon::prelude::*; #[inline(always)] -fn check_ingredient_type(ingredient_type: &TagType, input: ItemStack) -> bool { +fn check_ingredient_type(ingredient_type: &TagType, input: &ItemStack) -> bool { match ingredient_type { TagType::Tag(tag) => { let _items = match get_tag_values(RegistryKey::Item, tag) { @@ -18,11 +18,13 @@ fn check_ingredient_type(ingredient_type: &TagType, input: ItemStack) -> bool { false // items.iter().any(|tag| check_ingredient_type(&tag, input)) } - TagType::Item(item) => Item::from_name(item).is_some_and(|item| item.id == input.item.id), + TagType::Item(item) => { + Item::from_registry_key(item).is_some_and(|item| item.id == input.item.id) + } } } -pub fn check_if_matches_crafting(input: [[Option; 3]; 3]) -> Option { +pub fn check_if_matches_crafting(input: [[Option<&ItemStack>; 3]; 3]) -> Option { let input = flatten_3x3(input); RECIPES .par_iter() @@ -53,18 +55,18 @@ pub fn check_if_matches_crafting(input: [[Option; 3]; 3]) -> Option Some(ItemStack { - item: Item::from_name(id).unwrap(), + item: Item::from_registry_key(id).unwrap(), item_count: 1, }), RecipeResult::Many { id, count, .. } => Some(ItemStack { - item: Item::from_name(id).unwrap(), + item: Item::from_registry_key(id).unwrap(), item_count: *count, }), RecipeResult::Special => None, })? } -fn ingredient_slot_check(recipe_item: &RegistryEntryList, input: ItemStack) -> bool { +fn ingredient_slot_check(recipe_item: &RegistryEntryList, input: &ItemStack) -> bool { match recipe_item { RegistryEntryList::Single(ingredient) => check_ingredient_type(ingredient, input), RegistryEntryList::Many(ingredients) => ingredients @@ -73,7 +75,7 @@ fn ingredient_slot_check(recipe_item: &RegistryEntryList, input: ItemStack) -> b } } fn shapeless_crafting_match( - input: [[Option; 3]; 3], + input: [[Option<&ItemStack>; 3]; 3], pattern: &[[[Option; 3]; 3]], ) -> bool { let mut pattern: Vec = pattern diff --git a/pumpkin-inventory/src/drag_handler.rs b/pumpkin-inventory/src/drag_handler.rs index aa403772b..f3a3b4885 100644 --- a/pumpkin-inventory/src/drag_handler.rs +++ b/pumpkin-inventory/src/drag_handler.rs @@ -71,10 +71,6 @@ impl DragHandler { Err(InventoryError::MultiplePlayersDragging)? } let mut slots = container.all_slots(); - let slots_cloned: Vec> = slots - .iter() - .map(|stack| stack.map(|item| item.to_owned())) - .collect(); let Some(carried_item) = maybe_carried_item else { return Ok(()); }; @@ -83,27 +79,27 @@ impl DragHandler { // Checked in any function that uses this function. MouseDragType::Middle => { for slot in &drag.slots { - *slots[*slot] = *maybe_carried_item; + *slots[*slot] = maybe_carried_item.clone(); } } MouseDragType::Right => { - let mut single_item = *carried_item; - single_item.item_count = 1; - let changing_slots = - drag.possibly_changing_slots(&slots_cloned, carried_item.item.id); - changing_slots.for_each(|slot| { + drag.possibly_changing_slots(slots.as_ref(), carried_item.item.id); + changing_slots.into_iter().for_each(|slot| { if carried_item.item_count != 0 { carried_item.item_count -= 1; if let Some(stack) = &mut slots[slot] { // TODO: Check for stack max here - if stack.item_count + 1 < 64 { + if stack.item_count + 1 < stack.item.components.max_stack_size { stack.item_count += 1; } else { carried_item.item_count += 1; } } else { - *slots[slot] = Some(single_item) + *slots[slot] = Some(ItemStack { + item: carried_item.item.clone(), + item_count: 1, + }) } } }); @@ -116,9 +112,8 @@ impl DragHandler { // TODO: Handle dragging a stack with greater amount than item allows as max unstackable // In that specific case, follow MouseDragType::Right behaviours instead! - let changing_slots = - drag.possibly_changing_slots(&slots_cloned, carried_item.item.id); - let amount_of_slots = changing_slots.clone().count(); + let changing_slots = drag.possibly_changing_slots(&slots, carried_item.item.id); + let amount_of_slots = changing_slots.len(); let (amount_per_slot, remainder) = if amount_of_slots == 0 { // TODO: please work lol (1, 0) @@ -128,9 +123,13 @@ impl DragHandler { carried_item.item_count.rem_euclid(amount_of_slots as u8), ) }; - let mut item_in_each_slot = *carried_item; - item_in_each_slot.item_count = amount_per_slot; - changing_slots.for_each(|slot| *slots[slot] = Some(item_in_each_slot)); + changing_slots.into_iter().for_each(|slot| { + if let Some(stack) = slots[slot].as_mut() { + debug_assert!(stack.item.id == carried_item.item.id); + // TODO: Handle max stack size + stack.item_count += amount_per_slot; + } + }); if remainder > 0 { carried_item.item_count = remainder; @@ -150,24 +149,27 @@ struct Drag { } impl Drag { - fn possibly_changing_slots<'a>( - &'a self, - slots: &'a [Option], + fn possibly_changing_slots( + &self, + slots: &[&mut Option], carried_item_id: u16, - ) -> impl Iterator + 'a + Clone { - self.slots.iter().filter_map(move |slot_index| { - let slot = &slots[*slot_index]; + ) -> Box<[usize]> { + self.slots + .iter() + .filter_map(move |slot_index| { + let slot = &slots[*slot_index]; - match slot { - Some(item_slot) => { - if item_slot.item.id == carried_item_id { - Some(*slot_index) - } else { - None + match slot { + Some(item_slot) => { + if item_slot.item.id == carried_item_id { + Some(*slot_index) + } else { + None + } } + None => Some(*slot_index), } - None => Some(*slot_index), - } - }) + }) + .collect() } } diff --git a/pumpkin-inventory/src/lib.rs b/pumpkin-inventory/src/lib.rs index ef2b652e9..6fc2780d4 100644 --- a/pumpkin-inventory/src/lib.rs +++ b/pumpkin-inventory/src/lib.rs @@ -29,7 +29,7 @@ pub trait Container: Sync + Send { mouse_click: MouseClick, taking_crafted: bool, ) -> Result<(), InventoryError> { - let mut all_slots = self.all_slots(); + let all_slots = self.all_slots(); if slot > all_slots.len() { Err(InventoryError::InvalidSlot)? } @@ -50,9 +50,9 @@ pub trait Container: Sync + Send { Ok(()) } - fn all_slots(&mut self) -> Vec<&mut Option>; + fn all_slots(&mut self) -> Box<[&mut Option]>; - fn all_slots_ref(&self) -> Vec>; + fn all_slots_ref(&self) -> Box<[Option<&ItemStack>]>; fn clear_all_slots(&mut self) { let all_slots = self.all_slots(); @@ -61,11 +61,11 @@ pub trait Container: Sync + Send { } } - fn all_combinable_slots(&self) -> Vec> { + fn all_combinable_slots(&self) -> Box<[Option<&ItemStack>]> { self.all_slots_ref() } - fn all_combinable_slots_mut(&mut self) -> Vec<&mut Option> { + fn all_combinable_slots_mut(&mut self) -> Box<[&mut Option]> { self.all_slots() } @@ -85,10 +85,8 @@ pub trait Container: Sync + Send { false } - fn crafted_item_slot(&self) -> Option { - self.all_slots_ref() - .get(self.crafting_output_slot()?)? - .copied() + fn crafted_item_slot(&self) -> Option<&ItemStack> { + *self.all_slots_ref().get(self.crafting_output_slot()?)? } fn recipe_used(&mut self) {} @@ -109,13 +107,13 @@ impl Container for EmptyContainer { ); } - fn all_slots(&mut self) -> Vec<&mut Option> { + fn all_slots(&mut self) -> Box<[&mut Option]> { unreachable!( "you should never be able to get here because this type is always wrapped in an option" ); } - fn all_slots_ref(&self) -> Vec> { + fn all_slots_ref(&self) -> Box<[Option<&ItemStack>]> { unreachable!( "you should never be able to get here because this type is always wrapped in an option" ); @@ -127,19 +125,22 @@ pub fn handle_item_take( item_slot: &mut Option, mouse_click: MouseClick, ) { - let Some(item) = item_slot else { + let Some(item_stack) = item_slot else { return; }; - let mut new_item = *item; + let mut new_item = item_stack.clone(); match mouse_click { MouseClick::Left => { *item_slot = None; } MouseClick::Right => { - let half = item.item_count / 2; - item.item_count -= half; + let half = item_stack.item_count / 2; new_item.item_count = half; + item_stack.item_count -= half; + if item_stack.item_count == 0 { + *item_slot = None; + } } } *carried_item = Some(new_item); @@ -155,22 +156,24 @@ pub fn handle_item_change( if current.item.id == carried.item.id { combine_stacks(carried_slot, current, mouse_click); } else if mouse_click == MouseClick::Left { - let carried = *carried; - *carried_slot = Some(current.to_owned()); - *current_slot = Some(carried.to_owned()); + std::mem::swap(carried_slot, current_slot); } } // Put held stack into empty slot - (None, Some(carried)) => match mouse_click { + (None, Some(carried_item_stack)) => match mouse_click { MouseClick::Left => { - *current_slot = Some(carried.to_owned()); - *carried_slot = None; + std::mem::swap(carried_slot, current_slot); } MouseClick::Right => { - carried.item_count -= 1; - let mut new = *carried; - new.item_count = 1; - *current_slot = Some(new); + let new_stack = ItemStack { + item: carried_item_stack.item.clone(), + item_count: 1, + }; + *current_slot = Some(new_stack); + carried_item_stack.item_count -= 1; + if carried_item_stack.item_count == 0 { + *carried_slot = None; + } } }, // Take stack into carried @@ -188,21 +191,24 @@ pub fn combine_stacks( return; }; + debug_assert!(carried_item.item.id == slot.item.id); + let max_size = carried_item.item.components.max_stack_size; + let carried_change = match mouse_click { MouseClick::Left => carried_item.item_count, MouseClick::Right => 1, }; // TODO: Check for item stack max size here - if slot.item_count + carried_change <= 64 { + if slot.item_count + carried_change <= max_size { slot.item_count += carried_change; carried_item.item_count -= carried_change; if carried_item.item_count == 0 { *carried_slot = None; } } else { - let left_over = slot.item_count + carried_change - 64; - slot.item_count = 64; + let left_over = slot.item_count + carried_change - max_size; + slot.item_count = max_size; carried_item.item_count = left_over; } } @@ -243,24 +249,24 @@ impl<'a> Container for OptionallyCombinedContainer<'a, 'a> { .unwrap_or(self.inventory.window_name()) } - fn all_slots(&mut self) -> Vec<&mut Option> { + fn all_slots(&mut self) -> Box<[&mut Option]> { let slots = match &mut self.container { Some(container) => { - let mut slots = container.all_slots(); + let mut slots = container.all_slots().into_vec(); slots.extend(self.inventory.all_combinable_slots_mut()); - slots + slots.into_boxed_slice() } None => self.inventory.all_slots(), }; slots } - fn all_slots_ref(&self) -> Vec> { + fn all_slots_ref(&self) -> Box<[Option<&ItemStack>]> { match &self.container { Some(container) => { - let mut slots = container.all_slots_ref(); + let mut slots = container.all_slots_ref().into_vec(); slots.extend(self.inventory.all_combinable_slots()); - slots + slots.into_boxed_slice() } None => self.inventory.all_slots_ref(), } diff --git a/pumpkin-inventory/src/open_container.rs b/pumpkin-inventory/src/open_container.rs index d1dd35892..9089b369c 100644 --- a/pumpkin-inventory/src/open_container.rs +++ b/pumpkin-inventory/src/open_container.rs @@ -94,7 +94,7 @@ pub struct Chest([Option; 27]); impl Chest { pub fn new() -> Self { - Self([None; 27]) + Self([const { None }; 27]) } } impl Container for Chest { @@ -105,11 +105,11 @@ impl Container for Chest { fn window_name(&self) -> &'static str { "Chest" } - fn all_slots(&mut self) -> Vec<&mut Option> { + fn all_slots(&mut self) -> Box<[&mut Option]> { self.0.iter_mut().collect() } - fn all_slots_ref(&self) -> Vec> { + fn all_slots_ref(&self) -> Box<[Option<&ItemStack>]> { self.0.iter().map(|slot| slot.as_ref()).collect() } } @@ -120,6 +120,12 @@ pub struct CraftingTable { output: Option, } +impl CraftingTable { + const SLOT_OUTPUT: usize = 0; + const SLOT_INPUT_START: usize = 1; + const SLOT_INPUT_END: usize = 9; +} + impl Container for CraftingTable { fn window_type(&self) -> &'static WindowType { &WindowType::Crafting @@ -128,7 +134,7 @@ impl Container for CraftingTable { fn window_name(&self) -> &'static str { "Crafting Table" } - fn all_slots(&mut self) -> Vec<&mut Option> { + fn all_slots(&mut self) -> Box<[&mut Option]> { let slots = vec![&mut self.output]; let slots = slots .into_iter() @@ -137,7 +143,7 @@ impl Container for CraftingTable { slots } - fn all_slots_ref(&self) -> Vec> { + fn all_slots_ref(&self) -> Box<[Option<&ItemStack>]> { let slots = vec![self.output.as_ref()]; let slots = slots .into_iter() @@ -146,28 +152,49 @@ impl Container for CraftingTable { slots } - fn all_combinable_slots(&self) -> Vec> { + fn all_combinable_slots(&self) -> Box<[Option<&ItemStack>]> { self.input.iter().flatten().map(|s| s.as_ref()).collect() } - fn all_combinable_slots_mut(&mut self) -> Vec<&mut Option> { + fn all_combinable_slots_mut(&mut self) -> Box<[&mut Option]> { self.input.iter_mut().flatten().collect() } fn craft(&mut self) -> bool { - let old_output = self.output; - self.output = check_if_matches_crafting(self.input); - old_output != self.output + // TODO: Is there a better way to do this? + let check = [ + [ + self.input[0][0].as_ref(), + self.input[0][1].as_ref(), + self.input[0][2].as_ref(), + ], + [ + self.input[1][0].as_ref(), + self.input[1][1].as_ref(), + self.input[1][2].as_ref(), + ], + [ + self.input[2][0].as_ref(), + self.input[2][1].as_ref(), + self.input[2][2].as_ref(), + ], + ]; + + let new_output = check_if_matches_crafting(check); + let result = new_output != self.output || self.input.iter().flatten().any(|s| s.is_some()) - || self.output.is_some() + || new_output.is_some(); + + self.output = new_output; + result } fn crafting_output_slot(&self) -> Option { - Some(0) + Some(Self::SLOT_OUTPUT) } fn slot_in_crafting_input_slots(&self, slot: &usize) -> bool { - (1..10).contains(slot) + (Self::SLOT_INPUT_START..=Self::SLOT_INPUT_END).contains(slot) } fn recipe_used(&mut self) { self.input.iter_mut().flatten().for_each(|slot| { @@ -197,17 +224,11 @@ impl Container for Furnace { fn window_name(&self) -> &'static str { "Furnace" } - fn all_slots(&mut self) -> Vec<&mut Option> { - let mut slots = vec![&mut self.cook]; - slots.push(&mut self.fuel); - slots.push(&mut self.output); - slots + fn all_slots(&mut self) -> Box<[&mut Option]> { + Box::new([&mut self.cook, &mut self.fuel, &mut self.output]) } - fn all_slots_ref(&self) -> Vec> { - let mut slots = vec![self.cook.as_ref()]; - slots.push(self.fuel.as_ref()); - slots.push(self.output.as_ref()); - slots + fn all_slots_ref(&self) -> Box<[Option<&ItemStack>]> { + Box::new([self.cook.as_ref(), self.fuel.as_ref(), self.output.as_ref()]) } } diff --git a/pumpkin-inventory/src/player.rs b/pumpkin-inventory/src/player.rs index 47ac7aafc..d821ee8ea 100644 --- a/pumpkin-inventory/src/player.rs +++ b/pumpkin-inventory/src/player.rs @@ -17,6 +17,23 @@ use std::slice::IterMut; */ +pub const SLOT_CRAFT_OUTPUT: usize = 0; +pub const SLOT_CRAFT_INPUT_START: usize = 1; +pub const SLOT_CRAFT_INPUT_END: usize = 4; +pub const SLOT_HELM: usize = 5; +pub const SLOT_CHEST: usize = 6; +pub const SLOT_LEG: usize = 7; +pub const SLOT_BOOT: usize = 8; +pub const SLOT_INV_START: usize = 9; +pub const SLOT_INV_END: usize = 35; +pub const SLOT_HOTBAR_START: usize = 36; +pub const SLOT_HOTBAR_END: usize = 44; +pub const SLOT_OFFHAND: usize = 45; + +pub const SLOT_HOTBAR_INDEX: usize = SLOT_HOTBAR_END - SLOT_HOTBAR_START; +pub const SLOT_MAX: usize = SLOT_OFFHAND; +pub const SLOT_INDEX_OUTSIDE: i16 = -999; + pub struct PlayerInventory { // Main Inventory + Hotbar crafting: [Option; 4], @@ -25,7 +42,7 @@ pub struct PlayerInventory { armor: [Option; 4], offhand: Option, // current selected slot in hotbar - pub selected: u32, + pub selected: usize, 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: i32, @@ -42,10 +59,10 @@ impl PlayerInventory { pub fn new() -> Self { Self { - crafting: [None; 4], + crafting: [const { None }; 4], crafting_output: None, - items: [None; 36], - armor: [None; 4], + items: [const { None }; 36], + armor: [const { None }; 4], offhand: None, // TODO: What when player spawns in with an different index ? selected: 0, @@ -68,7 +85,7 @@ impl PlayerInventory { item_allowed_override: bool, ) -> Result<(), InventoryError> { if item_allowed_override { - if !(0..=45).contains(&slot) { + if !(0..=SLOT_MAX).contains(&slot) { Err(InventoryError::InvalidSlot)? } *self.all_slots()[slot] = item; @@ -87,44 +104,45 @@ impl PlayerInventory { &self, slot: usize, ) -> Result bool>, InventoryError> { - if !(0..=45).contains(&slot) { + if !(0..=SLOT_MAX).contains(&slot) { return Err(InventoryError::InvalidSlot); } Ok(Box::new(match slot { - 0..=4 | 9..=45 => |_| true, - 5 => |item: &ItemStack| item.is_helmet(), - 6 => |item: &ItemStack| item.is_chestplate(), - 7 => |item: &ItemStack| item.is_leggings(), - 8 => |item: &ItemStack| item.is_boots(), + SLOT_CRAFT_OUTPUT..=SLOT_CRAFT_INPUT_END | SLOT_INV_START..=SLOT_OFFHAND => |_| true, + SLOT_HELM => |item: &ItemStack| item.is_helmet(), + SLOT_CHEST => |item: &ItemStack| item.is_chestplate(), + SLOT_LEG => |item: &ItemStack| item.is_leggings(), + SLOT_BOOT => |item: &ItemStack| item.is_boots(), _ => unreachable!(), })) } pub fn get_slot(&mut self, slot: usize) -> Result<&mut Option, InventoryError> { match slot { - 0 => { + SLOT_CRAFT_OUTPUT => { // TODO: Add crafting check here Ok(&mut self.crafting_output) } - 1..=4 => Ok(&mut self.crafting[slot - 1]), - 5..=8 => Ok(&mut self.armor[slot - 5]), - 9..=44 => Ok(&mut self.items[slot - 9]), - 45 => Ok(&mut self.offhand), + SLOT_CRAFT_INPUT_START..=SLOT_CRAFT_INPUT_END => { + Ok(&mut self.crafting[slot - SLOT_CRAFT_INPUT_START]) + } + SLOT_HELM..=SLOT_BOOT => Ok(&mut self.armor[slot - SLOT_HELM]), + SLOT_INV_START..=SLOT_HOTBAR_END => Ok(&mut self.items[slot - SLOT_INV_START]), + SLOT_OFFHAND => Ok(&mut self.offhand), _ => Err(InventoryError::InvalidSlot), } } - pub fn set_selected(&mut self, slot: u32) { - assert!((0..9).contains(&slot)); + pub fn set_selected(&mut self, slot: usize) { + debug_assert!((0..=SLOT_HOTBAR_INDEX).contains(&slot)); self.selected = slot; } - pub fn get_selected(&self) -> u32 { - self.selected + 36 + pub fn get_selected_slot(&self) -> usize { + self.selected + SLOT_HOTBAR_START } - pub fn held_item(&self) -> Option<&ItemStack> { - debug_assert!((0..9).contains(&self.selected)); - self.items[self.selected as usize + 36 - 9].as_ref() + pub fn increment_state_id(&mut self) { + self.state_id = self.state_id % 100 + 1; } pub async fn get_mining_speed(&self, block_name: &str) -> f32 { @@ -132,9 +150,16 @@ impl PlayerInventory { .map_or_else(|| 1.0, |e| e.get_speed(block_name)) } + //NOTE: We actually want &mut Option instead of Option<&mut> pub fn held_item_mut(&mut self) -> &mut Option { - debug_assert!((0..9).contains(&self.selected)); - &mut self.items[self.selected as usize + 36 - 9] + debug_assert!((0..=SLOT_HOTBAR_INDEX).contains(&self.selected)); + &mut self.items[self.get_selected_slot() - SLOT_INV_START] + } + + #[inline] + pub fn held_item(&self) -> Option<&ItemStack> { + debug_assert!((0..=SLOT_HOTBAR_INDEX).contains(&self.selected)); + self.items[self.get_selected_slot() - SLOT_INV_START].as_ref() } pub fn decrease_current_stack(&mut self, amount: u8) -> bool { @@ -149,111 +174,105 @@ impl PlayerInventory { false } - /// Checks if we can merge an existing item into an Stack or if a any new Slot is empty - pub fn collect_item_slot(&self, item_id: u16) -> Option { - // Lets try to merge first - if let Some(stack) = self.get_nonfull_slot_with_item(item_id) { - return Some(stack); - } - if let Some(empty) = self.get_empty_slot() { - return Some(empty); - } - None - } - - pub fn get_empty_hotbar_slot(&self) -> u32 { - if self.items[self.selected as usize + 36 - 9].is_none() { + pub fn get_empty_hotbar_slot(&self) -> usize { + if self.held_item().is_none() { return self.selected; } - for slot in 0..9 { - if self.items[slot + 36 - 9].is_none() { - return slot as u32; + for slot in SLOT_HOTBAR_START..=SLOT_HOTBAR_END { + if self.items[slot - SLOT_INV_START].is_none() { + return slot - SLOT_HOTBAR_START; } } self.selected } - pub fn get_nonfull_slot_with_item(&self, item_id: u16) -> Option { - let max_stack = Item::from_id(item_id) - .unwrap_or(Item::AIR) - .components - .max_stack_size; - + pub fn get_slot_filtered(&self, filter: &F) -> Option + where + F: Fn(Option<&ItemStack>) -> bool, + { // Check selected slot - if let Some(item) = &self.items[self.selected as usize + 36 - 9] { - if item.item.id == item_id && item.item_count < max_stack { - // + 9 - 9 is 0 - return Some(self.selected as usize + 36); - } + if filter(self.items[self.get_selected_slot() - SLOT_INV_START].as_ref()) { + Some(self.get_selected_slot()) } - // Check hotbar slots (27-35) first - if let Some(index) = self.items[27..36].iter().position(|slot| { - slot.is_some_and(|item| item.item.id == item_id && item.item_count < max_stack) - }) { - return Some(index + 27 + 9); + else if let Some(index) = self.items + [SLOT_HOTBAR_START - SLOT_INV_START..=SLOT_HOTBAR_END - SLOT_INV_START] + .iter() + .enumerate() + .position(|(index, item_stack)| index != self.selected && filter(item_stack.as_ref())) + { + Some(index + SLOT_HOTBAR_START) } - // Then check main inventory slots (0-26) - if let Some(index) = self.items[0..27].iter().position(|slot| { - slot.is_some_and(|item| item.item.id == item_id && item.item_count < max_stack) - }) { - return Some(index + 9); + else if let Some(index) = self.items[0..=SLOT_INV_END - SLOT_INV_START] + .iter() + .position(|item_stack| filter(item_stack.as_ref())) + { + Some(index + SLOT_INV_START) + } + // Check offhand + else if filter(self.offhand.as_ref()) { + Some(SLOT_OFFHAND) + } else { + None } - - None } - pub fn get_slot_with_item(&self, item_id: u16) -> Option { - for slot in 9..=44 { - match &self.items[slot - 9] { - Some(item) if item.item.id == item_id => return Some(slot), - _ => continue, - } - } + pub fn get_nonfull_slot_with_item(&self, item_id: u16) -> Option { + let max_stack = Item::from_id(item_id) + .expect("We passed an invalid item id") + .components + .max_stack_size; - None + self.get_slot_filtered(&|item_stack| { + item_stack.is_some_and(|item_stack| { + item_stack.item.id == item_id && item_stack.item_count < max_stack + }) + }) } - pub fn get_empty_slot(&self) -> Option { - // Check hotbar slots (27-35) first - if let Some(index) = self.items[27..36].iter().position(|slot| slot.is_none()) { - return Some(index + 27 + 9); - } + /// Returns a slot that has an item with less than the max stack size, if none, returns an empty + /// slot, if none, returns None + pub fn get_pickup_item_slot(&self, item_id: u16) -> Option { + self.get_nonfull_slot_with_item(item_id) + .or_else(|| self.get_empty_slot()) + } - // Then check main inventory slots (0-26) - if let Some(index) = self.items[0..27].iter().position(|slot| slot.is_none()) { - return Some(index + 9); - } + pub fn get_slot_with_item(&self, item_id: u16) -> Option { + self.get_slot_filtered(&|item_stack| { + item_stack.is_some_and(|item_stack| item_stack.item.id == item_id) + }) + } - None + pub fn get_empty_slot(&self) -> Option { + self.get_slot_filtered(&|item_stack| item_stack.is_none()) } pub fn get_empty_slot_no_order(&self) -> Option { self.items .iter() .position(|slot| slot.is_none()) - .map(|index| index + 9) + .map(|index| index + SLOT_INV_START) } - pub fn slots(&self) -> Vec> { + pub fn slots(&self) -> Box<[Option<&ItemStack>]> { let mut slots = vec![self.crafting_output.as_ref()]; slots.extend(self.crafting.iter().map(|c| c.as_ref())); slots.extend(self.armor.iter().map(|c| c.as_ref())); slots.extend(self.items.iter().map(|c| c.as_ref())); slots.push(self.offhand.as_ref()); - slots + slots.into_boxed_slice() } - pub fn slots_mut(&mut self) -> Vec<&mut Option> { + pub fn slots_mut(&mut self) -> Box<[&mut Option]> { let mut slots = vec![&mut self.crafting_output]; slots.extend(self.crafting.iter_mut()); slots.extend(self.armor.iter_mut()); slots.extend(self.items.iter_mut()); slots.push(&mut self.offhand); - slots + slots.into_boxed_slice() } pub fn iter_items_mut(&mut self) -> IterMut> { @@ -263,7 +282,7 @@ impl PlayerInventory { pub fn slots_with_hotbar_first( &mut self, ) -> Chain>, IterMut>> { - let (items, hotbar) = self.items.split_at_mut(27); + let (items, hotbar) = self.items.split_at_mut(SLOT_HOTBAR_START - SLOT_INV_START); hotbar.iter_mut().chain(items) } } @@ -288,43 +307,47 @@ impl Container for PlayerInventory { let slot_condition = self.slot_condition(slot)?; let item_slot = self.get_slot(slot)?; if let Some(item) = carried_slot { + debug_assert!( + item.item_count > 0, + "We aren't setting the stack to None somewhere" + ); if slot_condition(item) { if invert { handle_item_change(item_slot, carried_slot, mouse_click); - return Ok(()); + } else { + handle_item_change(carried_slot, item_slot, mouse_click); } - handle_item_change(carried_slot, item_slot, mouse_click); + } else { + return Err(InventoryError::InvalidSlot); } + } else if invert { + handle_item_change(item_slot, carried_slot, mouse_click); } else { - if invert { - handle_item_change(item_slot, carried_slot, mouse_click); - return Ok(()); - } handle_item_change(carried_slot, item_slot, mouse_click) } Ok(()) } - fn all_slots(&mut self) -> Vec<&mut Option> { + fn all_slots(&mut self) -> Box<[&mut Option]> { self.slots_mut() } - fn all_slots_ref(&self) -> Vec> { + fn all_slots_ref(&self) -> Box<[Option<&ItemStack>]> { self.slots() } - fn all_combinable_slots(&self) -> Vec> { + fn all_combinable_slots(&self) -> Box<[Option<&ItemStack>]> { self.items.iter().map(|item| item.as_ref()).collect() } - fn all_combinable_slots_mut(&mut self) -> Vec<&mut Option> { + fn all_combinable_slots_mut(&mut self) -> Box<[&mut Option]> { self.items.iter_mut().collect() } fn craft(&mut self) -> bool { - let v1 = [self.crafting[0], self.crafting[1], None]; - let v2 = [self.crafting[2], self.crafting[3], None]; - let v3 = [None; 3]; + let v1 = [self.crafting[0].as_ref(), self.crafting[1].as_ref(), None]; + let v2 = [self.crafting[2].as_ref(), self.crafting[3].as_ref(), None]; + let v3 = [const { None }; 3]; let together = [v1, v2, v3]; self.crafting_output = check_if_matches_crafting(together); @@ -332,10 +355,10 @@ impl Container for PlayerInventory { } fn crafting_output_slot(&self) -> Option { - Some(0) + Some(SLOT_CRAFT_OUTPUT) } fn slot_in_crafting_input_slots(&self, slot: &usize) -> bool { - (1..=4).contains(slot) + (SLOT_CRAFT_INPUT_START..=SLOT_CRAFT_INPUT_END).contains(slot) } } diff --git a/pumpkin-inventory/src/window_property.rs b/pumpkin-inventory/src/window_property.rs index 9c7a1cc24..8d075255c 100644 --- a/pumpkin-inventory/src/window_property.rs +++ b/pumpkin-inventory/src/window_property.rs @@ -33,6 +33,7 @@ pub enum EnchantmentTable { EnchantmentLevel { slot: u8 }, } +// TODO: No more magic numbers impl WindowPropertyTrait for EnchantmentTable { fn to_id(self) -> i16 { use EnchantmentTable::*; diff --git a/pumpkin-macros/src/lib.rs b/pumpkin-macros/src/lib.rs index c4c24f6e3..15983195d 100644 --- a/pumpkin-macros/src/lib.rs +++ b/pumpkin-macros/src/lib.rs @@ -206,7 +206,7 @@ pub fn pumpkin_item(input: TokenStream, item: TokenStream) -> TokenStream { let input_string = input.to_string(); let packet_name = input_string.trim_matches('"'); - let item_id = Item::from_name(packet_name).unwrap(); + let item_id = Item::from_registry_key(packet_name).unwrap(); let id = item_id.id; let item: proc_macro2::TokenStream = item.into(); diff --git a/pumpkin-protocol/src/client/config/update_tags.rs b/pumpkin-protocol/src/client/config/update_tags.rs index 83d02efeb..9f7fe8bbb 100644 --- a/pumpkin-protocol/src/client/config/update_tags.rs +++ b/pumpkin-protocol/src/client/config/update_tags.rs @@ -34,18 +34,14 @@ impl ClientPacket for CUpdateTags<'_> { for (key, values) in values.iter() { // This is technically a Identifier but same thing p.put_string_len(key, u16::MAX as usize); - p.put_list(values, |p, v| { - if let Some(string_id) = v { - let id = match registry_key { - RegistryKey::Block => registry::get_block(string_id).unwrap().id as i32, - RegistryKey::Fluid => { - Fluid::ident_to_fluid_id(string_id).unwrap() as i32 - } - _ => unimplemented!(), - }; + p.put_list(values, |p, string_id| { + let id = match registry_key { + RegistryKey::Block => registry::get_block(string_id).unwrap().id as i32, + RegistryKey::Fluid => Fluid::ident_to_fluid_id(string_id).unwrap() as i32, + _ => unimplemented!(), + }; - p.put_var_int(&VarInt::from(id)); - } + p.put_var_int(&VarInt::from(id)); }); } }); diff --git a/pumpkin-protocol/src/codec/slot.rs b/pumpkin-protocol/src/codec/slot.rs index 864626b1f..7596fa76f 100644 --- a/pumpkin-protocol/src/codec/slot.rs +++ b/pumpkin-protocol/src/codec/slot.rs @@ -130,13 +130,38 @@ impl Serialize for Slot { } impl Slot { - pub fn to_item(self) -> Option { - let item_id = self.item_id?.0.try_into().unwrap(); - let item = Item::from_id(item_id)?; - Some(ItemStack { - item, - item_count: self.item_count.0.try_into().unwrap(), - }) + pub fn new(item_id: u16, count: u32) -> Self { + Slot { + item_count: count.into(), + item_id: Some((item_id as i32).into()), + // TODO: add these + num_components_to_add: None, + num_components_to_remove: None, + components_to_add: None, + components_to_remove: None, + } + } + + pub fn to_stack(self) -> Result, &'static str> { + let item_id = self.item_id; + let Some(item_id) = item_id else { + return Ok(None); + }; + let item_id = item_id.0.try_into().map_err(|_| "Item id too large")?; + let item = Item::from_id(item_id).ok_or("Item id invalid")?; + if self.item_count.0 > item.components.max_stack_size as i32 { + Err("Over sized stack") + } else { + let stack = ItemStack { + item, + item_count: self + .item_count + .0 + .try_into() + .map_err(|_| "Stack count too large")?, + }; + Ok(Some(stack)) + } } pub const fn empty() -> Self { @@ -153,15 +178,7 @@ impl Slot { impl From<&ItemStack> for Slot { fn from(item: &ItemStack) -> Self { - Slot { - item_count: item.item_count.into(), - item_id: Some(VarInt(item.item.id as i32)), - // TODO: add these - num_components_to_add: None, - num_components_to_remove: None, - components_to_add: None, - components_to_remove: None, - } + Slot::new(item.item.id, item.item_count as u32) } } diff --git a/pumpkin-protocol/src/server/play/click_container.rs b/pumpkin-protocol/src/server/play/click_container.rs index a5d3c9258..a246ec361 100644 --- a/pumpkin-protocol/src/server/play/click_container.rs +++ b/pumpkin-protocol/src/server/play/click_container.rs @@ -5,6 +5,7 @@ use pumpkin_macros::packet; use serde::de::SeqAccess; use serde::{Deserialize, de}; +#[derive(Debug)] #[packet(PLAY_CONTAINER_CLICK)] pub struct SClickContainer { pub window_id: VarInt, @@ -86,7 +87,7 @@ impl<'de> Deserialize<'de> for SClickContainer { } } -#[derive(Deserialize)] +#[derive(Deserialize, Debug)] 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, diff --git a/pumpkin-util/src/lib.rs b/pumpkin-util/src/lib.rs index 2e635f1f4..ab2b790bc 100644 --- a/pumpkin-util/src/lib.rs +++ b/pumpkin-util/src/lib.rs @@ -5,6 +5,8 @@ pub mod random; pub mod text; pub mod translation; +use std::ops::{Index, IndexMut}; + pub use gamemode::GameMode; pub use permission::PermissionLvl; @@ -25,6 +27,60 @@ pub enum ProfileAction { UsingBannedSkin, } +/// Takes a mutable reference of an index and returns a mutable "slice" where we can mutate both at +/// the same time +pub struct MutableSplitSlice<'a, T> { + start: &'a mut [T], + end: &'a mut [T], +} + +impl<'a, T> MutableSplitSlice<'a, T> { + pub fn extract_ith(base: &'a mut [T], index: usize) -> (&'a mut T, Self) { + let (start, end_inclusive) = base.split_at_mut(index); + let (value, end) = end_inclusive + .split_first_mut() + .expect("Index is not in base slice"); + + (value, Self { start, end }) + } + + pub fn len(&self) -> usize { + self.start.len() + self.end.len() + 1 + } + + pub fn is_empty(&self) -> bool { + false + } +} + +impl Index for MutableSplitSlice<'_, T> { + type Output = T; + + #[allow(clippy::comparison_chain)] + fn index(&self, index: usize) -> &Self::Output { + if index < self.start.len() { + &self.start[index] + } else if index == self.start.len() { + panic!("We tried to index into the element that was removed"); + } else { + &self.end[index - self.start.len() - 1] + } + } +} + +impl IndexMut for MutableSplitSlice<'_, T> { + #[allow(clippy::comparison_chain)] + fn index_mut(&mut self, index: usize) -> &mut Self::Output { + if index < self.start.len() { + &mut self.start[index] + } else if index == self.start.len() { + panic!("We tried to index into the element that was removed"); + } else { + &mut self.end[index - self.start.len() - 1] + } + } +} + #[macro_export] macro_rules! assert_eq_delta { ($x:expr, $y:expr, $d:expr) => { diff --git a/pumpkin-world/src/item/categories.rs b/pumpkin-world/src/item/categories.rs index 301ac4129..06936a7dd 100644 --- a/pumpkin-world/src/item/categories.rs +++ b/pumpkin-world/src/item/categories.rs @@ -1,69 +1,46 @@ +use pumpkin_data::tag::Tagable; + use crate::item::ItemStack; +const SWORDS_TAG: &str = "#minecraft:swords"; +const HEAD_ARMOR_TAG: &str = "#minecraft:head_armor"; +const CHEST_ARMOR_TAG: &str = "#minecraft:chest_armor"; +const LEG_ARMOR_TAG: &str = "#minecraft:leg_armor"; +const FOOT_ARMOR_TAG: &str = "#minecraft:foot_armor"; + impl ItemStack { + #[inline] pub fn is_sword(&self) -> bool { - [ - 818, // Wooden - 823, // Stone - 828, // Gold - 833, // Iron - 838, // Diamond - 843, // Netherite - ] - .contains(&self.item.id) + self.item.is_tagged_with(SWORDS_TAG).expect( + "This is a default minecraft tag that should have been gotten from the extractor", + ) } + #[inline] pub fn is_helmet(&self) -> bool { - [ - // Leather - 856, // Netherite - 876, // Turtle helmet - 794, // Chainmail - 860, // Diamond - 868, // Gold - 872, // Iron - 864, - ] - .contains(&self.item.id) + self.item.is_tagged_with(HEAD_ARMOR_TAG).expect( + "This is a default minecraft tag that should have been gotten from the extractor", + ) } + #[inline] pub fn is_chestplate(&self) -> bool { - [ - // Leather - 857, // Netherite - 877, // Chainmail - 861, // Diamond - 869, // Gold - 873, // Iron - 865, // Elytra - 773, - ] - .contains(&self.item.id) + self.item.is_tagged_with(CHEST_ARMOR_TAG).expect( + "This is a default minecraft tag that should have been gotten from the extractor", + ) } + #[inline] pub fn is_leggings(&self) -> bool { - [ - // Leather - 858, // Netherite - 878, // Chainmail - 862, // Diamond - 870, // Gold - 874, // Iron - 866, - ] - .contains(&self.item.id) + self.item.is_tagged_with(LEG_ARMOR_TAG).expect( + "This is a default minecraft tag that should have been gotten from the extractor", + ) } + #[inline] pub fn is_boots(&self) -> bool { - [ - // Leather - 859, // Netherite - 879, // Chainmail - 863, // Diamond - 871, // Gold - 875, // Iron - 867, - ] - .contains(&self.item.id) + self.item.is_tagged_with(FOOT_ARMOR_TAG).expect( + "This is a default minecraft tag that should have been gotten from the extractor", + ) } } diff --git a/pumpkin-world/src/item/mod.rs b/pumpkin-world/src/item/mod.rs index 4ea914258..44a5410d8 100644 --- a/pumpkin-world/src/item/mod.rs +++ b/pumpkin-world/src/item/mod.rs @@ -2,6 +2,7 @@ use pumpkin_data::item::Item; use pumpkin_data::tag::{RegistryKey, get_tag_values}; mod categories; + #[derive(serde::Deserialize, Debug, Clone, PartialEq, Eq)] #[serde(rename_all = "lowercase")] /// Item Rarity @@ -12,9 +13,10 @@ pub enum Rarity { Epic, } -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Debug)] pub struct ItemStack { pub item_count: u8, + // TODO: Should this be a ref? all of our items are const pub item: Item, } @@ -54,7 +56,7 @@ impl ItemStack { if let Some(blocks) = get_tag_values(RegistryKey::Block, entry.strip_prefix('#').unwrap()) { - if blocks.iter().flatten().any(|s| s == block) { + if blocks.iter().any(|s| s == block) { return speed; } } @@ -89,7 +91,7 @@ impl ItemStack { if let Some(blocks) = get_tag_values(RegistryKey::Block, entry.strip_prefix('#').unwrap()) { - if blocks.iter().flatten().any(|s| s == block) { + if blocks.iter().any(|s| s == block) { return correct_for_drops; } } diff --git a/pumpkin/src/block/mod.rs b/pumpkin/src/block/mod.rs index fe9af9cc5..b11fb929e 100644 --- a/pumpkin/src/block/mod.rs +++ b/pumpkin/src/block/mod.rs @@ -18,11 +18,9 @@ use properties::{ waterlog::Waterlogged, }; use pumpkin_data::entity::EntityType; -use pumpkin_data::item::Item; use pumpkin_util::math::position::BlockPos; use pumpkin_util::math::vector3::Vector3; use pumpkin_world::block::registry::{Block, State}; -use pumpkin_world::item::ItemStack; use rand::Rng; use crate::block::registry::BlockRegistry; @@ -68,10 +66,7 @@ pub async fn drop_loot( ); let entity = server.add_entity(pos, EntityType::ITEM, world); - let item_entity = Arc::new(ItemEntity::new( - entity, - ItemStack::new(1, Item::from_id(block.item_id).unwrap()), - )); + let item_entity = Arc::new(ItemEntity::new(entity, block.item_id, 1)); world.spawn_entity(item_entity.clone()).await; item_entity.send_meta_packet().await; diff --git a/pumpkin/src/command/args/resource/item.rs b/pumpkin/src/command/args/resource/item.rs index ea7fcf7d2..4988884c4 100644 --- a/pumpkin/src/command/args/resource/item.rs +++ b/pumpkin/src/command/args/resource/item.rs @@ -58,14 +58,15 @@ impl<'a> FindArg<'a> for ItemArgumentConsumer { fn find_arg(args: &'a ConsumedArgs, name: &str) -> Result { match args.get(name) { - Some(Arg::Item(name)) => Item::from_name(&name.replace("minecraft:", "")).map_or_else( - || { - Err(CommandError::GeneralCommandIssue(format!( - "Item {name} does not exist." - ))) - }, - |item| Ok((*name, item)), - ), + Some(Arg::Item(name)) => Item::from_registry_key(&name.replace("minecraft:", "")) + .map_or_else( + || { + Err(CommandError::GeneralCommandIssue(format!( + "Item {name} does not exist." + ))) + }, + |item| Ok((*name, item)), + ), _ => Err(CommandError::InvalidConsumption(Some(name.to_string()))), } } diff --git a/pumpkin/src/command/commands/give.rs b/pumpkin/src/command/commands/give.rs index 8ac2052eb..177fe9e64 100644 --- a/pumpkin/src/command/commands/give.rs +++ b/pumpkin/src/command/commands/give.rs @@ -54,7 +54,7 @@ impl CommandExecutor for Executor { }; for target in targets { - target.give_items(item, item_count as u32).await; + target.give_items(item.clone(), item_count as u32).await; } let msg = if targets.len() == 1 { TextComponent::translate( diff --git a/pumpkin/src/entity/item.rs b/pumpkin/src/entity/item.rs index 49d9e920b..558e8ff22 100644 --- a/pumpkin/src/entity/item.rs +++ b/pumpkin/src/entity/item.rs @@ -1,39 +1,41 @@ -use crate::server::Server; +use std::sync::{Arc, atomic::AtomicU32}; + use async_trait::async_trait; -use pumpkin_data::damage::DamageType; +use pumpkin_data::{damage::DamageType, item::Item}; use pumpkin_protocol::{ - client::play::{MetaDataType, Metadata}, + client::play::{CTakeItemEntity, MetaDataType, Metadata}, codec::slot::Slot, }; use pumpkin_world::item::ItemStack; -use std::sync::{ - Arc, - atomic::{AtomicI8, AtomicU8, AtomicU32}, -}; +use tokio::sync::Mutex; + +use crate::server::Server; use super::{Entity, EntityBase, living::LivingEntity, player::Player}; pub struct ItemEntity { entity: Entity, - item: ItemStack, - count: AtomicU8, + item: Item, item_age: AtomicU32, - pickup_delay: AtomicI8, + // These cannot be atomic values because we mutate their state based on what they are; we run + // into the ABA problem + item_count: Mutex, + pickup_delay: Mutex, } impl ItemEntity { - pub fn new(entity: Entity, stack: ItemStack) -> Self { + pub fn new(entity: Entity, item_id: u16, count: u32) -> Self { entity.yaw.store(rand::random::() * 360.0); Self { entity, - item: stack, - count: AtomicU8::new(stack.item_count), + item: Item::from_id(item_id).expect("We passed a bad item id into ItemEntity"), item_age: AtomicU32::new(0), - pickup_delay: AtomicI8::new(10), // Vanilla + item_count: Mutex::new(count), + pickup_delay: Mutex::new(10), // Vanilla pickup delay is 10 ticks } } pub async fn send_meta_packet(&self) { - let slot = Slot::from(&self.item); + let slot = Slot::new(self.item.id, *self.item_count.lock().await); self.entity .send_meta_data(&[Metadata::new(8, MetaDataType::ItemStack, &slot)]) .await; @@ -42,11 +44,11 @@ impl ItemEntity { #[async_trait] impl EntityBase for ItemEntity { - async fn tick(&self, _: &Server) { - if self.pickup_delay.load(std::sync::atomic::Ordering::Relaxed) > 0 { - self.pickup_delay - .fetch_sub(1, std::sync::atomic::Ordering::Relaxed); - } + async fn tick(&self, _server: &Server) { + { + let mut delay = self.pickup_delay.lock().await; + *delay = delay.saturating_sub(1); + }; let age = self .item_age @@ -60,48 +62,94 @@ impl EntityBase for ItemEntity { } async fn on_player_collision(&self, player: Arc) { - if self.pickup_delay.load(std::sync::atomic::Ordering::Relaxed) == 0 { + let can_pickup = { + let delay = self.pickup_delay.lock().await; + *delay == 0 + }; + + if can_pickup { let mut inv = player.inventory.lock().await; - let mut item = self.item; - // Check if we have space in inv - if let Some(slot) = inv.collect_item_slot(item.item.id) { - let max_stack = item.item.components.max_stack_size; - if let Some(stack) = inv.get_slot(slot).unwrap() { - if stack.item_count + self.count.load(std::sync::atomic::Ordering::Relaxed) - > max_stack - { - // Fill the stack to max and store the overflow - let overflow = stack.item_count - + self.count.load(std::sync::atomic::Ordering::Relaxed) - - max_stack; - - stack.item_count = max_stack; - item.item_count = stack.item_count; - - self.count - .store(overflow, std::sync::atomic::Ordering::Relaxed); + let mut total_pick_up = 0; + let mut slot_updates = Vec::new(); + let remove_entity = { + let mut stack_size = self.item_count.lock().await; + let max_stack = self.item.components.max_stack_size; + while *stack_size > 0 { + if let Some(slot) = inv.get_pickup_item_slot(self.item.id) { + // Fill the inventory while there are items in the stack and space in the inventory + let maybe_stack = inv + .get_slot(slot) + .expect("collect item slot returned an invalid slot"); + + if let Some(existing_stack) = maybe_stack { + // We have the item in this stack already + + // This is bounded to u8::MAX + let amount_to_fill = u32::from(max_stack - existing_stack.item_count); + // This is also bounded to u8::MAX since amount_to_fill is max u8::MAX + let amount_to_add = amount_to_fill.min(*stack_size); + // Therefore this is safe + + // Update referenced stack so next call to get_pickup_item_slot is + // correct + existing_stack.item_count += amount_to_add as u8; + total_pick_up += amount_to_add; + + debug_assert!(amount_to_add > 0); + *stack_size -= amount_to_add; + + slot_updates.push((slot, existing_stack.clone())); + } else { + // A new stack + + // This is bounded to u8::MAX + let amount_to_fill = u32::from(max_stack); + // This is also bounded to u8::MAX since amount_to_fill is max u8::MAX + let amount_to_add = amount_to_fill.min(*stack_size); + total_pick_up += amount_to_add; + + debug_assert!(amount_to_add > 0); + *stack_size -= amount_to_add; + + // Therefore this is safe + let item_stack = ItemStack::new(amount_to_add as u8, self.item.clone()); + + // Update referenced stack so next call to get_pickup_item_slot is + // correct + *maybe_stack = Some(item_stack.clone()); + + slot_updates.push((slot, item_stack)); + } } else { - // Add the item to the stack - stack.item_count += self.count.load(std::sync::atomic::Ordering::Relaxed); - item.item_count = stack.item_count; - - player - .living_entity - .pickup(&self.entity, u32::from(item.item_count)) - .await; - self.entity.remove().await; + // We can't pick anything else up + break; } - } else { - // Add the item as a new stack - item.item_count = self.count.load(std::sync::atomic::Ordering::Relaxed); - - player - .living_entity - .pickup(&self.entity, u32::from(item.item_count)) - .await; - self.entity.remove().await; } - player.update_single_slot(&mut inv, slot as i16, item).await; + + *stack_size == 0 + }; + + if total_pick_up > 0 { + player + .client + .send_packet(&CTakeItemEntity::new( + self.entity.entity_id.into(), + player.entity_id().into(), + total_pick_up.into(), + )) + .await; + } + + // TODO: Can we batch slot updates? + for (slot, stack) in slot_updates { + player.update_single_slot(&mut inv, slot, stack).await; + } + + if remove_entity { + self.entity.remove().await; + } else { + // Update entity + self.send_meta_packet().await; } } } diff --git a/pumpkin/src/entity/player.rs b/pumpkin/src/entity/player.rs index 4edaa4b0b..f881967c1 100644 --- a/pumpkin/src/entity/player.rs +++ b/pumpkin/src/entity/player.rs @@ -103,7 +103,7 @@ pub struct Player { /// The ID of the currently open container (if any). pub open_container: AtomicCell>, /// The item currently being held by the player. - pub carried_item: AtomicCell>, + pub carried_item: Mutex>, /// send `send_abilities_update` when changed /// The player's abilities and special powers. /// @@ -201,7 +201,7 @@ impl Player { tick_counter: AtomicI32::new(0), packet_sequence: AtomicI32::new(-1), start_mining_time: AtomicI32::new(0), - carried_item: AtomicCell::new(None), + carried_item: Mutex::new(None), experience_pick_up_delay: Mutex::new(0), teleport_id_count: AtomicI32::new(0), mining: AtomicBool::new(false), @@ -1018,22 +1018,24 @@ impl Player { .await; } - pub async fn drop_item(&self, server: &Server, stack: ItemStack) { + pub async fn drop_item(&self, server: &Server, item_id: u16, count: u32) { let entity = server.add_entity( self.living_entity.entity.pos.load(), EntityType::ITEM, &self.world().await, ); - let item_entity = Arc::new(ItemEntity::new(entity, stack)); + + // TODO: Merge stacks together + let item_entity = Arc::new(ItemEntity::new(entity, item_id, count)); self.world().await.spawn_entity(item_entity.clone()).await; item_entity.send_meta_packet().await; } pub async fn drop_held_item(&self, server: &Server, drop_stack: bool) { let mut inv = self.inventory.lock().await; - if let Some(item) = inv.held_item_mut() { - let drop_amount = if drop_stack { item.item_count } else { 1 }; - self.drop_item(server, ItemStack::new(drop_amount, item.item)) + if let Some(item_stack) = inv.held_item_mut() { + let drop_amount = if drop_stack { item_stack.item_count } else { 1 }; + self.drop_item(server, item_stack.item.id, u32::from(drop_amount)) .await; inv.decrease_current_stack(drop_amount); } @@ -1171,7 +1173,8 @@ impl NBTStorage for Player { async fn read_nbt(&mut self, nbt: &mut NbtCompound) { self.living_entity.read_nbt(nbt).await; - self.inventory.lock().await.selected = nbt.get_int("SelectedItemSlot").unwrap_or(0) as u32; + self.inventory.lock().await.selected = + nbt.get_int("SelectedItemSlot").unwrap_or(0) as usize; self.abilities.lock().await.read_nbt(nbt).await; // Load from total XP diff --git a/pumpkin/src/net/container.rs b/pumpkin/src/net/container.rs index afeff1ff0..ebe339bb0 100644 --- a/pumpkin/src/net/container.rs +++ b/pumpkin/src/net/container.rs @@ -7,6 +7,7 @@ use pumpkin_inventory::container_click::{ Click, ClickType, DropType, KeyClick, MouseClick, MouseDragState, MouseDragType, }; use pumpkin_inventory::drag_handler::DragHandler; +use pumpkin_inventory::player::{SLOT_BOOT, SLOT_CHEST, SLOT_HELM, SLOT_HOTBAR_START, SLOT_LEG}; use pumpkin_inventory::window_property::{WindowProperty, WindowPropertyTrait}; use pumpkin_inventory::{InventoryError, OptionallyCombinedContainer, container_click}; use pumpkin_protocol::client::play::{ @@ -15,15 +16,16 @@ use pumpkin_protocol::client::play::{ use pumpkin_protocol::codec::slot::Slot; use pumpkin_protocol::codec::var_int::VarInt; use pumpkin_protocol::server::play::SClickContainer; -use pumpkin_util::GameMode; use pumpkin_util::text::TextComponent; +use pumpkin_util::{GameMode, MutableSplitSlice}; use pumpkin_world::item::ItemStack; use std::sync::Arc; impl Player { pub async fn open_container(&self, server: &Server, window_type: WindowType) { let mut inventory = self.inventory().lock().await; - inventory.state_id = 0; + //inventory.state_id = 0; + inventory.increment_state_id(); inventory.total_opened_containers += 1; let mut container = self.get_open_container(server).await; let mut container = match container.as_mut() { @@ -65,13 +67,12 @@ impl Player { .map(Slot::from) .collect(); - let carried_item = self - .carried_item - .load() + let carried_item = self.carried_item.lock().await; + let carried_item = carried_item .as_ref() .map_or_else(Slot::empty, std::convert::Into::into); - inventory.state_id += 1; + inventory.increment_state_id(); let packet = CSetContainerContent::new( id.into(), (inventory.state_id).into(), @@ -142,7 +143,7 @@ impl Player { let combined = OptionallyCombinedContainer::new(&mut inventory, opened_container.as_deref_mut()); ( - combined.crafted_item_slot(), + combined.crafted_item_slot().cloned(), combined.crafting_output_slot(), ) }; @@ -277,16 +278,32 @@ impl Player { .await } ClickType::DropType(drop_type) => { - let carried_item = self.carried_item.load(); - if let Some(item) = carried_item { - match drop_type { - DropType::FullStack => self.drop_item(server, item).await, - DropType::SingleItem => { - let mut item = item; - item.item_count = 1; - self.drop_item(server, item).await; + if let container_click::Slot::Normal(slot) = click.slot { + let mut inventory = self.inventory().lock().await; + let mut container = + OptionallyCombinedContainer::new(&mut inventory, opened_container); + let slots = container.all_slots(); + + if let Some(item_stack) = slots[slot].as_mut() { + match drop_type { + DropType::FullStack => { + self.drop_item( + server, + item_stack.item.id, + u32::from(item_stack.item_count), + ) + .await; + *slots[slot] = None; + } + DropType::SingleItem => { + self.drop_item(server, item_stack.item.id, 1).await; + item_stack.item_count -= 1; + if item_stack.item_count == 0 { + *slots[slot] = None; + } + } } - }; + } } Ok(()) } @@ -303,26 +320,29 @@ impl Player { ) -> Result<(), InventoryError> { let mut inventory = self.inventory().lock().await; let mut container = OptionallyCombinedContainer::new(&mut inventory, opened_container); - let mut carried_item = self.carried_item.load(); + let mut carried_item = self.carried_item.lock().await; match slot { container_click::Slot::Normal(slot) => { - let res = container.handle_item_change( - &mut carried_item, - slot, - mouse_click, - taking_crafted, - ); - self.carried_item.store(carried_item); - res + container.handle_item_change(&mut carried_item, slot, mouse_click, taking_crafted) } container_click::Slot::OutsideInventory => { - if let Some(item) = carried_item { + if let Some(item_stack) = carried_item.as_mut() { match mouse_click { - MouseClick::Left => self.drop_item(server, item).await, + MouseClick::Left => { + self.drop_item( + server, + item_stack.item.id, + u32::from(item_stack.item_count), + ) + .await; + *carried_item = None; + } MouseClick::Right => { - let mut item = item; - item.item_count = 1; - self.drop_item(server, item).await; + self.drop_item(server, item_stack.item.id, 1).await; + item_stack.item_count -= 1; + if item_stack.item_count == 0 { + *carried_item = None; + } } }; } @@ -331,45 +351,123 @@ impl Player { } } + /// TODO: Allow equiping/de equiping armor and allow taking items from crafting grid async fn shift_mouse_click( &self, opened_container: Option<&mut Box>, slot: container_click::Slot, - taking_crafted: bool, + _taking_crafted: bool, ) -> Result<(), InventoryError> { let mut inventory = self.inventory().lock().await; + let has_container = opened_container.is_some(); + let container_size = opened_container + .as_ref() + .map_or(0, |c| c.all_slots_ref().len()); let mut container = OptionallyCombinedContainer::new(&mut inventory, opened_container); match slot { container_click::Slot::Normal(slot) => { - let all_slots = container.all_slots(); - if let Some(item_in_pressed_slot) = all_slots[slot].to_owned() { - let slots = all_slots.into_iter().enumerate(); - // Hotbar - let find_condition = |(slot_number, slot): (usize, &mut Option)| { - // TODO: Check for max item count here - match slot { - Some(item) => (item.item.id == item_in_pressed_slot.item.id - && item.item_count != 64) - .then_some(slot_number), - None => Some(slot_number), - } - }; + let mut all_slots = container.all_slots(); + let (item_stack, mut split_slice) = + MutableSplitSlice::extract_ith(&mut all_slots, slot); + let Some(clicked_item_stack) = item_stack else { + return Ok(()); + }; + + // Define the two inventories and determine which one contains the source slot + let (inv1_range, inv2_range) = if has_container { + // When container is open: + // Inv1 = Container slots (0 to container_size-1) + // Inv2 = Player inventory (container_size to end) + ((0..container_size), (container_size..split_slice.len())) + } else { + // When no container: + // Inv1 = Hotbar (36-45) + // Inv2 = Main inventory (9-36) + ((36..45), (9..36)) + }; - let slots = if slot > 35 { - slots.skip(9).find_map(find_condition) + // Determine which inventory we're moving from and to + let (source_inv, target_inv) = if inv1_range.contains(&slot) { + (&inv1_range, &inv2_range) + } else if inv2_range.contains(&slot) { + (&inv2_range, &inv1_range) + } else { + // When moving from top slots to inventory + (&(0..9), &(9..45)) + }; + + // If moving to hotbar, reverse the order to fill from right to left + let target_slots: Vec = + if has_container && source_inv.contains(&slot) && source_inv == &inv1_range { + target_inv.clone().rev().collect() } else { - slots.skip(36).rev().find_map(find_condition) + target_inv.clone().collect() }; - if let Some(slot) = slots { - let mut item_slot = container.all_slots()[slot].map(|i| i); - container.handle_item_change( - &mut item_slot, - slot, - MouseClick::Left, - taking_crafted, - )?; - *container.all_slots()[slot] = item_slot; + + //Handle armor slots + if !has_container { + let temp_item_stack = ItemStack::new(1, clicked_item_stack.item.clone()); + if slot != SLOT_HELM + && temp_item_stack.is_helmet() + && split_slice[SLOT_HELM].is_none() + { + *split_slice[SLOT_HELM] = Some(temp_item_stack); + **item_stack = None; + return Ok(()); + } else if slot != SLOT_CHEST + && temp_item_stack.is_chestplate() + && split_slice[SLOT_CHEST].is_none() + { + *split_slice[SLOT_CHEST] = Some(temp_item_stack); + **item_stack = None; + return Ok(()); + } else if slot != SLOT_LEG + && temp_item_stack.is_leggings() + && split_slice[SLOT_LEG].is_none() + { + *split_slice[SLOT_LEG] = Some(temp_item_stack); + **item_stack = None; + return Ok(()); + } else if slot != SLOT_BOOT + && temp_item_stack.is_boots() + && split_slice[SLOT_BOOT].is_none() + { + *split_slice[SLOT_BOOT] = Some(temp_item_stack); + **item_stack = None; + return Ok(()); + } + } + + // First try to stack with existing items + let max_stack_size = clicked_item_stack.item.components.max_stack_size; + for target_idx in &target_slots { + if let Some(target_item) = split_slice[*target_idx].as_mut() { + if target_item.item.id == clicked_item_stack.item.id + && target_item.item_count < max_stack_size + { + let space_in_stack = max_stack_size - target_item.item_count; + let amount_to_add = clicked_item_stack.item_count.min(space_in_stack); + target_item.item_count += amount_to_add; + clicked_item_stack.item_count -= amount_to_add; + + if clicked_item_stack.item_count == 0 { + **item_stack = None; + return Ok(()); + } + } + } + } + + // Then try to place in empty slots + for target_idx in target_slots { + if split_slice[target_idx].is_none() + || split_slice[target_idx] + .as_ref() + .is_some_and(|item| item.item_count == 0) + { + std::mem::swap(split_slice[target_idx], *item_stack); + return Ok(()); } } } @@ -386,11 +484,11 @@ impl Player { taking_crafted: bool, ) -> Result<(), InventoryError> { let changing_slot = match key_click { - KeyClick::Slot(slot) => slot, + KeyClick::Slot(slot) => slot as usize + SLOT_HOTBAR_START, KeyClick::Offhand => 45, }; let mut inventory = self.inventory().lock().await; - let mut changing_item_slot = inventory.get_slot(changing_slot as usize)?.to_owned(); + let mut changing_item_slot = inventory.get_slot(changing_slot)?.clone(); let mut container = OptionallyCombinedContainer::new(&mut inventory, opened_container); container.handle_item_change( @@ -399,7 +497,7 @@ impl Player { MouseClick::Left, taking_crafted, )?; - *inventory.get_slot(changing_slot as usize)? = changing_item_slot; + *inventory.get_slot(changing_slot)? = changing_item_slot; Ok(()) } @@ -414,7 +512,8 @@ impl Player { let mut inventory = self.inventory().lock().await; let mut container = OptionallyCombinedContainer::new(&mut inventory, opened_container); if let Some(Some(item)) = container.all_slots().get_mut(slot) { - self.carried_item.store(Some(item.to_owned())); + let mut carried_item = self.carried_item.lock().await; + *carried_item = Some(item.clone()); } Ok(()) } @@ -422,38 +521,38 @@ impl Player { async fn double_click( &self, opened_container: Option<&mut Box>, - slot: usize, + _slot: usize, ) -> Result<(), InventoryError> { let mut inventory = self.inventory().lock().await; let mut container = OptionallyCombinedContainer::new(&mut inventory, opened_container); - let mut slots = container.all_slots(); - - let Some(item) = slots.get_mut(slot) else { + let mut carried_item = self.carried_item.lock().await; + let Some(carried_item) = carried_item.as_mut() else { return Ok(()); }; - let Some(mut carried_item) = **item else { - return Ok(()); - }; - **item = None; - - for slot in slots.iter_mut().filter_map(|slot| slot.as_mut()) { - if slot.item.id == carried_item.item.id { - // TODO: Check for max stack size - if slot.item_count + carried_item.item_count <= 64 { - slot.item_count = 0; - carried_item.item_count = 64; - } else { - let to_remove = slot.item_count - (64 - carried_item.item_count); - slot.item_count -= to_remove; - carried_item.item_count += to_remove; - } - if carried_item.item_count == 64 { - break; + // Iterate directly over container slots to modify them in place' + for slot in container.all_slots() { + if let Some(itemstack) = slot { + if itemstack.item.id == carried_item.item.id { + if itemstack.item_count + carried_item.item_count + <= carried_item.item.components.max_stack_size + { + carried_item.item_count += itemstack.item_count; + *slot = None; + } else { + let overflow = itemstack.item_count + - (carried_item.item.components.max_stack_size + - carried_item.item_count); + carried_item.item_count = carried_item.item.components.max_stack_size; + itemstack.item_count = overflow; + } + + if carried_item.item_count == carried_item.item.components.max_stack_size { + break; + } } } } - self.carried_item.store(Some(carried_item)); Ok(()) } @@ -486,12 +585,10 @@ impl Player { let mut inventory = self.inventory().lock().await; let mut container = OptionallyCombinedContainer::new(&mut inventory, opened_container); - let mut carried_item = self.carried_item.load(); - let res = drag_handler + let mut carried_item = self.carried_item.lock().await; + drag_handler .apply_drag(&mut carried_item, &mut container, &container_id, player_id) - .await; - self.carried_item.store(carried_item); - res + .await } } } @@ -545,7 +642,7 @@ impl Player { let total_opened_containers = inventory.total_opened_containers; // Returns previous value - inventory.state_id += 1; + inventory.increment_state_id(); let packet = CSetContainerSlot::new( total_opened_containers as i8, (inventory.state_id) as i32, @@ -581,63 +678,47 @@ impl Player { } } - async fn pickup_items(&self, item: Item, mut amount: u32) { + // TODO: Use this method when actually picking up items instead of just the command + async fn pickup_items(&self, item: Item, amount: u32) { + let mut amount_left = amount; let max_stack = item.components.max_stack_size; let mut inventory = self.inventory().lock().await; - let slots = inventory.slots_with_hotbar_first(); - - let matching_slots = slots.filter_map(|slot| { - if let Some(item_slot) = slot.as_mut() { - (item_slot.item.id == item.id && item_slot.item_count < max_stack).then(|| { - let item_count = item_slot.item_count; - (item_slot, item_count) - }) - } else { - None - } - }); - for (slot, item_count) in matching_slots { - if amount == 0 { - return; - } - let amount_to_add = max_stack - item_count; - if let Some(amount_left) = amount.checked_sub(u32::from(amount_to_add)) { - amount = amount_left; - *slot = ItemStack { - item, - item_count: item.components.max_stack_size, - }; - } else { - *slot = ItemStack { - item, - item_count: max_stack - (amount_to_add - amount as u8), - }; - return; - } - } + while let Some(slot) = inventory.get_pickup_item_slot(item.id) { + let item_stack = inventory + .get_slot(slot) + .expect("We just called a method that said this was a valid slot"); - let empty_slots = inventory - .slots_with_hotbar_first() - .filter(|slot| slot.is_none()); - for slot in empty_slots { - if amount == 0 { - return; - } - if let Some(remaining_amount) = amount.checked_sub(u32::from(max_stack)) { - amount = remaining_amount; - *slot = Some(ItemStack { - item, + if let Some(item_stack) = item_stack { + let amount_to_add = max_stack - item_stack.item_count; + if let Some(new_amount_left) = amount_left.checked_sub(u32::from(amount_to_add)) { + item_stack.item_count = max_stack; + amount_left = new_amount_left; + } else { + // This is safe because amount left is less than amount_to_add which is a u8 + item_stack.item_count = max_stack - (amount_to_add - amount_left as u8); + // Return here because if we have less than the max amount left then the whole + // stack will be moved + return; + } + } else if let Some(new_amount_left) = amount_left.checked_sub(u32::from(max_stack)) { + *item_stack = Some(ItemStack { + item: item.clone(), item_count: max_stack, }); + amount_left = new_amount_left; } else { - *slot = Some(ItemStack { - item, - item_count: amount as u8, + *item_stack = Some(ItemStack { + item: item.clone(), + // This is safe because amount left is less than max_stack which is a u8 + item_count: amount_left as u8, }); + // Return here because if we have less than the max amount left then the whole + // stack will be moved return; } } + log::warn!( "{amount} items were discarded because dropping them to the ground is not implemented" ); diff --git a/pumpkin/src/net/packet/play.rs b/pumpkin/src/net/packet/play.rs index c376069f9..19eb946c8 100644 --- a/pumpkin/src/net/packet/play.rs +++ b/pumpkin/src/net/packet/play.rs @@ -20,7 +20,9 @@ use pumpkin_data::sound::Sound; use pumpkin_data::sound::SoundCategory; use pumpkin_data::world::CHAT; use pumpkin_inventory::InventoryError; -use pumpkin_inventory::player::PlayerInventory; +use pumpkin_inventory::player::{ + PlayerInventory, SLOT_HOTBAR_END, SLOT_HOTBAR_START, SLOT_OFFHAND, +}; use pumpkin_macros::block_entity; use pumpkin_protocol::client::play::{ CBlockEntityData, COpenSignEditor, CSetContainerSlot, CSetHeldItem, EquipmentSlot, @@ -395,20 +397,22 @@ impl Player { pub async fn update_single_slot( &self, - inventory: &mut tokio::sync::MutexGuard<'_, PlayerInventory>, - slot: i16, + inventory: &mut PlayerInventory, + slot: usize, stack: ItemStack, ) { - inventory.state_id += 1; + inventory.increment_state_id(); let slot_data = Slot::from(&stack); - let dest_packet = CSetContainerSlot::new(0, inventory.state_id as i32, slot, &slot_data); - self.client.send_packet(&dest_packet).await; - - if inventory - .set_slot(slot as usize, Some(stack), false) - .is_err() - { - log::error!("Pick item set slot error!"); + if let Err(err) = inventory.set_slot(slot, Some(stack), false) { + log::error!("Pick item set slot error: {}", err); + } else { + let dest_packet = CSetContainerSlot::new( + PlayerInventory::CONTAINER_ID, + inventory.state_id as i32, + slot as i16, + &slot_data, + ); + self.client.send_packet(&dest_packet).await; } } @@ -430,10 +434,10 @@ impl Player { let mut inventory = self.inventory().lock().await; let source_slot = inventory.get_slot_with_item(block.item_id); - let mut dest_slot = inventory.get_empty_hotbar_slot() as usize; + let mut dest_slot = inventory.get_empty_hotbar_slot(); - let dest_slot_data = match inventory.get_slot(dest_slot + 36) { - Ok(Some(stack)) => *stack, + let dest_slot_data = match inventory.get_slot(dest_slot + SLOT_HOTBAR_START) { + Ok(Some(stack)) => stack.clone(), _ => ItemStack::new(0, Item::AIR), }; @@ -443,35 +447,39 @@ impl Player { } match source_slot { - Some(slot_index) if (36..=44).contains(&slot_index) => { + Some(slot_index) if (SLOT_HOTBAR_START..=SLOT_HOTBAR_END).contains(&slot_index) => { // Case where item is in hotbar - dest_slot = slot_index - 36; + dest_slot = slot_index - SLOT_HOTBAR_START; } Some(slot_index) => { // Case where item is in inventory // Update destination slot let source_slot_data = match inventory.get_slot(slot_index) { - Ok(Some(stack)) => *stack, + Ok(Some(stack)) => stack.clone(), _ => return, }; - self.update_single_slot(&mut inventory, dest_slot as i16 + 36, source_slot_data) - .await; + self.update_single_slot( + &mut inventory, + dest_slot + SLOT_HOTBAR_START, + source_slot_data, + ) + .await; // Update source slot - self.update_single_slot(&mut inventory, slot_index as i16, dest_slot_data) + self.update_single_slot(&mut inventory, slot_index, dest_slot_data) .await; } None if self.gamemode.load() == GameMode::Creative => { // Case where item is not present, if in creative mode create the item let item_stack = ItemStack::new(1, Item::from_id(block.item_id).unwrap()); - self.update_single_slot(&mut inventory, dest_slot as i16 + 36, item_stack) + self.update_single_slot(&mut inventory, dest_slot + SLOT_HOTBAR_START, item_stack) .await; // Check if there is any empty slot in the player inventory if let Some(slot_index) = inventory.get_empty_slot_no_order() { - inventory.state_id += 1; - self.update_single_slot(&mut inventory, slot_index as i16, dest_slot_data) + inventory.increment_state_id(); + self.update_single_slot(&mut inventory, slot_index, dest_slot_data) .await; } } @@ -479,10 +487,10 @@ impl Player { } // Update held item - inventory.set_selected(dest_slot as u32); + inventory.set_selected(dest_slot); let empty = &ItemStack::new(0, Item::AIR); let stack = inventory.held_item().unwrap_or(empty); - let equipment = &[(EquipmentSlot::MainHand, *stack)]; + let equipment = &[(EquipmentSlot::MainHand, stack.clone())]; self.living_entity.send_equipment_changes(equipment).await; self.client .send_packet(&CSetHeldItem::new(dest_slot as i8)) @@ -1008,19 +1016,18 @@ impl Player { return Err(BlockPlacingError::InvalidBlockFace.into()); }; - let mut inventory = self.inventory().lock().await; - let entity = &self.living_entity.entity; - let world = &entity.world.read().await; - let slot_id = inventory.get_selected(); - let mut state_id = inventory.state_id; - let item_slot = *inventory.held_item_mut(); + let inventory = self.inventory().lock().await; + let slot_id = inventory.get_selected_slot(); + let held_item = inventory.held_item().cloned(); drop(inventory); + let entity = &self.living_entity.entity; + let world = &entity.world.read().await; let Ok(block) = world.get_block(&location).await else { return Err(BlockPlacingError::NoBaseBlock.into()); }; - let Some(stack) = item_slot else { + let Some(stack) = held_item else { if !self .living_entity .entity @@ -1082,6 +1089,7 @@ impl Player { // Decrease Block count if self.gamemode.load() != GameMode::Creative { let mut inventory = self.inventory().lock().await; + if !inventory.decrease_current_stack(1) { return Err(BlockPlacingError::InventoryInvalid.into()); } @@ -1090,8 +1098,8 @@ impl Player { .handle_decrease_item( server, slot_id as i16, - inventory.held_item(), - &mut state_id, + inventory.held_item().cloned().as_ref(), + &mut inventory.state_id, ) .await; } @@ -1140,10 +1148,10 @@ impl Player { return; } let mut inv = self.inventory().lock().await; - inv.set_selected(slot as u32); + inv.set_selected(slot as usize); let empty = &ItemStack::new(0, Item::AIR); let stack = inv.held_item().unwrap_or(empty); - let equipment = &[(EquipmentSlot::MainHand, *stack)]; + let equipment = &[(EquipmentSlot::MainHand, stack.clone())]; self.living_entity.send_equipment_changes(equipment).await; } @@ -1155,16 +1163,18 @@ impl Player { if self.gamemode.load() != GameMode::Creative { return Err(InventoryError::PermissionError); } - let valid_slot = packet.slot >= 0 && packet.slot <= 45; - let item_stack = packet.clicked_item.to_item(); + let valid_slot = packet.slot >= 0 && packet.slot as usize <= SLOT_OFFHAND; + // TODO: Handle error + let item_stack = packet.clicked_item.to_stack().unwrap(); if valid_slot { self.inventory() .lock() .await .set_slot(packet.slot as usize, item_stack, true)?; - } else { + } else if let Some(item_stack) = item_stack { // Item drop - self.drop_item(server, item_stack.unwrap()).await; + self.drop_item(server, item_stack.item.id, u32::from(item_stack.item_count)) + .await; }; Ok(()) } @@ -1180,9 +1190,6 @@ impl Player { // return; // }; // window_id 0 represents both 9x1 Generic AND inventory here - let mut inventory = self.inventory().lock().await; - - inventory.state_id = 0; let open_container = self.open_container.load(); if let Some(id) = open_container { let mut open_containers = server.open_containers.write().await; @@ -1198,6 +1205,13 @@ impl Player { } // Remove the player from the container container.remove_player(self.entity_id()); + + let mut inventory = self.inventory().lock().await; + if inventory.state_id >= 2 { + inventory.state_id -= 2; + } else { + inventory.state_id = 0; + } } self.open_container.store(None); }