Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Citizens Sentinels #17

Open
tajobe opened this issue Feb 16, 2020 · 3 comments
Open

Support Citizens Sentinels #17

tajobe opened this issue Feb 16, 2020 · 3 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@tajobe
Copy link
Member

tajobe commented Feb 16, 2020

We should be able to add a soft-dependency on citizens to add proper support for healthbars on their NPCs (Sentinels in particular)

According to @Maitlans, the API seems to treat them as players and the health shows as 0. See report in #15

@tajobe tajobe added enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers labels Feb 16, 2020
@tajobe tajobe self-assigned this Feb 19, 2020
@tajobe
Copy link
Member Author

tajobe commented Feb 20, 2020

After some testing, it seems like treating the Citizens NPCs as mobs doesn't fix the problem as setting the customName like a normal entity doesn't work (since they are actually Players as far as the server is concerned) and keeping it as a player isn't ideal since the vanilla hearts scoreboard doesn't seem to work with them.

It's worth noting that without any changes to the code, the following healthbars work for Citizens player NPCs (in the player-bar config):

  • SCOREBOARD type using PERCENT style
  • ACTION type

So, we can

  • Document the above (if you want citizens player NPCs to work, it uses the player-bar config with the above limitations or
  • Integrate more directly with citizens API to allow NAME type (via AbstractNPC#setName, this is speculative but looking at the api I guess it'd work)
  • If we find we are setting a scoreboard healthbar on a citizens player NPC, always set the score ourselves (rather than just for PERCENT)

Either way, if we intend to support it more directly, I'd probably suggest having them on a separate config....eg citizens-npc-bar. That could make citizens NPCs the only entities that support all 3 healthbar types. The risk/annoyance with integrating with Citizens can't really be overstated though...

  • Their maven repo is missing the metadata a normal repository should have and thus is frustrating to work with
  • They made breaking API changes (not just additive...seem to have removed NPC#getBukkitEntity) between versions 2.0.5 and 2.0.26-SNAPSHOT. Clearly not semantic versioning and that tiny mismatch between the API version I pulled in maven and the version of Citizens on the server caused exceptions and failures
  • Since Citizens makes heavy use of NMS, I doubt they'll be up to date super quickly. This isn't too bad for us as that would only mean that the Citizens integration would be broken, not the whole build/plugin, but still worth considering.

@Puremin0rez and @Maitlans, any ideas/preferences?

@tajobe
Copy link
Member Author

tajobe commented Feb 23, 2020

I took a stab at direct integration and setting the NPCs name does bad things to the NPC.
11K8xF6SHE

At this point, I'm probably leaning towards no direct integration and documenting the limitations as the scoreboard is unreliable as well, at least until someone more familiar with Citizens can come along and integrate. As it stands, even if I add a separate configuration section for citizens bars (ACTION works fine), they still seem to get a hearts scoreboard added (with 0 hearts) even if I never set a scoreboard on the NPCs. Better to just say we don't support it but X seems to work OK than to support an awful user experience.

@Maitlans
Copy link

Maitlans commented Mar 1, 2020

Thank you so much for the detailed post, thank you for attempting to add integration, I've never really seen a plugin with it integrated so it must be really hard or not possible at its current state. Yeah I agree, sorry for the late response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants