GH-579 Add repository for language. Remove user settings. #890
Conversation
WalkthroughThe pull request introduces a comprehensive refactoring of the language management system in the EternalCore API. The changes primarily focus on decoupling language-related functionality from user settings and introducing a more flexible, UUID-centric approach to language handling. Key modifications include the introduction of new interfaces like These changes represent a significant architectural shift towards a more modular and flexible language management approach in the application. Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
Rollczi
changed the title
Rollczi
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: 13
🔭 Outside diff range comments (3)
eternalcore-core/src/main/java/com/eternalcode/core/user/UserClientBukkitSettings.java (1)
Line range hint
11-21: Would you mind adding some class documentation?A brief comment explaining the class's purpose and its role in the new UUID-based system would help future developers understand the code better.
+/** + * Manages Bukkit player settings using UUID-based identification. + * Uses WeakReference to handle player instances efficiently. + */ class UserClientBukkitSettings implements UserClientSettings {eternalcore-core/src/main/java/com/eternalcode/core/feature/home/command/HomeArgument.java (1)
Line range hint
53-54: Small suggestion to simplify the homes string collection.You can make this a bit simpler by directly using the home name:
- .map(home -> home.getName()) + .map(Home::getName)eternalcore-core/src/main/java/com/eternalcode/core/viewer/BukkitViewerImpl.java (1)
Line range hint
44-46: Improve error message for unknown playerAdd the player's UUID to the error message to help with debugging.
- throw new IllegalStateException("Unknown player name"); + throw new IllegalStateException("Unknown player name for UUID: " + this.uuid);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (48)
eternalcore-api/src/main/java/com/eternalcode/core/feature/language/Language.java(3 hunks)eternalcore-api/src/main/java/com/eternalcode/core/feature/language/LanguageProvider.java(1 hunks)eternalcore-api/src/main/java/com/eternalcode/core/feature/language/LanguageService.java(1 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/argument/EnchantmentArgument.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/argument/GameModeArgument.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/argument/NoticeTypeArgument.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/argument/PlayerArgument.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/argument/UserArgument.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/AftPlaceholderSetup.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/container/DisposalCommand.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/item/enchant/EnchantArgument.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/item/give/GiveArgument.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/item/lore/ItemLoreArgument.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/mob/ButcherArgument.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/mob/MobEntityArgument.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/speed/SpeedArgument.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/speed/SpeedTypeArgument.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/time/TimeArgument.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/fullserverbypass/FullServerBypassController.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/home/HomePlaceholderSetup.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/home/command/HomeArgument.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/language/BukkitLanguageProvider.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageCommand.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageInventory.java(6 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageLoadController.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageRepository.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageRepositoryImpl.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageServiceImpl.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageSettings.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/language/config/LanguageConfiguration.java(3 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/RequesterArgument.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/WarpInventory.java(5 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/command/WarpArgument.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/command/WarpCommand.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/notice/NoticeService.java(5 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/TranslationManager.java(3 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/TranslationManagerSetup.java(3 hunks)eternalcore-core/src/main/java/com/eternalcode/core/user/User.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/user/UserClientBukkitSettings.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/user/UserClientNoneSettings.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/user/UserClientSettings.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/user/UserSettings.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/user/UserSettingsImpl.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/viewer/BukkitViewerImpl.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/viewer/BukkitViewerProvider.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/viewer/Viewer.java(0 hunks)
💤 Files with no reviewable changes (7)
- eternalcore-core/src/main/java/com/eternalcode/core/user/UserClientNoneSettings.java
- eternalcore-core/src/main/java/com/eternalcode/core/viewer/Viewer.java
- eternalcore-core/src/main/java/com/eternalcode/core/user/UserSettings.java
- eternalcore-core/src/main/java/com/eternalcode/core/user/UserClientSettings.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageSettings.java
- eternalcore-core/src/main/java/com/eternalcode/core/user/User.java
- eternalcore-core/src/main/java/com/eternalcode/core/user/UserSettingsImpl.java
🔇 Additional comments (51)
eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/argument/GameModeArgument.java (1)
25-26: Nice cleanup! 👍Removing the ViewerService dependency makes the code simpler and easier to maintain. This change fits well with the overall goal of cleaning up user settings.
eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/RequesterArgument.java (1)
29-30: Looks good! Nice cleanup of unused dependency.The removal of ViewerService makes the code simpler while keeping all the necessary functionality intact. 👍
eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/item/give/GiveArgument.java (1)
24-25: Nice cleanup!The constructor has been simplified by removing the unused ViewerService dependency. This change aligns well with the PR's goal of cleaning up user settings.
eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/item/enchant/EnchantArgument.java (1)
24-25: Nice cleanup! 👍The simplified constructor looks good - it's part of the larger effort to streamline how language features work.
eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/command/WarpArgument.java (1)
34-34: Nice cleanup! 👍The removal of ViewerService simplifies the code and aligns well with the project's goal of decoupling language management from user settings.
eternalcore-core/src/main/java/com/eternalcode/core/feature/afk/AftPlaceholderSetup.java (2)
22-24: Nice cleanup of the constructor!The simplified constructor makes the code cleaner and easier to understand.
34-36: Good switch to direct UUID usage!The code now gets translations in a simpler way using the player's UUID directly. Clean and effective!
eternalcore-core/src/main/java/com/eternalcode/core/user/UserClientBukkitSettings.java (1)
23-26: Nice work on the new isOnline() method!The implementation is clean and makes good use of the existing getPlayer() method.
eternalcore-core/src/main/java/com/eternalcode/core/feature/home/command/HomeArgument.java (2)
29-29: Nice addition of the ViewerService field!The new field follows good practices by being private and final.
38-38: Constructor changes look good!The initialization order is clear and all fields are properly set.
Also applies to: 41-41
eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/item/lore/ItemLoreArgument.java (2)
24-25: Nice simplification of the constructor!The removal of ViewerService dependency makes the code cleaner and easier to maintain.
24-25: 🧹 Nitpick (assertive)Consider making the suggestions list configurable
Instead of having a fixed list of suggestions (0-5), you could make this configurable through TranslationManager or a separate configuration.
- private static final List<Integer> suggestions = List.of(0, 1, 2, 3, 4, 5); + private final List<Integer> suggestions; + + @Inject + public ItemLoreArgument(TranslationManager translationManager) { + super(translationManager); + this.suggestions = translationManager.getConfiguration().getItemLoreSuggestions(); + }Likely invalid or redundant comment.
eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/argument/UserArgument.java (1)
25-26: Looks good - constructor simplified!The removal of ViewerService aligns well with the move towards UUID-based language handling. The constructor is now cleaner and has fewer dependencies.
eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/container/DisposalCommand.java (1)
28-33: Looking good! Nice cleanup of the constructor.The removal of UserManager makes the code simpler and more focused.
eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/argument/NoticeTypeArgument.java (1)
21-22: Nice cleanup! 👍Removing the unused ViewerService makes the code simpler and cleaner.
Let's check if AbstractViewerArgument was updated similarly:
✅ Verification successful
Changes look good! 👍
The parent class
AbstractViewerArgumentalso uses onlyTranslationManager, so everything is consistent.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if AbstractViewerArgument also had ViewerService removed ast-grep --pattern 'class AbstractViewerArgument { $$$ AbstractViewerArgument($$$) { $$$ } $$$ }'Length of output: 111
Script:
#!/bin/bash # Find and examine AbstractViewerArgument fd -e java | xargs rg "class AbstractViewerArgument" -A 10Length of output: 1728
eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/mob/MobEntityArgument.java (2)
25-26: Nice cleanup! 👍The constructor simplification looks good - removing unused dependencies makes the code cleaner and easier to maintain.
25-26: Double-check parent class usageLet's make sure the parent class
AbstractViewerArgumentworks correctly without ViewerService.✅ Verification successful
ViewerService removal is safe ✅
The parent class only needs TranslationManager to work properly, so removing ViewerService from the constructor is the right thing to do.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if AbstractViewerArgument uses ViewerService in any way ast-grep --pattern 'class AbstractViewerArgument { $$$ viewerService $$$ }' # Check for any direct references to ViewerService in the class rg "ViewerService" "eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/argument/AbstractViewerArgument.java"Length of output: 210
Script:
#!/bin/bash # Check if the file exists and show its content cat "eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/argument/AbstractViewerArgument.java" # Find all files that might contain AbstractViewerArgument as a backup fd "AbstractViewerArgument.java"Length of output: 1727
eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/speed/SpeedTypeArgument.java (2)
21-22: Nice cleanup! 👍The constructor is now simpler and focuses only on what it needs - the translation manager.
Line range hint
1-40: Double-check the parent class usageJust to be safe, could you verify that AbstractViewerArgument still works as expected with just the TranslationManager?
✅ Verification successful
Everything looks good with AbstractViewerArgument usage 👍
The parent class is designed to work with just the TranslationManager parameter, so your implementation is spot-on.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check the AbstractViewerArgument constructor requirements ast-grep --pattern 'class AbstractViewerArgument { $$$ AbstractViewerArgument($$$) { $$$ } $$$ }'Length of output: 111
Script:
#!/bin/bash # Find AbstractViewerArgument class file rg -l "class AbstractViewerArgument" # After finding the file, let's see its content rg -A 10 "class AbstractViewerArgument"Length of output: 1856
eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/argument/PlayerArgument.java (1)
31-31: LGTM! Simple and clean dependency removal.The removal of
ViewerServicealigns well with the PR's goal of removing user settings. The change looks straightforward and maintains the core functionality.Let's verify the parent class changes:
✅ Verification successful
All good! Parent class matches the changes perfectly
The parent class
AbstractViewerArgumentis already set up to work with just theTranslationManager, so your changes inPlayerArgumentare spot-on.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if AbstractViewerArgument has been updated to handle the removal of ViewerService ast-grep --pattern 'class AbstractViewerArgument { $$$ AbstractViewerArgument($$$) { $$$ } $$$ }'Length of output: 111
Script:
#!/bin/bash # Find AbstractViewerArgument file fd AbstractViewerArgument # Once found, examine the file content rg "class AbstractViewerArgument" -A 10 # Try modified ast-grep pattern with generic type ast-grep --pattern 'class AbstractViewerArgument<$_> { $$$ AbstractViewerArgument($$$) { $$$ } $$$ }'Length of output: 1961
eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/speed/SpeedArgument.java (1)
23-24: Nice cleanup! 👍The constructor is now simpler and more focused. This matches the overall goal of removing user settings.
Let's make sure this change didn't break anything:
✅ Verification successful
All good! The changes look safe 👍
The SpeedArgument is only used in annotations and is properly managed by dependency injection. No issues found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if any files still expect ViewerService in SpeedArgument constructor rg -l "new SpeedArgument.*ViewerService"Length of output: 42
Script:
#!/bin/bash # Find any SpeedArgument instantiation or reference rg "SpeedArgument" -B 2 -A 2 # Check for any dependency injection configurations fd ".*\.xml|.*Config.*\.java|.*Module.*\.java" --exec rg -l "SpeedArgument" # Look at the parent class to understand the constructor pattern ast-grep --pattern 'class AbstractViewerArgument'Length of output: 4632
eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/time/TimeArgument.java (1)
24-25: Nice cleanup! 👍The simplified constructor looks good - keeping just what's needed makes the code cleaner.
Let's double-check this change fits with other files:
✅ Verification successful
Looks good! The ViewerService removal is spot-on here 👍
The change is safe since TimeArgument wasn't using ViewerService functionality. Other files still have ViewerService because they actually need it for their operations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if other similar argument classes have been updated consistently ast-grep --pattern 'class $_ extends AbstractViewerArgument { $$$ @Inject public $_($_, ViewerService viewerService, $_) { $$$ } $$$ }'Length of output: 151
Script:
#!/bin/bash # Find AbstractViewerArgument class and its implementations echo "=== Finding AbstractViewerArgument class and its implementations ===" rg "class.*extends.*AbstractViewerArgument" -l echo -e "\n=== Checking for ViewerService usage in argument files ===" rg "ViewerService" --type java echo -e "\n=== Looking for AbstractViewerArgument definition ===" rg "class AbstractViewerArgument" --type javaLength of output: 6837
eternalcore-core/src/main/java/com/eternalcode/core/translation/TranslationManagerSetup.java (3)
4-4: LGTM! Clean import addition.
16-16: Clean method signature update!The addition of LanguageService parameter makes sense for the new language management approach.
25-25: Looks good! Let's verify the TranslationManager constructor.The updated instantiation looks correct, but let's make sure the TranslationManager class has the matching constructor.
✅ Verification successful
Constructor verified and matches perfectly! ✨
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if TranslationManager has the correct constructor ast-grep --pattern 'class TranslationManager { $$$ TranslationManager(LanguageService $_, Translation $_) { $$$ } $$$ }'Length of output: 134
Script:
#!/bin/bash # First, find the TranslationManager file fd TranslationManager.java # Then, let's look at its content fd TranslationManager.java --exec cat {}Length of output: 2701
eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/command/WarpCommand.java (2)
47-47: Nice simplification of the method signature!Removing the User parameter makes the code cleaner while maintaining functionality.
69-69: Let's double-check the language handlingThe change looks good, but let's make sure WarpInventory correctly handles the player's language.
✅ Verification successful
Language handling looks great!
The code properly handles player languages, including fallback to default language if needed. All text elements (titles, items, lore) are correctly translated based on the player's language settings.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if WarpInventory properly uses LanguageService ast-grep --pattern 'class WarpInventory { $$$ private final LanguageService languageService; $$$ openInventory(Player $_) { $$$ } }'Length of output: 148
Script:
#!/bin/bash # Find WarpInventory class file fd -e java "WarpInventory.java" # Search for WarpInventory class implementation ast-grep --pattern 'class WarpInventory { $$$ }'Length of output: 168
Script:
#!/bin/bash # Check WarpInventory implementation cat eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/WarpInventory.javaLength of output: 10680
eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/mob/ButcherArgument.java (2)
28-28: LGTM! Field is properly defined.The new ViewerService field follows good practices with proper access modifiers.
31-35: Check for affected constructor calls.The constructor parameter order has changed. Let's make sure all existing code is updated.
Good job simplifying the parent class dependency!
The changes to the super constructor call help decouple language management, which matches the PR's goals.
✅ Verification successful
All good! No manual constructor calls to update.
The class is managed by dependency injection, so we don't need to worry about updating constructor calls.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for constructor calls to verify they've been updated rg -l "new ButcherArgument\(" | xargs rg "new ButcherArgument\("Length of output: 66
Script:
#!/bin/bash # Search for any reference to ButcherArgument class rg "ButcherArgument" # Also search for potential Spring/DI configuration files fd -e xml -e java | grep -i "config\|bean" | xargs rg "ButcherArgument"Length of output: 1065
eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/argument/EnchantmentArgument.java (1)
23-24: Nice cleanup! 👍The constructor is now simpler and more focused, keeping only what it needs.
eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/argument/AbstractViewerArgument.java (2)
17-19: Constructor update looks goodSimplifying the constructor by removing the
ViewerServiceparameter makes the code cleaner.
23-28: Nice conditional translation handlingGood job checking if the sender is a player to retrieve the correct translation, and defaulting to
Language.DEFAULTwhen necessary.eternalcore-core/src/main/java/com/eternalcode/core/feature/afk/AfkKickController.java (1)
63-63: Looking good! Simple and clean change.The switch to using UUID for translation retrieval aligns perfectly with the new approach.
eternalcore-core/src/main/java/com/eternalcode/core/feature/fullserverbypass/FullServerBypassController.java (2)
50-50: Nice method rename!The new name 'getServerFullMessage' is clearer than 'extractServerFullMessage'.
69-69: Good consistency!Using UUID for translation retrieval matches the pattern used in other files.
eternalcore-core/src/main/java/com/eternalcode/core/viewer/BukkitViewerProvider.java (1)
63-63: Clean up: Removed redundant language parameterGood job removing the Language.DEFAULT parameter from all viewer creation calls. This simplifies the code and aligns with the new language management approach.
Also applies to: 68-68, 84-84
eternalcore-core/src/main/java/com/eternalcode/core/notice/NoticeService.java (2)
4-5: Good separation of concerns!Adding LanguageService as a dedicated dependency improves code organization.
Also applies to: 43-43, 54-54, 64-64
90-96: Check potential blocking operationThe
getLanguageNowmethod might be blocking. Consider using an async approach if possible.Run this script to check other usages of getLanguageNow:
eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/WarpInventory.java (1)
41-41: Nice addition of LanguageService!The dependency is properly injected through the constructor.
Also applies to: 52-52, 61-61
eternalcore-core/src/main/java/com/eternalcode/core/feature/language/config/LanguageConfiguration.java (1)
70-71: Looking good!Simple formatting improvements that make the code cleaner.
Also applies to: 80-81, 90-91
eternalcore-api/src/main/java/com/eternalcode/core/feature/language/LanguageService.java (1)
9-23: Good use of UUIDs and Asynchronous MethodsSwitching to
UUIDfor player identification and usingCompletableFuturefor asynchronous operations enhance flexibility and efficiency. Nice work!eternalcore-api/src/main/java/com/eternalcode/core/feature/language/Language.java (4)
17-21: Using Set for AliasesChanging
aliasesfrom aListto aSethelps prevent duplicates and improves performance. Good job!
28-29: Returning Unmodifiable SetProviding an unmodifiable set of aliases ensures the collection can't be modified externally. Nice touch!
37-47: Simplified Equality LogicThe updates in the
isEqualsmethod make the equality checks more efficient. Well done!
77-77: Modern Locale CreationUsing
Locale.of(this.lang)aligns with modern practices for creating locales. Looks great!eternalcore-core/src/main/java/com/eternalcode/core/translation/TranslationManager.java (3)
25-31: Updated Constructor with LanguageServiceAdding
LanguageServiceto the constructor centralizes language handling. This simplifies the code and improves maintainability.
39-42: Simplified Message RetrievalThe new
getMessages(UUID uniqueId)method streamlines message retrieval using UUIDs. Nicely done!
74-74: Immutable Language SetReturning an unmodifiable set of available languages ensures the integrity of the collection. Good practice!
eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageServiceImpl.java (1)
5-79: Great job on the refactoring!The implementation enhances language management with efficient caching and asynchronous operations.
eternalcore-api/src/main/java/com/eternalcode/core/feature/language/LanguageProvider.java (1)
1-9: Interface looks good!The
LanguageProviderinterface is clean and straightforward.eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageRepository.java (1)
1-15: Well-defined repository interface.The
LanguageRepositorymethods provide clear asynchronous operations for language management.
| UserArgument(TranslationManager translationManager, Server server, UserManager userManager) { | ||
| super(translationManager); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding @NotNull annotations
Since these parameters are required for the class to function, it might be helpful to mark them as non-null.
- UserArgument(TranslationManager translationManager, Server server, UserManager userManager) {
+ UserArgument(@NotNull TranslationManager translationManager, @NotNull Server server, @NotNull UserManager userManager) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| UserArgument(TranslationManager translationManager, Server server, UserManager userManager) { | |
| super(translationManager); | |
| UserArgument(@NotNull TranslationManager translationManager, @NotNull Server server, @NotNull UserManager userManager) { | |
| super(translationManager); |
| @@ -69,7 +69,7 @@ private String ownedHomes(Player targetPlayer) { | |||
|
|
|||
| User user = this.userManager.getOrCreate(targetPlayer.getUniqueId(), targetPlayer.getName()); | |||
|
|
|||
| Translation translation = this.translationManager.getMessages(user); | |||
| Translation translation = this.translationManager.getMessages(user.getUniqueId()); | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Nice update to use UUID for translation lookup!
The change looks good and matches the new UUID-based approach. Since we already have the UUID from targetPlayer.getUniqueId(), we could skip creating the User object if it's only needed for the UUID.
Here's a simpler version:
- User user = this.userManager.getOrCreate(targetPlayer.getUniqueId(), targetPlayer.getName());
- Translation translation = this.translationManager.getMessages(user.getUniqueId());
+ Translation translation = this.translationManager.getMessages(targetPlayer.getUniqueId());| .orElse(Language.DEFAULT); | ||
|
|
||
| Translation translation = this.translationManager.getMessages(language); | ||
| Translation translation = this.translationManager.getMessages(player.getUniqueId()); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding a fallback for translation retrieval.
The direct UUID approach looks good! You might want to add a simple null check to handle cases where translations aren't found, falling back to a default language.
- Translation translation = this.translationManager.getMessages(player.getUniqueId());
+ Translation translation = this.translationManager.getMessages(player.getUniqueId());
+ if (translation == null) {
+ translation = this.translationManager.getDefaultMessages();
+ }Committable suggestion skipped: line range outside the PR's diff.
| void execute(@Context Player player) { | ||
| this.languageInventory.open(player); |
There was a problem hiding this comment.
💡 Codebase verification
Hey there! You've got an unused import to clean up
The User import on line 3 isn't needed anymore since we're using Player directly now.
🔗 Analysis chain
Nice simplification of the command!
The change aligns well with moving away from user settings to a UUID-based approach.
Let's make sure all related components are updated:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if any remaining references to User exist in language-related files
rg -l "import.*User" $(fd -t f "Language.*java$")
Length of output: 1463
Script:
#!/bin/bash
# Check actual usage of User in LanguageCommand.java
rg "User" eternalcore-core/src/main/java/com/eternalcode/core/feature/language/LanguageCommand.java
Length of output: 138
| EnchantmentArgument(TranslationManager translationManager) { | ||
| super(translationManager); |
There was a problem hiding this comment.
💡 Codebase verification
ViewerService removal needs to be consistent across all argument classes
These files still use ViewerService and need similar updates:
eternalcore-core/src/main/java/com/eternalcode/core/feature/home/command/HomeArgument.javaeternalcore-core/src/main/java/com/eternalcode/core/feature/home/homeadmin/PlayerHomeMultiArgument.javaeternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/mob/ButcherArgument.java
🔗 Analysis chain
Let's double-check the ViewerService removal
The removal of ViewerService looks good here, but let's make sure it's consistent across related files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if any remaining ViewerService references exist in similar argument classes
rg -l "ViewerService" --type java | grep -i "argument.java"
# Look for any remaining constructor patterns with ViewerService
ast-grep --pattern 'class $_ extends AbstractViewerArgument {
$$$
$_($_ viewerService, $$$) {
$$$
}
}'
Length of output: 463
Script:
#!/bin/bash
# Check the content of files that mention ViewerService
for file in eternalcore-core/src/main/java/com/eternalcode/core/feature/home/command/HomeArgument.java \
eternalcore-core/src/main/java/com/eternalcode/core/feature/home/homeadmin/PlayerHomeMultiArgument.java \
eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/mob/ButcherArgument.java; do
echo "=== $file ==="
rg -A 5 "ViewerService" "$file"
done
# Also check if these files extend AbstractViewerArgument
ast-grep --pattern 'class $_ extends AbstractViewerArgument {
$$$
}'
Length of output: 4056
| catch (InvocationTargetException | NoSuchMethodException | IllegalAccessException exception) { | ||
| return Locale.of(player.getLocale()); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider handling specific exceptions separately
Instead of catching all exceptions together, handle each type separately for better error tracking.
- catch (InvocationTargetException | NoSuchMethodException | IllegalAccessException exception) {
+ catch (NoSuchMethodException e) {
+ return Locale.of(player.getLocale());
+ }
+ catch (InvocationTargetException | IllegalAccessException e) {
return Locale.of(player.getLocale());
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| catch (InvocationTargetException | NoSuchMethodException | IllegalAccessException exception) { | |
| return Locale.of(player.getLocale()); | |
| } | |
| catch (NoSuchMethodException e) { | |
| return Locale.of(player.getLocale()); | |
| } | |
| catch (InvocationTargetException | IllegalAccessException e) { | |
| return Locale.of(player.getLocale()); | |
| } |
| @DatabaseTable(tableName = "eternal_core_languages") | ||
| private static class LanguageTable { |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider making LanguageTable package-private
Since this is an internal implementation detail, consider removing the private modifier to make it package-private.
@DatabaseTable(tableName = "eternal_core_languages")
- private static class LanguageTable {
+ static class LanguageTable {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @DatabaseTable(tableName = "eternal_core_languages") | |
| private static class LanguageTable { | |
| @DatabaseTable(tableName = "eternal_core_languages") | |
| static class LanguageTable { |
| @Override | ||
| public CompletableFuture<Void> saveLanguage(UUID player, Language language) { | ||
| return save(LanguageTable.class, new LanguageTable(player, language)) | ||
| .thenApply(status -> null); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add null checks for parameters
Add validation for null parameters to prevent potential issues.
public CompletableFuture<Void> saveLanguage(UUID player, Language language) {
+ if (player == null || language == null) {
+ return CompletableFuture.failedFuture(new IllegalArgumentException("Player and language cannot be null"));
+ }
return save(LanguageTable.class, new LanguageTable(player, language))
.thenApply(status -> null);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Override | |
| public CompletableFuture<Void> saveLanguage(UUID player, Language language) { | |
| return save(LanguageTable.class, new LanguageTable(player, language)) | |
| .thenApply(status -> null); | |
| } | |
| @Override | |
| public CompletableFuture<Void> saveLanguage(UUID player, Language language) { | |
| if (player == null || language == null) { | |
| return CompletableFuture.failedFuture(new IllegalArgumentException("Player and language cannot be null")); | |
| } | |
| return save(LanguageTable.class, new LanguageTable(player, language)) | |
| .thenApply(status -> null); | |
| } |
| public void openInventory(Player player) { | ||
| this.languageService.getLanguage(player.getUniqueId()).whenComplete((language, throwable) -> { | ||
| if (language == null) { | ||
| language = Language.DEFAULT; | ||
| } | ||
|
|
||
| this.createInventory(player, language) | ||
| .open(player); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Let's make this code a bit simpler!
The completion handler can be more concise.
- public void openInventory(Player player) {
- this.languageService.getLanguage(player.getUniqueId()).whenComplete((language, throwable) -> {
- if (language == null) {
- language = Language.DEFAULT;
- }
-
- this.createInventory(player, language)
- .open(player);
- });
+ public void openInventory(Player player) {
+ this.languageService.getLanguage(player.getUniqueId())
+ .thenApply(lang -> lang != null ? lang : Language.DEFAULT)
+ .thenAccept(lang -> this.createInventory(player, lang).open(player));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public void openInventory(Player player) { | |
| this.languageService.getLanguage(player.getUniqueId()).whenComplete((language, throwable) -> { | |
| if (language == null) { | |
| language = Language.DEFAULT; | |
| } | |
| this.createInventory(player, language) | |
| .open(player); | |
| }); | |
| public void openInventory(Player player) { | |
| this.languageService.getLanguage(player.getUniqueId()) | |
| .thenApply(lang -> lang != null ? lang : Language.DEFAULT) | |
| .thenAccept(lang -> this.createInventory(player, lang).open(player)); | |
| } |
| CompletableFuture<Void> unloadLanguage(UUID uniqueId) { | ||
| return languageRepository.findLanguage(uniqueId) | ||
| .thenAccept(language -> cachedLanguages.remove(uniqueId)); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Simplify the unloadLanguage method.
Since you're removing the language from the cache, you might not need to query the languageRepository first.
Consider updating the method:
- CompletableFuture<Void> unloadLanguage(UUID uniqueId) {
- return languageRepository.findLanguage(uniqueId)
- .thenAccept(language -> cachedLanguages.remove(uniqueId));
- }
+ CompletableFuture<Void> unloadLanguage(UUID uniqueId) {
+ cachedLanguages.remove(uniqueId);
+ return CompletableFuture.completedFuture(null);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| CompletableFuture<Void> unloadLanguage(UUID uniqueId) { | |
| return languageRepository.findLanguage(uniqueId) | |
| .thenAccept(language -> cachedLanguages.remove(uniqueId)); | |
| } | |
| CompletableFuture<Void> unloadLanguage(UUID uniqueId) { | |
| cachedLanguages.remove(uniqueId); | |
| return CompletableFuture.completedFuture(null); | |
| } |
Resolve: #579