EssentialsX

EssentialsX

2M Downloads

Error with combust event

PhanaticD opened this issue · 16 comments

commented

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
commented

probably, theyre the same API so

commented

It looks like this is caused by an NPC shooting a flaming arrow.

Does this happen when running Spigot or Paper?

commented

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.

commented

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

commented

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.

commented

so you want me to do all the work for an error in your plugin basically lol

commented

@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.

commented

okay tested it same thing

commented

I'd think a nullcheck there would fix the issue, with no adverse side effects, unless Essentials#getUser(...) should be modified instead.

commented

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.

commented

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.

commented

dam this is old

commented

Is this issue even still relevant? I have no idea if this still happens.

commented

I have not noticed it

commented

Thought I’d give my own response in case it does still happen.

I have not noticed it

Citizens must have fixed it then?

commented

Closing since it seems like this may be fixed. If anyone encounters this issue, please feel free to let us know!