NeoForge icon indicating copy to clipboard operation
NeoForge copied to clipboard

Give command's "Fake" Entity assigning is too late

Open Soaryn opened this issue 10 months ago • 2 comments

Minecraft Version: 1.21.4 (valid to at least 1.20.1)

NeoForge Version: 21.4.75-beta

Steps to Reproduce:

  1. Set a break point in GiveCommand around line 70
  2. In game /give @s cobblestone 128 This should give the player 2 stacks of cobble, but notice that the "fake" entity is being initialized at the break point.
  3. Because the construction, post of events, and handling of cancellations all happen before the entity is told it is fake, event consumers will misfire.

Description of issue: Typical use of EntityJoinLevelEvent is to cut down on entity object churn when collecting items from say a mob farm. Canceling this event basically does just that; however, the give command creates "fake" entities when the player absorbs all of the command's items. We need a different way to either inform the consuming events that the entity is fake, or determine if the vanilla "fake item" for a single tick is a bug/feature.

Reference for problem: https://github.com/Soaryn/XyCraftTracker/issues/81

Soaryn avatar Mar 01 '25 09:03 Soaryn

The main issue from what I can see is that the ItemEntity sets all of its 'fake' settings after the entity has already been added to the world. If it was set beforehand, it would be relatively simple to just check if the pickupDelay is 32767, assuming that mods don't want call setNeverPickUp at any other point. Though it wouldn't be difficult to add another field that holds the true value in case that assumption is violated.

As for setting the fake item after being added to the world, I don't really see a reason why it couldn't be marked beforehand as the entity wouldn't be ticked before the method was called. The only reason I can see of why its set afterward is because its using the common ServerPlayer#createItemStackToDrop to construct the stack.

How to necessarily do such a thing is a point of contention. Capturing the 'fakeness' is possible if we use a ThreadLocal due to chunk load async. Another method would be to patch in a new drop method take contains a boolean on whether it's fake or not, though it's use case is only limited to the give command.

ChampionAsh5357 avatar Mar 02 '25 16:03 ChampionAsh5357

Was curious if any additional thought given to this one? Albeit it only happens with something like the /give command, but it seems quite a few things like to abuse that for quest reward systems.

Still a problem in 1.21.9

Soaryn avatar Oct 02 '25 06:10 Soaryn