-
-
Notifications
You must be signed in to change notification settings - Fork 17
GH-930 Add private chat toggle placeholder. #931
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThis update introduces a new way to show the status of private messages using placeholders. A new class is added to handle the setup for these placeholders. When the system starts, it registers two placeholders: one shows if private messages are enabled or disabled as "true" or "false," and the other gives a formatted message, like "Enabled" or "Disabled," in the player's language. The code also adds new fields and methods to handle these messages in both English and Polish. An extra helper class is included to manage the asynchronous fetching and formatting of the placeholder values. Assessment against linked issues
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ 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:
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
eternalcore-core/src/main/java/com/eternalcode/core/placeholder/AsyncPlaceholderValue.java (1)
16-28
: Consider adding JavaDoc comments for better clarity.Adding some documentation would help other developers understand how this class works and when to use it.
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/PrivateChatPlaceholderSetup.java (1)
38-43
: Consider adding error loggingWhile
AsyncPlaceholderValue
handles errors by showing an error message, it would be helpful to also log these errors for debugging.CompletableFuture<PrivateChatState> stateFuture = this.privateChatStateService.getChatState(playerId); +// Consider adding .exceptionally() handler for logging errors
📜 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/privatechat/PrivateChatPlaceholderSetup.java
(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/messages/ENPrivateMessages.java
(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/messages/PLPrivateChatMessages.java
(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/messages/PrivateChatMessages.java
(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/placeholder/AsyncPlaceholderValue.java
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/PrivateChatPlaceholderSetup.java (1)
eternalcore-core/src/main/java/com/eternalcode/core/placeholder/AsyncPlaceholderValue.java (1)
AsyncPlaceholderValue
(6-29)
🔇 Additional comments (9)
eternalcore-core/src/main/java/com/eternalcode/core/placeholder/AsyncPlaceholderValue.java (1)
1-29
: Nice implementation of the placeholder value handler.This class provides a simple way to display placeholder values that need to be loaded asynchronously. It handles loading states, errors, and formatting nicely.
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/messages/PrivateChatMessages.java (1)
28-31
: Good addition of placeholder methods.These methods will make it easy to get localized text for private message toggle status.
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/messages/ENPrivateMessages.java (1)
61-62
: Good implementation with clear color coding.The green/red colors make it easy to understand the status at a glance.
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/messages/PLPrivateChatMessages.java (1)
62-63
: Good Polish translation with matching colors.The translations are appropriate and maintain the same color scheme as the English version.
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/PrivateChatPlaceholderSetup.java (5)
1-19
: Nice job organizing the imports and class structure!The class is properly set up as a controller with the necessary imports. Good use of the dependency injection pattern with the
@Controller
annotation.
21-31
: Constructor looks good!You've correctly injected the services needed for this feature. The code follows the project's dependency injection pattern with the
@Inject
annotation.
33-45
: Good implementation of the basic placeholder!The registration and implementation of the
eternalcore_msgtoggle
placeholder looks clean. Good job usingAsyncPlaceholderValue
to handle the asynchronous state retrieval.
47-61
: Formatted placeholder implementation looks good!This nicely handles both the asynchronous state retrieval and proper translation. Good job making sure it's user-friendly with the formatted output.
36-44
: Check if naming follows conventionsVerify that the placeholder name
eternalcore_msgtoggle
follows the same naming pattern as other placeholders in your system.
UUID playerId = targetPlayer.getUniqueId(); | ||
CompletableFuture<PrivateChatState> stateFuture = this.privateChatStateService.getChatState(playerId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just inline
UUID playerId = targetPlayer.getUniqueId(); | |
CompletableFuture<PrivateChatState> stateFuture = this.privateChatStateService.getChatState(playerId); | |
CompletableFuture<PrivateChatState> stateFuture = this.privateChatStateService.getChatState(targetPlayer.getUniqueId()); |
UUID playerId = targetPlayer.getUniqueId(); | ||
Translation messages = this.translationManager.getMessages(playerId); | ||
CompletableFuture<PrivateChatState> stateFuture = this.privateChatStateService.getChatState(playerId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UUID playerId = targetPlayer.getUniqueId(); | |
Translation messages = this.translationManager.getMessages(playerId); | |
CompletableFuture<PrivateChatState> stateFuture = this.privateChatStateService.getChatState(playerId); | |
UUID playerId = targetPlayer.getUniqueId(); | |
Translation messages = this.translationManager.getMessages(playerId); | |
CompletableFuture<PrivateChatState> stateFuture = this.privateChatStateService.getChatState(playerId); |
} | ||
|
||
@Override | ||
public String toString() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that this name is good. I would suggest change to maybe "getValue" or smth like this.
public String toString() { | |
public String getValue() { |
Fixes: #930