GH-875 Add server capacity command. #899
Conversation
WalkthroughThis change introduces a new command for adjusting server capacity. The pull request adds a new method to the translation interface and corresponding fields in different language implementations to support messages for slot changes. It also updates the reflection utility with functions for dynamic method invocation. In the setslot feature package, new classes are added: a command handler to process slot commands, a service to update the server capacity using reflection, and a saver to persist the new capacity value into the server.properties file. Additionally, message classes for different locales are provided for notifications related to slot setting. Assessment against linked issues
Tip 🌐 Web search-backed reviews and chat
✨ 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 (
|
![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
🧹 Nitpick comments (5)
eternalcore-core/src/main/java/com/eternalcode/core/feature/servercapacity/messages/PLServerCapacityMessages.java (2)
8-11: Add class documentation.Consider adding a brief Javadoc comment to explain the purpose of this Polish message provider.
+/** + * Polish implementation of server capacity messages. + */ @Getter @Accessors(fluent = true) @Contextual public class PLServerCapacityMessages implements ServerCapacityMessages {
12-12: Consider making the field private and moving the message to a resource file.A few suggestions to improve this:
- Make the field private to better encapsulate it
- Consider moving the message text to a resource bundle for easier maintenance
- public Notice slotSaved = Notice.chat("<green>► <white>Sloty serwera zostały ustawione na <green>{SLOTS} <white>i zapisane!"); + private final Notice slotSaved = Notice.chat("<green>► <white>Sloty serwera zostały ustawione na <green>{SLOTS} <white>i zapisane!");eternalcore-core/src/main/java/com/eternalcode/core/util/ReflectUtil.java (1)
100-107: Consider enhancing getDeclaredMethod to support methods with parameters.The method currently only works for no-arg methods. This might be too limiting for general use.
Here's a simple way to make it more flexible:
- public static Method getDeclaredMethod(Class<?> clazz, String methodName) { + public static Method getDeclaredMethod(Class<?> clazz, String methodName, Class<?>... parameterTypes) { try { - return clazz.getDeclaredMethod(methodName); + return clazz.getDeclaredMethod(methodName, parameterTypes); } catch (NoSuchMethodException exception) { throw new IllegalArgumentException("Method " + methodName + " not found in class " + clazz.getName(), exception); } }eternalcore-core/src/main/java/com/eternalcode/core/feature/servercapacity/ServerCapacityService.java (1)
24-33: Set capacity logic is direct.Consider logging when the capacity is changed to help diagnose issues quickly.
eternalcore-core/src/main/java/com/eternalcode/core/feature/servercapacity/ServerCapacitySaver.java (1)
18-24: Consider a more descriptive constant name.The constant name could be more specific to better indicate its purpose.
- private static final String MAX_PLAYERS_PROPERTY = "max-players"; + private static final String SERVER_MAX_PLAYERS_PROPERTY = "max-players";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
eternalcore-core/src/main/java/com/eternalcode/core/feature/servercapacity/ServerCapacityCommand.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/servercapacity/ServerCapacitySaver.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/servercapacity/ServerCapacityService.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/servercapacity/messages/ENServerCapacityMessages.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/servercapacity/messages/PLServerCapacityMessages.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/servercapacity/messages/ServerCapacityMessages.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/Translation.java(2 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/util/ReflectUtil.java(2 hunks)
🔇 Additional comments (14)
eternalcore-core/src/main/java/com/eternalcode/core/util/ReflectUtil.java (1)
109-116: Add security checks before method invocation.Unrestricted reflection access could pose security risks. Consider adding checks for the calling context.
Let's check if this utility is used safely:
eternalcore-core/src/main/java/com/eternalcode/core/feature/servercapacity/ServerCapacityService.java (6)
11-12: Clean class definition.This service annotation and class naming look consistent and clear.
14-16: Good job initializing fields.The final fields keep the class thread-safe.
18-22: Constructor usage is straightforward.Injecting the saver here keeps things nicely organized.
35-41: Reflection call is concise.This method neatly encapsulates the reflection to get the player list object.
42-60: Clever field detection.Relying on the current max player check can be risky if multiple int fields share the same value. You might want a stricter approach or naming check.
62-69: Setting the new value.Exception handling is appropriate here. Looks good overall.
eternalcore-core/src/main/java/com/eternalcode/core/feature/servercapacity/messages/ServerCapacityMessages.java (1)
5-7: Interface is well-defined.A simple contract for capacity messages keeps it flexible.
eternalcore-core/src/main/java/com/eternalcode/core/feature/servercapacity/messages/ENServerCapacityMessages.java (1)
1-13: Clear and friendly messaging.Fluent accessors and a contextual approach keep this easy to understand.
eternalcore-core/src/main/java/com/eternalcode/core/feature/servercapacity/ServerCapacityCommand.java (2)
14-28: Looking good! Clean command setup.The command structure follows best practices with proper dependency injection and clear permission control.
34-40: Nice notification implementation!The code follows the builder pattern consistently and handles placeholders well.
eternalcore-core/src/main/java/com/eternalcode/core/translation/Translation.java (1)
449-450: Clean interface addition!The new method follows the existing pattern and is well-placed in the interface.
eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/ENTranslation.java (1)
849-851: Perfect translation implementation!The server capacity messages are properly integrated with clear documentation.
eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/PLTranslation.java (1)
873-875: Looks good!The new server capacity messages section is properly integrated into the translations.
...re-core/src/main/java/com/eternalcode/core/feature/servercapacity/ServerCapacityCommand.java
Outdated
Show resolved
Hide resolved
...core-core/src/main/java/com/eternalcode/core/feature/servercapacity/ServerCapacitySaver.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/feature/servercapacity/ServerCapacityCommand.java (2)
14-16: Consider making the permission more specific.The current permission 'eternalcore.servercapacity' could be more descriptive. Consider using 'eternalcore.servercapacity.set' to better reflect that this command modifies server settings.
30-48: Add an upper limit check for server slots.Thanks for adding the validation for negative numbers! To prevent potential server issues, it would be good to add an upper limit too (e.g., 1000 slots).
public void execute(@Context Viewer viewer, @Arg int slots) { if (slots <= 0) { this.noticeService.create() .notice(notice -> notice.argument().numberBiggerThanOrEqualZero()) .viewer(viewer) .send(); return; } + if (slots > 1000) { + this.noticeService.create() + .notice(notice -> notice.argument().numberTooBig()) + .viewer(viewer) + .send(); + return; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
eternalcore-core/src/main/java/com/eternalcode/core/feature/servercapacity/ServerCapacityCommand.java(1 hunks)
🔇 Additional comments (2)
eternalcore-core/src/main/java/com/eternalcode/core/feature/servercapacity/ServerCapacityCommand.java (2)
1-13: Looking good! All the right imports are here.The imports are well-organized and include everything needed for the command to work.
18-28: Nice work with dependency injection!The constructor and fields are well-structured, using final fields and proper dependency injection.
...re-core/src/main/java/com/eternalcode/core/feature/servercapacity/ServerCapacityService.java
Outdated
Show resolved
Hide resolved
...re-core/src/main/java/com/eternalcode/core/feature/servercapacity/ServerCapacityCommand.java
Outdated
Show resolved
Hide resolved
|
🐒 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
eternalcore-core/src/main/java/com/eternalcode/core/feature/setslot/messages/ENSetSlotMessages.java (1)
12-12: Consider making the field privateThe field could be private since we have @Getter already.
- public Notice slotSaved = Notice.chat("<green>► <white>Server slots have been set to <green>{SLOTS} <white>and saved!"); + private Notice slotSaved = Notice.chat("<green>► <white>Server slots have been set to <green>{SLOTS} <white>and saved!");eternalcore-core/src/main/java/com/eternalcode/core/feature/setslot/messages/PLSetSlotMessages.java (1)
12-12: Consider making the field privateJust like in ENSetSlotMessages, the field could be private since @Getter is used.
- public Notice slotSaved = Notice.chat("<green>► <white>Sloty serwera zostały ustawione na <green>{SLOTS} <white>i zapisane!"); + private Notice slotSaved = Notice.chat("<green>► <white>Sloty serwera zostały ustawione na <green>{SLOTS} <white>i zapisane!");eternalcore-core/src/main/java/com/eternalcode/core/feature/setslot/SetSlotCommand.java (1)
30-48: Consider adding a constant for the minimum slot value.Instead of using the magic number 0 directly, it would be clearer to define it as a constant.
Here's a simple way to improve it:
+ private static final int MIN_SLOTS = 0; + @Execute @Async @DescriptionDocs(description = "Set the max players on the server") public void execute(@Context Viewer viewer, @Arg int slots) { - if (slots <= 0) { + if (slots <= MIN_SLOTS) {eternalcore-core/src/main/java/com/eternalcode/core/feature/setslot/SetSlotService.java (1)
35-40: Potential method rename risk
The reflect call may fail if “getHandle” is changed in future server updates.eternalcore-core/src/main/java/com/eternalcode/core/feature/setslot/SetSlotSaver.java (1)
27-27: Consider making the properties file path configurableUsing a hardcoded path might cause issues if the server is run from a different directory.
- File propertiesFile = new File("server.properties"); + File propertiesFile = new File(server.getWorldContainer(), "server.properties");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
eternalcore-core/src/main/java/com/eternalcode/core/feature/setslot/SetSlotCommand.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/setslot/SetSlotSaver.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/setslot/SetSlotService.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/setslot/messages/ENSetSlotMessages.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/setslot/messages/PLSetSlotMessages.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/setslot/messages/SetSlotMessages.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/Translation.java(2 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)
🚧 Files skipped from review as they are similar to previous changes (3)
- eternalcore-core/src/main/java/com/eternalcode/core/translation/Translation.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 comments (10)
eternalcore-core/src/main/java/com/eternalcode/core/feature/setslot/messages/SetSlotMessages.java (1)
5-7: Nice and clean interface!The interface is simple and focused, which is exactly what we want.
eternalcore-core/src/main/java/com/eternalcode/core/feature/setslot/SetSlotCommand.java (3)
1-13: Looking good! All imports are in order.Everything is properly organized and includes just what we need.
18-28: Nice work with dependency injection!The constructor is clean and follows best practices.
14-16: Let's double-check the permission naming.The permission "eternalcore.setslot" looks good, but let's make sure it follows the project's standard pattern.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Check other permission patterns in the codebase rg "@Permission\(" -A 1Length of output: 30360
Permission naming verified
The permission "eternalcore.setslot" adheres to the same naming pattern found across the project. Comparing it with other commands (like "eternalcore.afk", "eternalcore.kill", and "eternalcore.spawn") confirms that it matches the established convention. No changes are needed.
eternalcore-core/src/main/java/com/eternalcode/core/feature/setslot/SetSlotService.java (5)
1-10: Neat import structure
This is clear and organized. Good work.
11-23: Class and dependency injection look solid
The service annotation and constructor injection are straightforward and tidy.
24-33: Reflection in setCapacity
Watch out for concurrency if multiple calls try to update this at once.
42-60: Field discovery by default value
This is clever, but be aware if server defaults shift, it might not locate the right field.
62-69: Exception handling is well-done
Errors will be clear and won’t go unnoticed.eternalcore-core/src/main/java/com/eternalcode/core/feature/setslot/SetSlotSaver.java (1)
1-24: Nice work on the class setup!The class is well-organized with proper dependency injection and clear naming.
eternalcore-core/src/main/java/com/eternalcode/core/feature/setslot/SetSlotSaver.java
Show resolved
Hide resolved
Fixes #875