feature(api): Allow overriding the locale used in the global translator

[kezz]

We often get requests for overriding the locale used in the GT and some platforms (e.g. Velocity) have methods for overriding this locale. It would be easiest for the implementing platforms if we added a way to handle this. It also prevents confusion in the implementing platforms about exactly what these "effective locale" methods do.

The only additional complexity would be for the implementing platforms to remove the override when a player disconnects.

[kermandev]

Wouldn't it be the simplest for the platform to change the effective locale from the pointers Identity.LOCALE? This would prevent an adventure from carrying additional state that platforms can overlook.

[kermandev]

Probably needs a warning in the javadocs or platform implementation that these must be unregistered by passing (null).

[kezz]

Wouldn't it be the simplest for the platform to change the effective locale from the pointers Identity.LOCALE?

This isn't a great idea because of a) the confusing nature of api to set an override (e.g. velocity), b) it breaks the expectations of existing api that should return the settings locale, and c) it isn't Cross-Platform if each implementation has their own way of setting overrides.

[kermandev]

Wouldn't it be the simplest for the platform to change the effective locale from the pointers Identity.LOCALE?

This isn't a great idea because of a) the confusing nature of api to set an override (e.g. velocity), b) it breaks the expectations of existing api that should return the settings locale, and c) it isn't Cross-Platform if each implementation has their own way of setting overrides.

Why not do something like Audience#audience(Locale, Audience...) or Audience.Builder#pointer(Identity.LOCALE, Locale.US) then? It would add everything you are looking for, and then you could wrap any audience to a specific locale? You could extend this to force state on more pointers, but that's another thing.

[kezz]

Why not do something like Audience#audience(Locale, Audience...) or Audience.Builder#pointer(Identity.LOCALE, Locale.US) then?

Yeah I did consider this but in this example both points a and b still apply.

[masmc05]

it breaks the expectations of existing api that should return the settings locale

Does it really break that expectation? The javadoc always said that it points to a locale, and whatever a platform considers that is an audience's locale (which also I don't think anyone ever guaranteed to be settings locale)

This approach is also extremely limiting for non-platform audiences that may not be bound to uuids. This api seemed to be more universal in usages as noted in Audience's javadocs

Audience is designed to be a universal interface for any player, command sender, console, or otherwise who can receive text, titles, boss bars, and other Minecraft media. It is also designed for a group of receivers such as a team, server, world, or permission.

But yet the api starts having player-bound usages and starts hardcoding group specific behaviour to just forwarding audience.

So in my opinion the current pr isn't a great solution to that existing problem, as trying to keep someone's unconfirmed expectations isn't worth bending written javadocs expectations.

[kezz]

Regardless of the exact wording of the Javadoc, it's abundantly clear that Identity#LOCALE points to the Locale the player has set, as it has done consistently for almost four years now. This is why we can see that both Paper (in the linked PR) and Velocity have chosen to add a separate method for changing the locale used for server side translation specifically. Adventure will not add the ability to change the locale of an audience mid-major update cycle, especially not when there are legitimate reasons to access the settings locale of a user (e.g. client side translation management).

If you are aware of any implementations that do not back a player instance by their UUID, please do let me know and we can change the implementation of this PR to use an identity backed map of sorts.

Adventure already has plenty of API that expects or performs differently for audience implementations directly backed by a player object (e.g. boss bar viewing, permissions check). I'm unsure what your comment there has to do with anything here. This "hardcoding" is simply to provide an easy means to bulk set locale overrides (which fwiw is another reason why having a non-identity based cache is useful, what would setting an override for a world do? and would the implementations have to now manage world evictions too?).

Overall, I appreciate both of your comments, but the goal of this PR is to solve a specific problem as addressed in the linked Paper PR, that adding platform specific API methods on the Player objects is confusing to developers. If we (as in Adventure) added non-platform specific API methods on Audience objects, that still leaves us with the main issue of confusing API. That is why this PR is as specific as possible, keeping the API surface changes directly to the one area that is impacted by the overrides.

[masmc05]

Adventure will not add the ability to change the locale of an audience mid-major update cycle,

I don't think anyone suggested that? Both here and in paper-contrib we just don't think it's really something on adventure to handle.
a) it's some data that isn't so much related to the translator but the audience, it's weird to be stored on translator
b) it's easy for platforms to somehow lose track and forget to remove it
c) Global translator currently just have the translators that are almost always registered on start and that's all, making it statefull all of a sudden is really really weird.
d) We always can have a race condition when someone put the override right after a player left, as the platform can't enforce some synchronization like main thread check on paper, so we easily get into a memory leak as the platform ofc won't randomly clear again the player data in the random static final place on adventure 10 minutes after the player left.

Then platforms could do any changes in more appropriate platform specific release cycles.

If you are aware of any implementations that do not back a player instance by their UUID

People may want to change the locale of console that is used by the global translator. A console ofc isn't backed by an uuid. Then you have the weirdness that changing the console's override locale did nothing and failed silently and you waste hours trying to find out what you did wrong.

which fwiw is another reason why having a non-identity based cache is useful, what would setting an override for a world do? and would the implementations have to now manage world evictions too?

Another point towards the weirdness of having adventure manage a random huge map with data without any memory leak protection. Let the audience implementation decide those details.

Adventure already has plenty of API that expects or performs differently for audience implementations directly backed by a player object (e.g. boss bar viewing, permissions check)

Those directly involve having a player, as without a player in context those are meaningless. Receiving a message can happen by many non-players, like rcon consoles, discord-chat webhook redirections etc, these aren't so player-bound.

that adding platform specific API methods on the Player objects is confusing to developers

I don't really agree that it's so confusing. The only current confusion comes from lack of possibility to override the locale by player preference which many want, and there are some debates on better naming of that api in the pr.

[masmc05]

As an alternative approach to solve this issue, what about creating

public interface TranslationPreProcessor {
    void process(Context ctx);

    interface Context {
        Audience audience();
        Locale locale();
        void locale(Locale newLocale);
        TranslatableComponent component();
        void component(TranslatableComponent component);
    }
}

Which is responsible of preprocessing context before translation, which may be registred by GlobalTranslator#registerPreProcessor(Key, TranslationPreProcessor) and GlobalTranslator#registerPreProcessorBefore(Key, Key, TranslationPreProcessor)

Since this data is already transitive, plugins will most probably already have their own storage for it anyway, which they will be able to query in the preprocessor to override the locale.

Translating not by audience but by locale skips this preprocessing.