[Discussion] Adapt or overhaul message sending system

[mdcfe]

Discussion

Note that this is a discussion thread, and does not currently indicate any planned features. Instead, it exists to allow people to comment and suggest changes to a particular part of EssentialsX in one consolidated thread.

Summary
Several changes to or a total overhaul of how EssentialsX constructs and sends messages. (To clarify; "messages" means any text sent from the plugin to a user/the console, not private messages.)

Problems with the current system
Currently, EssentialsX stores its messages in several messages[...].properties files. These are accessed with the static I18n.tl method used throughout the plugin, which loads the corresponding message from the default or user-created messages files, substitutes in any required parameters like player names or other translated messages, and returns it ready to be sent out. However, this has some flaws:

• Placeholders use a format that is not particularly descriptive (and feels non-standard)
  • You either have to guess what the placeholders do based on then default messages or read the EssentialsX source, both of which can be daunting for server admins and translators.
  • PlaceholderAPI isn't supported, and trying to add support was difficult as we needed to change every place a message was used in code just to provide a player to PlaceholderAPI.
• Colour codes are currently recorded as \u00a7x, where \u00a7 is the § symbol and x is the hexadecimal colour code.
  • If you haven't memorised these colour codes, editing EssentialsX messages becomes a tedious chore.
  • Update: EssentialsX now uses Crowdin, which alleviates some of the issues with this.
• There is absolutely no way to utilise Minecraft's modern chat features like hover/click events due to a lack of any JSON support.
  • This has been requested several times already, but isn't currently very easy to implement without significant changes.
• Usage messages can only be controlled via plugin.yml, not in the standard messages file.
  • These generally use English words, and translating these requires more steps than should be necessary.
  • Update: As of Add support for translating command description and usage #3284 this has been implemented

Possible improvements

• Allow JSON in messages files.
  • This finally gives people the ability to use hover and click events.
• Split messages across several files by category.
  • For example, we could have a "usage" file, an "errors" file, a "economy" file etc.
  • EssentialsX doesn't need to know where the messages come from as long as they're present.
  • Users can alter this structure however they want, and EssentialsX will just load all the files that match the right locale.
• Use a chat component library like md_5's chat API (shipped with Spigot but not CB) or KyoriPowered/text (well-known)
  • Picking one of these libraries could make supporting JSON features much easier.
  • We would need to shade in the library, as they are not shipped with Bukkit itself.
  • These libraries would likely need additional supporting code to allow for more advanced features such as the ones mentioned below.
• PlaceholderAPI support considered from day one.
  • PAPI includes supports for hundreds more placeholders than we could implement by ourselves.
  • PAPI requires a player to be provided as context, otherwise most expansions won't know what to retrieve placeholders for.
• Support named placeholders in messages files, like {target} or {location}
  • These are easier for translators and server admins to understand, compared to the cryptic {0} {1} {2} littered throughout the messages files.
  • These could be extended to support tiny script-like expressions, like {player is god} -> "true"/"false" or {target afkmessage} -> "getting lunch", thought this likely wouldn't be a priority.
    • This could even be how we specify which player is used for PAPI placeholders - for example, {target %luckperms_track%}.

Concerns

• Lots of servers already have pre-existing custom messages.properties files, and EssentialsX has several community translations in this format.
  • We'd need to either find a way of not breaking compatibility, or automatically migrating these files if we change the format.

---

This is the write-up of several thoughts I've had over the past few months. Any further suggestions or feedback on EssentialsX is welcome in the replies below.

[Phoenix616]

Regarding backwards compatibility of non-named placeholders in existing custom messages files:

This could be easily done by replacing both, the actual placeholder name and the legacy index placholder in messages, the only thing that would need to stay the same is the order in which placeholder replacements are passed to the translation method.

Possible (pseudo) code for the replacement assuming that you have a linked Map with placeholder name -> replacement value:

int i = 0;
for(Iterator<Map.Entry<String, String>> it = map.entrySet().iterator(); it.hasNext(); i++) {
    Map.Entry<String, String> e = it.next();
    string = string.replace("{" + e.getKey() + "}", e.getValue()).replace("{" + i + "}", e.getValue());
}

Then you would just change a translation method from something like tl("nickSet", player.getNick(), sender.getName()) to tl("nickSet", ImmutableMap.of("nickname", player.getNick(), "sender", sender.getName())) (Or a more convenient way of using a vararg to create the replacement map, that might break any usage of the old translation methods though in other plugins)

