Skip to content
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 InventoryAction wrong for Bundles #11313

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

notTamion
Copy link
Contributor

Follow-up pr to #10703. Fixes an issue i discovered while discussing the pr. Since craftbukkit manually calculates the InventoryAction for the click events it doesn't take bundles into account. I am not sure whether PICKUP_ONE_INTO_BUNDLE and PLACE_ONE_INTO_BUNDLE are needed since there isn't a way to explicitly get 1 item out of a bundle like with right clicking a normal stack (or whether the new enums are needed at all)

@notTamion notTamion requested a review from a team as a code owner August 21, 2024 13:52
@Leguan16
Copy link
Contributor

Is this worth updating given that bundles are still experimental?

@notTamion
Copy link
Contributor Author

Considering mojang is now also adding bundles to bedrock and have an experiment for further updates to it, i would definitely say that it is worth fixing this. As long as there aren't any changes in the way bundles are used in terms of how you pickup items and etc. i don't think this fix will pose an issue when updating mc versions.

Copy link
Member

@Machine-Maker Machine-Maker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you looked ahead to the 1.21.2 changes to the bundle? I haven't looked in depth at some of the functionality changes to see if any of these "actions" would no longer exist or be named something else.

Either way, these need to be annotated with the ApiStatus.Experimental annotation so we can make any future changes until the bundle isn't experimental anymore.

@notTamion
Copy link
Contributor Author

there is a small change in the experiment where right and left click were exchanged which here could just be fixed by making a change to an if statement. With my comment above i meant bigger changes for which we would need to delete an enum or move big portions of code around. We can ofc also wait for what mojang actually decides to do with bundles in 1.21.2 and decide what we do from there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Changes required
Development

Successfully merging this pull request may close these issues.

3 participants