HologramTrait aggressively warns about "too many NPCs than lines" despite that not being the case

[2008Choco]

The output of command /version on my server is: git-Tuinity-"23247c4" (MC: 1.16.5) The output of command /version citizens on my server is: 2.0.28-SNAPSHOT (build 2357) (though we were running 2.0.27-SNAPSHOT prior, I updated thinking it would resolve it but all it had done was clean up the error message)

Error:

[23:15:10 ERROR]: [Citizens] More hologram NPCs than lines for ID 0 lines [&e&lChallenges, &7&o(New challenges every day!)]
[23:15:10 ERROR]: [Citizens] More hologram NPCs than lines for ID 0 lines [&e&lChallenges, &7&o(New challenges every day!)]
[23:15:10 ERROR]: [Citizens] More hologram NPCs than lines for ID 0 lines [&e&lChallenges, &7&o(New challenges every day!)]
[23:15:10 ERROR]: [Citizens] More hologram NPCs than lines for ID 0 lines [&e&lChallenges, &7&o(New challenges every day!)]
[23:15:10 ERROR]: [Citizens] More hologram NPCs than lines for ID 0 lines [&e&lChallenges, &7&o(New challenges every day!)]
[23:15:10 ERROR]: [Citizens] More hologram NPCs than lines for ID 0 lines [&e&lChallenges, &7&o(New challenges every day!)]
[23:15:10 ERROR]: [Citizens] More hologram NPCs than lines for ID 0 lines [&e&lChallenges, &7&o(New challenges every day!)]
[23:15:10 ERROR]: [Citizens] More hologram NPCs than lines for ID 0 lines [&e&lChallenges, &7&o(New challenges every day!)]
[23:15:10 ERROR]: [Citizens] More hologram NPCs than lines for ID 0 lines [&e&lChallenges, &7&o(New challenges every day!)]
[23:15:10 ERROR]: [Citizens] More hologram NPCs than lines for ID 0 lines [&e&lChallenges, &7&o(New challenges every day!)]
[23:15:10 ERROR]: [Citizens] More hologram NPCs than lines for ID 0 lines [&e&lChallenges, &7&o(New challenges every day!)]
[23:15:10 ERROR]: [Citizens] More hologram NPCs than lines for ID 0 lines [&e&lChallenges, &7&o(New challenges every day!)]
[23:15:10 ERROR]: [Citizens] More hologram NPCs than lines for ID 0 lines [&e&lChallenges, &7&o(New challenges every day!)]
[23:15:10 ERROR]: [Citizens] More hologram NPCs than lines for ID 0 lines [&e&lChallenges, &7&o(New challenges every day!)]
[23:15:10 ERROR]: [Citizens] More hologram NPCs than lines for ID 0 lines [&e&lChallenges, &7&o(New challenges every day!)]
[23:15:10 ERROR]: [Citizens] More hologram NPCs than lines for ID 0 lines [&e&lChallenges, &7&o(New challenges every day!)]
[23:15:10 ERROR]: [Citizens] More hologram NPCs than lines for ID 0 lines [&e&lChallenges, &7&o(New challenges every day!)]
[23:15:10 ERROR]: [Citizens] More hologram NPCs than lines for ID 0 lines [&e&lChallenges, &7&o(New challenges every day!)]
[23:15:10 ERROR]: [Citizens] More hologram NPCs than lines for ID 0 lines [&e&lChallenges, &7&o(New challenges every day!)]
[23:15:10 ERROR]: [Citizens] More hologram NPCs than lines for ID 0 lines [&e&lChallenges, &7&o(New challenges every day!)]
[23:15:10 ERROR]: [Citizens] More hologram NPCs than lines for ID 0 lines [&e&lChallenges, &7&o(New challenges every day!)]
[23:15:10 ERROR]: [Citizens] More hologram NPCs than lines for ID 0 lines [&e&lChallenges, &7&o(New challenges every day!)]

