GH-950 Single Global Language from Config, Remove Per-User and GUI #950
Conversation
|
|
WalkthroughThis update removes all features related to managing individual player languages. It deletes the language service, repository, commands, and configuration for multiple languages. Instead, the system now uses a single language setting from the main configuration for everyone. Translation handling and GUIs are simplified to work with this one language, and all code that supported per-player language preferences is cleaned up or removed. The overall effect is a streamlined approach focusing on a global language rather than personalized language options. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
vLuckyyy
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
eternalcore-core/src/main/java/com/eternalcode/core/feature/fullserverbypass/FullServerBypassController.java (1)
57-71: Translation calls updated but logic can be simplified.Both branches now use the same global translation, making the user existence check redundant.
private String getServerFullMessage(Player player) { - Optional<User> userOption = this.userManager.getUser(player.getUniqueId()); - - if (userOption.isEmpty()) { - return Joiner.on("\n") - .join(this.translationManager.getMessages().player().fullServerSlots()) - .toString(); - } - - User user = userOption.get(); - return Joiner.on("\n") .join(this.translationManager.getMessages().player().fullServerSlots()) .toString(); }
♻️ Duplicate comments (1)
eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/WarpInventory.java (1)
255-261: Same casting issue here.The unsafe cast to
AbstractTranslationshould be avoided for the same reasons mentioned in theaddWarpmethod.
🧹 Nitpick comments (5)
eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/argument/AbstractViewerArgument.java (1)
22-30: Translation calls updated correctly.Both branches now properly use the global translation. Since both paths are identical, consider simplifying:
@Override protected ParseResult<T> parse(Invocation<CommandSender> invocation, Argument<T> context, String argument) { - if (invocation.sender() instanceof Player player) { - Translation translation = this.translationManager.getMessages(); - return this.parse(invocation, argument, translation); - } - - Translation translation = this.translationManager.getMessages(); + Translation translation = this.translationManager.getMessages(); return this.parse(invocation, argument, translation); }eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/ENTranslation.java (1)
24-24: Remove unused import.The
java.util.Localeimport is not used in this file and should be removed.-import java.util.Locale;eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/TranslationFactory.java (1)
10-17: Good implementation with flexible language matching.The case-insensitive comparison for language codes is a nice touch. Consider using English comments for consistency with the rest of the codebase.
- // Możesz tu dodać if-y dla własnych klas tłumaczeń + // You can add if statements here for your own translation classes if (languageCode.equalsIgnoreCase("pl") || languageCode.equalsIgnoreCase("pl-pl")) { return new PLTranslation(languageCode); } - // Domyślnie angielski + // Default to English return new ENTranslation(languageCode);eternalcore-core/src/main/java/com/eternalcode/core/translation/TranslationManager.java (2)
14-14: Consider thread safety for the mutable translation field.Since this field can be changed via
setTranslation(), consider making it volatile or using proper synchronization if this class is used in a multi-threaded environment.-private Translation translation; +private volatile Translation translation;
30-32: Document that locale parameter is ignored.Since this method now ignores the locale parameter and always returns the single translation, consider adding a comment to clarify this behavior.
@NotNull @Override +// Note: locale parameter is ignored in single-language mode public Translation provide(Locale locale) { return this.translation; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
eternalcore-api/src/main/java/com/eternalcode/core/EternalCoreApi.java(0 hunks)eternalcore-api/src/main/java/com/eternalcode/core/feature/language/LanguageService.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/EternalCoreApiImpl.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/argument/AbstractViewerArgument.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/contextual/PlayerContextual.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/contextual/UserContextual.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/configuration/implementation/PluginConfiguration.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/afk/AfkKickController.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/afk/AfkPlaceholderSetup.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/container/DisposalCommand.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/fullserverbypass/FullServerBypassController.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/home/HomePlaceholderSetup.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/language/BukkitLanguageProvider.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageCommand.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageInventory.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageLoadController.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageRepository.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageRepositoryImpl.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageServiceImpl.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/language/config/LanguageConfigItem.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/language/config/LanguageConfiguration.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/WarpInventory.java(3 hunks)eternalcore-core/src/main/java/com/eternalcode/core/notice/NoticeService.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/AbstractTranslation.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/Translation.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/TranslationManager.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/TranslationManagerSetup.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/ENTranslation.java(3 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/PLTranslation.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/TranslationFactory.java(1 hunks)
💤 Files with no reviewable changes (12)
- eternalcore-core/src/main/java/com/eternalcode/core/EternalCoreApiImpl.java
- eternalcore-api/src/main/java/com/eternalcode/core/EternalCoreApi.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageCommand.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/language/config/LanguageConfigItem.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/language/BukkitLanguageProvider.java
- eternalcore-api/src/main/java/com/eternalcode/core/feature/language/LanguageService.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageRepository.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageServiceImpl.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageRepositoryImpl.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageInventory.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/language/config/LanguageConfiguration.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageLoadController.java
🧰 Additional context used
🧠 Learnings (3)
eternalcore-core/src/main/java/com/eternalcode/core/translation/AbstractTranslation.java (1)
Learnt from: vLuckyyy
PR: EternalCodeTeam/EternalCore#865
File: eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/item/give/GiveService.java:0-0
Timestamp: 2024-10-14T20:11:13.610Z
Learning: In the 'EternalCore' project, avoid using null in method signatures in Java code.
eternalcore-core/src/main/java/com/eternalcode/core/notice/NoticeService.java (1)
Learnt from: vLuckyyy
PR: EternalCodeTeam/EternalCore#865
File: eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/item/give/GiveService.java:0-0
Timestamp: 2024-10-14T20:11:13.610Z
Learning: In the 'EternalCore' project, avoid using null in method signatures in Java code.
eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/PLTranslation.java (1)
Learnt from: CitralFlo
PR: EternalCodeTeam/EternalCore#896
File: eternalcore-api/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggle.java:7-8
Timestamp: 2025-01-22T22:25:29.027Z
Learning: In the EternalCore project, when a class has a default no-args constructor, avoid making fields final as they need to support initialization through both the default and parameterized constructors.
🧬 Code Graph Analysis (1)
eternalcore-core/src/main/java/com/eternalcode/core/translation/TranslationManagerSetup.java (1)
eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/TranslationFactory.java (1)
TranslationFactory(5-18)
🔇 Additional comments (18)
eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/contextual/UserContextual.java (1)
35-35: Translation method call updated correctly.The change from
getDefaultMessages()togetMessages()aligns with the simplified translation system.eternalcore-core/src/main/java/com/eternalcode/core/feature/afk/AfkKickController.java (1)
63-63: Per-user language selection removed correctly.The change from
getMessages(user.getUniqueId())togetMessages()successfully removes player-specific language handling as intended.eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/contextual/PlayerContextual.java (1)
30-30: Translation method call updated correctly.The change from
getDefaultMessages()togetMessages()aligns with the simplified translation system.eternalcore-core/src/main/java/com/eternalcode/core/feature/home/HomePlaceholderSetup.java (1)
57-57: Per-player language selection removed correctly.The change from
getMessages(targetPlayer.getUniqueId())togetMessages()successfully removes player-specific language handling for home placeholders.eternalcore-core/src/main/java/com/eternalcode/core/feature/container/DisposalCommand.java (1)
38-38: Per-player language selection removed correctly.The change from
getMessages(player.getUniqueId())togetMessages()successfully removes player-specific language handling for the disposal command.eternalcore-core/src/main/java/com/eternalcode/core/feature/afk/AfkPlaceholderSetup.java (1)
34-34: Good refactoring to single global translation.The change from player-specific to global translation retrieval is correct and aligns with the simplified language system.
eternalcore-core/src/main/java/com/eternalcode/core/configuration/implementation/PluginConfiguration.java (1)
50-51: Let’s list all translation files and pull out their language codes:#!/bin/bash find . -type f \( -iname "*.properties" -o -iname "*.yml" -o -iname "*.yaml" -o -iname "*.json" \) \ | sed -n 's/.*[_\/]\([[:alpha:]]\{2\}\)\.\(properties\|yml\|yaml\|json\)/\1/p' \ | sort -ueternalcore-core/src/main/java/com/eternalcode/core/translation/Translation.java (1)
186-186: Good simplification of language handling.Changing from Language enum to String makes the interface more flexible and aligns with the simplified language system.
eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/ENTranslation.java (2)
41-43: Good simplification of the constructor.The change from
Languageenum toString languageCodealigns well with the overall refactor goals.
125-128: LGTM on the new getLanguage() method.The implementation correctly returns the language code string as expected by the new interface.
eternalcore-core/src/main/java/com/eternalcode/core/notice/NoticeService.java (2)
43-51: Good removal of LanguageService dependency.The constructor simplification correctly removes the unused language service parameter.
85-87: LGTM on the simplified locale provider.Returning
Locale.ROOTis appropriate for the new single-language system.eternalcore-core/src/main/java/com/eternalcode/core/translation/AbstractTranslation.java (2)
10-18: Excellent refactor of the language handling.The change from
Languageenum toString languageCodeis clean, and the new method namegetLanguageCode()is more descriptive.
20-23: Good update to resource path construction.Using the language code directly in the file path is simpler and more straightforward.
eternalcore-core/src/main/java/com/eternalcode/core/translation/TranslationManagerSetup.java (1)
13-18: Perfect simplification of translation management.The new single-language approach is much cleaner. Getting the language from plugin configuration and creating one translation instance is exactly what this refactor aimed for.
eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/PLTranslation.java (2)
41-43: Constructor change looks good!The new public constructor properly accepts and passes the language code to the superclass, aligning with the single-language refactoring.
571-574: Clean implementation of getLanguage method.The method correctly returns the language code from the superclass, maintaining consistency with the new design.
eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/WarpInventory.java (1)
74-76: Good simplification of inventory opening.Removing the async language fetching makes the code simpler and more direct, which aligns well with the single-language approach.
eternalcore-core/src/main/java/com/eternalcode/core/translation/TranslationManager.java
Show resolved
Hide resolved
eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/WarpInventory.java
Show resolved
Hide resolved
There was a problem hiding this comment.
In my opinion this is a very good change, our language system was a bit overengineered and as you described there are special plugins for this. I looked through the code and nothing caught my eye, but I need to be corrected because I haven't done anything for a long time
...e-core/src/main/java/com/eternalcode/core/translation/implementation/TranslationFactory.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
eternalcore-core/src/main/java/com/eternalcode/core/translation/Language.java (2)
28-42: Consider making the language list more flexible.The method works well but hardcodes the language list to
List.of(PL, EN). This limits adding new languages without code changes.Consider storing predefined languages in a static collection or making this more configurable.
+ private static final List<Language> PREDEFINED_LANGUAGES = List.of(PL, EN); + public static Language fromString(String lang) { if (lang == null || lang.isBlank()) { return DEFAULT; } String lower = lang.toLowerCase(); - for (Language language : List.of(PL, EN)) { + for (Language language : PREDEFINED_LANGUAGES) { if (language.lang.equalsIgnoreCase(lower) || language.aliases.contains(lower)) { return language; } } return new Language(lower, List.of()); }
14-14: Note: DEFAULT language has empty string as language code.Using
Locale.ROOTcreates a Language with an empty string as the language code. This might be intentional, but worth documenting if this behavior is expected.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
eternalcore-api/src/main/java/com/eternalcode/core/feature/language/LanguageProvider.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/argument/AbstractViewerArgument.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/configuration/ConfigurationManager.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/configuration/composer/LanguageComposer.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/AbstractTranslation.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/Language.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/Translation.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/TranslationManagerSetup.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/ENTranslation.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/PLTranslation.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/TranslationFactory.java(1 hunks)eternalcore-core/src/test/java/com/eternalcode/core/configuration/language/LanguageComposerTest.java(1 hunks)eternalcore-core/src/test/java/com/eternalcode/core/test/MockViewer.java(0 hunks)
💤 Files with no reviewable changes (4)
- eternalcore-core/src/test/java/com/eternalcode/core/test/MockViewer.java
- eternalcore-core/src/main/java/com/eternalcode/core/translation/AbstractTranslation.java
- eternalcore-core/src/main/java/com/eternalcode/core/translation/Translation.java
- eternalcore-api/src/main/java/com/eternalcode/core/feature/language/LanguageProvider.java
✅ Files skipped from review due to trivial changes (3)
- eternalcore-core/src/test/java/com/eternalcode/core/configuration/language/LanguageComposerTest.java
- eternalcore-core/src/main/java/com/eternalcode/core/configuration/composer/LanguageComposer.java
- eternalcore-core/src/main/java/com/eternalcode/core/configuration/ConfigurationManager.java
🚧 Files skipped from review as they are similar to previous changes (5)
- eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/TranslationFactory.java
- eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/argument/AbstractViewerArgument.java
- eternalcore-core/src/main/java/com/eternalcode/core/translation/TranslationManagerSetup.java
- eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/ENTranslation.java
- eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/PLTranslation.java
🧰 Additional context used
🧠 Learnings (1)
eternalcore-core/src/main/java/com/eternalcode/core/translation/Language.java (1)
Learnt from: vLuckyyy
PR: EternalCodeTeam/EternalCore#856
File: eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/data/WarpDataConfig.java:19-20
Timestamp: 2025-01-12T13:03:07.239Z
Learning: Configuration classes in EternalCore (those implementing ReloadableConfig and annotated with @ConfigurationFile) don't need thread-safe collections as they are managed by the configuration system which handles synchronization.
🔇 Additional comments (2)
eternalcore-core/src/main/java/com/eternalcode/core/translation/Language.java (2)
1-1: Package relocation looks good.Moving from the feature package to translation package makes sense for this refactoring.
24-26: Simple and clean factory method.The
fromLocalemethod correctly extracts the language code and creates a Language instance with no aliases.
| @@ -47,6 +48,12 @@ public class PluginConfiguration implements ReloadableConfig { | |||
| @Description("# Whether the player should receive information about new plugin updates upon joining the server") | |||
| public boolean shouldReceivePluginUpdates = true; | |||
|
|
|||
| @Description({ | |||
| "# Language, currently supported languages: en, pl", | |||
| "# You need restart server after change." | |||
There was a problem hiding this comment.
| "# You need restart server after change." | |
| "# You need to restart the server after changes." |
|
|
||
| this.configurationManager.save(translation); | ||
| } | ||
| AbstractTranslation translation = (AbstractTranslation) this.translationManager.getMessages(); |
There was a problem hiding this comment.
Just to be noted:
After changing the language warp GUI is empty because warps are now saved only in 1 config file. I know we are looking forward to improving warp.yml!!!!
This PR completely refactors the EternalCore language/translation system to be as simple and user-friendly as possible.
All legacy features related to per-user language, GUI selection, client detection, and language databases have been removed.
Key Changes
Removed all per-user language logic:
No more language selection per player, no database, no client detection, no GUI.
Removed all code and config for
language.yml,LanguageService,LanguageProvider, and related classes.TranslationManagernow uses only a single language, set globally inconfig.yml.NoticeServiceand other components now always use the global language.Translation classes (e.g.
ENTranslation,PLTranslation) now use a string language code, not an enum.It is now possible to add any number of custom languages without touching the codebase.
How the new language system works
The language is set globally in
config.yml:The plugin loads the corresponding translation file from
lang/<code>-messages.yml.All users see the same language, regardless of their client or preferences.
To add a new language, just add a new translation file and set the code in config.
No GUI, no per-user settings, no database, no detection—just one config option.
Why?