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

Changes on EntityPickupItemEvent not reflected #10065

Open
rainbowdashlabs opened this issue Dec 21, 2023 · 2 comments · May be fixed by #10256
Open

Changes on EntityPickupItemEvent not reflected #10065

rainbowdashlabs opened this issue Dec 21, 2023 · 2 comments · May be fixed by #10256
Labels
status: accepted Disputed bug is accepted as valid or Feature accepted as desired to be added. type: bug Something doesn't work as it was intended to. version: 1.20.2 Game version 1.20.2

Comments

@rainbowdashlabs
Copy link

Expected behavior

Changes done in the event to the item stack associated with the event should be reflected on the actual picked up item.

Observed/Actual behavior

The changes are not reflected and are ignored.

Steps/models to reproduce

Example code: https://code.lewds.de/BQ7OpU

Output showing that even though the item was changed it didnt end up in the inventory
image

The whole process is:

  • I drop a diamond to a zombie
  • When picking up an item I set a key inside the pdc.
  • I check whether the key is actually there. It is.
  • I check after two ticks if the item in the hand. It is not.
  • The zombie gets killed after 20 ticks
  • The zombie drops a diamond without any pdc keys set.
  • I pick the diamond up. The key is added again and present in my inventory.
  • I drop the now tagged item to a zombie again.
  • The zombie picks up my item and the key is now actually there

Plugin and Datapack List

Only the example plugin above.

Paper version

1This server is running Paper version git-Paper-318 (MC: 1.20.2) (Implementing API version 1.20.2-R0.1-SNAPSHOT) (Git: 9271ee7)
You are running the latest version
Previous version: git-Paper-196 (MC: 1.20.1)

Other

No response

@rainbowdashlabs rainbowdashlabs added status: needs triage type: bug Something doesn't work as it was intended to. labels Dec 21, 2023
@lynxplay
Copy link
Contributor

https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/browse/nms-patches/net/minecraft/world/entity/EntityInsentient.patch#159-188

While upstream passes the itementity, which intern is used for the event, the item stack passed to the method is used instead of the "updated" one from the item entity.

The method also receives a copy of the item entities itemstack, so we don't magically update it with a craft mirror rn either.

@lynxplay lynxplay added status: accepted Disputed bug is accepted as valid or Feature accepted as desired to be added. version: 1.20 and removed status: needs triage labels Dec 21, 2023
@Machine-Maker Machine-Maker changed the title Changes on ItemPickupEvent not reflected Changes on EntityPickupItemEvent not reflected Dec 21, 2023
@Machine-Maker
Copy link
Member

The EntityPickupItemEvent is just wrong right now. It is incorrect to pass the Item (entity) as the thing being picked up because in vanilla, the stack actually being kept by the mob isn't always the full stack in the entity. Piglins, for example, can pickup 1 item at a time from an item entity and the event doesn't give you that context.

I think the solution here, is to add an ItemStack field to that event, deprecate getItem() (just to rename it to getSourceItem) and tell people to check the stack instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepted Disputed bug is accepted as valid or Feature accepted as desired to be added. type: bug Something doesn't work as it was intended to. version: 1.20.2 Game version 1.20.2
Projects
Status: ✅ Accepted
Development

Successfully merging a pull request may close this issue.

4 participants