Ghost NPCs created on Removing/Skins not Updating #1035

Open
Ditronian opened this Issue Jan 3, 2017 · 14 comments

Projects

None yet

3 participants

@Ditronian
Ditronian commented Jan 3, 2017 edited

Greetings again,

This has been a string of related problems that we've made do with in the past, but has becoming an increasing annoyance as our server population grows in size. Citizens has had a recurring issue of creating 'ghost npcs' (for lack of a better term), in random instances. 90% of the time, creating/removing NPCs works fine, however once one such ghost is created any removal of a NPC will leave an error throwing ghost in its place, and any npc skins you set become unviewable until a restart. Furthermore if you perform a /citizens reload while this is happening it will duplicate all recently created NPCs with ghosts.

A server reboot will clear out all the ghosts and load the new skins on any NPCs, however this no guarantee that citizens will then let you set new skins, or remove new npcs afterwards. Unfortunately all the errors seem to be caused by the ghost's existence itself, and are not causes of the ghost itself. Its getting to the point where we have to reboot the server every time we make a new batch of NPCs.

We are on the latest development build, but this has been an issue we've had for a couple months now across a swathe of builds.

Spigot: 1.11 (occurred as well on 1.10.2)
Citizens: 2.0.21 latest development build.

Ghost Error #1:

[15:38:49] [Server thread/INFO]: [Citizens] Exception while updating 555: null.
[15:38:49] [Server thread/WARN]: java.lang.NullPointerException

Ghost Error #2:

[15:41:10] [Server thread/INFO]: [Citizens] Exception while updating 569: null.
[15:41:10] [Server thread/WARN]: java.lang.NullPointerException
[15:41:10] [Server thread/WARN]: at org.mcmonkey.sentinel.SentinelTrait.isTargeted(SentinelTrait.java:1264)
[15:41:10] [Server thread/WARN]: at org.mcmonkey.sentinel.SentinelTrait.shouldTarget(SentinelTrait.java:1159)
[15:41:10] [Server thread/WARN]: at org.mcmonkey.sentinel.SentinelTrait.findBestTarget(SentinelTrait.java:1324)
[15:41:10] [Server thread/WARN]: at org.mcmonkey.sentinel.SentinelTrait.runUpdate(SentinelTrait.java:1388)
[15:41:10] [Server thread/WARN]: at org.mcmonkey.sentinel.SentinelTrait.run(SentinelTrait.java:1481)
[15:41:10] [Server thread/WARN]: at net.citizensnpcs.api.npc.AbstractNPC.update(AbstractNPC.java:430)
[15:41:10] [Server thread/WARN]: at net.citizensnpcs.npc.CitizensNPC.update(CitizensNPC.java:268)
[15:41:10] [Server thread/WARN]: at net.citizensnpcs.nms.v1_11_R1.entity.EntityHumanNPC.A_(EntityHumanNPC.java:140)
[15:41:10] [Server thread/WARN]: at net.minecraft.server.v1_11_R1.World.entityJoinedWorld(World.java:1652)
[15:41:10] [Server thread/WARN]: at net.citizensnpcs.nms.v1_11_R1.util.NMSImpl.tick(NMSImpl.java:980)
[15:41:10] [Server thread/WARN]: at net.citizensnpcs.util.NMS.tick(NMS.java:270)
[15:41:10] [Server thread/WARN]: at net.citizensnpcs.util.PlayerUpdateTask.run(PlayerUpdateTask.java:36)
[15:41:10] [Server thread/WARN]: at org.bukkit.craftbukkit.v1_11_R1.scheduler.CraftTask.run(CraftTask.java:71)
[15:41:10] [Server thread/WARN]: at org.bukkit.craftbukkit.v1_11_R1.scheduler.CraftScheduler.mainThreadHeartbeat(CraftScheduler.java:353)
[15:41:10] [Server thread/WARN]: at net.minecraft.server.v1_11_R1.MinecraftServer.D(MinecraftServer.java:730)
[15:41:10] [Server thread/WARN]: at net.minecraft.server.v1_11_R1.DedicatedServer.D(DedicatedServer.java:399)
[15:41:10] [Server thread/WARN]: at net.minecraft.server.v1_11_R1.MinecraftServer.C(MinecraftServer.java:675)
[15:41:10] [Server thread/WARN]: at net.minecraft.server.v1_11_R1.MinecraftServer.run(MinecraftServer.java:574)
[15:41:10] [Server thread/WARN]: at java.lang.Thread.run(Thread.java:745)
[15:41:10] [Server thread/INFO]: [Citizens] Exception while updating 561: null.
[15:41:10] [Server thread/WARN]: java.lang.NullPointerException
[15:41:10] [Server thread/WARN]: at org.mcmonkey.sentinel.SentinelTrait.isTargeted(SentinelTrait.java:1264)
[15:41:10] [Server thread/WARN]: at org.mcmonkey.sentinel.SentinelTrait.shouldTarget(SentinelTrait.java:1159)
[15:41:10] [Server thread/WARN]: at org.mcmonkey.sentinel.SentinelTrait.findBestTarget(SentinelTrait.java:1324)
[15:41:10] [Server thread/WARN]: at org.mcmonkey.sentinel.SentinelTrait.runUpdate(SentinelTrait.java:1388)
[15:41:10] [Server thread/WARN]: at org.mcmonkey.sentinel.SentinelTrait.run(SentinelTrait.java:1481)
[15:41:10] [Server thread/WARN]: at net.citizensnpcs.api.npc.AbstractNPC.update(AbstractNPC.java:430)
[15:41:10] [Server thread/WARN]: at net.citizensnpcs.npc.CitizensNPC.update(CitizensNPC.java:268)
[15:41:10] [Server thread/WARN]: at net.citizensnpcs.nms.v1_11_R1.entity.EntityHumanNPC.A_(EntityHumanNPC.java:140)
[15:41:10] [Server thread/WARN]: at net.minecraft.server.v1_11_R1.World.entityJoinedWorld(World.java:1652)
[15:41:10] [Server thread/WARN]: at net.citizensnpcs.nms.v1_11_R1.util.NMSImpl.tick(NMSImpl.java:980)
[15:41:10] [Server thread/WARN]: at net.citizensnpcs.util.NMS.tick(NMS.java:270)
[15:41:10] [Server thread/WARN]: at net.citizensnpcs.util.PlayerUpdateTask.run(PlayerUpdateTask.java:36)
[15:41:10] [Server thread/WARN]: at org.bukkit.craftbukkit.v1_11_R1.scheduler.CraftTask.run(CraftTask.java:71)
[15:41:10] [Server thread/WARN]: at org.bukkit.craftbukkit.v1_11_R1.scheduler.CraftScheduler.mainThreadHeartbeat(CraftScheduler.java:353)
[15:41:10] [Server thread/WARN]: at net.minecraft.server.v1_11_R1.MinecraftServer.D(MinecraftServer.java:730)
[15:41:10] [Server thread/WARN]: at net.minecraft.server.v1_11_R1.DedicatedServer.D(DedicatedServer.java:399)
[15:41:10] [Server thread/WARN]: at net.minecraft.server.v1_11_R1.MinecraftServer.C(MinecraftServer.java:675)
[15:41:10] [Server thread/WARN]: at net.minecraft.server.v1_11_R1.MinecraftServer.run(MinecraftServer.java:574)
[15:41:10] [Server thread/WARN]: at java.lang.Thread.run(Thread.java:745)

