[20.6] Add events to determine if item pickup should be allowed or not

[ApexModder]

This PR implements a few new events fired during item pickup. ItemPickupAllowedEvent - This event is used to determine if pickup should be allowed or not, via the event Result. ALLOW - Forcefully allow pickup (if possible) DEFAULT - Default behavior DENY - Disallow pickup

ItemPickupEvent - This event is fired so that modders can listen in on when an item was picked up and why.

These events come with the following sub-classes to know the target attempting to pickup the item, ByPlayer / ByMob and ByHopper.

Modders are welcome to implement and fire their own sub classes for more mod specific events. For example ItemPickupAllowedByBeltEvent / ItemPickupByBeltEvent for create belts.

---

The following events have been marked as deprecated as they should be replaced by the above events. While these events are now legacy and marked as deprecated, they are still in use and take priority over the newer events. This is to lessen the amount of broken mods caused by this change as possible.

• EntityItemPickupEvent - Replaced by ItemPickupAllowedEvent.ByPlayer
• PlayerEvent.ItemPickupEvent - Replaced with ItemPickupEvent.ByPlayer

---

A new test mod as been introduced to test the above event types DisallowItemPickupTest. When this test is enabled the following should be true

• Players should not be allowed to pickup any item if they are sneaking.
• Hoppers should not be allowed to pickup #gems/diamond.
• Mobs should be forcefully allowed to pickup items when standing on minecraft:stone
• Mobs should not be allowed to pickup items when standing on minecraft:diamond_block

[neoforged-pr-publishing[bot]]

• [x] Publish PR to GitHub Packages

Last commit published: d9844b8a63fecd497efbcb87826e3182a1b65d8d.

PR Publishing

The artifacts published by this PR:

• :package: net.neoforged:neoforge:20.6.45-beta-pr-692-item-entity-no-pickup
• :package: net.neoforged:testframework:20.6.45-beta-pr-692-item-entity-no-pickup

Repository Declaration

In order to use the artifacts published by the PR, add the following repository to your buildscript:

repositories {
    maven {
        name 'Maven for PR #692' // https://github.com/neoforged/NeoForge/pull/692
        url 'https://prmaven.neoforged.net/NeoForge/pr692'
        content {
            includeModule('net.neoforged', 'neoforge')
            includeModule('net.neoforged', 'testframework')
        }
    }
}

MDK installation

In order to setup a MDK using the latest PR version, run the following commands in a terminal.
The script works on both *nix and Windows as long as you have the JDK bin folder on the path.
The script will clone the MDK in a folder named NeoForge-pr692.
On Powershell you will need to remove the -L flag from the curl invocation.

mkdir NeoForge-pr692
cd NeoForge-pr692
curl -L https://prmaven.neoforged.net/NeoForge/pr692/net/neoforged/neoforge/20.6.45-beta-pr-692-item-entity-no-pickup/mdk-pr692.zip -o mdk.zip
jar xf mdk.zip
rm mdk.zip || del mdk.zip

To test a production environment, you can download the installer from here.

[Technici4n]

This is a vanilla feature which disallows only players by default

I find that a bit concerning. Isn't this a behavior change compared to vanilla?

[Matyrobbrt]

We should not diverge from vanilla. Was this "bug" reported to Mojang and if so, what is its status?

[sciwhiz12]

This is definitely a behavior change compared to vanilla. See the following Mojira bug reports which were closed as "Works As Intended":

• https://bugs.mojang.com/browse/MC-47547 "Items with the PickupDelay tag set to 32767 can still be picked up by hoppers."
• https://bugs.mojang.com/browse/MC-125496 "Hoppers ignore PickupDelay and Owner of item entities making the tags useless"

Note the comment by a Mojang employee, Searge, on the first issue:

The tag [referring to PickupDelay] only prevents items from being picked up by players walking over the item on the ground.

I would expressly deny this PR based on that alone, as we still hold to the principle that unmodified NeoForge (i.e., without any additional mods) should be as close to vanilla's behavior as possible.

[Shadows-of-Fire]

Could guard it with a config option or a mod internal flag, but no we can't have it enabled by default due to Mojang's stance here.

[Technici4n]

Please no random config options (baby zombie spawn chance anyone?) anymore...

[marchermans]

We should make this, if we consider it all, a different option. To allow modders to spawn items which can not be picked up at all. Aka vanity items.

[marchermans]

We should make this, if we consider it all, a different option. To allow modders to spawn items which can not be picked up at all. Aka vanity items.

