GH-870 Add /msgtoggle command #896
Conversation
WalkthroughThis update introduces a new 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 3
🧹 Nitpick comments (6)
eternalcore-api/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggleService.java (1)
6-12: Add documentation to explain the interface's purpose.Adding some simple comments would help other developers understand what this interface does and how to use it.
public interface MsgToggleService { + /** + * Checks if a player has message toggle enabled + * @param uuid Player's UUID + * @return Future containing true if messages are toggled off + */ CompletableFuture<Boolean> hasMsgToggled(UUID uuid); + /** + * Updates a player's message toggle state + * @param uuid Player's UUID + * @param toggle True to enable message blocking, false to disable + */ void toggleMsg(UUID uuid, boolean toggle); }eternalcore-core/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggleRepository.java (1)
10-10: Make method name clearer.The name
setToggledOffmight be confusing when used withfalse. Consider renaming it tosetToggleStateto better reflect its purpose.- CompletableFuture<Void> setToggledOff(UUID uuid, boolean toggledOff); + CompletableFuture<Void> setToggleState(UUID uuid, boolean enabled);eternalcore-core/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggleServiceImpl.java (1)
26-29: Consider removing @Blocking annotationThe @Blocking annotation might be misleading here since the repository operation could be asynchronous.
eternalcore-core/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggleRepositoryOrmLite.java (1)
31-34: Simplify CompletableFuture chainThe
thenApply(status -> null)can be replaced withthenRun(() -> {})for better readability.- public CompletableFuture<Void> setToggledOff(UUID uuid, boolean toggledOff) { - return this.save(MsgToggleWrapper.class, new MsgToggleWrapper(uuid, toggledOff)) - .thenApply(status -> null); - } + public CompletableFuture<Void> setToggledOff(UUID uuid, boolean toggledOff) { + return this.save(MsgToggleWrapper.class, new MsgToggleWrapper(uuid, toggledOff)) + .thenRun(() -> {}); + }eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/PrivateChatServiceImpl.java (1)
62-78: Improve async operation flowThe nested async operations can be flattened using CompletableFuture's composition methods for better readability.
- this.msgToggleService.hasMsgToggled(uniqueId).thenAccept(hasMsgToggled -> { - if (hasMsgToggled) { - this.noticeService.player(sender.getUniqueId(), translation -> translation.privateChat().msgToggledOff()); - - return; - } - - this.ignoreService.isIgnored(uniqueId, sender.getUniqueId()).thenAccept(isIgnored -> { - if (!isIgnored) { - this.replies.put(uniqueId, sender.getUniqueId()); - this.replies.put(sender.getUniqueId(), uniqueId); - } - - PrivateChatEvent event = new PrivateChatEvent(sender.getUniqueId(), uniqueId, message); - this.eventCaller.callEvent(event); - this.presenter.onPrivate(new PrivateMessage(sender, target, event.getContent(), this.socialSpy, isIgnored)); - }); - }); + this.msgToggleService.hasMsgToggled(uniqueId) + .thenCompose(hasMsgToggled -> { + if (hasMsgToggled) { + this.noticeService.player(sender.getUniqueId(), translation -> translation.privateChat().msgToggledOff()); + return CompletableFuture.completedFuture(null); + } + return this.ignoreService.isIgnored(uniqueId, sender.getUniqueId()) + .thenAccept(isIgnored -> { + if (!isIgnored) { + this.replies.put(uniqueId, sender.getUniqueId()); + this.replies.put(sender.getUniqueId(), uniqueId); + } + PrivateChatEvent event = new PrivateChatEvent(sender.getUniqueId(), uniqueId, message); + this.eventCaller.callEvent(event); + this.presenter.onPrivate(new PrivateMessage(sender, target, event.getContent(), this.socialSpy, isIgnored)); + }); + });eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/ENTranslation.java (1)
499-507: Consider standardizing color usage across messages.While the messages are clear, the color usage could be more consistent:
- Some messages use
<red>disabledwhile others use<green>enabled- The player name highlighting varies between messages
Consider standardizing to:
- public Notice msgToggledOff = Notice.chat("<red>► <dark_red>This player has disabled private messages!"); + public Notice msgToggledOff = Notice.chat("<red>► <white>This player has <red>disabled</red> private messages!"); - public Notice msgToggledSelf = Notice.chat("<green>► <white>Private messages have been <red>disabled<white>!"); + public Notice msgToggledSelf = Notice.chat("<green>► <white>Private messages have been <red>disabled</red>!"); - public Notice msgUntoggleSelf = Notice.chat("<green>► <white>Private messages have been <green>enabled<white>!"); + public Notice msgUntoggleSelf = Notice.chat("<green>► <white>Private messages have been <green>enabled</green>!");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
README.md(1 hunks)eternalcore-api/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggle.java(1 hunks)eternalcore-api/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggleService.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggleCommand.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggleRepository.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggleRepositoryOrmLite.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggleServiceImpl.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggleWrapper.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/PrivateChatServiceImpl.java(4 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/Translation.java(1 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(1 hunks)
🔇 Additional comments (7)
eternalcore-core/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggleServiceImpl.java (1)
20-23: Implementation looks good!Clean and straightforward implementation of the message toggle service.
eternalcore-core/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggleRepositoryOrmLite.java (1)
23-28: Verify default value choiceThe method returns true by default, which means messages are blocked by default. Is this the intended behavior?
eternalcore-core/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggleCommand.java (1)
14-16: Well-structured command implementation!Clear command descriptions and good separation between self and other-player operations.
Also applies to: 32-34, 47-48, 57-59, 69-71
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/PrivateChatServiceImpl.java (1)
41-42: Clean integration of MsgToggleService!The message toggle feature is well integrated with the existing private chat system.
Also applies to: 48-48
README.md (1)
43-43: LGTM! Documentation updated appropriately.The new /msgtoggle command is properly documented in the features list.
eternalcore-core/src/main/java/com/eternalcode/core/translation/Translation.java (1)
263-267: LGTM! Well-structured interface additions.The new message toggle methods follow a consistent naming pattern and clearly indicate their purpose.
eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/PLTranslation.java (1)
508-515: LGTM! Well-structured Polish translations.The Polish translations are properly localized while maintaining consistency with the English version.
eternalcore-api/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggle.java
Outdated
Show resolved
Hide resolved
eternalcore-core/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggleWrapper.java
Outdated
Show resolved
Hide resolved
eternalcore-core/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggleCommand.java
Outdated
Show resolved
Hide resolved
…toggle/MsgToggle.java Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
eternalcore-core/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggleCommand.java
Outdated
Show resolved
Hide resolved
eternalcore-core/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggleWrapper.java
Outdated
Show resolved
Hide resolved
eternalcore-api/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggle.java
Outdated
Show resolved
Hide resolved
eternalcore-api/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggle.java
Outdated
Show resolved
Hide resolved
eternalcore-api/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggle.java
Outdated
Show resolved
Hide resolved
...re-core/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggleRepositoryOrmLite.java
Outdated
Show resolved
Hide resolved
eternalcore-core/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggleWrapper.java
Outdated
Show resolved
Hide resolved
eternalcore-core/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggleWrapper.java
Outdated
Show resolved
Hide resolved
eternalcore-core/src/main/java/com/eternalcode/core/feature/msgtoggle/MsgToggleWrapper.java
Outdated
Show resolved
Hide resolved
...core-core/src/main/java/com/eternalcode/core/feature/privatechat/PrivateChatServiceImpl.java
Outdated
Show resolved
Hide resolved
...pi/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleState.java
Outdated
Show resolved
Hide resolved
.../src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleCommand.java
Outdated
Show resolved
Hide resolved
.../src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleCommand.java
Outdated
Show resolved
Hide resolved
...pi/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleState.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleState.java (2)
3-5: Clarify the Javadoc description.The Javadoc should clearly state what
ENABLEDandDISABLEDmean in terms of message reception.-/** - * Enum representing state of blocking incoming private messages by the player. - */ +/** + * Enum representing whether a player can receive private messages. + */
8-16: Improve state documentation.The Javadoc for each state should be more explicit about its effect on message reception.
- /** - * State that represents that the player can be messaged by other players. - */ - ENABLED, - - /** - * State that represents that the player has disabled incoming private messages. - */ - DISABLED + /** + * Player can receive private messages. + */ + ENABLED, + + /** + * Player cannot receive private messages. + */ + DISABLEDeternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleCommand.java (1)
77-77: Use reference equality for enum comparison.Use
==instead ofequals()for enum comparison.- if (state.equals(PrivateChatToggleState.DISABLED)) { + if (state == PrivateChatToggleState.DISABLED) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleState.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/PrivateChatServiceImpl.java(4 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleCommand.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleRepositoryOrmLite.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/ENTranslation.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleRepositoryOrmLite.java
🧰 Additional context used
📓 Learnings (2)
eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleState.java (2)
Learnt from: CitralFlo
PR: EternalCodeTeam/EternalCore#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.
Learnt from: CitralFlo
PR: EternalCodeTeam/EternalCore#896
File: eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleCommand.java:1-1
Timestamp: 2025-01-30T19:49:57.192Z
Learning: In the EternalCore project's private chat toggle feature, PrivateChatToggleState.ON means messages are blocked, while OFF means messages are allowed.
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleCommand.java (2)
Learnt from: CitralFlo
PR: EternalCodeTeam/EternalCore#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.
Learnt from: CitralFlo
PR: EternalCodeTeam/EternalCore#896
File: eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleCommand.java:1-1
Timestamp: 2025-01-30T19:49:57.192Z
Learning: In the EternalCore project's private chat toggle feature, PrivateChatToggleState.ON means messages are blocked, while OFF means messages are allowed.
🔇 Additional comments (5)
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleCommand.java (1)
39-46: 🛠️ Refactor suggestionUse reference equality for enum comparison and add error handling.
Use
==for enum comparison and handle potential async failures.- privateChatToggleState.thenAccept(toggleState -> { - if (toggleState == PrivateChatToggleState.DISABLED) { - this.enable(uniqueId); - } - else { - this.disable(uniqueId); - } - }); + privateChatToggleState + .thenAccept(toggleState -> { + if (toggleState == PrivateChatToggleState.DISABLED) { + this.enable(uniqueId); + } + else { + this.disable(uniqueId); + } + }) + .exceptionally(throwable -> { + this.noticeService.create() + .notice(translation -> translation.general().error()) + .player(uniqueId) + .send(); + return null; + });Likely invalid or redundant comment.
eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/ENTranslation.java (1)
500-508: LGTM!The messages are clear and provide good feedback to users about their private message settings.
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/PrivateChatServiceImpl.java (3)
5-6: Looks good! Clean import and field additions.The new imports and field for the toggle service are well-organized and properly placed.
Also applies to: 29-29
42-44: Nice work on the constructor updates!The new toggle service is properly injected and initialized.
Also applies to: 49-49
61-80: Let's make the async code more robust!The toggle check works, but we should handle potential errors and simplify the nested operations.
Previous review comments already suggested:
- Adding exception handling for the async operations
- Using
thenComposeto flatten the nested structurePlease refer to the previous review comments for the detailed implementation suggestions.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleState.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/PrivateChatServiceImpl.java(4 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleCommand.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleState.java
🧰 Additional context used
📓 Learnings (1)
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleCommand.java (2)
Learnt from: CitralFlo
PR: EternalCodeTeam/EternalCore#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.
Learnt from: CitralFlo
PR: EternalCodeTeam/EternalCore#896
File: eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleCommand.java:1-1
Timestamp: 2025-01-30T19:49:57.192Z
Learning: In the EternalCore project's private chat toggle feature, PrivateChatToggleState.ON means messages are blocked, while OFF means messages are allowed.
🔇 Additional comments (6)
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/PrivateChatServiceImpl.java (3)
5-6: LGTM!The new imports and field for toggle service are properly organized.
Also applies to: 29-29
42-43: LGTM!Constructor properly initializes the new toggle service.
Also applies to: 49-49
63-80: 🛠️ Refactor suggestionAdd error handling for async operations.
The async operations could fail silently. Add error handling to inform users when operations fail.
this.privateChatToggleService.getPrivateChatToggleState(uniqueId).thenAccept(privateChatToggleState -> { if (privateChatToggleState.equals(PrivateChatToggleState.DISABLED)) { this.noticeService.player(sender.getUniqueId(), translation -> translation.privateChat().receiverDisabledMessages()); return; } this.ignoreService.isIgnored(uniqueId, sender.getUniqueId()).thenAccept(isIgnored -> { // ... existing code ... - }); + }).exceptionally(throwable -> { + this.noticeService.player(sender.getUniqueId(), translation -> translation.general().error()); + return null; + }); - }); + }).exceptionally(throwable -> { + this.noticeService.player(sender.getUniqueId(), translation -> translation.general().error()); + return null; + });Likely invalid or redundant comment.
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleCommand.java (3)
76-101: LGTM!The method is well-structured with clear separation of concerns.
103-126: LGTM!The utility methods are well-implemented with clear responsibilities.
36-49: 🛠️ Refactor suggestionAdd error handling and optimize state comparison.
The async operation needs error handling, and state comparison can be simplified.
if (state == null) { CompletableFuture<PrivateChatToggleState> privateChatToggleState = this.privateChatToggleService.getPrivateChatToggleState(sender.getUniqueId()); privateChatToggleState.thenAccept(toggleState -> { - if (toggleState == PrivateChatToggleState.DISABLED) { + if (toggleState == PrivateChatToggleState.DISABLED) { this.enable(uniqueId); } else { this.disable(uniqueId); } - }); + }).exceptionally(throwable -> { + this.noticeService.create() + .notice(translation -> translation.general().error()) + .sender(sender) + .send(); + return null; + });Likely invalid or redundant comment.
.../src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleCommand.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/PrivateChatServiceImpl.java (1)
42-43: Consider adding null check for the new service.The constructor should validate that the injected service is not null.
PrivateChatServiceImpl( NoticeService noticeService, IgnoreService ignoreService, UserManager userManager, EventCaller eventCaller, PrivateChatToggleService privateChatToggleService ) { + if (privateChatToggleService == null) { + throw new IllegalArgumentException("privateChatToggleService cannot be null"); + } this.privateChatToggleService = privateChatToggleService;Also applies to: 49-49
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleState.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/PrivateChatServiceImpl.java(4 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleCommand.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleRepositoryOrmLite.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleServiceImpl.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleState.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleServiceImpl.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleRepositoryOrmLite.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleCommand.java
🔇 Additional comments (3)
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/PrivateChatServiceImpl.java (3)
5-6: Looks good!Clean additions of necessary imports and field for the toggle service.
Also applies to: 29-29
64-64: Align toggle state naming with PR description.The PR description uses ON/OFF but the code uses DISABLE. Consider using consistent terminology.
Should this be
PrivateChatToggleState.ONinstead ofDISABLE?
63-80: Handle CompletableFuture exceptions and simplify nested async operations.The code could benefit from better error handling and simpler async flow.
Previous review comments already suggested:
- Adding exception handling for CompletableFuture
- Using thenCompose to simplify nested async operations
...ore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggle.java
Outdated
Show resolved
Hide resolved
…State instead of toggle
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateRepository.java (1)
11-11: Improve parameter name clarityThe parameter name
toggledOffdoesn't clearly represent its purpose. Consider renaming it tostateto match its type and usage.- CompletableFuture<Void> setPrivateChatState(UUID uuid, PrivateChatState toggledOff); + CompletableFuture<Void> setPrivateChatState(UUID uuid, PrivateChatState state);eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateWrapper.java (1)
27-29: Rename method to match return typeThe method name
isEnabledsuggests a boolean return but returns PrivateChatState.- PrivateChatState isEnabled() { + PrivateChatState getState() { return this.state; }eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateServiceImpl.java (1)
13-13: Consider adding cache eviction strategyThe cache might grow too large over time. Consider adding size limits or TTL for entries.
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/PrivateChatServiceImpl.java (1)
63-80: Simplify nested async operations.The nested async operations can be flattened using
thenComposefor better readability.- this.privateChatStateService.getPrivateChatState(uniqueId).thenAccept(privateChatState -> { - if (privateChatState == PrivateChatState.DISABLE) { - this.noticeService.player(sender.getUniqueId(), translation -> translation.privateChat().receiverDisabledMessages()); - return; - } - this.ignoreService.isIgnored(uniqueId, sender.getUniqueId()).thenAccept(isIgnored -> { + this.privateChatStateService.getPrivateChatState(uniqueId) + .thenCompose(privateChatState -> { + if (privateChatState == PrivateChatState.DISABLE) { + this.noticeService.player(sender.getUniqueId(), + translation -> translation.privateChat().receiverDisabledMessages()); + return CompletableFuture.completedFuture(null); + } + return this.ignoreService.isIgnored(uniqueId, sender.getUniqueId()) + .thenAccept(isIgnored -> {eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateCommand.java (1)
105-121: Reduce code duplication in notification logic.The enable and disable methods share similar notification logic that can be extracted.
+ private void sendNotification(UUID uniqueId, boolean enabled) { + this.noticeService.create() + .notice(translation -> enabled + ? translation.privateChat().selfMessagesEnabled() + : translation.privateChat().selfMessagesDisabled()) + .player(uniqueId) + .send(); + } + private void enable(UUID uniqueId) { this.privateChatStateService.togglePrivateChat(uniqueId, PrivateChatState.ENABLE); - this.noticeService.create() - .notice(translation -> translation.privateChat().selfMessagesEnabled()) - .player(uniqueId) - .send(); + this.sendNotification(uniqueId, true); } private void disable(UUID uniqueId) { this.privateChatStateService.togglePrivateChat(uniqueId, PrivateChatState.DISABLE); - this.noticeService.create() - .notice(translation -> translation.privateChat().selfMessagesDisabled()) - .player(uniqueId) - .send(); + this.sendNotification(uniqueId, false); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatState.java(1 hunks)eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateService.java(1 hunks)eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggle.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/PrivateChatServiceImpl.java(4 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateCommand.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateRepository.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateRepositoryOrmLite.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateServiceImpl.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateWrapper.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatState.java
🚧 Files skipped from review as they are similar to previous changes (1)
- eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggle.java
🧰 Additional context used
📓 Learnings (5)
eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateService.java (1)
Learnt from: CitralFlo
PR: EternalCodeTeam/EternalCore#896
File: eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleServiceImpl.java:24-27
Timestamp: 2025-01-28T21:35:47.750Z
Learning: In EternalCore, async operations using CompletableFuture should remain non-blocking. Avoid using .join() or similar blocking operations to maintain better performance and scalability.
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateServiceImpl.java (1)
Learnt from: CitralFlo
PR: EternalCodeTeam/EternalCore#896
File: eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleServiceImpl.java:24-27
Timestamp: 2025-01-28T21:35:47.750Z
Learning: In EternalCore, async operations using CompletableFuture should remain non-blocking. Avoid using .join() or similar blocking operations to maintain better performance and scalability.
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateRepositoryOrmLite.java (1)
Learnt from: CitralFlo
PR: EternalCodeTeam/EternalCore#896
File: eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggle.java:10-11
Timestamp: 2025-01-28T21:35:04.888Z
Learning: Fields in classes used with ORM Lite repository must have package-private (default) access modifier, not private, to allow ORM Lite to access them properly.
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateWrapper.java (1)
Learnt from: CitralFlo
PR: EternalCodeTeam/EternalCore#896
File: eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggle.java:10-11
Timestamp: 2025-01-28T21:35:04.888Z
Learning: Fields in classes used with ORM Lite repository must have package-private (default) access modifier, not private, to allow ORM Lite to access them properly.
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateCommand.java (2)
Learnt from: CitralFlo
PR: EternalCodeTeam/EternalCore#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.
Learnt from: CitralFlo
PR: EternalCodeTeam/EternalCore#896
File: eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleCommand.java:1-1
Timestamp: 2025-01-30T19:49:57.192Z
Learning: In the EternalCore project's private chat toggle feature, PrivateChatToggleState.ON means messages are blocked, while OFF means messages are allowed.
...i/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateService.java
Outdated
Show resolved
Hide resolved
...e/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateWrapper.java
Show resolved
Hide resolved
...c/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateServiceImpl.java
Outdated
Show resolved
Hide resolved
.../java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateRepositoryOrmLite.java
Outdated
Show resolved
Hide resolved
.../java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateRepositoryOrmLite.java
Show resolved
Hide resolved
...e/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateCommand.java
Outdated
Show resolved
Hide resolved
# Conflicts: # 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
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/messages/ENPrivateMessages.java (1)
51-51: Fix typo in unIgnorePlayer messageThere's a typo in the word "uningored".
- public Notice unIgnorePlayer = Notice.chat("<red>► <dark_red>{PLAYER} <red>player has been uningored!"); + public Notice unIgnorePlayer = Notice.chat("<red>► <dark_red>{PLAYER} <red>player has been unignored!");eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/messages/PLPrivateChatMessages.java (1)
52-52: Fix spacing in unIgnorePlayer messageThe words "Od ignorowano" should be combined.
- public Notice unIgnorePlayer = Notice.chat("<red>► <dark_red>Od ignorowano gracza <red>{PLAYER}<dark_red>!"); + public Notice unIgnorePlayer = Notice.chat("<red>► <dark_red>Odignorowano gracza <red>{PLAYER}<dark_red>!");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md(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)
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
🔇 Additional comments (3)
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/messages/PrivateChatMessages.java (1)
14-18: Nice work on the new message methods!The new methods are well-organized and their names clearly describe their purpose.
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/messages/ENPrivateMessages.java (1)
27-34: Great job on the new message fields!The messages are clear and follow the existing color scheme consistently.
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/messages/PLPrivateChatMessages.java (1)
32-39: Well-done on the Polish translations!The messages maintain consistent formatting and are properly translated.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/messages/ENPrivateMessages.java (1)
32-34: Small spacing tweak neededThere's an extra space before "for" in both messages. Let's make them more consistent:
- public Notice otherMessagesDisabled = Notice.chat("<green>► <white>Private messages have been <red>disabled <white>for <green>{PLAYER}<white>!"); - public Notice otherMessagesEnabled = Notice.chat("<green>► <white>Private messages have been <green>enabled <white>for <green>{PLAYER}<white>!"); + public Notice otherMessagesDisabled = Notice.chat("<green>► <white>Private messages have been <red>disabled<white> for <green>{PLAYER}<white>!"); + public Notice otherMessagesEnabled = Notice.chat("<green>► <white>Private messages have been <green>enabled<white> for <green>{PLAYER}<white>!");eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateServiceImpl.java (1)
22-29: Cache usage
To further optimize, consider storing the state from the repository in your cache for future lookups.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateService.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/messages/ENPrivateMessages.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateRepositoryOrmLite.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateServiceImpl.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateRepositoryOrmLite.java
🧰 Additional context used
🧠 Learnings (2)
eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateService.java (1)
Learnt from: CitralFlo
PR: EternalCodeTeam/EternalCore#896
File: eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleServiceImpl.java:24-27
Timestamp: 2025-01-28T21:35:47.750Z
Learning: In EternalCore, async operations using CompletableFuture should remain non-blocking. Avoid using .join() or similar blocking operations to maintain better performance and scalability.
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateServiceImpl.java (1)
Learnt from: CitralFlo
PR: EternalCodeTeam/EternalCore#896
File: eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleServiceImpl.java:24-27
Timestamp: 2025-01-28T21:35:47.750Z
Learning: In EternalCore, async operations using CompletableFuture should remain non-blocking. Avoid using .join() or similar blocking operations to maintain better performance and scalability.
🔇 Additional comments (12)
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/messages/ENPrivateMessages.java (2)
27-31: Nice work on the new message templates!The messages are clear and use consistent colors to show different states.
51-51: Good catch on the spelling fixes!The spelling corrections in the unignore messages look good.
Also applies to: 54-54
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateServiceImpl.java (5)
1-8: Looks tidy so far
Package and imports are well-organized. Everything appears good here.
9-10: Service annotation
Neat use of @service, which makes the implementation easy to spot.
12-14: Fields are straightforward
The final fields are nicely named and make the code easy to follow.
15-19: Dependency injection
Your constructor injection is set up cleanly.
31-40: Error rollback
Removing the cached state on failure helps keep data fresh and avoids confusion. Nicely done.eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateService.java (5)
1-5: Async imports
Great to see CompletableFuture encouraged for non-blocking flows.
6-8: Helpful documentation
The quick summary ensures team members know what this service does.
9-16: Interface clarity
The descriptions make it simple to understand each method’s purpose.
17-17: Non-blocking retrieval
Using CompletableFuture for getPrivateChatState aligns with modern async practices.
19-25: Toggle method
Keeping togglePrivateChat async is a consistent and flexible design decision.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateService.java (1)
6-17: Add documentation about default state and null handling.The documentation would be more helpful if it mentioned:
- The default state when a player hasn't set any preference
- How null values are handled
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateServiceImpl.java (1)
13-13: Consider adding cache eviction strategy.The cache might grow indefinitely since there's no eviction policy. Consider using CacheBuilder with expiration:
- private final ConcurrentHashMap<UUID, PrivateChatState> cachedToggleStates; + private final Cache<UUID, PrivateChatState> cachedToggleStates = CacheBuilder.newBuilder() + .expireAfterAccess(Duration.ofHours(1)) + .build();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateService.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/PrivateChatServiceImpl.java(4 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateCommand.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateRepositoryOrmLite.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateServiceImpl.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateWrapper.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateRepositoryOrmLite.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateCommand.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateWrapper.java
🧰 Additional context used
🧠 Learnings (1)
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateServiceImpl.java (1)
Learnt from: CitralFlo
PR: EternalCodeTeam/EternalCore#896
File: eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleServiceImpl.java:24-27
Timestamp: 2025-01-28T21:35:47.750Z
Learning: In EternalCore, async operations using CompletableFuture should remain non-blocking. Avoid using .join() or similar blocking operations to maintain better performance and scalability.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (17)
...c/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateServiceImpl.java
Show resolved
Hide resolved
...core-core/src/main/java/com/eternalcode/core/feature/privatechat/PrivateChatServiceImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateServiceImpl.java (1)
37-42:⚠️ Potential issueFix error handling in setChatState
Returning null on error could cause NullPointerException in the calling code.
return this.msgToggleRepository.setPrivateChatState(playerUniqueId, state) .exceptionally(throwable -> { this.cachedToggleStates.remove(playerUniqueId); - return null; + throw new CompletionException("Failed to set chat state", throwable); });
🧹 Nitpick comments (3)
eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatState.java (1)
18-20: Consider making invert() publicThe invert() method could be useful for external code that needs to toggle states.
- PrivateChatState invert() { + public PrivateChatState invert() {eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateServiceImpl.java (2)
8-8: Remove unused importConcurrentSkipListMap is not used in this class.
-import java.util.concurrent.ConcurrentSkipListMap;
13-14: Consider adding cache evictionThe cache might grow indefinitely. Consider adding a mechanism to remove old entries.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatState.java(1 hunks)eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateService.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateCommand.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateServiceImpl.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateCommand.java
🧰 Additional context used
🧠 Learnings (3)
eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatState.java (2)
Learnt from: CitralFlo
PR: EternalCodeTeam/EternalCore#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.
Learnt from: CitralFlo
PR: EternalCodeTeam/EternalCore#896
File: eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleCommand.java:1-1
Timestamp: 2025-01-30T19:49:57.192Z
Learning: In the EternalCore project's private chat toggle feature, PrivateChatToggleState.ON means messages are blocked, while OFF means messages are allowed.
eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateService.java (1)
Learnt from: CitralFlo
PR: EternalCodeTeam/EternalCore#896
File: eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleServiceImpl.java:24-27
Timestamp: 2025-01-28T21:35:47.750Z
Learning: In EternalCore, async operations using CompletableFuture should remain non-blocking. Avoid using .join() or similar blocking operations to maintain better performance and scalability.
eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateServiceImpl.java (1)
Learnt from: CitralFlo
PR: EternalCodeTeam/EternalCore#896
File: eternalcore-core/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatToggleServiceImpl.java:24-27
Timestamp: 2025-01-28T21:35:47.750Z
Learning: In EternalCore, async operations using CompletableFuture should remain non-blocking. Avoid using .join() or similar blocking operations to maintain better performance and scalability.
🔇 Additional comments (1)
eternalcore-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatStateService.java (1)
9-35: Well-designed async interface!The interface is well structured with clear documentation and proper async patterns using CompletableFuture.
...core-api/src/main/java/com/eternalcode/core/feature/privatechat/toggle/PrivateChatState.java
Show resolved
Hide resolved
To clarify:
ON - true - means that the player has turned on msgToggle and has blocked getting private messages
OFF - false - means that the player has turned off msgToggle and has allowed getting private messages