Skip to content

Commit

Permalink
Fix dupe, double click, and shift click (re-PR) (#591)
Browse files Browse the repository at this point in the history 
* 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 <[email protected]>
Co-authored-by: Alexander Medvedev <[email protected]>
  • Loading branch information
@kralverde @4lve @Snowiiii
3 people authored Mar 1, 2025
1 parent a60a3ba commit d518732
Show file tree
Hide file tree
Showing 23 changed files with 859 additions and 564 deletions.
18 changes: 16 additions & 2 deletions pumpkin-data/build/item.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
});
Expand All @@ -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,
}

Expand Down Expand Up @@ -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<Self> {
pub fn from_registry_key(name: &str) -> Option<Self> {
match name {
#type_from_name
_ => None
Expand All @@ -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
}
}
}
}
19 changes: 15 additions & 4 deletions pumpkin-data/src/tag.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool> {
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<HashMap<RegistryKey, HashMap<String, Vec<Option<String>>>>> =
pub static TAGS: LazyLock<HashMap<RegistryKey, HashMap<String, Vec<String>>>> =
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<Option<String>>> {
pub fn get_tag_values(tag_category: RegistryKey, tag: &str) -> Option<&Vec<String>> {
TAGS.get(&tag_category)
.expect("Should deserialize all tag categories")
.get(tag)
Expand Down
52 changes: 38 additions & 14 deletions pumpkin-inventory/src/container_click.rs
View file
Original file line number Diff line number Diff line change
@@ -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<Self, InventoryError> {
match mode {
Expand All @@ -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,
Expand All @@ -29,15 +37,15 @@ impl Click {

fn new_normal_click(button: i8, slot: i16) -> Result<Self, InventoryError> {
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 {
Expand All @@ -55,8 +63,10 @@ impl Click {

fn new_key_click(button: i8, slot: i16) -> Result<Self, InventoryError> {
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)?,
};

Expand All @@ -66,15 +76,18 @@ impl Click {
})
}

fn new_drop_item(button: i8) -> Result<Self, InventoryError> {
let drop_type = match button {
0 => DropType::SingleItem,
1 => DropType::FullStack,
_ => Err(InventoryError::InvalidPacket)?,
fn new_drop_item(button: i8, slot: i16) -> Result<Self, InventoryError> {
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,
})
}

Expand Down Expand Up @@ -120,7 +133,7 @@ pub enum KeyClick {
Slot(u8),
Offhand,
}
#[derive(Copy, Clone)]
#[derive(Debug, Copy, Clone)]
pub enum Slot {
Normal(usize),
OutsideInventory,
Expand All @@ -131,6 +144,17 @@ pub enum DropType {
SingleItem,
FullStack,
}

impl DropType {
fn from_i8(value: i8) -> Result<Self, InventoryError> {
Ok(match value {
0 => Self::SingleItem,
1 => Self::FullStack,
_ => return Err(InventoryError::InvalidPacket),
})
}
}

#[derive(Debug, PartialEq)]
pub enum MouseDragType {
Left,
Expand Down
16 changes: 9 additions & 7 deletions pumpkin-inventory/src/crafting.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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<ItemStack>; 3]; 3]) -> Option<ItemStack> {
pub fn check_if_matches_crafting(input: [[Option<&ItemStack>; 3]; 3]) -> Option<ItemStack> {
let input = flatten_3x3(input);
RECIPES
.par_iter()
Expand Down Expand Up @@ -53,18 +55,18 @@ pub fn check_if_matches_crafting(input: [[Option<ItemStack>; 3]; 3]) -> Option<I
})
.map(|recipe| match recipe.result() {
RecipeResult::Single { id, .. } => 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
Expand All @@ -73,7 +75,7 @@ fn ingredient_slot_check(recipe_item: &RegistryEntryList, input: ItemStack) -> b
}
}
fn shapeless_crafting_match(
input: [[Option<ItemStack>; 3]; 3],
input: [[Option<&ItemStack>; 3]; 3],
pattern: &[[[Option<RegistryEntryList>; 3]; 3]],
) -> bool {
let mut pattern: Vec<RegistryEntryList> = pattern
Expand Down
68 changes: 35 additions & 33 deletions pumpkin-inventory/src/drag_handler.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,6 @@ impl DragHandler {
Err(InventoryError::MultiplePlayersDragging)?
}
let mut slots = container.all_slots();
let slots_cloned: Vec<Option<ItemStack>> = slots
.iter()
.map(|stack| stack.map(|item| item.to_owned()))
.collect();
let Some(carried_item) = maybe_carried_item else {
return Ok(());
};
Expand All @@ -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,
})
}
}
});
Expand All @@ -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)
Expand All @@ -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;
Expand All @@ -150,24 +149,27 @@ struct Drag {
}

impl Drag {
fn possibly_changing_slots<'a>(
&'a self,
slots: &'a [Option<ItemStack>],
fn possibly_changing_slots(
&self,
slots: &[&mut Option<ItemStack>],
carried_item_id: u16,
) -> impl Iterator<Item = usize> + '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()
}
}
Loading

0 comments on commit d518732

Please sign in to comment.