CitizensDev / Citizens2

Citizens - the premier plugin and API for creating server-side NPCs in Minecraft.
https://citizensnpcs.co
Open Software License 3.0
589 stars 313 forks source link

NPCs fail to spawn due to trial fix. #2668

Closed mibby closed 3 years ago

mibby commented 3 years ago

Citizens 2.0.28 dev 2383 Purpur dev 1394 (Paper 1.17.1) https://github.com/CitizensDev/Citizens2/commit/18423134fafd858d031a84f5bda97c923b9738b6

Feedback for the trial fix. The change breaks NPCs spawning after server initialization. No NPCs are present in my spawn area. Downgrading to dev 2382, NPCs are able to spawn & appear again.

mcmonkey4eva commented 3 years ago

CitizensNPC#despawn calls remove() on the entity controller, but never nulls it: https://github.com/CitizensDev/Citizens2/blob/18423134fafd858d031a84f5bda97c923b9738b6/main/src/main/java/net/citizensnpcs/npc/CitizensNPC.java#L101-L105

This works for most entity types because AbstractEntityController nulls its internal reference: https://github.com/CitizensDev/Citizens2/blob/18423134fafd858d031a84f5bda97c923b9738b6/main/src/main/java/net/citizensnpcs/npc/AbstractEntityController.java#L27-L31

However HumanController in particular does not: https://github.com/CitizensDev/Citizens2/blob/18423134fafd858d031a84f5bda97c923b9738b6/v1_17_R1/src/main/java/net/citizensnpcs/nms/v1_17_R1/entity/HumanController.java#L82-L94


The most immediate correction to make is either

A: in HumanController#remove, null the bukkitEntity field (technically a private field of the superclass, so that access level would need to change probably?) B: in CitizensNPC#despawn, null the entityController field after calling remove

mcmonkey4eva commented 3 years ago

Correction A was implemented via https://github.com/CitizensDev/Citizens2/commit/cfacb52d292eedf1882885e3f62727b7c2c745d7 (for MC 1.17.1 at least, needs to be replicated on other modules as well), for build 2384+ from https://ci.citizensnpcs.co/job/Citizens2/

Confirmed on my own test server that the one loading issue I was able to replicate was no longer happening and NPCs are now spawning and respawning without any trouble.

mibby commented 3 years ago

Can confirm commit https://github.com/CitizensDev/Citizens2/commit/cfacb52d292eedf1882885e3f62727b7c2c745d7 seems to resolve the problem, thanks! NPCs are visible as of dev 2385.