Stumped on InventoryDragEvent manipulation - CB implementation bugged?

Discussion in 'Plugin Development' started by AnorZaken, Aug 24, 2014.

Thread Status:
Not open for further replies.
  1. Offline

    AnorZaken

    Edit: The ticket has been created and is here: https://bukkit.atlassian.net/browse/BUKKIT-5785
    -----

    If you don't cancel the event craftbukkit will overwrite any change you've made to the draged slots.
    If you cancel the event craftbukkit will overwrite any change you've made to the item on the cursor.

    Really...?

    The API javadoc even suggest they should both be changeable (and why indeed shouldn't they??)

    But the actual implementation says trololol! >:[

    A. If the event-result isn't Denied, craftbukkit overwrites changes made to the dragged slots ("draggedSlots" is computed before the event is fired) and performs any change to the item on the cursor that plugins wanted.
    = You are allowed to change cursor item but not slot contents.

    B. If the event-result is Denied, craftbukkit leaves the slots alone, but forces an overwrite of the item on the cursor, completely disregarding any change to the cursor item that was requested by the plugins that processed the event.
    = You are allowed to change slot contents but not the cursor.

    > This is not sane!

    One of the above behaviors needs to change:
    Either the loop in A should use the "eventmap" variable and let plugins control the slot contents through the event (seems sane especially if you consider how this propagates updated information as the event travels between listeners), or B should not ignore changes made to the cursor item. Probably both actually, but currently the issue with B is what is most frustrating! "Denied" != Revert , don't randomly revert part of the changes manually performed by the plugins! They where probably done for sane reasons. Verify that no change was made (to the cursor) by the plugins before performing a revert (as many other events correctly does).

    ...
    As it stands it is impossible for me to perform an InventoryDragEvent where I allow the drag but manipulate the contents of the dragged slots. Only way to do it is to set the result to disabled - but then I get item duplication because the original stack cant be removed from the cursor!

    It's driving me nuts! Am I missing something here?
    Did the implementation really make this simple thing impossible for no reason (aka is this a bug)?

    What is the solution/best workaround?
    I'm thinking it's to set the cursor item to null with a 1-tick delay, but what if the user closes the inventory really fast? That opens up a small possibility of duplication! :(
    Other option is to manipulate the slots with a 1-tick delay, but again this opens up for possible issues!
    I'm open to suggestions at this point... :(

    *bump*
    As justification for requiring a change of this behavior look at it's sibling, InventoryClickEvent, setting the result to Denied doesn't force any revert actions on the server state, and allows plugins full control of the outcome.

    EDIT by Moderator: merged posts, please use the edit button instead of double posting.
     
    Last edited by a moderator: Jun 10, 2016
  2. Offline

    Necrodoom

    You should make a ticket on the tracker for this.
     
  3. Offline

    AnorZaken

    Thanks, I suspected that was the case, but wanted a second opinion before I did. :)
     
  4. Offline

    fireblast709

    Scroll up, read the code, realize that the event called at that line is an InventoryClickEvent.

    Next up, clone CB, do a global search over the whole NMS package for an InventoryDragEvent, and find the following:
    https://github.com/Bukkit/CraftBukk...net/minecraft/server/Container.java#L183-L206

    Seems perfectly fine to me.
     
  5. Offline

    AnorZaken

    Um
    Umm.... you didn't read what I wrote at all. No offense but read what you quoted... and you linked to the exact same CB-code that I did... so if that code seems sane to you then I assume you could just as well have been the one that wrote it. Read what I said, then read the code you (and I) linked to more carefully. You will see the code does exactly what I claimed it does - and that ain't sane.

    Edit: The ticket has been created and is here: https://bukkit.atlassian.net/browse/BUKKIT-5785
     
  6. Offline

    fireblast709

    No I wasn't, xD. I simply misread a lot.
     
    AnorZaken likes this.
  7. Offline

    AnorZaken

    No harm done. The quote-thing was slightly amusing. :p
     
Thread Status:
Not open for further replies.

Share This Page