From 64197733346bbcabc3daa65f6648c1a21e26b328 Mon Sep 17 00:00:00 2001 From: Sho Sakuma Date: Sun, 30 Aug 2026 20:43:18 +0900 Subject: feat(engine): give both platforms one debug switch to read "Debug" had grown into three unconnected mechanisms: a boolean injected into RomanjiConverter, a one-off configuration dump at startup, and KtorSimpleLogger calls that Paper's stock log4j configuration never printed. Turning `debug: true` on told an operator nothing about the Velocity handshake, which is what #228 is actually about. The vocabulary goes in engine because it is a contract before it is an implementation: config.yml and the proxy's system property have to agree on what `velocity,protocol` means, and a category added on one side must exist on the other. Only the writing is left to the platforms. The message is a lambda so that the chat path - instrumented once per message - costs nothing while debugging is off. The `if (debugMode) logger.info("... $word")` guards this replaces built their strings either way. Both platform loggers write at INFO rather than at the level the content deserves, because neither Paper's nor Velocity's stock logging configuration prints anything below it, and asking operators to edit log4j before they can answer a bug report is the friction this exists to remove. Refs #228 Co-Authored-By: Claude --- .../lunaticChat/engine/debug/DebugCategories.kt | 59 +++++++++++++++++ .../lunaticChat/engine/debug/DebugCategory.kt | 29 +++++++++ .../m1sk9/lunaticChat/engine/debug/DebugLogger.kt | 27 ++++++++ .../m1sk9/lunaticChat/engine/debug/DebugState.kt | 32 +++++++++ .../engine/debug/DebugCategoriesTest.kt | 76 ++++++++++++++++++++++ .../lunaticChat/engine/debug/DebugStateTest.kt | 45 +++++++++++++ 6 files changed, 268 insertions(+) create mode 100644 engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategories.kt create mode 100644 engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategory.kt create mode 100644 engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugLogger.kt create mode 100644 engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugState.kt create mode 100644 engine/src/test/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategoriesTest.kt create mode 100644 engine/src/test/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugStateTest.kt (limited to 'engine/src') diff --git a/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategories.kt b/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategories.kt new file mode 100644 index 0000000..3231295 --- /dev/null +++ b/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategories.kt @@ -0,0 +1,59 @@ +package dev.m1sk9.lunaticChat.engine.debug + +/** + * Reads a debug switch written as text. + * + * Shared by both platforms so that `debug: velocity,protocol` in config.yml and + * `-Dlunaticchat.debug=velocity,protocol` on the proxy mean the same thing. A grammar owned by one + * side would drift from the other the first time a category was added. + */ +object DebugCategories { + /** What a debug switch asked for. */ + data class Parsed( + val enabled: Boolean, + val categories: Set, + /** Names that match no category, reported to the operator rather than thrown. */ + val unknown: List, + ) { + /** The categories that actually log. Empty while [enabled] is false, whatever was listed. */ + val active: Set get() = if (enabled) categories else emptySet() + } + + private val allWords = setOf("true", "yes", "on", "y", "all") + private val noneWords = setOf("false", "no", "off", "n", "none") + + /** + * Reads a whole switch: a boolean spelling, or a comma-separated list of category keys. + * + * The boolean spellings match the ones config.yml already accepts elsewhere: `debug: yes` + * worked while Bukkit read the file as YAML 1.1, and reading it as a category name instead + * would silently switch debugging off for a file that had it on. + */ + fun parse(raw: String): Parsed { + val trimmed = raw.trim() + val word = trimmed.lowercase() + return when { + trimmed.isEmpty() || word in noneWords -> Parsed(false, DebugCategory.entries.toSet(), emptyList()) + word in allWords -> Parsed(true, DebugCategory.entries.toSet(), emptyList()) + else -> resolve(trimmed.split(',')) + } + } + + /** Reads category keys that arrived already separated, as config.yml's `categories:` list does. */ + fun resolve(names: Iterable): Parsed { + val categories = LinkedHashSet() + val unknown = mutableListOf() + + names + .map { it.trim() } + .filter { it.isNotEmpty() } + .forEach { name -> + val category = DebugCategory.fromKey(name) + if (category == null) unknown += name else categories += category + } + + // A switch that named nothing this build knows is off rather than on-with-no-categories: + // reporting it as enabled would put "debug is on" in /lc status for a typo that logs nothing. + return Parsed(enabled = categories.isNotEmpty(), categories = categories, unknown = unknown) + } +} diff --git a/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategory.kt b/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategory.kt new file mode 100644 index 0000000..9c12a92 --- /dev/null +++ b/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategory.kt @@ -0,0 +1,29 @@ +package dev.m1sk9.lunaticChat.engine.debug + +/** + * The area of the plugin a debug line belongs to. + * + * Categories exist so that instrumenting the chat path - which runs once per message - does not + * make the log unusable for someone chasing a handshake. [key] is the spelling operators write in + * config.yml and on the Velocity command line; the enum name is never exposed to them. + */ +enum class DebugCategory( + val key: String, +) { + CONFIG("config"), + CHAT("chat"), + CHANNEL("channel"), + CONVERSION("conversion"), + PROTOCOL("protocol"), + VELOCITY("velocity"), + STORAGE("storage"), + COMMAND("command"), + ; + + companion object { + private val byKey = entries.associateBy { it.key } + + /** The category [key] names, or null when no category is spelled that way. */ + fun fromKey(key: String): DebugCategory? = byKey[key.trim().lowercase()] + } +} diff --git a/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugLogger.kt b/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugLogger.kt new file mode 100644 index 0000000..5f235f6 --- /dev/null +++ b/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugLogger.kt @@ -0,0 +1,27 @@ +package dev.m1sk9.lunaticChat.engine.debug + +/** + * Writes a diagnostic line, but only for the categories that are switched on. + * + * [message] is a lambda rather than a String so that instrumenting a path taken once per chat + * message costs nothing while debugging is off: the interpolation never runs. The + * `if (debugMode) logger.info("... $word")` guards this replaced paid for their strings either way. + */ +interface DebugLogger { + fun isEnabled(category: DebugCategory): Boolean + + fun log( + category: DebugCategory, + message: () -> String, + ) + + /** A logger for callers that have no debug state to consult, such as tests of unrelated code. */ + object Disabled : DebugLogger { + override fun isEnabled(category: DebugCategory): Boolean = false + + override fun log( + category: DebugCategory, + message: () -> String, + ) = Unit + } +} diff --git a/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugState.kt b/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugState.kt new file mode 100644 index 0000000..b4fe9db --- /dev/null +++ b/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugState.kt @@ -0,0 +1,32 @@ +package dev.m1sk9.lunaticChat.engine.debug + +/** + * The debug categories currently switched on, which a reload or `/lc debug` may replace while the + * server runs. + * + * Held apart from the configuration for the same reason the message formats are: taking this type + * says the value is live, while taking a configuration says it was frozen at startup. Every writer + * replaces the whole set under a lock, so a reader never sees a half-built one. + */ +class DebugState( + initial: Set = emptySet(), +) { + @Volatile + var enabled: Set = initial.toSet() + private set + + fun isEnabled(category: DebugCategory): Boolean = category in enabled + + @Synchronized + fun replace(categories: Set) { + enabled = categories.toSet() + } + + @Synchronized + fun set( + category: DebugCategory, + on: Boolean, + ) { + enabled = if (on) enabled + category else enabled - category + } +} diff --git a/engine/src/test/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategoriesTest.kt b/engine/src/test/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategoriesTest.kt new file mode 100644 index 0000000..44ccb86 --- /dev/null +++ b/engine/src/test/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategoriesTest.kt @@ -0,0 +1,76 @@ +package dev.m1sk9.lunaticChat.engine.debug + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class DebugCategoriesTest { + @Test + fun `a true spelling switches every category on`() { + listOf("true", "TRUE", "yes", "on", "all", " true ").forEach { raw -> + val parsed = DebugCategories.parse(raw) + + assertTrue(parsed.enabled, raw) + assertEquals(DebugCategory.entries.toSet(), parsed.active, raw) + } + } + + @Test + fun `a false spelling switches every category off`() { + listOf("false", "FALSE", "no", "off", "none", "", " ").forEach { raw -> + val parsed = DebugCategories.parse(raw) + + assertFalse(parsed.enabled, raw) + assertEquals(emptySet(), parsed.active, raw) + } + } + + @Test + fun `a comma separated list switches on exactly the categories it names`() { + val parsed = DebugCategories.parse("velocity,protocol") + + assertTrue(parsed.enabled) + assertEquals(setOf(DebugCategory.VELOCITY, DebugCategory.PROTOCOL), parsed.active) + assertEquals(emptyList(), parsed.unknown) + } + + @Test + fun `surrounding whitespace, casing and empty entries are ignored`() { + val parsed = DebugCategories.parse(" Velocity , , PROTOCOL ,") + + assertEquals(setOf(DebugCategory.VELOCITY, DebugCategory.PROTOCOL), parsed.active) + assertEquals(emptyList(), parsed.unknown) + } + + @Test + fun `an unknown name is reported without discarding the categories beside it`() { + val parsed = DebugCategories.parse("velocity,proxy") + + assertEquals(setOf(DebugCategory.VELOCITY), parsed.active) + assertEquals(listOf("proxy"), parsed.unknown) + } + + @Test + fun `a switch naming only unknown categories logs nothing`() { + val parsed = DebugCategories.parse("maybe") + + assertEquals(emptySet(), parsed.active) + assertEquals(listOf("maybe"), parsed.unknown) + } + + @Test + fun `resolve reads a list the way parse reads the same names spelled with commas`() { + assertEquals( + DebugCategories.parse("velocity,protocol"), + DebugCategories.resolve(listOf("velocity", "protocol")), + ) + } + + @Test + fun `every category is reachable by the key operators write`() { + DebugCategory.entries.forEach { category -> + assertEquals(category, DebugCategory.fromKey(category.key)) + } + } +} diff --git a/engine/src/test/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugStateTest.kt b/engine/src/test/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugStateTest.kt new file mode 100644 index 0000000..ae1456d --- /dev/null +++ b/engine/src/test/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugStateTest.kt @@ -0,0 +1,45 @@ +package dev.m1sk9.lunaticChat.engine.debug + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class DebugStateTest { + @Test + fun `a state starts on the categories it was built with`() { + val state = DebugState(setOf(DebugCategory.VELOCITY)) + + assertTrue(state.isEnabled(DebugCategory.VELOCITY)) + assertFalse(state.isEnabled(DebugCategory.CHAT)) + } + + @Test + fun `replace drops the categories it does not list`() { + val state = DebugState(setOf(DebugCategory.VELOCITY, DebugCategory.CHAT)) + + state.replace(setOf(DebugCategory.PROTOCOL)) + + assertEquals(setOf(DebugCategory.PROTOCOL), state.enabled) + } + + @Test + fun `set switches one category without touching the others`() { + val state = DebugState(setOf(DebugCategory.VELOCITY)) + + state.set(DebugCategory.CHAT, on = true) + state.set(DebugCategory.VELOCITY, on = false) + + assertEquals(setOf(DebugCategory.CHAT), state.enabled) + } + + @Test + fun `a state built from a set is unaffected by later changes to it`() { + val categories = mutableSetOf(DebugCategory.VELOCITY) + val state = DebugState(categories) + + categories += DebugCategory.CHAT + + assertEquals(setOf(DebugCategory.VELOCITY), state.enabled) + } +} -- cgit v1.2.1 From 85fad9cf81c45b16dc9d0c3310bdbf0eea694723 Mon Sep 17 00:00:00 2001 From: Sho Sakuma Date: Sun, 30 Aug 2026 20:43:36 +0900 Subject: feat(paper): make the debug switch categorised, live and dumpable config.yml now accepts `debug: velocity,protocol` and a block with `enabled` and `categories` beside the boolean it always took. Categories exist so that instrumenting the chat path does not bury the handshake an operator is chasing under one line per message. The setting is read by inspecting the YAML node by hand rather than through a data class, because one key has to accept both a scalar and a map and kaml picks the input to build from the descriptor kind alone - a class descriptor rejects `debug: true` before the serializer is ever called. An unknown category name is collected rather than thrown: throwing would make ConfigManager prune the whole `debug` key and switch off the categories the operator did spell correctly. `debug` moves from RESTART_REQUIRED to applied, since a reload only has to replace a DebugState. `/lc debug` switches a category for the running server and deliberately does not write config.yml back - re-emitting a commented YAML document from a parsed tree loses every comment and every bit of formatting the operator put there, and a restart or `/lc reload` putting the file back in charge is the safer default anyway. `/lc dump` writes the report a bug report needs: versions, enabled features, connection state and how full each store is. It carries no message text, player name or UUID, because it exists to be pasted into a public issue by someone who is not reading it first. Its reply names the file relative to the server directory - the command can be granted to someone with no filesystem access, to whom an absolute path discloses where the server is installed and nothing they can act on. The KtorSimpleLogger calls in ChannelMessageHandler and ChannelMessageLogger are folded into the same logger, so there is one place that decides whether a diagnostic line is printed. Refs #228 Co-Authored-By: Claude --- .../lunaticChat/engine/permission/LunaticChatPermissionNode.kt | 4 ++++ .../engine/permission/LunaticChatPermissionNodeTest.kt | 8 ++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) (limited to 'engine/src') diff --git a/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/permission/LunaticChatPermissionNode.kt b/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/permission/LunaticChatPermissionNode.kt index d12bb2d..9831b3c 100644 --- a/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/permission/LunaticChatPermissionNode.kt +++ b/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/permission/LunaticChatPermissionNode.kt @@ -15,6 +15,10 @@ sealed class LunaticChatPermissionNode( object Reload : LunaticChatPermissionNode("lunaticchat.command.lc.reload") + object Debug : LunaticChatPermissionNode("lunaticchat.command.lc.debug") + + object Dump : LunaticChatPermissionNode("lunaticchat.command.lc.dump") + object Channel : LunaticChatPermissionNode("lunaticchat.command.lc.channel") object ChannelCreate : LunaticChatPermissionNode("lunaticchat.command.lc.channel.create") diff --git a/engine/src/test/kotlin/dev/m1sk9/lunaticChat/engine/permission/LunaticChatPermissionNodeTest.kt b/engine/src/test/kotlin/dev/m1sk9/lunaticChat/engine/permission/LunaticChatPermissionNodeTest.kt index 39f2604..e5e43cb 100644 --- a/engine/src/test/kotlin/dev/m1sk9/lunaticChat/engine/permission/LunaticChatPermissionNodeTest.kt +++ b/engine/src/test/kotlin/dev/m1sk9/lunaticChat/engine/permission/LunaticChatPermissionNodeTest.kt @@ -13,6 +13,8 @@ class LunaticChatPermissionNodeTest { LunaticChatPermissionNode.Settings, LunaticChatPermissionNode.Status, LunaticChatPermissionNode.Reload, + LunaticChatPermissionNode.Debug, + LunaticChatPermissionNode.Dump, LunaticChatPermissionNode.Channel, LunaticChatPermissionNode.ChannelCreate, LunaticChatPermissionNode.ChannelList, @@ -45,8 +47,8 @@ class LunaticChatPermissionNodeTest { } @Test - fun `should have 25 permission nodes`() { - assertEquals(25, allNodes.size) + fun `should have 27 permission nodes`() { + assertEquals(27, allNodes.size) } @Test @@ -56,6 +58,8 @@ class LunaticChatPermissionNodeTest { assertEquals("lunaticchat.command.reply", LunaticChatPermissionNode.Reply.permissionNode) assertEquals("lunaticchat.command.lc.settings", LunaticChatPermissionNode.Settings.permissionNode) assertEquals("lunaticchat.command.lc.reload", LunaticChatPermissionNode.Reload.permissionNode) + assertEquals("lunaticchat.command.lc.debug", LunaticChatPermissionNode.Debug.permissionNode) + assertEquals("lunaticchat.command.lc.dump", LunaticChatPermissionNode.Dump.permissionNode) assertEquals("lunaticchat.command.lc.channel.create", LunaticChatPermissionNode.ChannelCreate.permissionNode) assertEquals("lunaticchat.command.lcv.status", LunaticChatPermissionNode.VelocityStatus.permissionNode) assertEquals("lunaticchat.spy", LunaticChatPermissionNode.Spy.permissionNode) -- cgit v1.2.1 From 1c79bee8f3c8fe20582afa96527cd298c043daa5 Mon Sep 17 00:00:00 2001 From: Sho Sakuma Date: Sun, 30 Aug 2026 21:11:46 +0900 Subject: fix(paper): stop the command category logging on every tree build Three findings from review of #296. Brigadier evaluates a `requires` predicate when it builds the command tree it sends to a player - on every join and every updateCommands() - not when someone is refused. The denial line sitting there put one entry per gated node per player in the log, burying the command results the category exists to show. It is dropped rather than moved: a hidden node is never reached by an execution, so there is no refusal path to log from. `all` was honoured by `debug: all` and `/lc debug all on` but not by `categories: [all]`, which resolved to an unknown name - leaving `enabled: true` with nothing switched on, reported as off by /lc status, explained only by a warning in the log. The word now lives in one place that all three read. Two dumps in the same second left one file, because the name is only accurate to the second and the write replaces. Reproducing a bug means dumping either side of the step that triggers it, so the second one now takes a suffix. Co-Authored-By: Claude --- .../m1sk9/lunaticChat/engine/debug/DebugCategories.kt | 13 ++++++++++++- .../dev/m1sk9/lunaticChat/engine/debug/DebugCategory.kt | 3 +++ .../lunaticChat/engine/debug/DebugCategoriesTest.kt | 17 +++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) (limited to 'engine/src') diff --git a/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategories.kt b/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategories.kt index 3231295..c3d7d95 100644 --- a/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategories.kt +++ b/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategories.kt @@ -19,7 +19,10 @@ object DebugCategories { val active: Set get() = if (enabled) categories else emptySet() } - private val allWords = setOf("true", "yes", "on", "y", "all") + /** The word that stands for every category at once, wherever a category may be named. */ + const val ALL = "all" + + private val allWords = setOf("true", "yes", "on", "y", ALL) private val noneWords = setOf("false", "no", "off", "n", "none") /** @@ -48,6 +51,14 @@ object DebugCategories { .map { it.trim() } .filter { it.isNotEmpty() } .forEach { name -> + // [ALL] is honoured here as well as in [parse] and /lc debug: an operator who has + // read `debug: all` or `/lc debug all on` writes `categories: [all]` next, and + // reporting that as an unknown name would leave them with a switch that logs + // nothing while saying it is on. + if (name.equals(ALL, ignoreCase = true)) { + categories += DebugCategory.entries + return@forEach + } val category = DebugCategory.fromKey(name) if (category == null) unknown += name else categories += category } diff --git a/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategory.kt b/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategory.kt index 9c12a92..40cc0e8 100644 --- a/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategory.kt +++ b/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategory.kt @@ -23,6 +23,9 @@ enum class DebugCategory( companion object { private val byKey = entries.associateBy { it.key } + /** Every [key], for the "known categories are ..." half of a message about a typo. */ + val keyList: String = entries.joinToString(", ") { it.key } + /** The category [key] names, or null when no category is spelled that way. */ fun fromKey(key: String): DebugCategory? = byKey[key.trim().lowercase()] } diff --git a/engine/src/test/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategoriesTest.kt b/engine/src/test/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategoriesTest.kt index 44ccb86..5931c60 100644 --- a/engine/src/test/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategoriesTest.kt +++ b/engine/src/test/kotlin/dev/m1sk9/lunaticChat/engine/debug/DebugCategoriesTest.kt @@ -67,6 +67,23 @@ class DebugCategoriesTest { ) } + @Test + fun `all names every category wherever a category may be named`() { + // The same word works as `debug: all`, as `categories: [all]` and as `/lc debug all on`; + // reading it as a category name in only two of the three would log nothing for the third. + assertEquals(DebugCategory.entries.toSet(), DebugCategories.parse("all").active) + assertEquals(DebugCategory.entries.toSet(), DebugCategories.resolve(listOf("all")).active) + assertEquals(emptyList(), DebugCategories.resolve(listOf("ALL")).unknown) + } + + @Test + fun `all beside a named category still names every category`() { + val parsed = DebugCategories.resolve(listOf("velocity", "all")) + + assertEquals(DebugCategory.entries.toSet(), parsed.active) + assertEquals(emptyList(), parsed.unknown) + } + @Test fun `every category is reachable by the key operators write`() { DebugCategory.entries.forEach { category -> -- cgit v1.2.1