SpongePowered / SpongeAPI

A Minecraft plugin API
http://www.spongepowered.org/
MIT License
1.14k stars 342 forks source link

Player#getLocale should return the server default (by configuration) if not ready. #1567

Open ryantheleach opened 7 years ago

ryantheleach commented 7 years ago

https://github.com/SpongePowered/SpongeCommon/issues/1145

Player#getLocale will always return en_US when a client joins - the Player's locale gets updated after joining, when packet 0x04/CPacketClientSettings is sent to the server.

Would it be feasible to replace the method with an optional returning method, instead of defaulting to en_US? That + documentation would make it crystal clear to people attempting to use it on join.

kashike commented 7 years ago

A locale must be available, as it is possible to send messages before the server is aware of the client's real locale.

ryantheleach commented 7 years ago

Sure it's possible to send messages, but that doesn't mean it needs to be exposed to plugins while it's potentially incorrect.

If they want to send messages, they can already without the Locale.

If they want to send localized messages, before they have the players locale, then they can make their own assumption or use a configured default?

If people are against the idea, another option could be to have a default locale configured, so that a french hosted server, targetted at french players, could always default to french.

Super minor nitpick I guess, but it would be cool to see a better localization story for minecraft mods / plugins then what's been seen previously.

kashike commented 7 years ago

@ryantheleach Do keep in mind that plugins can send translated messages, in which case a locale is always required.

A default locale option wouldn't be extremely useful, seeing as it'd only be useful when first logging in.

kashike commented 7 years ago

thoughts @Minecrell?

parlough commented 7 years ago

I feel like making it return an Optional is unnecessary, and kind of a pain. There is no problem defaulting to something for the short time where it is not set, as messages may need to be sent and some systems need a locale, and in most cases English is used anyway.

This will just make it so there's a bunch of player.getLocale().get() calls everywhere whenever the locale is needed, since 99% of the time it is available.

If anything, a Javadoc could just state this default either on the getLocale() method or on the joining events.

ryantheleach commented 7 years ago

@Meronat more like player.getLocale().orElse(Locales.Default) but I understand your point, which is why I offered that maybe Sponge should be able to offer a non US based default, by configuration.

kashike commented 7 years ago

I think adding a comment to getLocale for Players is a good idea.

parlough commented 7 years ago

Offering a configuration option for a default language is not a bad idea, as it could be of use elsewhere in Sponge and in plugins.

ST-DDT commented 7 years ago

I think so too. We should either default to a configurable or the system locale and add a comment in the javadocs. Maybe we should also state that plugins that want to use the locale should use the scheduler and wait x/a few ticks for it to become available.

I also considered using a locale that represent the absent/unset locale like xx_yy. But i guess this requires special handling as using Optionals does.

ryantheleach commented 6 years ago

Has the situation changed at all with the packet sequence for logins under 1.13?

kashike commented 6 years ago

No, @ryantheleach.

ejm commented 6 years ago

Why not have something along the lines of Optional<Locale> getLocaleSafe() for those who need to know whether or not a player is actually in the default locale and then something along the lines of

default Locale getLocale() {
    return getLocaleSafe().orElse(Locales.Default);
}

This way you don't break existing code but you also make it clear to those who need to know whether or not the language presented is actually the language of the client or the default?

kashike commented 6 years ago

The client always sends a language. It is impossible to know if a user has explicitly chosen en_us or if it is the default.