@Ditronian

Renaming a NPC when citizens is bugging out like this is similarly a bad idea, as despite it showing the new correct name in chat, visibly it just conjoins it to the end of the previous name above the NPCs head. This fixes itself with a server shutdown (Citizens Reload does not suffice).

@fullwall
Member
fullwall commented Jan 6, 2017

That error is from Sentinel: it seems that you're not using the newest version of Sentinel since the line numbers don't match up. Would it be possible to update Sentinel and get a new version of that error? For the NPC ghosts: they just randomly appear on /npc create or do you have to do anything extra to get it to happen?

@Ditronian
Ditronian commented Jan 6, 2017 edited

I've updated sentinel but haven't seen that error make a come back so far, but I will keep an eye out for it. (I expect it'll only happen when a Sentinel NPC is the ghost)

They randomly remain as ghosts on /npc remove, and can also be created as duplicates of existing/recently made NPCs on /citizens reload. Ghosts can be selected with /npc sel and a few other commands (which have no effect), however any other command returns the message 'You must have an npc selected to execute that command'

Ghost NPC

In the above screenshot you can see I created two NPCs, Greyhaven Archer and then Bob. Greyhaven Archer removed correctly, however Bob left behind a ghost npc. As stated above you can reselect Bob, and even reperform the /npc remove command on him, but not much else registers him as existing.

Performing a citizens save + reload after Bob's ghost forming resulted in this, the only error during this round of testing:

12:29:03] [Server thread/INFO]: Ditronian issued server command: /citizens save
[12:29:06] [Server thread/INFO]: Ditronian issued server command: /citizens reload
[12:29:07] [Server thread/WARN]: Force-added player with duplicate UUID 5f771923-af57-25b0-b10f-828b1e7078e3

The citizens reload command also brought the ACTUAL Bob who was removed back into existence (which correct me if Im wrong but the /citizens save should've prevented this). So now I'm left with a revived from the dead Bob and a Ghost Bob. Using denizen I confirmed that the UUID in the above error is assigned to Bob.

@mcmonkey4eva
Member

Are you perhaps in offline mode?
That's an offline mode UUID in that log I think. (5f771923-af57-25b0-b10f-828b1e7078e3)

@Ditronian

No, the server is in online mode.

@fullwall
Member
fullwall commented Jan 6, 2017 edited

How quickly are you using /npc remove after creating the NPC? Are they doing anything while you /npc remove them?

@Ditronian
Ditronian commented Jan 6, 2017 edited

It varies, Bob in the above screenshot existed for just a few minutes and had no other npc commands used on him other than /npc create and /npc remove, so he didn't have anything special on him and was not moving. That being said a lot of our NPCs are Sentinels, so this problem also effects them.

/citizens reload has never 'duplicated' an old NPC with a ghost to my knowledge, when it does this the NPCs were all created after the last server restart. Old NPCs similarly are much more stable in terms of successfully performing /npc remove on them.

The one issue that for sure does effect old NPCs is when trying to rename them.

Name Merger

This is a month old denizen NPC who we tried to rename, unfortunately in his case a restart did not fix it. You can see his old name conjoined to the new name above his head. If only I could make names that long intentionally.

@fullwall
Member
fullwall commented Jan 6, 2017 edited

Looking at that bug I wonder if this is related to scoreboards. They wouldn't happen to have the same name would they?

@Ditronian
Ditronian commented Jan 6, 2017 edited

Not sure what you mean exactly by 'they', but there are several NPCs with this name (There was only one Bob) if that's what you mean. I just went and checked them all and noticed they are all showing this same bug.

We went through and renamed them all last night (The previous name was too long, which occasionally causes another bug where it only displays the last couple of letters). A curious thing to note is last night the names were displaying the new name correctly.

Since then we've restarted to load the new sentinel version, and performed a couple /citizens reload commands for this testing. One of those must have brought on the name bug. That being said, another group of NPCs that were having the name bug last night, were fixed by today's restart.

If I had to reckon if we were to restart again this name bug would go away like the last ones.

@fullwall
Member
fullwall commented Jan 7, 2017

I mean do all the ghost NPCs have duplicate names with each other? If so, it may be worth disabling player-scoreboard-teams in the config and/or adding colour codes to the ends of their names to avoid the scoreboard bug.

@Ditronian
Ditronian commented Jan 7, 2017 edited

Some of them do, others are one of a kind NPCs. I went ahead and disabled the config option since I doubt its something we make use of anyway. Several restarts later I gave up on trying to change those bugged out NPC names, and set them back to their old names (which ironically 'fixes' the bug).

After 5-6 initial successful NPC creations/removals, I can confirm the ghost NPC issue also still exists.

I decided to take note of the NPC ids before removing them this time, so I can now say for sure that despite the Ghosts existing, they are being properly removed from the Citizens saves file after /citizens save. This I assume is why the restart fixes the ghost problem, and a citizens reload does not.

@fullwall
Member
fullwall commented Jan 7, 2017

Yes, /citizens reload does not remove the entities it doesn't know about (the ghosts). I'll keep trying to reproduce but no luck so far...

@Ditronian
Ditronian commented Jan 7, 2017 edited

Yeah its a tricky problem, but it happens very regularly for us. Especially after performing a /citizens reload (which we must do regularly to set new NPC skins unfortunately). Maybe its an issue that pops up after there are a certain number of NPCs?

At this point we apparently have just over 500 according to the console startup, though this issue existed well before then.

Here's an image of a batch of NPCs I recently made, after doing /citizens save and a /citizens reload. Every Orc was duplicated on top of each other.

OrcsGoneWild

Until one of them has a /npc remove done on them (which won't actually remove him), Citizens sees both NPC copies as the same NPC. The only difference I can tell right off the bat is, only one of them retains any pathfinding (the two Orcs in the foreground are the same NPC).

@Ditronian Ditronian closed this Jan 7, 2017
@Ditronian Ditronian reopened this Jan 7, 2017
@Ditronian
Ditronian commented Jan 7, 2017 edited

I went ahead and tried messing around on my localhost server to see what issues I can duplicate, and while I've not seen any ghosts, /citizens save continues to not be overriding removed NPCs properly.

I can remove a NPC with /npc remove, perform /citizens save, and when I do /citizens reload all removed NPCs return.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment