Add permissions for individual colors

[Pokechu22]

This PR adds permissions for each individual chat color (and rewrites+adds tests for the chat formatting code).

New permissions suffixes for each code are added to essentials.msg, essentials.chat, essentials.nick, and essentials.signs.

[Pokechu22]

Oh, I should be clear: the perms right now are in the form of e.g. essentials.chat.code.0, essentials.chat.code.k, etc. I'm not sure if this is the best format; I could maybe swap it out for the bukkit enum names instead.

[mdcfe]

I think enum names would be better, especially considering that &k is .magic for nicks (as far as I know).

[Pokechu22]

Done (e.g. essentials.chat.black, essentials.chat.magic).

I guess I should check: is there another place I need to declare these permissions for permissions plugins to properly detect them?

[Pokechu22]

Just realized that this doesn't work right with escaping, will correct...

[SupaHam]

Hey Pokechu22. Thanks for a great contribution once again. I have a few concerns:

1. As it stands I believe there is no change in current behaviour based on any given permission set, correct?
2. When given .black they'll be given black. What if we wanted to enable a wildcard feature where you have the ability to negate .black but keep all. If I'm not mistaken a user would currently need to define all chat styling permissions but the ones they don't want.

What I would do is when running permission checks on ChatColor.values() I would check for explicit permission setting alongside the permission value. So

1. when a player doesn't have .black set, it's a NOOP;
2. when they have .black set, and value is true, it is added to supported set;
3. when they have .black set, and value is false, it is not added to supported set.

EDIT: this would require the adding of a method in User that runs Permissible#isPermissionSet

[Pokechu22]

As it stands I believe there is no change in current behaviour based on any given permission set, correct?

Yea, the existing permissions will continue to behave exactly the same.

When given .black they'll be given black. What if we wanted to enable a wildcard feature where you have the ability to negate .black but keep all. If I'm not mistaken a user would currently need to define all chat styling permissions but the ones they don't want.

Yes, currently that's how this works, and it's definitely suboptimal. I'll try to replace it. (Might also make sense to add a .all permission for that purpose, since right now you need to have .color, .format, and .magic)

[Pokechu22]

... on second thought, an all permission is completely useless given that wildcards exist.

[Pokechu22]

Ah, right, that's the problem that I was thinking of:

I made it so that magic isn't a category, but instead is just checked by the loop. If I switch to checking whether or not a permission is set, then using an * perm would not include magic. Just making magic a manually-checked category again would solve that.

[SupaHam]

Lastly, may we change this to comply with Minecraft's vanilla colour names and not Bukkit? I believe this is the correct step forward in making the feature more accessible in general.

magic would become obfuscated.

[mdcfe]

If we're going to change one thing (in this case, the name for &k-style formatting) from Bukkit naming to vanilla naming then we should be following that convention consistently throughout the plugin. (Not that I see this as a necessary change, but still.)

[SupaHam]

Are you referring to anything specifically?

…

On Aug 12, 2017 5:44 PM, "md678685" ***@***.***> wrote: If we're going to change one thing from Bukkit naming to vanilla naming then we should be following that convention consistently throughout the plugin. (Not that I see this as a necessary change, but still.) — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1441 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABPV0RNE4ke19plC_Q1cK0jrMEaByDrvks5sXdaKgaJpZM4OuW5b> .

[mdcfe]

@SupaHam My bad regarding consistency, didn't fully read the first comment of the PR.

However, I still don't think we should change the name of the permission for magic/obfuscated text. It's been around for long enough and I don't see why we should just break people's permission setups. (Especially considering that by this point we don't generally tend to support the BukkitDev/Spigot releases in which server owners would normally expect to be informed of the breaking changes, but that's starting to go into another argument about the flaws of EssentialsX's versioning.)

[SupaHam]

I forgot that .magic already exists. I agree that it should stay the same to minimise maintenance required from users. Ignore any change to .magic, but since colour permissions are an addition, make sure they are similar to vanilla naming.

[Pokechu22]

Yea, it should be easy to leave .magic alone and add a .obfuscated permission (I already set up groups of a sort; I can special-case obfuscated too).

[Pokechu22]

Done. One other thought - should I leave § as-is, or should I replace it with \u00a7?

[SupaHam]

This sorta goes back to the .colour .color problem that we resolved a while back. When you have alias permissions it becomes more of a burden. In some cases, you have to make sure you've disabled both permissions to produce the desired behaviour.

[mdcfe]

I'm not sure what needs doing for this PR to be ready. From what I can tell, this is ready aside from this comment suggesting ca856a8 should be changed or reverted?

Edit: I'll update this to master when ready to merge.

[Pokechu22]

Yeah, I'm not sure what's left either. If you want, I can revert that, but it's been a long enough time that I'm not really sure.

[mdcfe]

Looking over this again, I think the issue was that having both magic and obfuscated would potentially confuse server owners as to which one should be set/revoked. I'm happy to merge this as-is, though, because as far as I can tell it doesn't matter as long as one is set.

[Pokechu22]

Yeah, I think the point of that change was to allow using either magic or obfuscated, but generally you'd only set one of them.