SpongeForge icon indicating copy to clipboard operation
SpongeForge copied to clipboard

Duplication bug with Soul Bound (EnderIO)

Open runescapejon opened this issue 5 years ago • 12 comments

I am currently running

  • SpongeForge version:
  • Forge version: 14.23.5.2838
  • Java version: 8
  • Operating System: windows 10
  • Plugins/Mods:
  • spongeforge-1.12.2-2838-7.1.7-RC3847
  • Nucleus-1.12.1-S7.1-MC1.12.2-plugin
  • LuckPerms-Sponge-4.3.109
  • EnderIO-1.12.2-5.0.46
  • EnderCore-1.12.2-0.5.59

Issue Description So, let me provide the step to reproduce this bug.

  1. You would need to spawn a anvil (vanilla minecraft), Soul Bound enchantment book (EnderIO). Then get an armor piece for example boots. Soul Bound is when the player dies, any item enchanted with Soulbound will be kept in inventory.

  2. put boots (example) in the anvil then the Soul Bound enchantment book, then enchant it.

  3. Make sure you have permission to nucleus.inventory.keepondeath from Nucleus You can type on a clean test server lp user {name} permission set nucleus.inventory.keepondeath

  4. Then Die, you will have the boots in your inventory armor slots and you will have an extra boot duplication glitch in your hotbar.

This seem to be relate to https://github.com/SpongePowered/SpongeForge/issues/2874 EDIT: Make sure on your test server you have gamerule keepinventory false

runescapejon avatar Jul 19 '19 18:07 runescapejon

Pretty sure I may have fixed this more recently, can you re-test with latest?

gabizou avatar Aug 02 '19 16:08 gabizou

I can still able to reproduce it using spongeforge-1.12.2-2838-7.1.7-RC3870 which is the current latest version

runescapejon avatar Aug 02 '19 19:08 runescapejon

I'm also having this issue with soulbound from enderio and cofh. The items get duped. Server is on Nucleus 1.13.2 and Sponge RC3903.

jinkhya avatar Aug 27 '19 16:08 jinkhya

Still an issue on spongeforge-1.12.2-2838-7.1.9-RC3988

runescapejon avatar Jan 26 '20 14:01 runescapejon

Having this issue with Spongeforge-1.12.2-2838-7.1.10

Spontini avatar Mar 27 '20 05:03 Spontini

I'm very late to this bug, Having the same issue. Not quite sure about the versions, but we're running FTB Revelation with the latest sponge and nucleus with the keep inventory perms.

ZeroCool5254 avatar May 21 '20 14:05 ZeroCool5254

There is a vanilla gamerule for "keep inventory on death", which we need to check to avoid duping stuff. nucleus probably uses the same mechanism to preserve the inventory, but doesn't set the gamerule (because it's not global).

If possible, I'd recommend to temporarily enable the gamerule while firing Forge events about the death when the dying player has that permission. Then Forge mods will have a chance of knowing about it.

HenryLoenwind avatar May 21 '20 21:05 HenryLoenwind

If possible, I'd recommend to temporarily enable the gamerule while firing Forge events about the death when the dying player has that permission. Then Forge mods will have a chance of knowing about it.

That's already what happens.

dualspiral avatar May 21 '20 21:05 dualspiral

I tried to follow the program flow there, but just on github the code is a bit too fragmented. Could you please look up if both net.minecraftforge.event.entity.player.PlayerDropsEvent and net.minecraftforge.event.entity.player.PlayerEvent.Clone are fired inside that context?

PS: To understand the flow:

PlayerDropsEvent is called first. It gets a player object "old" with an empty inventory and a list of items to be dropped. Ender IO goes through the list and moves items to the inventory. At no point any itemstack exists in two locations at the same time (i.e. we don't dupe stacks here).

PlayerEvent.Clone is called second. It gets the "old" player object and a new cloned player object. Both inventories are empty by default, the "old" player object and inventory will be discarded after the event is done, the "new" player object will be put into the world. With Ender IO, the "old" inventory will have the soulbound items. We then move them from the old to the new inventory. Again we move them, not copy them.

Please note again that we never copy an item---we only move them around. If they get duped, that means that the calling code copies them from its own data storage. Which, honestly, it should not do. If an item is to not be dropped, it should show up in the "old" inventory and not the list for the first event and in the "new" inventory but not the "old" one for the second event. (I'm quite sure vanilla code also copies stuff, but we're quite used to vanilla code being unnecessarily convoluted (especially after Forge's events are patched in) and don't need to copy all its flaws...)

HenryLoenwind avatar May 22 '20 09:05 HenryLoenwind

I have tested and initially, I set the "gamerule KeepInventory true" which didn't work for some reason and that's why I ended up using the nucleus perm. I later noticed that keepinventory didn't work because of the case sensitivity. It should be "gamerule keepInventory true". OFC with sponge it's a per-world permission but that fixed the dupe bug for me.

ZeroCool5254 avatar May 22 '20 10:05 ZeroCool5254

There's an additional bug here:

When the vanilla gamerule keepInventory is enabled, SpongeForge still fires the PlayerDropsEvent. That event should not be fired if the player doesn't actually drop stuff (Forge doesn't fire it with keepInventory).

While Ender IO's Soulbound is protected against this, COFH's Soulbound isn't and will happily dupe items. But people will still report issues with COFH's Soulbound to us (Ender IO), so I just wasted a couple of hours on finding this bug. I'm a teensy tiny bit annoyed with the random weird incompatible behaviour SpongeForge introduces and am contemplating blacklisting it in Ender IO. If you cannot play nicely with others, just play alone.

HenryLoenwind avatar Dec 25 '20 15:12 HenryLoenwind

What we're doing is allowing plugins to set KEEP_INVENTORY for an individual user on death.

So the bug is probably this then: https://github.com/SpongePowered/Sponge/blob/3e618117627c3557e29e32ed35681033b3d07cc9/src/main/java/org/spongepowered/common/event/tracking/phase/entity/EntityDeathState.java#L126 Calling the Sponge event always causes the Forge event to also fire.

Faithcaio avatar Dec 25 '20 19:12 Faithcaio