-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Effective Locales #12448
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Effective Locales #12448
Conversation
paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java
Outdated
Show resolved
Hide resolved
paper-server/patches/sources/net/minecraft/server/level/ServerPlayer.java.patch
Show resolved
Hide resolved
|
I am semi uncertain on the naming "effective locale". This differs drastically on backend servers like paper, where basically none of the messages sent are actually translated on the server side beyond plugin messages. Command feedback, server system messages, etc are all not translated on the server but instead resolved by the client itself. |
How about just mentioning what that locale actually does in the javadoc? |
|
Well, javadocs are a nice tool. But they are generally a second resort and do not save us from poorly named methods. |
| - public String language = "en_us"; | ||
| + public String language = null; // Paper - default to null | ||
| + public java.util.Locale adventure$locale = java.util.Locale.US; // Paper | ||
| + @Nullable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try put this in the same line because the comment only apply to the field and not this.
I mean, I agree. But then again, there are no better name suggestions yet. And I think it makes sense to mention it regardless |
|
Yea certainly a good point to expand the javadocs here regardless 👍 |
|
Seeing as Velocity also has this and Adventure has a lot of requests of users wanting to override the locale used for server-side translating, I've gone ahead and opened a PR to implement this in Adventure directly, which also solves the issue this PR has about confusing developers. PaperMC/adventure#1221 |
|
How about Player.get/setServerSideLocale() ? |
|
Yea, setServerSideLocale is a nice name, however the adventure PR might be nicer. |
|
Seeing above, I am going to go ahead and close this PR. Thank you so much for your contribution, but this will be much better supported natively by the adventure PR listed above. |
Adds Player#setEffectiveLocale(Locale) / Player#getEffectiveLocale() to set and get a player-specific locale, which is used for server-side translations.
Closes #10570