Mithion / ArsMagica2

Ars Magica 2 Bug Tracker
126 stars 188 forks source link

Gravestones not seeing Soulbound via Armor Infusion #1344

Open theorbtwo opened 8 years ago

theorbtwo commented 8 years ago

To reproduce -- this was a fresh instance, with current forge and mods (AnimationAPI 1.2.4, Ars Magica 1.4.0.008, GraveStone 2.15.2, forge 10.13.4.1564). 1: Start a new world, creative mode, cheats enabled. 2: Use an anvil and an enchanged book of soulbound to give yourself a soulbound helmet. 3: Switch back to survival (/gamemode 0), and throw yourself off of a tower of dirt. 4: Note that you keep your helmet. 5: Pick up the other stuff, exit minecraft. 6: Use an NBT editor to edit level.dat to change the armor experience points on your breastplate. (I multiplied by a thousand.) 7: Reload. Notice that you get a breastplate with a large number of xp, but a low level, because the nbt lists them seperately. Kill yourself again and they will resync. 8: Use an armor infusion table to infuse the breastplate with soulbound. 9: Throw yourself off the tower again. 10: You will keep your helmet, but your breastplate (and other inventory) will end up in the gravestone. Desired behavior: Keeping the helmet and breastplate, other inventory ending up in the gravestone.

I theorize that this is because infused enchantments don't count as enchantments: https://github.com/NightKosh/GraveStone-mod/blob/4a7473e329de9c5ef32d013b2d751f71b2c13622/src/main/java/gravestone/core/compatibility/GSCompatibilityisArsMagica.java#L40 appears to be the relevant code.

Suggested fixes: 1: Make infused enchantments count as normal enchantments, possibly with an override to keep them from showing the shimmery overlay and to not block normal enchantments on an item that already has infusions. 2: Create an open API that allows for declaring callbacks that should be called to decide if a gravestone-type mod should ignore a given ItemStack. Hopefully this would allow Ars Magica and EnderIO (at least) to declare to GraveStone and OpenBlocks (at least) that things with their versions of the soulbound enchantment shouldn't be put in a gravestone, but allowed to drop as normal. (GraveStone contains code that checks for enchantment.enderio.soulBound and am2.enchantments.EnchantmentSoulbound by name, which seems delicate. This would replace all of that.) 3: Provide an API from Ars Magica to check for infusion enchantments, and make gravestone use it.

theorbtwo commented 8 years ago

See also initial report on twitter: https://twitter.com/theorbtwo/status/665280425829470208

Lonemind commented 8 years ago

The item loss isn't unique to the infusion enchantments. The item loss happens with all soulbound items. It has been occuring for at least a year. I too suspect that AM2 and Gravestones are not talking properly. The soulbound support that is in Gravestones is actually support for Ender IO's soulbound I believe. Not AM2.

jjtParadox commented 8 years ago

Because AM2 now handles death at a higher priority then other mods, this is no longer an issue. However if wanted, the Gravestones mod could add a check for ArmorHelper.isInfusionPreset(stack, GenericImbuement.soulbound). This accomplishes what was originally wanted from this issue, but may not be needed anymore. As such, this issue can be closed and possibly forwarded to the author of Gravestones.

Sunconure11 commented 8 years ago

This happens with openblocks gravestones too.

Lonemind commented 8 years ago

It seems that soulbound is working well in the overworld now. They still get deleted when you die in another dimension but respawn in the overworld. I suspect it would work fine if you respawn in the same dimension you die in.

jjtParadox commented 8 years ago

Make sure you've updated to the most recent version of Ars Magica, as we did target that bug in this most recent update (version *.009). If you are on the most recent version, can you write some updated steps to reproduce the bug? It helps a lot :smiley: