NullPointerException when you set enchanted item to air

Fusezion commented 11 months ago

Skript/Server Version

[01:21:10 INFO]: [Skript] Skript's aliases can be found here: https://github.com/SkriptLang/skript-aliases
[01:21:10 INFO]: [Skript] Skript's documentation can be found here: https://docs.skriptlang.org/
[01:21:10 INFO]: [Skript] Skript's tutorials can be found here: https://docs.skriptlang.org/tutorials
[01:21:10 INFO]: [Skript] Server Version: git-Paper-318 (MC: 1.20.2)
[01:21:10 INFO]: [Skript] Skript Version: 2.8.0-dev (selfbuilt-unknown)
[01:21:10 INFO]: [Skript] Installed Skript Addons:
[01:21:10 INFO]: [Skript]  - skript-gui v1.3 (https://github.com/APickledWalrus/skript-gui)
[01:21:10 INFO]: [Skript]  - skript-reflect v2.4-beta1 (https://github.com/SkriptLang/skript-reflect)
[01:21:10 INFO]: [Skript]  - SkBee v3.0.0-beta2 (https://github.com/ShaneBeee/SkBee)
[01:21:10 INFO]: [Skript] Installed dependencies: None

Bug Description

When you set enchanted item in an on item enchant event to air or any other invalid item, instead of setting slot to air or silently failing you're instead given a console stacktrace for null pointer when it attempts to add enchantments.

Expected Behavior

Silently error in console since why not

Steps to Reproduce

on item enchant:
    set enchanted item to air # void air also does this which is not an item but a block

Errors or Screenshots

Failed to handle packet net.minecraft.network.protocol.game.PacketPlayInEnchantItem@211f4f8c, suppressing error
java.lang.NullPointerException: Cannot invoke "org.bukkit.inventory.meta.ItemMeta.addEnchant(org.bukkit.enchantments.Enchantment, int, boolean)" because "itemMeta" is null
        at org.bukkit.craftbukkit.v1_20_R2.inventory.CraftItemStack.addUnsafeEnchantment(CraftItemStack.java:207) ~[paper-1.20.2.jar:git-Paper-318] 
        at net.minecraft.world.inventory.EnchantmentMenu.lambda$clickMenuButton$1(EnchantmentMenu.java:280) ~[?:?]
        at net.minecraft.world.inventory.ContainerLevelAccess.lambda$execute$0(ContainerLevelAccess.java:85) ~[?:?]
        at net.minecraft.world.inventory.ContainerLevelAccess$2.evaluate(ContainerLevelAccess.java:72) ~[?:?]
        at net.minecraft.world.inventory.ContainerLevelAccess.execute(ContainerLevelAccess.java:84) ~[?:?]
        at net.minecraft.world.inventory.EnchantmentMenu.clickMenuButton(EnchantmentMenu.java:231) ~[?:?]
        at net.minecraft.server.network.ServerGamePacketListenerImpl.handleContainerButtonClick(ServerGamePacketListenerImpl.java:3261) ~[?:?]      
        at net.minecraft.network.protocol.game.ServerboundContainerButtonClickPacket.handle(ServerboundContainerButtonClickPacket.java:17) ~[?:?]   
        at net.minecraft.network.protocol.game.ServerboundContainerButtonClickPacket.a(ServerboundContainerButtonClickPacket.java:11) ~[?:?]        
        at net.minecraft.network.protocol.PacketUtils.lambda$ensureRunningOnSameThread$0(PacketUtils.java:53) ~[?:?]
        at net.minecraft.server.TickTask.run(TickTask.java:18) ~[paper-1.20.2.jar:git-Paper-318]
        at net.minecraft.util.thread.BlockableEventLoop.doRunTask(BlockableEventLoop.java:153) ~[?:?]
        at net.minecraft.util.thread.ReentrantBlockableEventLoop.doRunTask(ReentrantBlockableEventLoop.java:24) ~[?:?]
        at net.minecraft.server.MinecraftServer.doRunTask(MinecraftServer.java:1324) ~[paper-1.20.2.jar:git-Paper-318]
        at net.minecraft.server.MinecraftServer.d(MinecraftServer.java:193) ~[paper-1.20.2.jar:git-Paper-318]
        at net.minecraft.util.thread.BlockableEventLoop.pollTask(BlockableEventLoop.java:126) ~[?:?]
        at net.minecraft.server.MinecraftServer.pollTaskInternal(MinecraftServer.java:1301) ~[paper-1.20.2.jar:git-Paper-318]
        at net.minecraft.server.MinecraftServer.pollTask(MinecraftServer.java:1294) ~[paper-1.20.2.jar:git-Paper-318]
        at net.minecraft.util.thread.BlockableEventLoop.managedBlock(BlockableEventLoop.java:136) ~[?:?]
        at net.minecraft.server.MinecraftServer.waitUntilNextTick(MinecraftServer.java:1272) ~[paper-1.20.2.jar:git-Paper-318]
        at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:1160) ~[paper-1.20.2.jar:git-Paper-318]
        at net.minecraft.server.MinecraftServer.lambda$spin$0(MinecraftServer.java:315) ~[paper-1.20.2.jar:git-Paper-318]
        at java.lang.Thread.run(Thread.java:833) ~[?:?]

Other

I'll be completely honest I am unsure how you should go about fixing this, as the error doesn't stem from enchanted item but rather the item being nulled out when applied enchantments attempt to be added.

After a bit further testing I've learned that if I do clear applied enchantments when I set it to air it does fail silently so possible adding as getMaterial() and comparing #isItem() and !#isAir() if either fail clear applied enchantments?

I would probably label this a server issue rather than skript but this is the easiest solution I've found for now

Agreement

[X] I have read the guidelines above and affirm I am following them with this report.

NotSoDelayed commented 11 months ago

There’s no way we could suppress this exception as you’re setting the ItemStack to Air in the event which then resulting Minecraft’s attempt to apply the enchant into a non-existent ItemMeta (Air does not have ItemMeta, highly probable).

sovdeeth commented 11 months ago

Yeah, looking into this, it seems like something spigot/paper should defend against, rather than Skript. I don't see how we'd be able to solve this without just not allowing the item to be set to Air. Leaving open for others' opinions.

For example, clearing the applied enchants fails because of:

set enchanted item to air
...
set enchanted item to stone # now no enchants are applied

Moderocky commented 11 months ago

This doesn't seem like something we really should be trying to work around.

SkriptLang / Skript