zDevelopers / QuartzLib

A set of tools to develop plugins using the Bukkit API
https://dev.zcraft.fr/quartzlib
Other
19 stars 27 forks source link

Potential conflict with Denizen due to zLib's glow effet on splatter maps #25

Closed nyvlunx closed 3 years ago

nyvlunx commented 6 years ago

Hello, I got some error,

I was in creative mode : https://pastebin.com/CT8hpebM

Then I looked if it was related to denizen in this issues : https://github.com/DenizenScript/Denizen-For-Bukkit/issues/1720 The answer was You somehow have a nonsense item with invalid enchantments on it, which Spigot internals then crash on because they can't process the item.

Regards

AmauryCarrade commented 6 years ago

There is indeed a non-standard enchant in the splatter maps. This being said, the enchant is correctly registered into Bukkit, so tools processing the items should be able to process them correctly.

Proof of that, I created an NBT-aware multiple-inventories plugin, saving and restoring items including weird ones, and there is no problem processing these enchanted items.

I will look into Bukkit/Spigot/Minecraft code to check if there is something I can do about that.

bibo38 commented 6 years ago

@AmauryCarrade The problem is the lazy registration of the Enchantment in the getGlow method of zLib. How to reproduce:

  1. Get some plugin, which executes the getEnchantments() method: TestPlugin
  2. Use the /tomap command to get a map item (which is enchanted with the GlowEffect)
  3. Restart the server (Reloading does not unregister the Enchantment).
  4. Don't use any commands from the ImageToMap-Plugin (To prevent getGlow from registering the enchantment)
  5. Let some plugin call the getEnchantments() method on the enchanted map in your hand (in my TestPlugin execute /ptest)
  6. You'll get an Exception, because the Enchantment-ID from the item is not registered in Bukkit (see: CraftItemStack)

How to fix: Rewrite the GlowEffect class to require an explicit registration (maybe with a static variabe), before it can be used. Now every Plugin should call the registration method in it's onEnable method.

AmauryCarrade commented 6 years ago

Nice catch, thanks for digging out. That should be easy to fix. Not in a static variable, else, it will not be executed at startup but the first time the class loads; instead, in zLib's startup process.

bibo38 commented 6 years ago

Oh, I didn't see, that zLib has a startup process.

But be aware, that uninstalling this plugin can cause the same exception. Maybe the glowing effect can be better achieved with a nonsense enchant (like Infinity on anything except Bows) and hiding the enchant attribute.