Error with combust event
PhanaticD opened this issue · 16 comments
Essentials 2.0.1-b601
CraftBukkit version git-TacoSpigot-"ca59736" (MC: 1.8.8) (Implementing API version 1.8.8-R0.2-SNAPSHOT)
the following issue occurs with bots from citizens2 plugin shooting flaming arrows, not sure if its from hitting the player or from the player hitting the bot
[22:09:50 ERROR]: Could not pass event EntityCombustByEntityEvent to Essentials v2.0.1-b601
org.bukkit.event.EventException
at org.bukkit.plugin.java.JavaPluginLoader$1.execute(JavaPluginLoader.java:302) ~[patched.jar:git-TacoSpigot-"ca59736"]
at co.aikar.timings.TimedEventExecutor.execute(TimedEventExecutor.java:78) ~[patched.jar:git-TacoSpigot-"ca59736"]
at org.bukkit.plugin.RegisteredListener.callEvent(RegisteredListener.java:62) ~[patched.jar:git-TacoSpigot-"ca59736"]
at org.bukkit.plugin.SimplePluginManager.fireEvent(SimplePluginManager.java:501) [patched.jar:git-TacoSpigot-"ca59736"]
at org.bukkit.plugin.SimplePluginManager.callEvent(SimplePluginManager.java:486) [patched.jar:git-TacoSpigot-"ca59736"]
at net.minecraft.server.v1_8_R3.EntityArrow.t_(EntityArrow.java:264) [patched.jar:git-TacoSpigot-"ca59736"]
at net.minecraft.server.v1_8_R3.World.entityJoinedWorld(World.java:1649) [patched.jar:git-TacoSpigot-"ca59736"]
at net.minecraft.server.v1_8_R3.World.g(World.java:1616) [patched.jar:git-TacoSpigot-"ca59736"]
at net.minecraft.server.v1_8_R3.World.tickEntities(World.java:1453) [patched.jar:git-TacoSpigot-"ca59736"]
at net.minecraft.server.v1_8_R3.WorldServer.tickEntities(WorldServer.java:600) [patched.jar:git-TacoSpigot-"ca59736"]
at net.minecraft.server.v1_8_R3.MinecraftServer.B(MinecraftServer.java:846) [patched.jar:git-TacoSpigot-"ca59736"]
at net.minecraft.server.v1_8_R3.DedicatedServer.B(DedicatedServer.java:378) [patched.jar:git-TacoSpigot-"ca59736"]
at net.minecraft.server.v1_8_R3.MinecraftServer.A(MinecraftServer.java:713) [patched.jar:git-TacoSpigot-"ca59736"]
at net.minecraft.server.v1_8_R3.MinecraftServer.run(MinecraftServer.java:616) [patched.jar:git-TacoSpigot-"ca59736"]
at java.lang.Thread.run(Thread.java:745) [?:1.8.0_111]
Caused by: java.lang.NullPointerException
at com.earth2me.essentials.EssentialsEntityListener.onEntityCombustByEntity(EssentialsEntityListener.java:131) ~[?:?]
at sun.reflect.GeneratedMethodAccessor265.invoke(Unknown Source) ~[?:?]
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_111]
at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_111]
at org.bukkit.plugin.java.JavaPluginLoader$1.execute(JavaPluginLoader.java:300) ~[patched.jar:git-TacoSpigot-"ca59736"]
... 14 more
It looks like this is caused by an NPC shooting a flaming arrow.
Does this happen when running Spigot or Paper?
Any chance you could test with Spigot and/or Paper? There's a chance that either Paper or TacoSpigot optimises something that breaks the check.
no I most likely will not, for 1.8 servers i use tacospigot as its the best option for 1.8 that i can see available
We can't narrow down the issue unless we can confirm which server implementation causes it, as the patches used to build for TacoSpigot is split across five separate codebases, and it's not possible for me to run each server individually to test them all at the moment.
@PhanaticD He's not going to create his own server and test it 3 times. He's a busy man. All he's asking is if it would work with paper/spigot. If it works, then they would work on it. If it didn't work, then they would work on it. Also its not "all the work" its just 1 error. Plus its a free plugin so why not.
I'd think a nullcheck there would fix the issue, with no adverse side effects, unless Essentials#getUser(...) should be modified instead.
getUser
should probably be called with a Player
not a UUID if we want to create a User, but I'm not sure that that's the ideal solution here or if we should just do a null check.
Alternatively, we could implement an IUser that is specific to Citizens NPCs, but I don't know whether this belongs in EssentialsX - we could include hooks so that this could be done in a separate compatibility plugin.
Hm, I don’t think a null check would be appropriate then. I also don’t think a User should be created for NPCs, I think that would lead to issues down the line if the NPCs on the server (either through deletion or some bug) gets desynced with what Essentials has. If Essentials were to fix this, then the best way would be through a dummy IUser for NPCs.
On the other hand, I can see difficulty with adoption if there isn’t one agreed-upon method of identifying NPCs. For example, if a new NPC plugin needed Essentials to hook into it in order to figure out which entities are NPCs, problems can start to arise there. I’m not familiar with NPC plugins, but I’m sure that not everybody plays nicely like Citizens does by setting the metadata tag.
If Citizens should take the burden for fixing this issue, what should they do? They could stop implementing Player of course, but that makes it hard to interact with the internals as far as managing NPCs goes. They could possibly set the shooter of the arrow to a dummy entity instead of using the NPC? Perhaps something like:
class DummyNpcEntity implements Entity {
// Noop Methods
public NPC getActualNpc() {
return npc;
}
}
Then whenever they fire an arrow simply use the dummy entity holder to indicate that an NPC fired it. I don’t think I like this idea though, it seems to be very convoluted and might break other plugins in the process.
IMHO this should fall on the Citizens people to figure out. The fact that Citizens is messing around with the way the Bukkit API should conventionally work (ie NPCs are literally Players) in a way that breaks plugins using the API correctly is a strong case in my eyes that the burden should lie with Citizens, not Essentials.
Thought I’d give my own response in case it does still happen.
I have not noticed it
Citizens must have fixed it then?