-
-
Notifications
You must be signed in to change notification settings - Fork 184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix dupe, doubble click and shift click #577
base: master
Are you sure you want to change the base?
Conversation
Does this fix any issues ?, Maybe #547 |
Yep, it fixes that issue |
I notice that our currently implementation has a bug which disallows the player to interact with an item the first time they open there Inventory. Can you may fix this as well? |
Ye that's odd, but that was a bug before I think cause even logging all clickevents it won't show the first interaction |
That issue is caused because the state id is off sync |
I fixed so it works first try now, and I also added partial support for equipping armor with shift clicking |
Nice, It fixed now. Great job @4lve. So i have one last bug i would love to have fixed 😅. Maybe you noticed that when you begin to drag an item in inventory and then close the inventory while having the stack dragged, it is not reseted but still being dragged when open the inventory the next time |
@Snowiiii Thx, I could maybe fix that in another PR |
0 => DropType::SingleItem, | ||
1 => DropType::FullStack, | ||
_ => Err(InventoryError::InvalidPacket)?, | ||
}; | ||
let slot = match slot { | ||
-999 => Slot::OutsideInventory, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you create constants for these magic numbers? like left click, right click, outside
818, // Wooden | ||
823, // Stone | ||
828, // Gold | ||
833, // Iron | ||
838, // Diamond | ||
843, // Netherite | ||
Item::WOODEN_SWORD.id, // Wooden | ||
Item::STONE_SWORD.id, // Stone | ||
Item::GOLDEN_SWORD.id, // Gold | ||
Item::IRON_SWORD.id, // Iron | ||
Item::DIAMOND_SWORD.id, // Diamond | ||
Item::NETHERITE_SWORD.id, // Netherite |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use our item tags for this instead of explicitly writing them out. See #566 (Ideally with my recommendations to @Jayryn) See also https://minecraft.wiki/w/Item_tag_(Java_Edition)
pub fn increment_state_id(&mut self) { | ||
self.state_id = self.state_id % 100 + 1; | ||
} | ||
|
||
pub fn held_item(&self) -> Option<&ItemStack> { | ||
debug_assert!((0..9).contains(&self.selected)); | ||
self.items[self.selected as usize + 36 - 9].as_ref() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also make constants for the inventory locations? Like MAX_SLOT, HOTBAR_SLOTS = (0..9), etc.
Hey @4lve I made a review, should be good to merge after. The item dragging can be another PR. Could you help me understand what the state_id does? At the end there was something weird with |
Idk, I just found that the state id decreases by 2 after closing a container. Found it by trail and error |
Description
This PR fixes:
Testing
Please follow our Coding Guidelines