/give will give twice if the item hasCustomEntity

[mezz]

The expected behavior:

CommandGive.processCommand will add an item to the player's inventory, and also spawn an item.

if (adding item succeeded) 
    item entity is spawned and modified so that it instantly disappears, giving a visual effect.
else 
    item entity is spawned and given an owner and no pickup delay.

The problem:

The item entity that CommandGive has is not the same one that is spawned if the item hasCustomEntity.
This is because entityplayer.dropPlayerItemWithRandomChoice returns the regular EntityItem instead of the custom one that is actually spawned.
CommandGive sets the special properties on the wrong item entity, so the custom entity that is spawned hangs around like a regular dropped item.

After trying to PR:

I think this is not simple to fix because Item.createEntity returns an Entity, and all the relevant methods require an EntityItem.

[LexManos]

I'm not sure there is a way to fix this, if anyone has any ideas let me know.
The spawned one by /give is simply a 'here is a item in your face!' graphic...

[Spartan322]

The only possible way I see to fix this so far is to force Item.createEntity to return an EntityItem (but that of course requires breaking mods which use that method), the next best thing is to detect if the result from createEntity is an instance of EntityItem and handle it if it is (that seems better then nothing), and otherwise ignore it (unless that's what already happens, in which case I was useless).

Another thing you could do is deprecate createEntity and ask for something like createEntityItem instead, but that's more of a workaround (sorry for stating the obvious)

[LexManos]

Sadly the only real way to do it is to capture all entities added to world in this event.
Then add a scheduled task to the next tick to kill all those entities.
I don't feel comfortable doing that dirty of a hack.
Also, FORCING people to use EntityItem for this dirty hack of a visual thing is just stupid... I'd rather have the item not exist int he world at all then have this issue.
Which actually we could do by checking if hasCustomEntity == false before spawning the popup.

[mezz]

There is still the "useful" case of having a full inventory. When that happens,
item entity is spawned and given an owner and no pickup delay.
It has the same problem with the custom entities. It's not super important, but it has real functionality unlike the visual thing.

I don't know the disadvantages of restricting to EntityItem (is there any example?), but it seems like the correct way.
It's not much different from Entity besides the pickup logic, but is does prevent dropping an EntityLiving or something. The only example I could think of was Forestry's butterflies but those are actually using EntityItem, and use Item.onEntityItemUpdate to spawn a butterfly entity when conditions are right.

[mezz]

After sleeping on the issue I think I agree with lex, the easiest solution is to check for custom entity and prevent the item spawning.

[LexManos]

The 'real value' is fine, and honestly would work just fine the entity just lands on the ground and if its custom use the custom mechanic to pick it up. That part i'm not worried about it's the stupid visual one that I am.

[stale]

This issue has been automatically marked as stale because it has not had activity in a long time, and will be closed if no further activity occurs. If this issue was overlooked, forgotten, or should remain open for any other reason, please reply here to call attention to it and remove the stale status. Thank you for your contributions.

[mezz]

Tested in 1.12 and this still happens with items that have a custom entity item like tinker's tools.

Github image upload isn't working for me today so here's a linked screenshot instead:
https://i.imgur.com/tMm2Ldb.png

[stale]

This issue has been automatically marked as stale because it has not had activity in a long time. If this issue is still relevant and should remain open, please reply with a short explanation (e.g. "I have checked the code and this issue is still relevant because ___." or "Here's a screenshot of this issue on the latest version"). Thank you for your contributions!

[mezz]

Lex, any thoughts on if we can fix this in 1.13?
I still sort of think Item.createEntity should drop EntityItem. I'm not sure what could need the flexibility of dropping Entity.

[stale]

This issue has been automatically marked as stale because it has not had activity in a long time. If this issue is still relevant and should remain open, please reply with a short explanation (e.g. "I have checked the code and this issue is still relevant because ___." or "Here's a screenshot of this issue on the latest version"). Thank you for your contributions!

[mezz]

This doesn't appear to be an issue in 1.13

[maxoupidou]

Hello I propose a solution to the problem of the double item give during the give command, it may not be suitable for everyone but it works:
rather than making a classic give: /give @p tconstruct:shuriken 1 0 {display:{Name:"§fShuriken epineux"},StatsOriginal:{AttackSpeedMultiplier:1.0f,...

I invoke the object on the reader and there are no duplicates
exemple : summon Item ~ ~1 ~ {Item:{id:"tconstruct:shuriken",Count:1,tag:{display:{Name:"§fShuriken epineux"},StatsOriginal:{AttackSpeedMultiplier:1.0f,....

to make it appear on a player, I can do: /execute {player} ~ ~ ~ summon Item ~ ~1 ~ {Item:{id:"tconstruct:shuriken",Count:1,tag:{display:{Name:"§fShuriken epineux"},StatsOriginal:{AttackSpeedMultiplier:1.0f,...

I hope I was able to help you

[williewillus]

This was fixed 4 years ago.

[maxoupidou]

This was fixed 4 years ago.

Maybe in the new version but not in 1.12.2 for example

[TheCurle]

We stopped developing 1.12 many, many, many years ago.