GH-997 Add elder guardian command along with appropriate translations #1061
Conversation
WalkthroughThis update adds a new "troll" section to the translation system for Elder Guardian messages in English and Polish, including interfaces and message classes. A new command allows showing the Elder Guardian overlay to players, with an optional silent mode. The old Estimated code review effort3 (~45 minutes) 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ 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 (
|
📦 Development Build ReadyWarning Do not use this build in production. It is for testing purposes only and may be unstable. |
Jakubk15
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: 0
🧹 Nitpick comments (1)
eternalcore-core/src/main/java/com/eternalcode/core/translation/Translation.java (1)
24-24: Check if this import is needed.The EntityDamageEvent import appears unrelated to the elder guardian troll feature. Verify this import is actually needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
eternalcore-core/src/main/java/com/eternalcode/core/feature/troll/ElderGuardianCommand.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/Translation.java(4 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/ENTranslation.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/PLTranslation.java(2 hunks)eternalcore-paper/src/main/java/com/eternalcode/containers/AdditionalContainerPaper.java(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
eternalcore-core/src/main/java/com/eternalcode/core/translation/Translation.java (1)
Learnt from: CitralFlo
PR: #896
File: eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleCommand.java:92-102
Timestamp: 2025-01-28T21:37:36.945Z
Learning: In the EternalCore plugin's private chat feature, PrivateChatToggleState.ON means message blocking is enabled (no messages will be received), while PrivateChatToggleState.OFF means messages are allowed.
eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/PLTranslation.java (1)
Learnt from: CitralFlo
PR: #896
File: eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleCommand.java:92-102
Timestamp: 2025-01-28T21:37:36.945Z
Learning: In the EternalCore plugin's private chat feature, PrivateChatToggleState.ON means message blocking is enabled (no messages will be received), while PrivateChatToggleState.OFF means messages are allowed.
🧬 Code Graph Analysis (1)
eternalcore-core/src/main/java/com/eternalcode/core/feature/troll/ElderGuardianCommand.java (1)
eternalcore-paper/src/main/java/com/eternalcode/containers/AdditionalContainerPaper.java (1)
AdditionalContainerPaper(11-43)
🔇 Additional comments (8)
eternalcore-paper/src/main/java/com/eternalcode/containers/AdditionalContainerPaper.java (1)
20-21: LGTM! Clean container implementation.The new elder guardian containers follow the established pattern and correctly use the boolean parameter to control silent mode.
eternalcore-core/src/main/java/com/eternalcode/core/translation/Translation.java (2)
194-197: Good interface design for the troll section.The TrollSection interface is well-structured and follows the established patterns used by other message sections in the translation system.
254-254: Consistent method addition.The troll() method follows the same pattern as other section accessors in the interface.
eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/ENTranslation.java (1)
566-575: Well-implemented English translation section.The ENTrollSection provides clear, properly formatted messages that follow the established styling patterns. Good use of placeholders and distinction between normal and silent modes.
eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/PLTranslation.java (1)
590-599: Good Polish translation implementation.The PLTrollSection provides appropriate Polish translations while maintaining consistency with the English implementation. The distinction between normal and silent modes ("po cichu") is clear.
eternalcore-core/src/main/java/com/eternalcode/core/feature/troll/ElderGuardianCommand.java (3)
16-22: Well-structured command definition.Good use of annotations with proper permission setup and clear documentation. The command name and alias are intuitive.
24-29: Clean dependency injection setup.Proper constructor injection pattern for the NoticeService dependency.
31-52: Solid command execution logic.The implementation correctly handles both silent and normal modes, uses the appropriate containers, and provides clear feedback to the command sender with proper placeholders.
|
|
There was a problem hiding this comment.
The command is also incomplete regarding version support. The @Compatibility annotation (which I mentioned twice) is missing, which specifies the version range in which the command should be enabled. The methods Player#showElderGuardian and Player#showElderGuardian(boolean) are available since version 1.19.2.
Sources: https://jd.papermc.io/paper/1.19.2/org/bukkit/entity/Player.html#showElderGuardian(boolean)
https://jd.papermc.io/paper/1.18.2/
eternalcore-core/src/main/java/com/eternalcode/core/feature/troll/ElderGuardianCommand.java
Outdated
Show resolved
Hide resolved
eternalcore-core/src/main/java/com/eternalcode/core/feature/troll/ElderGuardianCommand.java
Show resolved
Hide resolved
...alcore-core/src/main/java/com/eternalcode/core/translation/implementation/ENTranslation.java
Show resolved
Hide resolved
...alcore-core/src/main/java/com/eternalcode/core/translation/implementation/ENTranslation.java
Outdated
Show resolved
Hide resolved
eternalcore-paper/src/main/java/com/eternalcode/containers/AdditionalContainerPaper.java
Outdated
Show resolved
Hide resolved
CitralFlo
left a comment
CitralFlo
left a comment
There was a problem hiding this comment.
Nice!
I am wondering is there a better way to name the package instead of troll.
I know that in Essential it's fun like commands for fun.
Also consider moving catboy command to this package.
@Rollczi is /catboy a fun or troll command :P
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
eternalcore-paper/src/main/java/com/eternalcode/paper/PaperFeature.java (1)
11-11: Consider removing unused generic parameter.The generic type
Tisn't used meaningfully in the class. You could simplify this to justPaperFeaturewithout the generic.-public abstract class PaperFeature<T> { +public abstract class PaperFeature {eternalcore-paper/src/main/java/com/eternalcode/paper/PaperContainer.java (1)
18-21: Simplify the PaperFeature instantiation.You don't need the anonymous class since you're not adding any additional logic.
- this.feature = new PaperFeature<>(action, name) { - }; + this.feature = new PaperFeature<>(action, name);Note: This will require making PaperFeature a concrete class instead of abstract, or creating a simple concrete implementation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
eternalcore-core/src/main/java/com/eternalcode/core/feature/container/AnvilCommand.java(3 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/container/CartographyTableCommand.java(3 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/container/GrindstoneCommand.java(3 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/container/LoomCommand.java(3 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/container/SmithingTableCommand.java(3 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/container/StonecutterCommand.java(3 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/troll/elderguardian/ElderGuardianCommand.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/troll/elderguardian/messages/ENElderGuardianMessages.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/troll/elderguardian/messages/ElderGuardianMessages.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/troll/elderguardian/messages/PLElderGuardianMessages.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/Translation.java(4 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-paper/src/main/java/com/eternalcode/containers/AdditionalContainerPaper.java(0 hunks)eternalcore-paper/src/main/java/com/eternalcode/paper/PaperContainer.java(1 hunks)eternalcore-paper/src/main/java/com/eternalcode/paper/PaperFeature.java(1 hunks)eternalcore-paper/src/main/java/com/eternalcode/paper/PaperOverlay.java(1 hunks)
💤 Files with no reviewable changes (1)
- eternalcore-paper/src/main/java/com/eternalcode/containers/AdditionalContainerPaper.java
✅ Files skipped from review due to trivial changes (11)
- eternalcore-core/src/main/java/com/eternalcode/core/feature/container/GrindstoneCommand.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/container/StonecutterCommand.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/container/LoomCommand.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/container/CartographyTableCommand.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/container/SmithingTableCommand.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/container/AnvilCommand.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/troll/elderguardian/messages/ElderGuardianMessages.java
- eternalcore-paper/src/main/java/com/eternalcode/paper/PaperOverlay.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/troll/elderguardian/messages/PLElderGuardianMessages.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/troll/elderguardian/messages/ENElderGuardianMessages.java
- eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/ENTranslation.java
🚧 Files skipped from review as they are similar to previous changes (2)
- eternalcore-core/src/main/java/com/eternalcode/core/translation/Translation.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/feature/troll/elderguardian/ElderGuardianCommand.java (1)
Learnt from: vLuckyyy
PR: #865
File: eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/item/give/GiveService.java:61-72
Timestamp: 2024-10-14T20:14:03.215Z
Learning: In the 'EternalCore' project, input validation for command parameters like amount is performed by the command library, so methods such as giveItems do not need to add additional validation for these parameters.
🪛 GitHub Actions: Java CI with Gradle
eternalcore-core/src/main/java/com/eternalcode/core/feature/troll/elderguardian/ElderGuardianCommand.java
[error] 8-8: Compilation failed: package com.eternalcode.overlays does not exist. Import statement 'import com.eternalcode.overlays.AdditionalOverlayPaper;' cannot be resolved.
🔇 Additional comments (6)
eternalcore-core/src/main/java/com/eternalcode/core/feature/troll/elderguardian/ElderGuardianCommand.java (2)
17-27: Good command structure and dependency injection.The command follows proper patterns with appropriate annotations, permission checks, and dependency injection.
29-49: Clean command execution logic with proper translations.The execute method handles both silent and regular modes correctly, with proper translation integration and placeholder usage.
eternalcore-paper/src/main/java/com/eternalcode/paper/PaperFeature.java (2)
13-14: Good use of static environment detection.Smart to detect the environment once and reuse it for all feature checks.
24-33: Excellent error handling and environment validation.The method properly checks for Paper compatibility and provides clear feedback to both players and administrators when features aren't supported.
eternalcore-paper/src/main/java/com/eternalcode/paper/PaperContainer.java (2)
7-15: Well-organized enum with all major container types.Good coverage of container types with descriptive names and proper lambda usage.
23-25: Clean delegation pattern.Simple and effective delegation to the underlying PaperFeature.
...ore/src/main/java/com/eternalcode/core/feature/troll/elderguardian/ElderGuardianCommand.java
Outdated
Show resolved
Hide resolved
...ore/src/main/java/com/eternalcode/core/feature/troll/elderguardian/ElderGuardianCommand.java
Outdated
Show resolved
Hide resolved
Will do in another PR probably |
This pull request introduces a new "Elder Guardian" trolling feature, allowing players to show an Elder Guardian effect to others, either visibly or silently. It includes changes to implement the command, integrate it into the translation system, and add support for the feature in the container utilities.
New Feature: Elder Guardian Command
ElderGuardianCommandclass to handle the new/elderguardiancommand, including support for a silent flag (-s) and permission checks (eternalcore.troll.elderguardian). The command usesAdditionalContainerPaperto trigger the Elder Guardian effect and sends notices to the player. (ElderGuardianCommand.java)Translation System Updates
TrollSectioninterface to theTranslationsystem, with methods for elder guardian notices (elderGuardianShownandelderGuardianShownSilently). (Translation.java) [1] [2]TrollSectionin the English (ENTranslation.java) and Polish (PLTranslation.java) translation files, with appropriate messages for both languages. (ENTranslation.java,PLTranslation.java) [1] [2]Container Utility Enhancements
ELDER_GUARDIANandELDER_GUARDIAN_SILENTcontainers toAdditionalContainerPaper, enabling the Elder Guardian effect with or without visibility. (AdditionalContainerPaper.java)Minor Code Cleanups
Translation.javaandPLTranslation.javafor consistency. (Translation.java,PLTranslation.java) [1] [2]