Possible item loss when cancelling PlayerDropItemEvent

[aerulion]

Expected behavior

Assuming a player stands inside a pile of dropped items, then starts moving an item in his inventory with his cursor and thereby filling the empty slot with any of the dropped items. If the player now clicks outside his inventory to drop the cursor item, the event should get cancelled and the item should remain attached to the cursor.

Observed/Actual behavior

When clicking outside the inventory the item will simple get deleted if the players inventory is full.

Steps/models to reproduce

1. Cancel all PlayerDropItemEvents.
2. Stand in a pile of dropped items with a full inventory.
3. Move an item in your inventory with your cursor.
4. Click outside the inventory in order to drop the item.
5. Watch the item being deleted.

Plugin and Datapack List

Just a test plugin cancelling the event.

Paper version

This server is running Paper version git-Paper-283 (MC: 1.18.2) (Implementing API version 1.18.2-R0.1-SNAPSHOT) (Git: f8e8d6c)
You are running the latest version
Previous version: git-Paper-277 (MC: 1.18.2)

Other

No response

[Machine-Maker]

Just to note, you have to drop an item that won’t combine with any other stack in the inventory, cause if it does, it’ll go there and not be deleted. But I did confirm this happens.

[aerulion]

Another case I noticed is that the item will also get deleted when closing the inventory with an item attached to the cursor, but I haven't had the time to test whether this will be covered by your linked pr.

[Machine-Maker]

Another case I noticed is that the item will also get deleted when closing the inventory with an item attached to the cursor, but I haven't had the time to test whether this will be covered by your linked pr.

yeah its not, and yeah it will get deleted now. What do you think should happen? Should it stay on the cursor even tho the window is closed? that doesn't seem right. maybe that specific case ignores cancellation?

[aerulion]

Hmm, i don't think generally ignoring cancellation in this case would be a good idea, since I could get abused if it is intended for a player to not drop any items at all (for example mass dropping items with hack clients). The only option that comes to my mind would be either just letting the item get deleted and "accepting current behavior" or providing an option to set something like PlayerDropItemEvent#ignoreCancellationWithFullInventory(boolean ignore) and letting the user decide individually for each event / use case.

[imDaniX]

yeah its not, and yeah it will get deleted now. What do you think should happen? Should it stay on the cursor even tho the window is closed? that doesn't seem right. maybe that specific case ignores cancellation?

While I do share an opinion that it doesn't seem right, I don't think there's a way around it. Both removing the item and ignoring cancellation do sound like a very unexpected behavior - it means that we'll lost an item either way, while with the "stay" behavior we still could get an item using Player#getItemOnCursor.

[Machine-Maker]

Yeah I guess I'll do that. not a great solution either way.