Relevant code:

    // A lot of variables here are from a parent class and is why you may not see them declared anywhere.
    // I've prepended "super" to these variables
    // Everything else works quite well, the only issue is towards the bottom of this snippet, HologramTrait

    @Override
    public NPC spawn() {
        // name = String[]
        net.citizensnpcs.api.npc.NPC npcHandle = registry.getHandle().createNPC(super.entityType, (super.name != null && super.name.length == 1) ? super.name[0] : "");

        // Player skin textures
        if (super.entityType == EntityType.PLAYER) {
            // Set skin based on texture and signature
            if (super.skinTextureData != null && super.skinSignature != null) {
                npcHandle.getOrAddTrait(SkinTrait.class).setSkinPersistent(Integer.toString(npcHandle.getId()), super.skinSignature, super.skinTextureData);
            }

            // Set skin based on user name
            else if (super.skinName != null) {
                npcHandle.getOrAddTrait(SkinTrait.class).setSkinName(super.skinName);
            }
        }

        // Glowing colour
        if (super.glowColor != null) {
            npcHandle.data().setPersistent(net.citizensnpcs.api.npc.NPC.GLOWING_METADATA, true);
            npcHandle.getOrAddTrait(ScoreboardTrait.class).setColor(super.glowColor);
        }

        // Blindness (not looking at players)
        if (!super.blind) {
            npcHandle.getOrAddTrait(LookClose.class).lookClose(true);
        }

        if (super.silent) {
            npcHandle.data().setPersistent(net.citizensnpcs.api.npc.NPC.SILENT_METADATA, true);
        }

        npcHandle.spawn(location); // Actually attempt to spawn the NPC

        // Equipment
        if (super.helmet != null || super.chestplate != null || super.leggings != null || super.boots != null) {
            Equipment equipment = npcHandle.getOrAddTrait(Equipment.class);

            // Private method to just equipment.setSlot(slot, item); if it's not null
            this.setSlotIfNotNull(equipment, EquipmentSlot.HELMET, super.helmet);
            this.setSlotIfNotNull(equipment, EquipmentSlot.CHESTPLATE, super.chestplate);
            this.setSlotIfNotNull(equipment, EquipmentSlot.LEGGINGS, super.leggings);
            this.setSlotIfNotNull(equipment, EquipmentSlot.BOOTS, super.boots);
        }

        // (!!! !!! !!! THIS IS THE IMPORTANT PART WHERE I'M HAVING ISSUES !!! !!! !!!)
        if (name != null && name.length > 1) {
            HologramTrait traitHologram = npcHandle.getOrAddTrait(HologramTrait.class);

            // Hiding the default name tag because we want to use more than one line
            // If either of these are unset, an empty line is displayed over the NPC. I don't want that. I just want the holograms
            npcHandle.setAlwaysUseNameHologram(true);
            npcHandle.data().setPersistent(net.citizensnpcs.api.npc.NPC.NAMEPLATE_VISIBLE_METADATA, false);

            // I have also tried including traitHologram.clear() here to no avail
            traitHologram.setLineHeight(0.25D);
            traitHologram.setDirection(HologramDirection.TOP_DOWN);

            for (String line : super.name) {
                traitHologram.addLine(line);
            }
        }

        CitizensNPC npc = new CitizensNPC(super.key, npcHandle, registry, super.interactions);
        this.registry.addNPC(npc);
        return npc;
    }

The error repeats until the NPC is successfully spawned. For just one NPC, it generates roughly 20 errors. I've jerry-rigged the code in numerous different formations, commented out a number of different lines, did nothing more than some simple getOrAddTrait() and addLine() calls and I continue to receive the same error each and every time. Perhaps I'm misunderstanding the API but after browsing the source of HologramTrait, I cannot understand for what reason 3 holograms would be created instead of just 2 despite only 2 lines being added to the NPC.

Perhaps I'm misunderstanding how to use the HologramTrait API, but I've toyed with the spawning of holograms far too much to believe this is a fault on my part :/ Any help or insight would be appreciated.

[mcmonkey4eva]

My guess is this might be a bug in Citizens, related to your NPC not necessarily actually being spawned when your hologram code runs.

This would bug because: https://github.com/CitizensDev/Citizens2/blob/14a6004adf96d27e3da909a568518a4d0354fe56/main/src/main/java/net/citizensnpcs/trait/HologramTrait.java#L54-L58

The addLine section calls a fake-respawn, even if the NPC is not spawned. This means when the NPC later actually spawns, it's effectively a duplicated onSpawn call, doubling the number of NPCs. The solution naturally is that code should have an isSpawned check around the onDespawn and onSpawn calls. A similar check would need to be added to all other matching sections, or the 'fake respawn' logic be refactored into its own method called by each part.

[2008Choco]

That's a good lead and was my initial thought as well, though the "3 NPCs > 2 lines" really threw me off. This would apply also to pretty much anything else calling onDespawn() and onSpawn() though (including the setLineHeight() and setDirection() methods) so preferably those methods themselves would include a check for isSpawned()

[mcmonkey4eva]

https://github.com/CitizensDev/Citizens2/commit/11c48c4e09cf83a5113b172cc037342044e5ed6d is a much quicker and dirtier version of the solution but should work and is available in build 2364+ from https://ci.citizensnpcs.co/job/Citizens2/