PlaceholderAPI support for announcements.yml

[Andre601]

Foreword

I suggested this already in the MobArena Discord server, but are now creating this issue for proper tracking and discussion since the message can get lost in the Discord chat after a while.

Idea

MobArena should support PlaceholderAPI placeholders within its announcements.yml messages (At least for those that are for players).
This would allow larger customization of the messages to f.e. display the Player's nickname or in my case a custom font image from ItemsAdder.

Through the chat with you (garbagemule) did I came up with possible ideas on how to implement such a feature.

1) 1st-party support using classes

MobArena could have an Interface containing send methods (And perhaps also a default method to resolve the announcements file) that two classes would then implement.
One class is for normal sending of the messages while the other does the same, but first uses PlaceholderAPI.setPlaceholders(Player, String) on the message to parse eventual placeholders.

Which class would be used can be determined during the plugin's onEnable() phase by checking if PlaceholderAPI is enabled.

Quick Mockup to show the idea:

private MessageHandler messageHandler; // Interface to use

@Override
public void onEnable() {
    if(getServer().getPluginManager().isPluginEnabled("PlaceholderAPI")) {
        messageHandler = new PAPIMessageHandler(); // Class inheriting MessageHandler that parses placeholders
    } else {
        messageHandler = new DefaultMessageHandler(); // Default handler without placeholder parsing.
    }
}

This way could MobArena call a method like MessageHandler#sendMessage(Player, String) and placeholder would automatically be parsed by PlaceholderAPI.

The downside would be A) You have to add PlaceholderAPI as a dependency to your maven project, tho it only needs to be provided and not compiled into the jar, and as a soft-depend into the plugin.yml and B) if the PluginLoader is going crazy due to a plugin depending on another plugin that actually depends on the first one, could PlaceholderAPI enable AFTER MobArena, resulting in the enabled-check returning false. I encountered this issue quite a bit in my life and the only workaround is doing the check once the server is done loading.

2) Events for Announcements

MobArena could offer events that would be called whenever an announcement is being sent.
One event (Here called MobArenaPreAnnounceEvent) would be called before the message is send and another (Called MobArenaAnnouncedEvent) afterwards.

A separate plugin could then hook into MobArena, listen for the MobArenaPreAnnounceEvent and modify the text by parsing the placeholders in it.

Benefit of this option would be that MobArena doesn't have to do the parsing itself and could leave it up to an external plugin to do the job.

Other issues

I'll take the time to also point out some issues with the current system used that should be reworked for the future.

The first one being the usage of an enum. It's not a good idea, especially if the goal is to allow people to modify the text themself. Another issue is the fact that values of the enum (The actual strings) are modified, which shouldn't be done in an enum. Enums are meant to be and give constant values, so allowing modification of those is bad.

Next is the usage of % as a placeholder a really bad one.
It's quite hard for people to tell what the % symbol will be replaced with. Instead should MobArena use some named placeholders like {arena} to more easily display what the placeholder is meant to display when parsed.

Finally, maybe take the time to restructure the YAML file? I know it's auto-generated from the enum, but this could be replaced by keeping the original YAML file in the jar and copy it over into the plugin folder on first startup.
That way could the file be a bit more structured (i.e. putting messages into specific sections to categorize them) and easier to read.

[garbagemule]

Thanks for taking the time to riff on some ideas for how this could be implemented.

As mentioned in #743, slapping an extra dependency in pom.xml is not a good solution for MobArena, because it lowers maintainability. That rules out the first solution.

Before I address the second proposal, I'd like to just briefly mention a third idea: an extensible framework inside MobArena itself that other plugins can hook into and augment, similar to how the Things and Formulas APIs are designed. This form of dependency inversion would make for a very classic programming model where an external plugin would grab a hold of MobArena's messaging subsystem and register a "resolver" on it. I especially like this approach for its clear intent: you register a resolver within MobArena rather than listening for an arbitrary event. Perhaps the biggest problem I've seen with this approach is the atrocious plugin lifecycle in the Bukkit API. Vault is a great example. MobArena basically supports all economy plugins on paper, but because there is no guarantee that MobArena will load after an external-to-Vault economy plugin without setting an explicit soft dependency, when MobArena tries to resolve the economy instance, it might not exist yet. The de facto solution to this is an awkward, fragmented initialization phase with "next tick" delays that make everything so much more difficult than it should have to be.

The second proposal of an event-driven design definitely fits in nicely with the Bukkit API itself, and it also tackles the problem via dependency inversion like the framework idea, keeping the core plugin lean. The biggest benefit of an event-driven system is that "initialization" is really just an event handler registration, which means load order is only semi-relevant (PAPI/MobArena load before the external plugin via (soft) dependencies). As long as MobArena doesn't send any PAPI-ful messages before the external plugin has had a chance to register its event handler, it should work fine. It does feel a little clunky to use the event bus for this kind of thing, though. The performance impact is probably negligible compared to all the entity events, and I'm guessing the actual placeholder resolution is significantly heavier than the overhead of firing an event.

I think the second and third approaches are definitely worth a try, but it probably requires a rework of the messaging system first, since it's very rigid, not very well suited for real templating, and well... it's over 11 years old. An overhaul of the messaging system should at minimum give us the opportunity to crowd source translations somehow, which would require an actual source file to translate, so yeah, the YAML file has to be explicit at the very least.

[Andre601]

MobArena basically supports all economy plugins on paper, but because there is no guarantee that MobArena will load after an external-to-Vault economy plugin without setting an explicit soft dependency, when MobArena tries to resolve the economy instance, it might not exist yet. The de facto solution to this is an awkward, fragmented initialization phase with "next tick" delays that make everything so much more difficult than it should have to be.

What you could do is either listen for the ServerLoadEvent or on older versions set up a scheduler to run asap. Both would trigger when the server is "done", so you could then check for the economy plugins in question to be active without having a (soft)depend on them... It also bypasses issues of circular dependencies, where it would cause broken load-orders that completely ignore a plugins depend on another one...

I use that myself in one of my plugins to have it hook into a plugin (ProtocolLib in this case) once the server is done doing all its stuff. It gives me the guarantee that the plugin is loaded at that point and ready. And the softdepend is only there to avoid warnings in the process.

I feel like this would be a good aproach on avoiding this issue of "Plugin X is not ready when I load"... Just an idea tho.

The performance impact is probably negligible compared to all the entity events, and I'm guessing the actual placeholder resolution is significantly heavier than the overhead of firing an event.

The placeholder resolution is relatively small in performance impact. It all just depends on what placeholder expansion is doing what actions and if they have any means of reducing performance heavy stuff (i.e. cache values).

I think the second and third approaches are definitely worth a try, but it probably requires a rework of the messaging system first, since it's very rigid, not very well suited for real templating, and well... it's over 11 years old. An overhaul of the messaging system should at minimum give us the opportunity to crowd source translations somehow, which would require an actual source file to translate, so yeah, the YAML file has to be explicit at the very least.

Maybe consider a per-player translation system for that then? Like that messages are send in the language of the player (if available) based on the locale they use, which you should be able to obtain through the Bukkit/Spigot API from what I know.
This, however, would then require you to store each language in their own file, so having it in a dedicated sub-folder would be beneficial for it.

In terms of translation would I also recommend using and setting up a page like Crowdin (They offer open-source licenses for their services) or Weblate, so translations may be a bit easier to do for people who don't like editing files directly. It's not a requirement tho.