Skip to content

GH-1196 Refactor sound sending system #1196

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+57 −5
Open

GH-1196 Refactor sound sending system #1196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b6bdebc
Refactor sound system
Pawelusz Oct 6, 2025
c22eaca
moved sound sending functions e.g. (PlayerQuitSoundController -> Play…
Pawelusz Oct 6, 2025
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
28 changes: 27 additions & 1 deletion eternalcore-core/src/main/java/com/eternalcode/core/feature/chat/ChatManagerController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import static com.eternalcode.core.feature.chat.ChatManagerController.CHAT_SLOWMODE_BYPASS_PERMISSION;

import com.eternalcode.annotations.scan.permission.PermissionDocs;
import com.eternalcode.core.configuration.implementation.PluginConfiguration;
import com.eternalcode.core.event.EventCaller;
import com.eternalcode.core.feature.chat.event.restrict.ChatRestrictCause;
import com.eternalcode.core.feature.chat.event.restrict.ChatRestrictEvent;
Expand All @@ -13,6 +14,8 @@
import com.eternalcode.core.util.DurationUtil;
import java.time.Duration;
import java.util.UUID;

import org.bukkit.Server;
import org.bukkit.entity.Player;
import org.bukkit.event.EventHandler;
import org.bukkit.event.EventPriority;
Expand All @@ -34,18 +37,24 @@ class ChatManagerController implements Listener {
private final ChatSettings chatSettings;
private final NoticeService noticeService;
private final EventCaller eventCaller;
private final PluginConfiguration config;
private final Server server;

@Inject
ChatManagerController(
ChatService chatService,
ChatSettings chatSettings,
NoticeService noticeService,
EventCaller eventCaller
EventCaller eventCaller,
PluginConfiguration config,
Server server
) {
this.chatService = chatService;
this.chatSettings = chatSettings;
this.noticeService = noticeService;
this.eventCaller = eventCaller;
this.config = config;
this.server = server;
}

@EventHandler(ignoreCancelled = true, priority = EventPriority.HIGHEST)
Expand Down Expand Up @@ -90,4 +99,21 @@ void onChatSlowMode(AsyncPlayerChatEvent event) {
void markUseChat(AsyncPlayerChatEvent event) {
this.chatService.markUseChat(event.getPlayer().getUniqueId());
}

@EventHandler(ignoreCancelled = true, priority = EventPriority.MONITOR)
void sendSound(AsyncPlayerChatEvent event) {
PluginConfiguration.Sounds sound = this.config.sound;

if (!sound.enableAfterChatMessage) {
return;
}

if (!this.chatSettings.chatEnabled()) {
return;
}
Comment on lines +111 to +113
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This check if (!this.chatSettings.chatEnabled()) appears to be redundant and might cause unintended behavior. The event handler is already set to ignoreCancelled = true, so it won't run if the chat event is cancelled (for example, when chat is disabled for a user without bypass permissions).

However, for a user with bypass permissions, their message will go through, but this check will prevent the sound from playing. It seems more consistent to play the sound if the message is successfully sent. I recommend removing this conditional block to fix this.


for (Player online : this.server.getOnlinePlayers()) {
online.playSound(online.getLocation(), sound.afterChatMessage, sound.afterChatMessageVolume, sound.afterChatMessagePitch);
}
}
}
17 changes: 15 additions & 2 deletions ...nmessage/PlayerJoinMessageController.java → .../session/PlayerJoinMessageController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package com.eternalcode.core.feature.joinmessage;
package com.eternalcode.core.feature.session;


import com.eternalcode.commons.RandomElementUtil;
import com.eternalcode.core.configuration.implementation.PluginConfiguration;
import com.eternalcode.core.feature.vanish.VanishService;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.injector.annotations.component.Controller;
import com.eternalcode.core.notice.NoticeService;
import org.bukkit.Server;
import org.bukkit.entity.Player;
import org.bukkit.event.EventHandler;
import org.bukkit.event.Listener;
Expand All @@ -17,11 +19,15 @@ class PlayerJoinMessageController implements Listener {

private final NoticeService noticeService;
private final VanishService vanishService;
private final PluginConfiguration config;
private final Server server;

@Inject
PlayerJoinMessageController(NoticeService noticeService, VanishService vanishService) {
PlayerJoinMessageController(NoticeService noticeService, VanishService vanishService, PluginConfiguration config, Server server) {
this.noticeService = noticeService;
this.vanishService = vanishService;
this.config = config;
this.server = server;
}

@EventHandler
Expand All @@ -42,6 +48,13 @@ void onPlayerJoin(PlayerJoinEvent event) {
}

event.setJoinMessage(StringUtils.EMPTY);
PluginConfiguration.Sounds sound = this.config.sound;

if (sound.enabledAfterJoin) {
for (Player online : this.server.getOnlinePlayers()) {
online.playSound(online.getLocation(), sound.afterJoin, sound.afterJoinVolume, sound.afterJoinPitch);
}
Comment on lines +54 to +56
Copy link
Contributor

Choose a reason for hiding this comment

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

high

This logic for iterating over all online players to play a sound is duplicated in PlayerQuitMessageController.java (lines 46-48) and ChatManagerController.java (lines 115-117). To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, I suggest extracting this into a shared service, for example, a SoundService.

This would centralize the sound playing logic, making it easier to manage and modify in the future. Here's an example of how such a service could look:

@Service
public class SoundService {
    private final Server server;

    @Inject
    public SoundService(Server server) {
        this.server = server;
    }

    public void playSoundToAll(Sound sound, float volume, float pitch) {
        if (sound == null) {
            return;
        }
        for (Player online : this.server.getOnlinePlayers()) {
            online.playSound(online.getLocation(), sound, volume, pitch);
        }
    }
}

You could then inject SoundService into the controllers and simplify the code to a single method call, like this.soundService.playSoundToAll(sound.afterJoin, sound.afterJoinVolume, sound.afterJoinPitch);.

}

this.noticeService.create()
.noticeOptional(translation -> RandomElementUtil.randomElement(translation.event().joinMessage()))
Expand Down
17 changes: 15 additions & 2 deletions ...tmessage/PlayerQuitMessageController.java → .../session/PlayerQuitMessageController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package com.eternalcode.core.feature.quitmessage;
package com.eternalcode.core.feature.session;


import com.eternalcode.commons.RandomElementUtil;
import com.eternalcode.core.configuration.implementation.PluginConfiguration;
import com.eternalcode.core.feature.vanish.VanishService;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.injector.annotations.component.Controller;
import com.eternalcode.core.notice.NoticeService;
import org.bukkit.Server;
import org.bukkit.entity.Player;
import org.bukkit.event.EventHandler;
import org.bukkit.event.Listener;
Expand All @@ -17,11 +19,15 @@ class PlayerQuitMessageController implements Listener {

private final NoticeService noticeService;
private final VanishService vanishService;
private final PluginConfiguration config;
private final Server server;

@Inject
PlayerQuitMessageController(NoticeService noticeService, VanishService vanishService) {
PlayerQuitMessageController(NoticeService noticeService, VanishService vanishService, PluginConfiguration config, Server server) {
this.noticeService = noticeService;
this.vanishService = vanishService;
this.config = config;
this.server = server;
}

@EventHandler
Expand All @@ -34,6 +40,13 @@ void onPlayerQuit(PlayerQuitEvent event) {
}

event.setQuitMessage(StringUtils.EMPTY);
PluginConfiguration.Sounds sound = this.config.sound;

if (sound.enableAfterQuit) {
for (Player online : this.server.getOnlinePlayers()) {
online.playSound(online.getLocation(), sound.afterQuit, sound.afterQuitVolume, sound.afterQuitPitch);
}
}

this.noticeService.create()
.noticeOptional(translation -> RandomElementUtil.randomElement(translation.event().quitMessage()))
Expand Down