Skip to content
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

GH-870 Add /msgtoggle command #896

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ Get the latest development builds from our [GitHub Actions](https://github.com/E
- Chat On/Off Switch
- Chat Slow Mode
- /ignore and /unignore (with -all option)
- /msg, /socialspy, and /reply commands
- /msg, /msgtoggle, /socialspy, and /reply commands
- /helpop command
- Advanced Notification System allowing you to customize every message to your liking (Title, Subtitle, Actionbar, Chat, etc.)
- :hammer: Open Utility Blocks with simple commands like `/workbench`
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package com.eternalcode.core.feature.privatechat.toggle;

import java.util.UUID;

/**
* Represents a player's private chat toggle state.
*/
public class PrivateChatToggle {

UUID uuid;
PrivateChatToggleState state;
CitralFlo marked this conversation as resolved.
Show resolved Hide resolved

public PrivateChatToggle(UUID uuid, PrivateChatToggleState toggle) {
this.uuid = uuid;
this.state = toggle;
}

public UUID getUuid() {
return uuid;
}

public PrivateChatToggleState getState() {
return state;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package com.eternalcode.core.feature.privatechat.toggle;

import java.util.UUID;
import java.util.concurrent.CompletableFuture;

/**
* This Service manages the state of player's private chat messages blocking
*/
public interface PrivateChatToggleService {

/**
* Checks status of player's private chat messages blocking.
*
* @param uuid player's UUID.
* @return state of player's private chat messages blocking.
*/
CompletableFuture<PrivateChatToggleState> getPrivateChatToggleState(UUID uuid);

/**
* Sets blocking of incoming private messages.
*
* @param uuid player's UUID.
* @param toggle desired state of player's private chat messages blocking.
*/
void togglePrivateChat(UUID uuid, PrivateChatToggleState toggle);

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package com.eternalcode.core.feature.privatechat.toggle;

/**
* Enum representing state of blocking incoming private messages by the player.
*/
public enum PrivateChatToggleState {

/**
* State that represents that the player has blocked incoming private messages.
*/
ON,

/**
* State that represents that the player has not blocked incoming private messages.
*/
OFF
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import com.eternalcode.core.event.EventCaller;
import com.eternalcode.core.feature.ignore.IgnoreService;
import com.eternalcode.core.feature.privatechat.toggle.PrivateChatToggleService;
import com.eternalcode.core.feature.privatechat.toggle.PrivateChatToggleState;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.injector.annotations.component.Service;
import com.eternalcode.core.notice.NoticeService;
Expand All @@ -24,6 +26,7 @@ class PrivateChatServiceImpl implements PrivateChatService {
private final UserManager userManager;
private final PrivateChatPresenter presenter;
private final EventCaller eventCaller;
private final PrivateChatToggleService privateChatToggleService;

private final Cache<UUID, UUID> replies = CacheBuilder.newBuilder()
.expireAfterWrite(Duration.ofHours(1))
Expand All @@ -36,12 +39,14 @@ class PrivateChatServiceImpl implements PrivateChatService {
NoticeService noticeService,
IgnoreService ignoreService,
UserManager userManager,
EventCaller eventCaller
EventCaller eventCaller,
PrivateChatToggleService privateChatToggleService
) {
this.noticeService = noticeService;
this.ignoreService = ignoreService;
this.userManager = userManager;
this.eventCaller = eventCaller;
this.privateChatToggleService = privateChatToggleService;

this.presenter = new PrivateChatPresenter(noticeService);
}
Expand All @@ -53,15 +58,25 @@ void privateMessage(User sender, User target, String message) {
return;
}

this.ignoreService.isIgnored(target.getUniqueId(), sender.getUniqueId()).thenAccept(isIgnored -> {
if (!isIgnored) {
this.replies.put(target.getUniqueId(), sender.getUniqueId());
this.replies.put(sender.getUniqueId(), target.getUniqueId());
UUID uniqueId = target.getUniqueId();

this.privateChatToggleService.getPrivateChatToggleState(uniqueId).thenAccept(privateChatToggleState -> {
if (privateChatToggleState.equals(PrivateChatToggleState.ON)) {
CitralFlo marked this conversation as resolved.
Show resolved Hide resolved
this.noticeService.player(sender.getUniqueId(), translation -> translation.privateChat().receiverDisabledMessages());

return;
}

PrivateChatEvent event = new PrivateChatEvent(sender.getUniqueId(), target.getUniqueId(), message);
this.eventCaller.callEvent(event);
this.presenter.onPrivate(new PrivateMessage(sender, target, event.getContent(), this.socialSpy, isIgnored));
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));
Comment on lines +63 to +78
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trzeba jeszcze przemyśleć trochę to api serwisu - czy jest sens robić 2 osobne serwisy (dla toggle) i czy da się te rzeczy zrobić ładniej bo teraz robi sam się trochę śmietnik w PrivateChatService

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tbh I don't understand. Please elaborate

});
vLuckyyy marked this conversation as resolved.
Show resolved Hide resolved
});
}

Expand Down
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe smth more like this:

@Command(name = "msgtoggle")
@Permission("eternalcore.msgtoggle")
public class PrivateChatToggleCommand {

    private final PrivateChatToggleService privateChatToggleService;
    private final NoticeService noticeService;

    @Inject
    public PrivateChatToggleCommand(PrivateChatToggleService privateChatToggleService, NoticeService noticeService) {
        this.privateChatToggleService = privateChatToggleService;
        this.noticeService = noticeService;
    }

    @Execute
    @DescriptionDocs(description = "Toggle receiving private messages on and off")
    public void execute(@Context Player player, @OptionalArg PrivateChatToggleState state) {
        UUID uniqueId = player.getUniqueId();

        if (state == null) {
            CompletableFuture<PrivateChatToggleState> futureState = this.privateChatToggleService.getPrivateChatToggleState(uniqueId);

            futureState.thenAccept(currentState -> {
                if (currentState == PrivateChatToggleState.OFF) {
                    this.toggleOn(uniqueId);
                    return;
                }
                this.toggleOff(uniqueId);
            });

            return;
        }

        if (state == PrivateChatToggleState.OFF) {
            this.toggleOff(uniqueId);
            return;
        }

        this.toggleOn(uniqueId);
    }

    @Execute
    @Permission("eternalcore.msgtoggle.other")
    @DescriptionDocs(description = "Toggle private messages for another player", arguments = "<player> <toggle>")
    public void other(@Context CommandSender sender, @Arg("player") Player target, @OptionalArg PrivateChatToggleState state) {
        UUID targetUuid = target.getUniqueId();

        if (state == null) {
            CompletableFuture<PrivateChatToggleState> futureState = this.privateChatToggleService.getPrivateChatToggleState(targetUuid);

            futureState.thenAccept(currentState -> {
                PrivateChatToggleState newState = currentState == PrivateChatToggleState.OFF ? PrivateChatToggleState.ON : PrivateChatToggleState.OFF;
                this.setTargetState(sender, target, newState);
            });

            return;
        }

        this.setTargetState(sender, target, state);
    }

    private void setTargetState(CommandSender sender, Player target, @NotNull PrivateChatToggleState state) {
        UUID targetUuid = target.getUniqueId();

        if (state == PrivateChatToggleState.OFF) {
            this.toggleOff(targetUuid);
        } else {
            this.toggleOn(targetUuid);
        }
        boolean isDifferentSenderAndTarget = !(sender instanceof Player) || !((Player) sender).getUniqueId().equals(target.getUniqueId());

        if (isDifferentSenderAndTarget) {
            String messageKey = state == PrivateChatToggleState.OFF
                ? translation -> translation.privateChat().otherMessagesDisabled()
                : translation -> translation.privateChat().otherMessagesEnabled();

            this.noticeService.create()
                .notice(messageKey)
                .placeholder("{PLAYER}", target.getName())
                .send(sender);
        }
    }

    private void toggleOn(UUID uniqueId) {
        this.privateChatToggleService.togglePrivateChat(uniqueId, PrivateChatToggleState.ON);

        this.noticeService.create()
            .notice(translation -> translation.privateChat().selfMessagesEnabled())
            .player(uniqueId)
            .send();
    }

    private void toggleOff(UUID uniqueId) {
        this.privateChatToggleService.togglePrivateChat(uniqueId, PrivateChatToggleState.OFF);

        this.noticeService.create()
            .notice(translation -> translation.privateChat().selfMessagesDisabled())
            .player(uniqueId)
            .send();
    }
}

I think the overall command flow can be simplified

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai please explain the difference between my class and vLuckyyy's class

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CitralFlo Here are the key differences between the two implementations:

  1. Naming: The suggested version uses more descriptive parameter names (player/sender/target instead of context/player).

  2. Code Organization: The suggested version:

    • Simplifies the code with early returns
    • Combines message handling into a single setTargetState method
    • Uses more concise conditions with ternary operators
  3. Potential Issue: There seems to be a logic reversal in the suggested version:

    • Your version: ON means messages are blocked, OFF means messages are allowed
    • Suggested version: ON enables messages, OFF disables messages
      This reversal might cause confusion with the rest of the codebase.

The suggested version offers better code organization, but I'd recommend keeping your toggle logic (ON=blocked, OFF=allowed) to maintain consistency with the existing implementation.


✏️ Learnings added
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.021Z
Learning: In the EternalCore project's private chat toggle feature, PrivateChatToggleState.ON means messages are blocked, while OFF means messages are allowed.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
package com.eternalcode.core.feature.privatechat.toggle;

import com.eternalcode.annotations.scan.command.DescriptionDocs;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.notice.NoticeService;
import dev.rollczi.litecommands.annotations.argument.Arg;
import dev.rollczi.litecommands.annotations.command.Command;
import dev.rollczi.litecommands.annotations.context.Context;
import dev.rollczi.litecommands.annotations.execute.Execute;
import dev.rollczi.litecommands.annotations.optional.OptionalArg;
import dev.rollczi.litecommands.annotations.permission.Permission;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import org.bukkit.command.CommandSender;
import org.bukkit.entity.Player;
import org.jetbrains.annotations.NotNull;

@Command(name = "msgtoggle")
@Permission("eternalcore.msgtoggle")
public class PrivateChatToggleCommand {

private final PrivateChatToggleService privateChatToggleService;
private final NoticeService noticeService;

@Inject
public PrivateChatToggleCommand(PrivateChatToggleService privateChatToggleService, NoticeService noticeService) {
this.privateChatToggleService = privateChatToggleService;
this.noticeService = noticeService;
}

@Execute
@DescriptionDocs(description = "Toggle receiving private messages on and off")
public void execute(@Context Player context, @OptionalArg PrivateChatToggleState state) {
UUID uniqueId = context.getUniqueId();

if (state == null) {
CompletableFuture<PrivateChatToggleState> privateChatToggleState = this.privateChatToggleService.getPrivateChatToggleState(context.getUniqueId());

privateChatToggleState.thenAccept(toggleState -> {
if (toggleState.equals(PrivateChatToggleState.OFF)) {
this.toggleOn(uniqueId);
}
else {
this.toggleOff(uniqueId);
}
});

return;
}
CitralFlo marked this conversation as resolved.
Show resolved Hide resolved

if (state.equals(PrivateChatToggleState.OFF)) {
this.toggleOn(uniqueId);
}
else {
this.toggleOff(uniqueId);
}
}

@Execute
@Permission("eternalcore.msgtoggle.other")
@DescriptionDocs(description = "Toggle private messages for other player", arguments = "<player> <toggle>")
public void other(@Context CommandSender context, @Arg("player") Player player, @OptionalArg PrivateChatToggleState state) {

UUID uniqueId = player.getUniqueId();

if (state == null) {
CompletableFuture<PrivateChatToggleState> privateChatToggleState = this.privateChatToggleService.getPrivateChatToggleState(uniqueId);
privateChatToggleState.thenAccept(toggleState -> handleToggle(context, player, toggleState, uniqueId));

return;
}

handleToggle(context, player, state, uniqueId);
}

private void handleToggle(CommandSender context, Player player, @NotNull PrivateChatToggleState state, UUID uniqueId) {
if (state.equals(PrivateChatToggleState.OFF)) {
this.toggleOn(uniqueId);

if (this.isCommandSenderSameAsTarget(context, player)) {
this.noticeService.create()
.notice(translation -> translation.privateChat().otherMessagesDisabled())
.sender(context)
.placeholder("{PLAYER}", player.getName())
.send();
}
}
CitralFlo marked this conversation as resolved.
Show resolved Hide resolved
else {
this.toggleOff(uniqueId);

if (this.isCommandSenderSameAsTarget(context, player)) {
this.noticeService.create()
.notice(translation -> translation.privateChat().otherMessagesEnabled())
.sender(context)
.placeholder("{PLAYER}", player.getName())
.send();
}
}
}

private void toggleOn(UUID uniqueId) {
this.privateChatToggleService.togglePrivateChat(uniqueId, PrivateChatToggleState.ON);

this.noticeService.create()
.notice(translation -> translation.privateChat().selfMessagesDisabled())
.player(uniqueId)
.send();
}

private void toggleOff(UUID uniqueId) {
this.privateChatToggleService.togglePrivateChat(uniqueId, PrivateChatToggleState.OFF);

this.noticeService.create()
.notice(translation -> translation.privateChat().selfMessagesEnabled())
.player(uniqueId)
.send();
}

private boolean isCommandSenderSameAsTarget(CommandSender context, Player player) {
if (context instanceof Player commandSender) {
return commandSender.getUniqueId().equals(player.getUniqueId());
}
return false;
}
Comment on lines +119 to +124
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not nedded

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is connected to logic of sending messages. When CommandSender and target are the same. Player receives two messages instead of one. example: ./msgtoggle CitralFlo sent by CitralFlo will result in two messages.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i mean not nedded as external method, because it's used one time only

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.eternalcode.core.feature.privatechat.toggle;


import java.util.UUID;
import java.util.concurrent.CompletableFuture;

interface PrivateChatToggleRepository {

CompletableFuture<PrivateChatToggleState> getPrivateChatToggleState(UUID uuid);

CompletableFuture<Void> setPrivateChatToggle(UUID uuid, PrivateChatToggleState toggledOff);

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package com.eternalcode.core.feature.privatechat.toggle;

import com.eternalcode.commons.scheduler.Scheduler;
import com.eternalcode.core.database.DatabaseManager;
import com.eternalcode.core.database.wrapper.AbstractRepositoryOrmLite;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.injector.annotations.component.Repository;
import com.j256.ormlite.table.TableUtils;
import java.sql.SQLException;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;

@Repository
class PrivateChatToggleRepositoryOrmLite extends AbstractRepositoryOrmLite implements PrivateChatToggleRepository {

@Inject
private PrivateChatToggleRepositoryOrmLite(DatabaseManager databaseManager, Scheduler scheduler) throws SQLException {
super(databaseManager, scheduler);
TableUtils.createTableIfNotExists(databaseManager.connectionSource(), PrivateChatToggleWrapper.class);
}
CitralFlo marked this conversation as resolved.
Show resolved Hide resolved
CitralFlo marked this conversation as resolved.
Show resolved Hide resolved

@Override
public CompletableFuture<PrivateChatToggleState> getPrivateChatToggleState(UUID uuid) {
return this.selectSafe(PrivateChatToggleWrapper.class, uuid)
.thenApply(
optional -> optional.map(PrivateChatToggleWrapper::isEnabled).orElse(PrivateChatToggleState.OFF)
);
}

@Override
public CompletableFuture<Void> setPrivateChatToggle(UUID uuid, PrivateChatToggleState toggledOff) {
return this.save(PrivateChatToggleWrapper.class, new PrivateChatToggleWrapper(uuid, toggledOff))
.thenApply(status -> null);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package com.eternalcode.core.feature.privatechat.toggle;

import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.injector.annotations.component.Service;
import java.util.HashMap;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;

@Service
public class PrivateChatToggleServiceImpl implements PrivateChatToggleService {

private final PrivateChatToggleRepository msgToggleRepository;
CitralFlo marked this conversation as resolved.
Show resolved Hide resolved
private final ConcurrentHashMap<UUID, PrivateChatToggleState> cachedToggleStates;

@Inject
public PrivateChatToggleServiceImpl(PrivateChatToggleRepository msgToggleRepository) {
this.cachedToggleStates = new ConcurrentHashMap<>();
this.msgToggleRepository = msgToggleRepository;
}


@Override
public CompletableFuture<PrivateChatToggleState> getPrivateChatToggleState(UUID uuid) {
if (this.cachedToggleStates.containsKey(uuid)) {
return CompletableFuture.completedFuture(this.cachedToggleStates.get(uuid));
}

return this.msgToggleRepository.getPrivateChatToggleState(uuid);
}

@Override
public void togglePrivateChat(UUID uuid, PrivateChatToggleState toggle) {
this.cachedToggleStates.put(uuid, toggle);
this.msgToggleRepository.setPrivateChatToggle(uuid, toggle);
}


}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package com.eternalcode.core.feature.privatechat.toggle;

import com.j256.ormlite.field.DatabaseField;
import com.j256.ormlite.table.DatabaseTable;
import java.util.UUID;

@DatabaseTable(tableName = "eternal_core_private_chat_toggle")
class PrivateChatToggleWrapper {

@DatabaseField(columnName = "id", id = true)
private UUID uniqueId;

@DatabaseField(columnName = "enabled")
private PrivateChatToggleState state;

PrivateChatToggleWrapper(UUID id, PrivateChatToggleState enabled) {
this.uniqueId = id;
this.state = enabled;
}

PrivateChatToggleWrapper() {}

static PrivateChatToggleWrapper from(PrivateChatToggle msgToggle) {
return new PrivateChatToggleWrapper(msgToggle.getUuid(), msgToggle.getState());
}

PrivateChatToggleState isEnabled() {
return this.state;
}

void setState(PrivateChatToggleState state) {
this.state = state;
}
}
Loading