Paper
Paper copied to clipboard
Fix cancelling PlayerDropItemEvent for carried items
Fixes https://github.com/PaperMC/Paper/issues/7726
Now if the PlayerDropItemEvent is cancelled and the drop is trigged by clicking outside a menu, the item remains on the cursor instead of going back into the inventory or being deleted if the inventory is full.
There needs to be a consensus on what happens if the player closes the inventory with the item in their cursor and the drop item event is cancelled (and the inventory is full). Thoughts on what should happen?
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
This issue has been automatically closed because it has not had activity in a long time. If the issue still applies to the most recent supported version, please open a new issue referencing this original issue.
Yeah, I made some changes cause I spotted a bug in what I was doing before, but I didn't test those. I guess I broke it more.
Ok, fixed that issue. Also I changed what the cursor is set to if the event is cancelled. It gets the itemstack from the item drop. So if an event handler changes the Item entity and then cancels the event it uses that itemstack in the item entity. Is that right? I don't know if we do that elsewhere, take an action based on the event if its cancelled.
Since I came across this issue again, how about being able to provide a consumer for the itemstack, that will only accept the item if it has no other place to go. That would allow for plugin authors to handle the "lost" item as they wish. For example adding it to a "stash" of items, which may be used in other places across the server to prevent item loss in the event of any unexpected errors.
I like this idea. But what should the default behavior be? Just delete the item?
I guess so, maybe add a warning to the javadoc that one has to be cautious when cancelling the event and may need to provide further handling to prevent item loss? (And that is "known" behavior anyways right now)
Ok, added that overflow consumer. Tested it as well, setting the consumer to drop the itemstack in the world anyways. By default, it just sets the count of the stack to 0 to "delete" the itemstack.
Any chance this can get merged at some point? Adding an overflow consumer seems like the best option imo.