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 CraftBukkit drag system #10703

Merged
merged 9 commits into from
Aug 17, 2024
Merged

Conversation

notTamion
Copy link
Contributor

resolves #9915

@notTamion notTamion requested a review from a team as a code owner May 12, 2024 13:36
@Machine-Maker
Copy link
Member

Machine-Maker commented May 12, 2024

I feel like despite what CB's comment says about being unable to call the click event, we could call it by just mimicking some of the checks in the case QUICK_CRAFT in ServerGamePacketListenerImpl.

int currentStatus = this.player.containerMenu.quickcraftStatus;
int newStatus = AbstractContainerMenu.getQuickcraftHeader(packet.getButtonNum());
if ((currentStatus != 1 || newStatus != 2 && currentStatus != newStatus)) {
} else if (this.player.containerMenu.getCarried().isEmpty()) {
} else if (newStatus == 0) {
} else if (newStatus == 1) {
} else if (newStatus == 2) {
    if (!this.player.containerMenu.quickcraftSlots.isEmpty()) {
        if (this.player.containerMenu.quickcraftSlots.size() == 1) {
            // fire "click" event
        }
    }
}

Just put this right before the call to AbstractContainerMenu#clicked

@notTamion
Copy link
Contributor Author

I am having a bit of trouble understanding what you mean exactly. but if i am interpreting your comment correctly that wouldn't work because we don't have access to the new Items that early in the stack. i should probably also mention that cb's comment is wrong (assuming we want to call InventoryDragEvent if the mouse is moved in just the same slot, this way we don't break the existing calls for InventoryDragEvent and this also allows plugins to check whether or not the single item was dragged into the slot or clicked which might be useful for some developers)

@notTamion notTamion force-pushed the fix-craftbukkit-drag branch 2 times, most recently from 30e4070 to 7fe1055 Compare May 26, 2024 21:39
@notTamion
Copy link
Contributor Author

@Machine-Maker i was able to construct a packet that seems to be calling the event correctly aswell as not breaking vanilla logic (i think) like you proposed yesterday.

Copy link
Contributor

@lynxplay lynxplay left a comment

Choose a reason for hiding this comment

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

LGTM.
Current drawback being that single slot "draws" do no longer
trigger the draw event. E.g. what is just a click event but with
a held down right click in a single slot.

Up for a core/maintainer to decide if the minimal break from spigot is fine
here, imo it is.

@lynxplay lynxplay merged commit 4401748 into PaperMC:master Aug 17, 2024
3 checks passed
@notTamion notTamion deleted the fix-craftbukkit-drag branch August 18, 2024 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

PaperMC breaks the intended Vanilla behavior of emptying bundles.
4 participants