Whitelist Citizens NPCs from arena entity purging at plugin initialization and reload.

[mibby]

RE: Discord #testers discussion.

Would be nice to decouple Citizens NPCs from the arena entity purging at plugin initialization / mobarena reload to serve as a sort of whitelist to keep important NPCs alive in the arena. So if you have class upgrade shops via plugins like ShopKeeper, those entities can remain existing.

Citizen NPCs can be checked via their metadata, i.e. https://github.com/EngineHub/WorldGuard/commit/d24157e693ec0c6735881159011a448c3e402fb9

[garbagemule]

Alrighty, so the idea here is to actually reduce the otherwise gnarly and unwanted plugin integrations with a simple metadata check, which is completely decoupled from any specific NPC plugin. As long as the NPC plugin adds a metadata property with the key "NPC", this approach should work.

Here's what I'd like to see investigated before I implement this:

• What, exactly, are we doing with these NPCs? We're not removing them during cleanup, and I guess we're not including them as actual parts of the arena, aside from the fact that they can be interacted with? Are they automatically protected from explosions, skeleton arrows, fire, etc.?
• Some sort of profiling of the current cleanup process. This is probably best sampled from a real server with a realistic setup. What we're looking for is a sort of benchmark of the cleanup process, so we can compare the performance impact of making these metadata checks during an otherwise heavy operation.
• What entity types are NPCs? Can they be anything? Or are they always a Player type, or are they a custom type? This is important for limiting the scope of the metadata checks (no point in checking if a Projectile like an arrow has a NPC metadata, because it probably doesn't).

[blablubbabc]

For consideration:

• It would be nice to not limit this to Citizens NPC's "NPC" key. Different NPC plugins will likely use different keys to avoid collisions and interoperability problems with plugins that assume that some specific key belongs to a specific NPC plugin (since there is no clear definition for what it means for an entity to be tagged with "NPC", without considering which plugins actually add this key). Shopkeepers for example uses the key "shopkeeper" for its spawned living entities to not collide with Citizens NPCs. A setting that allows server admins to specify a list of metadata keys that are meant to be ignored would be useful.
• It is probably safe to assume that these entities get protected from "natural" (vanilla minecraft) dangers by the NPC plugins themselves. If a different plugin is however implementing new damage sources directly (eg. manually changing or removing entities hit by some projectile or similar), that plugin would be responsible for ignoring these NPC entities.
• I cannot comment on the performance aspect specific for MobArena. But it would be up to the server admin to specify the list of keys to check for. If the list is empty, the impact should be comparable to checking a simple flag per entity. Regarding the performance impact for a single metadata key lookup, that should probably be comparable to creating the key for the entity and performing a simple map lookup inside Bukkit. Whether this is acceptable depends on how many entities MobArena has to deal with. My guess would be that it doesn't matter, but it would indeed be interesting to do the benchmarking to be sure.
• Well, for the current usecase it would probably suffice to limit it to LivingEntities. I am not aware of any other entity types that would benefit from this right now. This can probably be looked into, if further usecases come up in the future.

[graywolf336]

My two cents, maybe add the ability to "whitelist" a list of metadata keys to ignore them? I know that's an extremely simplified solution, but it would leave it up to the server administrator to take care and ensure their server is handled. Then by default, we whitelist Citizens...

[x0Z3ro0x]

Just an outsider looking in, but from my own experience, this kind of implementation is a lot of un-needed work. This is just my honest opinion and here is why I think this:

Every time we personally build an arena, we don't make it just some simple box. After all MC is 50% about the builds. Instead we have the arena inside the actual build. Think of it like a Colosseum (which is where we take inspiration) in the fact that you have the main arena in the middle, and around it is spectator stands, shops, etc. The only region effected at that point is the arena, the builds around the arena that make up the entirety of the build remain unaffected from Mob Arena's functions thus working around NPC's being killed in a kill zone and making your arena's much nicer for players.

Win win.

With that being said, I can understand why users want this feature as is does make arena's a lot less sophisticated. Thus lowering the amount of work needed to create an arena. Apologies if I offended anyone, I was just sharing my two cents on this amazing plugin! Thanks for reading.

[Andre601]

I want to quickly give some of my info here regarding NPCs and their attributes. Most if not all of the info is based on Citizens and my experience with it. Some experience comes from providing support for AdvancedTabOverlay, a tab list plugin, which ignores NPC players.

NPCs from citizens can be checked in 2 specific ways:

1. Check for the "NPC" metadata being present as already pointed out, or
2. Check if the UUID associated with the player is a v2 UUID. Actual players use v3 UUIDs and Citizens intentionally uses v2. Other NPC plugins may as-well, but that would need to be checked.

Regarding whether NPCs are safe from damage/danger or not: At least in Citizens are NPCs usually protected and invulnerable, meaning no harm can be done unless their gamemode is altered.

Next, regarding possible uses: An NPC could be placed within a lobby, which itself could be inside the MobArena region (Maybe because for styling or to allow players to already see the arena?), giving players some info, maybe even a shop or something? There certainly are more use-cases here that I can't think of.