diff options
10 files changed, 100 insertions, 39 deletions
diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/LunaticChat.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/LunaticChat.kt index 7b2f3a0..d51a254 100644 --- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/LunaticChat.kt +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/LunaticChat.kt @@ -40,7 +40,10 @@ class LunaticChat : private lateinit var services: ServiceContainer private lateinit var configuration: LunaticChatConfiguration private lateinit var serviceInitializer: ServiceInitializer - private lateinit var pluginScope: PluginCoroutineScope + + // Read by commands that must not block the tick thread. + lateinit var pluginScope: PluginCoroutineScope + private set private var updateChecker: UpdateChecker? = null private val updateAvailable = AtomicBoolean(false) diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/handler/DirectMessageHandler.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/handler/DirectMessageHandler.kt index ba0b5f8..b3266fb 100644 --- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/handler/DirectMessageHandler.kt +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/handler/DirectMessageHandler.kt @@ -107,7 +107,7 @@ class DirectMessageHandler( * * @return true if message was sent successfully */ - fun sendDirectMessage( + suspend fun sendDirectMessage( sender: Player, recipient: Player, message: String, @@ -145,7 +145,7 @@ class DirectMessageHandler( * @return the message body to relay (romaji-converted if applicable), since the * receiving server has no access to the sender's settings. */ - fun handleOutgoingCrossServerMessage( + suspend fun handleOutgoingCrossServerMessage( sender: Player, targetName: String, targetServerName: String, @@ -196,7 +196,7 @@ class DirectMessageHandler( lastMessager[recipient.uniqueId] = ReplyTarget.Remote(senderName, sourceServerName) } - private fun convertIfEnabled( + private suspend fun convertIfEnabled( message: String, enabled: Boolean, ): String = diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/command/impl/ReplyCommand.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/command/impl/ReplyCommand.kt index 86cac38..c530ce9 100644 --- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/command/impl/ReplyCommand.kt +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/command/impl/ReplyCommand.kt @@ -16,6 +16,8 @@ import dev.m1sk9.lunaticChat.paper.i18n.LanguageManager import dev.m1sk9.lunaticChat.paper.velocity.CrossServerDirectMessageManager import io.papermc.paper.command.brigadier.CommandSourceStack import io.papermc.paper.command.brigadier.Commands +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.launch import org.bukkit.Bukkit @Command( @@ -30,6 +32,9 @@ class ReplyCommand( private val dmHandler: DirectMessageHandler, override val languageManager: LanguageManager, private val crossServerDirectMessageManager: CrossServerDirectMessageManager? = null, + // Delivery is dispatched here rather than run inline: romaji conversion can reach the Google + // IME API, and a command executor runs on the tick thread. + private val scope: CoroutineScope = plugin.pluginScope.scope, ) : LunaticCommand(plugin) { override val description: String get() = languageManager.getMessage("commandDescription.reply") @@ -65,14 +70,14 @@ class ReplyCommand( val recipient = Bukkit.getPlayer(target.uuid) ?: return fail("directMessage.replyTargetNotFound") - dmHandler.sendDirectMessage(sender, recipient, message) + scope.launch { dmHandler.sendDirectMessage(sender, recipient, message) } CommandResult.Success } is ReplyTarget.Remote -> { val manager = crossServerDirectMessageManager ?: return fail("directMessage.replyTargetNotFound") - manager.sendCrossServerMessage(sender, target.playerName, target.serverName, message) + scope.launch { manager.sendCrossServerMessage(sender, target.playerName, target.serverName, message) } CommandResult.Success } } diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/command/impl/TellCommand.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/command/impl/TellCommand.kt index 990399e..bd98f22 100644 --- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/command/impl/TellCommand.kt +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/command/impl/TellCommand.kt @@ -18,6 +18,8 @@ import dev.m1sk9.lunaticChat.paper.velocity.CrossServerDirectMessageManager import dev.m1sk9.lunaticChat.paper.velocity.RemotePlayerRegistry import io.papermc.paper.command.brigadier.CommandSourceStack import io.papermc.paper.command.brigadier.Commands +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.launch import org.bukkit.Bukkit import org.bukkit.entity.Player import java.util.concurrent.CompletableFuture @@ -37,6 +39,9 @@ class TellCommand( private val crossServerDirectMessageManager: CrossServerDirectMessageManager? = null, private val remotePlayerRegistry: RemotePlayerRegistry? = null, private val localServerName: String = "", + // Delivery is dispatched here rather than run inline: romaji conversion can reach the Google + // IME API, and a command executor runs on the tick thread. + private val scope: CoroutineScope = plugin.pluginScope.scope, ) : LunaticCommand(plugin) { override val description: String get() = languageManager.getMessage("commandDescription.tell") @@ -99,7 +104,7 @@ class TellCommand( ) { return fail("directMessage.yourself") } - manager.sendCrossServerMessage(sender, name, server, message) + scope.launch { manager.sendCrossServerMessage(sender, name, server, message) } return CommandResult.Success } @@ -111,7 +116,7 @@ class TellCommand( return fail("directMessage.yourself") } - directMessageHandler.sendDirectMessage(sender, recipient, message) + scope.launch { directMessageHandler.sendDirectMessage(sender, recipient, message) } return CommandResult.Success } diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomajiConversionHelper.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomajiConversionHelper.kt index 841061a..8c656ec 100644 --- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomajiConversionHelper.kt +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomajiConversionHelper.kt @@ -12,15 +12,26 @@ import kotlinx.coroutines.withTimeoutOrNull * @param timeoutMs Timeout in milliseconds for the conversion (default: 1000ms). * @return The message with conversion appended (e.g., "hello §e(こんにちは)"), or the original message. */ -fun convertWithRomaji( +suspend fun convertWithRomaji( message: String, converter: RomanjiConverter, timeoutMs: Long = 1000, ): String = runCatching { - runBlocking { - withTimeoutOrNull(timeoutMs) { - converter.convert(message) - }?.let { "$message §e($it)" } ?: message - } + withTimeoutOrNull(timeoutMs) { + converter.convert(message) + }?.let { "$message §e($it)" } ?: message }.getOrElse { message } + +/** + * Blocking form of [convertWithRomaji], for callers that cannot suspend. + * + * AsyncChatEvent is the only such caller: it has to decide whether to cancel the event and what + * body to set before the handler returns, and it already runs off the tick thread. Command + * handlers do run on the tick thread and must use the suspending form instead. + */ +fun convertWithRomajiBlocking( + message: String, + converter: RomanjiConverter, + timeoutMs: Long = 1000, +): String = runBlocking { convertWithRomaji(message, converter, timeoutMs) } diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/listener/PlayerChatListener.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/listener/PlayerChatListener.kt index 3bd2d4a..2652bed 100644 --- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/listener/PlayerChatListener.kt +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/listener/PlayerChatListener.kt @@ -4,7 +4,7 @@ import dev.m1sk9.lunaticChat.paper.chat.channel.ChannelManager import dev.m1sk9.lunaticChat.paper.chat.handler.ChannelMessageHandler import dev.m1sk9.lunaticChat.paper.config.LunaticChatConfiguration import dev.m1sk9.lunaticChat.paper.converter.RomanjiConverter -import dev.m1sk9.lunaticChat.paper.converter.convertWithRomaji +import dev.m1sk9.lunaticChat.paper.converter.convertWithRomajiBlocking import dev.m1sk9.lunaticChat.paper.settings.PlayerSettingsManager import dev.m1sk9.lunaticChat.paper.velocity.CrossServerChatManager import io.papermc.paper.event.player.AsyncChatEvent @@ -74,7 +74,7 @@ class PlayerChatListener( val displayMessage = if (settings.japaneseConversionEnabled && romajiConverter != null) { - convertWithRomaji(messageWithoutPrefix, romajiConverter) + convertWithRomajiBlocking(messageWithoutPrefix, romajiConverter) } else { messageWithoutPrefix } diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/velocity/CrossServerDirectMessageManager.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/velocity/CrossServerDirectMessageManager.kt index 76cc879..307ca8c 100644 --- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/velocity/CrossServerDirectMessageManager.kt +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/velocity/CrossServerDirectMessageManager.kt @@ -39,7 +39,7 @@ class CrossServerDirectMessageManager( * notification and reply recording are handled by [DirectMessageHandler]; * the (possibly romaji-converted) body is what gets relayed. */ - fun sendCrossServerMessage( + suspend fun sendCrossServerMessage( sender: Player, targetName: String, targetServerName: String, diff --git a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/chat/handler/DirectMessageHandlerTest.kt b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/chat/handler/DirectMessageHandlerTest.kt index e8764b2..27a4b9c 100644 --- a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/chat/handler/DirectMessageHandlerTest.kt +++ b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/chat/handler/DirectMessageHandlerTest.kt @@ -11,6 +11,7 @@ import io.mockk.every import io.mockk.mockk import io.mockk.mockkStatic import io.mockk.unmockkStatic +import kotlinx.coroutines.runBlocking import org.bukkit.Bukkit import kotlin.test.Test import kotlin.test.assertEquals @@ -23,6 +24,8 @@ import kotlin.test.assertTrue * Validates message handling with dependency injection and conversion features. */ class DirectMessageHandlerTest { + private fun <T> sync(block: suspend () -> T): T = runBlocking { block() } + private fun createHandler( configuration: dev.m1sk9.lunaticChat.paper.config.LunaticChatConfiguration? = null, settingsManager: PlayerSettingsManager? = null, @@ -40,7 +43,7 @@ class DirectMessageHandlerTest { val sender = TestUtils.createMockPlayer() val recipient = TestUtils.createMockPlayer() - val result = handler.sendDirectMessage(sender, recipient, "Test message") + val result = sync { handler.sendDirectMessage(sender, recipient, "Test message") } assertTrue(result) } @@ -62,7 +65,7 @@ class DirectMessageHandlerTest { val sender = TestUtils.createMockPlayer(name = "Alice") val recipient = TestUtils.createMockPlayer(name = "Bob") - val result = handler.sendDirectMessage(sender, recipient, "Test") + val result = sync { handler.sendDirectMessage(sender, recipient, "Test") } assertTrue(result) } @@ -81,7 +84,7 @@ class DirectMessageHandlerTest { val sender = TestUtils.createMockPlayer() val recipient = TestUtils.createMockPlayer() - val result = handler.sendDirectMessage(sender, recipient, "konnichiwa") + val result = sync { handler.sendDirectMessage(sender, recipient, "konnichiwa") } assertTrue(result) } @@ -107,7 +110,7 @@ class DirectMessageHandlerTest { val recipient = TestUtils.createMockPlayer() // Should not throw exception and should complete quickly (within timeout) - val result = handler.sendDirectMessage(sender, recipient, "konnichiwa") + val result = sync { handler.sendDirectMessage(sender, recipient, "konnichiwa") } assertTrue(result) } @@ -122,7 +125,7 @@ class DirectMessageHandlerTest { // This validates Issue #1 refactoring - ConfigManager DI val sender = TestUtils.createMockPlayer() val recipient = TestUtils.createMockPlayer() - val result = handler.sendDirectMessage(sender, recipient, "Test") + val result = sync { handler.sendDirectMessage(sender, recipient, "Test") } assertTrue(result) } @@ -143,7 +146,7 @@ class DirectMessageHandlerTest { handler.remotePlayerRegistry = registry val sender = TestUtils.createMockPlayer(name = "Alice") - val relayed = handler.handleOutgoingCrossServerMessage(sender, "Bob", "survival", "hi") + val relayed = sync { handler.handleOutgoingCrossServerMessage(sender, "Bob", "survival", "hi") } assertEquals("hi", relayed) val target = handler.getReplyTarget(sender) @@ -174,7 +177,7 @@ class DirectMessageHandlerTest { handler.remotePlayerRegistry = RemotePlayerRegistry(localServerName = "lobby") // empty roster val sender = TestUtils.createMockPlayer(name = "Alice") - handler.handleOutgoingCrossServerMessage(sender, "Bob", "survival", "hi") + sync { handler.handleOutgoingCrossServerMessage(sender, "Bob", "survival", "hi") } assertNull(handler.getReplyTarget(sender)) } @@ -188,7 +191,7 @@ class DirectMessageHandlerTest { val recipient = TestUtils.createMockPlayer(name = "Bob") every { Bukkit.getPlayer(recipient.uniqueId) } returns recipient - handler.sendDirectMessage(sender, recipient, "hi") + sync { handler.sendDirectMessage(sender, recipient, "hi") } val target = handler.getReplyTarget(sender) assertIs<ReplyTarget.Local>(target) @@ -206,7 +209,7 @@ class DirectMessageHandlerTest { handler.remotePlayerRegistry = registry val sender = TestUtils.createMockPlayer(name = "Alice") - handler.handleOutgoingCrossServerMessage(sender, "Bob", "survival", "hi") + sync { handler.handleOutgoingCrossServerMessage(sender, "Bob", "survival", "hi") } handler.clearPlayer(sender) assertNull(handler.getReplyTarget(sender)) @@ -225,7 +228,7 @@ class DirectMessageHandlerTest { val sender = TestUtils.createMockPlayer() val recipient = TestUtils.createMockPlayer() - val result = handler.sendDirectMessage(sender, recipient, "Test") + val result = sync { handler.sendDirectMessage(sender, recipient, "Test") } assertTrue(result) } diff --git a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/command/impl/TellCommandTest.kt b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/command/impl/TellCommandTest.kt index 208d4bf..cea9d6f 100644 --- a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/command/impl/TellCommandTest.kt +++ b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/command/impl/TellCommandTest.kt @@ -7,9 +7,12 @@ import dev.m1sk9.lunaticChat.paper.chat.handler.DirectMessageHandler import dev.m1sk9.lunaticChat.paper.command.core.CommandContext import dev.m1sk9.lunaticChat.paper.i18n.LanguageManager import dev.m1sk9.lunaticChat.paper.velocity.CrossServerDirectMessageManager +import io.mockk.coVerify import io.mockk.every import io.mockk.mockk -import io.mockk.verify +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.test.StandardTestDispatcher import kotlin.test.Test import kotlin.test.assertIs @@ -17,6 +20,9 @@ class TellCommandTest { private fun createCommand( crossServerManager: CrossServerDirectMessageManager? = null, localServerName: String = "lobby", + // Unconfined runs the dispatched delivery inline, so a test can assert on it right after + // execute() returns. + scope: CoroutineScope = CoroutineScope(Dispatchers.Unconfined), ): TellDeps { val plugin = mockk<LunaticChat>(relaxed = true) val dmHandler = mockk<DirectMessageHandler>(relaxed = true) @@ -28,7 +34,16 @@ class TellCommandTest { val ctx = mockk<CommandContext>(relaxed = true) every { ctx.requirePlayer() } returns sender - val command = TellCommand(plugin, dmHandler, languageManager, crossServerManager, null, localServerName) + val command = + TellCommand( + plugin, + dmHandler, + languageManager, + crossServerManager, + null, + localServerName, + scope, + ) return TellDeps(command, ctx, dmHandler, crossServerManager, sender) } @@ -57,7 +72,7 @@ class TellCommandTest { val result = deps.command.execute(deps.ctx, "Bob@survival", "hello") assertIs<CommandResult.Success>(result) - verify { manager.sendCrossServerMessage(deps.sender, "Bob", "survival", "hello") } + coVerify { manager.sendCrossServerMessage(deps.sender, "Bob", "survival", "hello") } } @Test @@ -69,7 +84,7 @@ class TellCommandTest { val result = deps.command.execute(deps.ctx, "Alice@lobby", "hello") assertIs<CommandResult.Failure>(result) - verify(exactly = 0) { manager.sendCrossServerMessage(any(), any(), any(), any()) } + coVerify(exactly = 0) { manager.sendCrossServerMessage(any(), any(), any(), any()) } } @Test @@ -80,7 +95,7 @@ class TellCommandTest { val result = deps.command.execute(deps.ctx, "Bob@", "hello") assertIs<CommandResult.Failure>(result) - verify(exactly = 0) { manager.sendCrossServerMessage(any(), any(), any(), any()) } + coVerify(exactly = 0) { manager.sendCrossServerMessage(any(), any(), any(), any()) } } @Test @@ -91,7 +106,7 @@ class TellCommandTest { val result = deps.command.parseAndExecute(deps.ctx, "Bob@survival hello there") assertIs<CommandResult.Success>(result) - verify { manager.sendCrossServerMessage(deps.sender, "Bob", "survival", "hello there") } + coVerify { manager.sendCrossServerMessage(deps.sender, "Bob", "survival", "hello there") } } @Test @@ -102,6 +117,22 @@ class TellCommandTest { val result = deps.command.parseAndExecute(deps.ctx, "Bob@survival") assertIs<CommandResult.Failure>(result) - verify(exactly = 0) { manager.sendCrossServerMessage(any(), any(), any(), any()) } + coVerify(exactly = 0) { manager.sendCrossServerMessage(any(), any(), any(), any()) } + } + + @Test + fun `execute returns without waiting for delivery`() { + val dispatcher = StandardTestDispatcher() + val manager = mockk<CrossServerDirectMessageManager>(relaxed = true) + val deps = createCommand(crossServerManager = manager, scope = CoroutineScope(dispatcher)) + + val result = deps.command.execute(deps.ctx, "Bob@survival", "hello") + + // Delivery can reach the Google IME API; the command must not hold the tick thread for it. + assertIs<CommandResult.Success>(result) + coVerify(exactly = 0) { manager.sendCrossServerMessage(any(), any(), any(), any()) } + + dispatcher.scheduler.advanceUntilIdle() + coVerify { manager.sendCrossServerMessage(deps.sender, "Bob", "survival", "hello") } } } diff --git a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/velocity/CrossServerDirectMessageManagerTest.kt b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/velocity/CrossServerDirectMessageManagerTest.kt index cf9f529..03938fc 100644 --- a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/velocity/CrossServerDirectMessageManagerTest.kt +++ b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/velocity/CrossServerDirectMessageManagerTest.kt @@ -7,6 +7,7 @@ import dev.m1sk9.lunaticChat.paper.i18n.LanguageManager import io.mockk.every import io.mockk.mockk import io.mockk.verify +import kotlinx.coroutines.runBlocking import net.kyori.adventure.text.Component import org.bukkit.plugin.Plugin import java.util.UUID @@ -14,6 +15,8 @@ import java.util.logging.Logger import kotlin.test.Test class CrossServerDirectMessageManagerTest { + private fun <T> sync(block: suspend () -> T): T = runBlocking { block() } + private class Fixture( cacheSize: Int = 100, ) { @@ -53,11 +56,11 @@ class CrossServerDirectMessageManagerTest { fun `sendCrossServerMessage relays via plugin channel and delegates display`() { val f = Fixture() val sender = TestUtils.createMockPlayer(name = "Alice") - every { f.dmHandler.handleOutgoingCrossServerMessage(sender, "Bob", "survival", "hi") } returns "hi" + every { sync { f.dmHandler.handleOutgoingCrossServerMessage(sender, "Bob", "survival", "hi") } } returns "hi" - f.manager.sendCrossServerMessage(sender, "Bob", "survival", "hi") + sync { f.manager.sendCrossServerMessage(sender, "Bob", "survival", "hi") } - verify { f.dmHandler.handleOutgoingCrossServerMessage(sender, "Bob", "survival", "hi") } + verify { sync { f.dmHandler.handleOutgoingCrossServerMessage(sender, "Bob", "survival", "hi") } } verify { sender.sendPluginMessage(f.plugin, "lunaticchat:main", any<ByteArray>()) } } @@ -140,9 +143,9 @@ class CrossServerDirectMessageManagerTest { fun `sendCrossServerMessage prunes the dedup cache when over capacity`() { val f = Fixture(cacheSize = 1) val sender = TestUtils.createMockPlayer(name = "Alice") - every { f.dmHandler.handleOutgoingCrossServerMessage(any(), any(), any(), any()) } returns "hi" + every { sync { f.dmHandler.handleOutgoingCrossServerMessage(any(), any(), any(), any()) } } returns "hi" - repeat(3) { f.manager.sendCrossServerMessage(sender, "Bob$it", "survival", "hi") } + repeat(3) { sync { f.manager.sendCrossServerMessage(sender, "Bob$it", "survival", "hi") } } verify(atLeast = 1) { sender.sendPluginMessage(f.plugin, "lunaticchat:main", any<ByteArray>()) } } |