And with option I mean a different method on the entity. Not a config option

[Shadows-of-Fire]

I think having the option as a different NBT key is a good solution. Similar to the mostly-agreed upon "no magnetism" tag

[sciwhiz12]

Aside from what @marchermans suggested, another reason I could see for implementing a patch similar to this would be a potential fix for MC-120507 ("Item duplication via /give"), where a hopper minecart is able to pick up the "fake item" created by /give (with infinite pickup delay and one-tick life remaining). That bug would affect anything else which does the same logic as /give[^give] in giving the player items.

I agree with @Shadows-of-Fire that this should be a separate NBT key on the item entity, to differentiate it from vanilla's PickupDelay that affects only players. I'd expect methods on ItemEntity to set and query that NBT key (and to allow us to change the backing implementation in the future, if need be).

[^give]: /give works by trying to add the items to the player's inventory directly. If that works (because the player has room remaining in their inventory, including if they are in creative mode), then a "fake item" entity is created (with infinite pickup delay and one-tick life remaining) for visual effect. Otherwise, it drops whatever cannot be placed directly into the inventory onto the ground as item entities (pickupable immediately and only by the target of the /give command).

[ApexModder]

As requested, i have moved the logic off to being its own nbt key instead of hooking into vanillas pickupDelay. pickupsAllowed which is set to true by default, allowing pickups to occur, when set to false all pickups are disallowed taking priority over the infinite pickupDelay logic.

This new flag can be set from either the entities nbt {NeoForge_PickUpsAllowed:0b} or via ItemEntity.setPickupsAllowed(false)

Edit: Although after making these changes I am thinking that this should maybe be pushed off to some event, maybe modify the existing pickup events to allow them to be used with more than just players? This would allow more flexibility over when a item should be picked up or not, rather than a simple on/off toggle flag.

[ApexModder]

I have migrated this PR so that rather than it being a simple on/off flag in the ItemEntity, modders can now listen for the ItemPickupAllowedEvent event and change the result based on whether or not item pickup should be allowed.

ALLOW - Forcefully allow pickup (if possible) DEFAULT - Default behavior DENY - Disallow item pickup

I have also added a ItemPickupEvent which is fired when ever some source pickups an item entity. Sub classes for these events exist to know the target trying to pickup the item ByPlayer/ByMob/ByHopper. For more mod specific types (belts, ender hopper, obsidian pipes) modders should be firing their own set of ItemPickupAllowedEvent/ItemPickupEvent.

With these event changes I have also deprecated the events EntityItemPickupEvent/PlayerEvent.ItemPickupEvent as they have been replaced with the above events. These legacy events are are still being used and take priority during player pickup, so that existing mods should not be broken by this change.

[Shadows-of-Fire]

Going to stop that review early. I think this has gotten too complex for the intended goal, which was just to mark certain items as being unable for pickup.

If this was trimmed to a simple ItemAllowPickupEvent (without context of who is doing the pickup), I could see this being a reasonable solution, but as-is I would not accept this solution.

[ApexModder]

All changes from previous commits have been reverted and a simplified ItemAllowPickupEvent has been implemented, where when cancelled item pickup is disallowed.

EntityItemPickupEvent has been marked deprecated as it would eventually be fully replaced by ItemAllowPickupEvent. With this deprecation EventHooks.onItemPickup has also been marked as deprecated and modified to check both events for if item pickup is allowed, where EntityItemPickupEvent takes priority over ItemAllowPickupEvent.

[Mrbysco]

Another use-case would be for mods that add drops to Glow Squids. Avoiding zombies from pickup up the added drops so that you won't have massive hordes of zombies holding the mob cap hostage.

Image example

[Shadows-of-Fire]

@ApexModder The PR needs to be rebased against 1.20.x to resolve conflicts.

[ApexModder]

Rebased onto latest 20.6 changes 👍 Wont lie I kinda forgot this PR was open, been too focused on 20.5/6 stuff recently

[embeddedt]

Something went wrong with the rebase; there are numerous unrelated changes showing in the Files tab.

[Matyrobbrt]

Why is the event with more context (the player) being deprecated? This seems like a downgrade in functionality.

[ApexModder]

The original implementation of this PR had the context of who is picking up the item, the deprecation is a remnant of that original implementation, now that you mention it though the deprecation doesn't make much sense since the event has been stripped down and lost that context.

[ApexModder]

Closing as PR went stale and overall seems very controversial