[mdcfe]

Backwards compatibility

@Phoenix616 This looks like a good way to handle backwards-compatibility with older messages - I'll keep a note of that.

Placeholder "expressions"

...tiny script-like expressions, like {player is god} -> "true"/"false" or {target afkmessage} -> "getting lunch"

While this seemed like a good idea at the time I wrote the issue, in retrospect I think including this in EssentialsX itself is both overcomplicated and unnecessary. PlaceholderAPI already covers pretty much everything I can think of in this regard, so we'd just be reinventing the wheel. In addition, I can't see a reason for most users to want this over proper PAPI support.

---

Started writing this up earlier then had to go out, so here's the rest:

JSON and click/hover

It probably isn't necessary to support full JSON in the messages files themselves. Instead, we could use expressions (somewhat inspired by Markdown) like so:

exampleUrl=&6Here's a [URL example](url https://essentialsx.github.io)!
exampleSuggest=&eUse [&f/helpop](suggest /helpop)&e to contact staff.
exampleHover=&cHere's an |error|(&bHere's some more info.)!

File structure

If we split messages into separate files, we'll probably want to reorganise where we keep them. This could be achieved by moving them into messages/<language>/<category>. properties.

If we're going to stick with an enhanced version of "legacy" formatting codes as described above, we likely won't need to move away from . properties files.

However, if we want to allow admins to specify their messages in actual JSON (eg. to use existing JSON, or to include keybinds or translated game text), it might be preferable to use either JSON or HOCON for messages files, allowing chat component JSON to be pasted straight in without escaping. It wouldn't be user- or translator-friendly to require all messages be defined in JSON as many won't need to use hover or click events, so the option to use legacy codes would need to remain.

Alternatively, we could extend the legacy formatting further to allow inserting these special components into otherwise normal messages without needing JSON. This would have the added benefit of not needing to add support for JSON to text commands (like /info, /rules, /custom etc) separately to translations.

[Phoenix616]

JSON and click/hover

It probably isn't necessary to support full JSON in the messages files themselves. Instead, we could use expressions (somewhat inspired by Markdown) like so:

exampleUrl=&6Here's a [URL example](url https://essentialsx.github.io)!
exampleSuggest=&eUse [&f/helpop](suggest /helpop)&e to contact staff.
exampleHover=&cHere's an |error|(&bHere's some more info.)!

You might be interested in my own markdown-inspired library: https://github.com/Phoenix616/MineDown, either to use directly (depends on bungee-chat though) or for some ideas on what to do with the syntax. Paper also has some plans to add a syntax for components that is easier to use than pure json.

[mdcfe]

@Phoenix616 I was aware of MineDown before, but hadn't really looked into it. The main issue with depending on it is that I was leaning towards using kashike's library, since there's a set of adapters to use it with both CraftBukkit and Spigot, in case people stick with CB. Otherwise, the syntax looks quite good - I'm particularly a fan of how hover and click are done.

Regarding Paper's proposed syntax: It looks interesting, but I'm not sure whether it's the best route for us. While I agree that it's easier to use than raw JSON, I think it's still too verbose - the syntax seems to rely on XML-like tags (eg {bold}text{/bold}), which I find hard to read in a long string and could put off some users. In contrast, MineDown's syntax is much more concise and readable, more like Markdown itself (which is, I suppose, what you were going for).

However, if Paper does eventually implement it, it might be worth considering; I think it could be confusing to users if they end up having to learn multiple separate syntaxes for different plugins.

[Andre601]

I appreciate this issue for discussion and want to make a smaller correction to one of the statements in the "possible improvements" section:

• PAPI requires a player to be provided as context, otherwise most expansions won't know what to retrieve placeholders for.

This is partially true. While many plugins and placeholder indeed require a player to return certain values does PlaceholderAPI not (anymore) require a player... At least not an online player that is.
An offline player (or as far as I know even null) can be provided instead and it's up to the actual expansion to handle the player.
I just wanted to point this out since there are quite a few expansions out there that aren't completely related to players and therefore not always require one to be present...

I can't really see any actual reason why it would be important for EssentialsX to handle the placeholder that properly... If EssX can should it provide the player, otherwhise null. It's then up to the expansion to handle this and either return something, nothing (empty string) or error out...

I hope this makes kinda sense what I typed here... Just gave some random thoughts I had.