| Age | Commit message (Collapse) | Author |
|---|
|
|
|
v26.2.build.96-stable (#270)
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
|
|
v26.2.build.93-stable (#269)
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
|
|
fix: Show the actual plugin versions on the download and compatibility pages, and mark feature-short pairs
|
|
The matrix marked every accepted pair with a plain tick, which reads as
"everything works". It does not. ProtocolVersion bumps PATCH for sub-channels a
peer can safely ignore, so Paper 1.3.0 (protocol 1.0.1) against Velocity 1.1.0
(1.0.0) completes the handshake and then silently drops cross-server direct
messages — the very feature that PATCH was bumped for. An operator reading the
tick had no way to learn that, and the pages around it repeated the claim.
The handshake is settled by MAJOR and MINOR alone, so any difference left once a
pair is accepted is a feature the newer end offers and the older will never
answer. Those pairs now carry a warning that says which end lags. Naming the
feature would take a protocol-version-to-feature table on the website, which
would drift from the protocol it describes, so the warning stays general and
leaves the reader one hop from the compatibility page.
The handshake verdict keeps the three checks that mirror Velocity's gate, with
the new one layered after them, so the mirror stays honest. isCompatible now
holds for a degraded pair, which does connect.
Cells are built once per pair in a computed instead of recomputing on each of the
template's reads, which a third state would otherwise have multiplied.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
Biome 2.5.7 deprecates `linter.rules.recommended` in favour of `preset` and will
drop it in the next major, so the config emitted a deprecation notice on every
run. Renaming it now keeps `bun run lint` and `format:check` clean and spares a
forced edit at the next upgrade; the rule set it selects is unchanged.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
`includes` stopped at `.vitepress/theme/*.ts`, so everything under
`components/` went unchecked — including the release and compatibility logic
this branch touches. Widening it to `**/*.ts` takes the check from five files to
nine.
`.vue` stays out. Biome does not resolve `<script setup>` bindings used from the
template, so it reports the components' `t`, `data`, `loading` and formatters as
unused and offers to delete them; admitting those files would mean turning off
noUnusedVariables and noUnusedImports to buy nothing.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
The badge read `v1.3.0~`, pointing at a Velocity release that does not exist.
The feature needs both halves, and the Velocity half first shipped as 1.2.0
inside the unified v1.3.0 release, so a reader hunting for Velocity v1.3.0 finds
nothing and cannot tell which proxy build carries the relay.
Also refreshes the gradle.properties example, which still showed the versions
current when it was written.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
The download cards and the compatibility matrix derived each plugin's version
from the release tag, which cannot hold for a unified `vX.Y.Z` release: it
carries both JARs and their versions need not agree. v1.3.0 shipped Paper 1.3.0
alongside Velocity 1.2.0.
So the download page advertised Paper v1.2.2 and Velocity v1.1.0, two
generations stale, because it searched `paper/v` and `velocity/v` tags first and
fell back to a unified tag only when none existed. The matrix meanwhile put a
Velocity v1.3.0 on its axis that was never released, leaving no row for the
proxy build operators actually run.
Both now pick releases by the JARs attached to them and take the version from
the file name, the only place it is stated. Tag shape stops mattering: a
platform-specific tag and a unified one are alike just releases that happen to
carry a given JAR.
Releases rank by publication time rather than by API response order, which
follows tag creation instead: velocity/v1.1.0 precedes the later-published
paper/v1.2.2.
A version is listed once even when a later unified release re-attaches an
unchanged JAR, which release.yaml does whenever only the other platform was
bumped. Otherwise the matrix would carry one version twice, under two protocol
versions if the protocol had moved in between.
Tests run in CI from here on. They need an explicit path because Bun does not
discover tests inside dot-directories.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
docs: Move release notes onto the documentation site and correct the docs against the implementation
|
|
The links were written before the English pages existed, so GitHub and Modrinth
sent an international audience to Japanese notes. Both now point at the English
pages; the Japanese ones remain reachable from the site's own locale switch.
Also stops the changelog template from being served. It lives under `src/`, so
VitePress published it as a page of empty headings at /assets/changelog-default,
where it was reachable and indexable. `srcExclude` keeps it a template.
The tag table on the changelog overview claimed `vX.Y.Z` was last used for
v1.0.0 and retired, which contradicts release.yaml and the release skill still
supporting it; it now says what the tag is for.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
The `@<server>` argument resolves against the server names registered in
`velocity.toml`, not the `features.velocityIntegration.serverName` of the
destination: PresenceTracker publishes `serverInfo.name` and
CrossServerDirectMessageRelay matches the target against the same, so the
previous text had it exactly backwards. Following it on a proxy where the two
names differ produces SERVER_NOT_FOUND every time, and points the reader at the
wrong file to fix. `serverName` still matters, for `{server}` and for
recognising local players, so what it does is stated instead.
Also corrects two claims made while widening these pages:
- `/lcv status` reports only this server's protocol version, not both sides'.
The Velocity bug template leaned on it as its most important field, so a
reporter would have pasted output that cannot answer the compatibility
question. It now also says to run it after a player has joined, since the
handshake waits for one - the same caveat the README install steps needed.
- Spies see pre-conversion text for direct messages only. Channel messages are
converted before the handler runs, so spies get the same formatted string the
members do.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
Every release emitted a skeleton of six empty headings that had to be filled in
by hand on the draft, and Modrinth linked to the GitHub release, so a reader
needed two hops to reach notes that might never have been written.
Both now link straight to the changelog page for the released version, built
from the tag, so the notes live in exactly one place. This makes the release
depend on that page being deployed, which the release skill now checks for.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
Release notes were written into the GitHub release body, which means they exist
once, in English only, and are awkward to revise after publishing. Putting them
on the site gives each release a Japanese and an English page that can be
corrected later and linked from anywhere.
The overview page states the versioning rules the version numbers alone do not
convey: that dropping a supported platform is a MINOR bump, that the Paper and
Velocity builds advance separately, and that the plugin version says nothing
about compatibility.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
Several statements were wrong rather than merely thin, and each would have set
the wrong expectation:
- the conversion example implied romaji replaces the input, when the result is
appended in parentheses and both are sent
- the handshake was described as happening at startup, when it waits for the
first player to join, so a DISCONNECTED status on an empty server read as a
fault
- cache eviction was called oldest-first, which the unordered in-memory map
cannot provide
- the Velocity settings table omitted crossServerDirectMessage
- ConfigManager was described as reading Bukkit's FileConfiguration, and the
settings storage as recovering from a backup that does not exist
It also documents behaviour that had no mention anywhere: the `!` force-global
prefix, that spy sees channel messages and not only DMs, the nightly build
warnings, the overall 1000ms conversion budget that makes api.timeout above it
ineffective, and the data files the plugin writes.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
The v1.3.0 entry listed three changes out of the roughly forty that landed since
paper/v1.2.2, so the config.yml resilience work and every performance fix went
unrecorded.
Its Velocity note also predated moving Ktor out of the shared module and
understated the JAR reduction as roughly 1 MB. Measured against the released
asset it is about 5.6 MiB, from 8.2 MiB down to 2.6 MiB.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
Installation only linked out to the download page, and Features restated the
site's feature cards without saying what any of them do. The README now carries
the requirements, the per-platform install steps, and a Configuration section -
including that most features are off by default and that applying a change needs
a restart, which nothing in the repository stated anywhere.
The Japanese translation is dropped rather than kept in sync; lc.m1sk9.dev
serves Japanese documentation already.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
CONTRIBUTING pointed contributors at `./x start`, which now fails without a
platform argument, and credited LuckPerms with our coding conventions.
SECURITY.md still assumed a single Paper-shaped release: it scoped support to
one api-version and offered only email. It now states support per platform,
records that a protocol-version mismatch refusing to relay chat is intended
behaviour rather than a vulnerability, and points at GitHub private
vulnerability reporting, which is enabled on this repository.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
The single template asked for a Paper version unconditionally, so Folia and
Velocity operators had nowhere to describe the environment that actually
matters. Splitting it also lets each template carry its own Module label rather
than leaving that to triage.
The Velocity template requires the output of `/lcv status`, since compatibility
is decided by the protocol version rather than the plugin version, and asks for
the serverName of each backend, which is what cross-server DM routing matches
against.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
The file had drifted far enough to mislead rather than help. It documented
`./x start` without the platform argument the script now requires, pointed at a
`docs/` directory that is `website/`, and placed the romaji converter in engine
after it moved to platform-paper.
It also said nothing about the conventions that are easiest to violate: the
annotation-driven command framework, the Folia thread boundaries, and the
per-setting config fallback that must not be traded back for null-assertions.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
refactor: fix a delivery-queue regression and lift durability into the storage layer
|
|
It was the one file store without a test, so the round trip it exists for - and
its two fallbacks, an absent file and an unparseable one - were only asserted
through PlayerSettingsManager. The write failure matters most: loading a torn
file discards every player's settings, so a failed save must leave the previous
file untouched.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
Making a conversion timeout an ordinary exception put it in the same arm as a
hard API failure, where caching the hiragana fallback is deliberate - so one slow
reply recorded the unconverted form and that word rendered as hiragana for the
life of the cache. A timeout says the request was slow, not that the word has no
conversion, so it now returns the fallback without caching it and the next
message asks again.
The retry test builds its own remembering cache: the shared fixture's get()
always returns null, so against it the API is called every time and the test
would have passed even with the timeout cached.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
- getDirectMessageSpyPlayers had no callers left; handing out a copy of the spy
map is the shape notifySpies was introduced to replace.
- pluginScope was widened to public for "commands that must not block the tick
thread", but commands take the delivery queue and only LunaticChat reads it.
- getPlayerChannels returned a Result that cannot fail, so three callers carried
unreachable error paths and channel.status.error could never be shown.
- The cross-server managers were gated on velocityIntegration.enabled as well as
on a manager that is non-null only when it is enabled, letting the two
conditions disagree.
- LenientBoolean's non-YamlInput fallback was observationally identical to the
cast failing, since both land in ConfigManager's catch-all.
- sendCrossServerMessage caught its own failures underneath the delivery queue,
which already reports them without stopping the sender's later messages. The
second boundary is what forced a CancellationException clause here.
- handleOutgoingCrossServerMessage still recorded the reply target after the
commands took that over, so remote targets were recorded twice - re-inserting
entries clearPlayer had swept, which is the bug the local path was fixed for.
- The reason delivery is queued was written out in both command constructors and
twice more in KDoc; it now lives where the queueing happens.
Spy notification also defers its notice lookup and member set until a spy is
actually online, which is not the normal case.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
Atomicity was opt-in per write site, so a file added later was safe only if its
author noticed the convention. Worse, DebouncedSaver drops a request while one is
pending and so serves exactly one file - a rule held up only by the wiring
happening to construct a separate saver per file, and written down nowhere. A
FileStore now owns its file, its atomic write and its own saver, so neither can
be got wrong by wiring; writeTextAtomically is internal to the package.
Taking the Bukkit plugin out of DebouncedSaver in favour of an AsyncScheduler
makes the debounce testable at all: ChannelStorage's "the snapshot is taken when
the write runs" now runs against the real thing rather than a mocked saver.
Teardown gets the same treatment. The five services with shutdown work spelled it
saveToDisk() three times and shutdown() twice, and the list of them was
hand-maintained against a fourteen-field container - so a new service was not
stopped unless someone remembered a second place. They now implement
StoppableService and register as they are built, and shutdown iterates that list.
stop() delegates rather than renames, because the conversion cache's periodic
flush is a different caller from shutdown.
Also here, on files this commit already touches: player settings carry a dirty
flag, since updateSettings is the only writer and queues its own save, so every
quit was re-serializing every stored player to write identical bytes; and
setPlayerChannel returns early when nothing moved, because the quit path clears
the active channel for every player whether or not they had one, and a mass
disconnect paid a full snapshot per player in one tick.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
An item takes up to the conversion timeout to drain, far slower than a player
can send, so an unbounded queue grew for as long as a macro ran - each item
holding its sender and recipient alive and arriving minutes after it was typed.
Refusing the overflow is at least visible to the player.
Releasing a player now cancels their worker instead of letting the backlog run.
Finishing it would spend a round trip per item writing to somebody who has left,
and keep both players reachable until it drained.
Chosen over DROP_LATEST on the channel: that reports success to trySend and
drops silently, which would make the warning about discarded work unreachable in
the case it was written for.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
withTimeout reports a timeout as a CancellationException, which callers must
rethrow rather than degrade. With api.timeout below convertWithRomaji's budget
the client's timeout therefore travelled through convertWord, through
withTimeoutOrNull - which rethrows a timeout belonging to another coroutine -
and into the queue worker, which read it as shutdown and ended its loop. The
channel stayed registered with nothing reading it, so every later message from
that player was buffered and never delivered: exactly the failure the worker's
own guard exists to prevent. A timeout is now an ordinary exception, so
cancellation once again means only cancellation.
The concurrency limiter also no longer covers the cache lookup. Its four
permits are shared by the whole server and held for a full round trip, so
cached words queued behind in-flight requests for a permit they did not need
and a message whose every word was cached could still exhaust the caller's
budget and go out unconverted.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
refactor: put config, protocol and module boundaries where they belong
|
|
Catching YamlException at the parse step covered kaml but not the scanner
underneath it: a file saved as UTF-16, or one truncated with NUL padding after an
unclean shutdown, fails inside snakeyaml-engine's reader with an exception that
is not a YamlException. It escaped onEnable and Bukkit disabled the plugin over a
config file - the very failure reading per setting was meant to prevent, and one
the catch-all this replaced had handled. The same gap swallowed the serializer's
own non-YamlException fallback.
A UTF-8 BOM is also stripped before parsing. It otherwise stays on the first key,
which strictMode = false drops as an unknown setting without logging anything, so
the operator sees exactly one setting ignored and no reason why.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
The key is gone from config.yml, but four pages still documented it with a
default of 2, so operators would keep setting something nothing reads.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
kaml rejects a document as a whole, so one unreadable value lost every other
setting the operator had written: velocityIntegration off, serverName "Unknown"
and channel chat disabled, behind one vague SEVERE line. The hand-written mapper
this replaced defaulted per key, so reading the file directly had quietly made
config.yml far more brittle than it was.
The offending setting is now dropped by the path its parse error carries and the
rest of the document is decoded, so the operator loses the one value they got
wrong and is told which. Only a document that is not YAML at all still costs
them everything.
Booleans also accept the YAML 1.1 spellings again. Bukkit read config.yml as
YAML 1.1, where `yes`, `no`, `on` and `off` are booleans; under kaml's YAML 1.2
they became strings, so `checkForUpdates: no` would have reset to its default -
which is the opposite of what the file says.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
readText sat outside ConfigManager, so the documented "fall back to defaults"
never covered the read itself. saveDefaultConfig only logs when it fails to
write the file, so the read can still find nothing there - and the IOException
then escaped onEnable and Paper disabled the plugin outright.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
Both platform modules described the engine dependency as providing
serialization, coroutines and ktor. Moving romaji conversion out of engine left
it exposing serialization alone, so the note now points readers at dependencies
they have to declare themselves.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
CommandResult carried Adventure Components, which was engine's last
Minecraft dependency and the reason CLAUDE.md's "engine has no Minecraft
platform dependencies" was not quite true. It also meant a command could
not report a result without having already decided how it looks: every
site had to pick formatError versus format before it could return.
Results now carry text, and LunaticCommandBase.handleResult is the single
place that styles it - error red for Failure, normal for
SuccessWithMessage. The fail()/ok() helpers from #260 already funnelled
every call site through two functions, so this is a change to those two
plus the one command that composes its own success text.
engine's dependency list is down to kotlinx-serialization, and nothing
under engine/src references net.kyori, org.bukkit, com.velocitypowered or
io.papermc.
Not done: the review also proposed collapsing the per-command
`when (error)` blocks into one exception-to-key table. Those blocks pick
wording, not just a key - "only owners can delete this channel" reads
differently in the ban command than the delete command - so a shared table
would hand every caller the same sentence and need per-command overrides
on top. Left alone deliberately.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
Every setting's default was written three times - in config.yml, in the
ConfigManager getter call, and on the data class - and copying the file
key by key is what made that necessary. They had already drifted:
checkForUpdates defaulted to false in ConfigManager while both config.yml
and the data class said true.
Worse, features.channelChat.messageLogging was documented in config.yml
with three settings and never parsed at all. ConfigManager did not build
it, so ChannelMessageLoggingConfig() always won and an operator editing
retentionDays or maxFileSizeMB changed nothing. Those settings now take
effect - the documented behaviour, but a real change for anyone whose file
disagrees with the defaults.
KAML deserializes the file straight into the tree, the same way player
settings and channel data are already read, so a default now lives only on
the data class. Two consequences worth stating:
- japaneseConversion.cache and .api are nested classes now, because the
data has to match the file rather than the file being flattened by hand
on the way in. The YAML is unchanged.
- api.retryAttempts is gone from config.yml. It was parsed and stored, but
never reached GoogleIMEClient or RomanjiConverter, so it documented a
knob that did nothing.
Unknown keys are ignored and a malformed file falls back to defaults with
a log line, so neither an old config nor a typo stops the server booting.
The tests parse real YAML instead of a mocked FileConfiguration, which
lets them cover what the mock could not: a partial file, a retired key, a
malformed document, and - the one that would have caught the drift above -
that the bundled config.yml equals the declared defaults.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
DirectMessageError.reason was a String backed by two constants, so the
receiving side matched one case and let everything else fall through to
"the target is offline". Adding a third reason on the proxy would have
shipped it to Paper servers that silently reported the wrong thing - the
one string-keyed dispatch sitting next to a protocol layer whose messages
are otherwise a sealed hierarchy with exhaustiveness checking.
As an enum, the reader must decide what to show for each case, and
CrossServerDirectMessageManager's when no longer needs an else.
The wire format is unchanged: kotlinx serializes an enum as its name, so
the existing snapshots still decode. What did need care is the reverse
direction - a reason from a newer proxy would now fail to parse, where the
String version degraded. The property has a default and the codec enables
coerceInputValues, so an unknown reason lands on TARGET_OFFLINE, exactly
the old else branch. There is a compatibility test for that case.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
Closes #259.
engine exposed ktor through api(), so both platforms inherited the client
and its CIO engine. The only thing in engine that used ktor was
GoogleIMEClient, and the only module that used GoogleIMEClient was
platform-paper - Velocity was shipping roughly six megabytes of HTTP
client to support a Paper-only feature. Same story for
kotlinx-coroutines-core, which Velocity does not use at all.
Romaji conversion is a Paper feature, so the converter package now lives
in platform-paper alongside the ConversionCache and RomanjiConverter that
were already there. engine keeps kotlinx-serialization on api(), which is
genuine shared surface: the plugin messaging protocol is built on it.
The velocity shadow jar goes from 7,618,405 to 2,769,395 bytes, and no
longer contains io/ktor at all.
CacheData's tests were sitting inside engine's SettingsDataClassesTest,
which is unrelated to settings; they move with the class.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
|
|
perf: take chat and command hot paths off the tick thread
|
|
Neither ChannelStorage.saveToDisk nor ConversionCache had a test, so the
round-trip they exist for - and the reasons they skip work, discard a file, or
leave the previous one intact - were only asserted through their callers.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
Both commands already record the conversation on the command thread before
queueing the delivery, for the same reason recordRemoteRecipient exists: /reply
reads the target there. Recording it again inside the queued work was not only
redundant but late, re-inserting entries that clearPlayer had already swept - so
lastMessager grew by one dead UUID every time a recipient quit mid-delivery.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
Cancelling the scope kills the worker coroutines but does not close their
channels, so trySend kept reporting success for work nothing would ever read.
The warning that exists precisely to avoid dropping a message in silence was
therefore unreachable in the case it was written for.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
The catch-all around the Google IME call also caught the CancellationException
from the caller's timeout, and then cached the unconverted hiragana. Since the
words of a message are now converted concurrently, one timeout pinned every
word of that message to its hiragana form for the life of the cache instead of
just the word that timed out.
The same swallowing let a cancelled delivery carry on past the plugin scope
being cancelled, and reported the shutdown of an in-flight cross-server
message as a SEVERE delivery failure.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
The steps ran as one statement each, so the first exception escaped
onDisable and took the rest with it - leaving the channel message logger
unflushed and the Velocity connection to be torn down by the server rather
than by us. Each step is independent, so a failure is now reported and the
remaining ones still run.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
Writing to a fixed sibling only moved the interleaving from the destination
to the temporary file: two saves racing there published mixed content, and
the losing move then failed with the temporary file already gone. Each write
now gets a unique temporary file, and falls back to a non-atomic replace on
the network mounts that refuse an atomic rename.
settings.yml and the conversion cache were still written in place. Both are
discarded wholesale when they do not parse, so a torn file silently costs
every player's settings or the whole accumulated cache.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
Findings from reviewing the branch, in descending order of how much they
could hurt.
A message that threw ended the consumer loop while its channel stayed
registered in the map. Every later message from that player was then
buffered with nothing reading it, so their /tell and /reply stopped working
silently until they reconnected - one bad message format was enough. The
CoroutineExceptionHandler added earlier reported the throw but could not
bring the worker back. Each item now runs inside its own guard, so a failure
costs one message instead of the player's whole session.
Reply targets were recorded inside the queued delivery, but /reply reads
them on the command thread. /tell followed straight by /r therefore failed
with "nobody to reply to" for as long as the conversion took - up to the
full timeout. They are recorded before the work is queued now, which is
also where the sender expects the effect to happen.
Three narrower ones:
- A refused trySend was discarded without a word. It can happen once the
scope is cancelled at shutdown, and dropping a player's message in
silence is the worst way to handle it.
- Parallel conversion opened one API request per word with no ceiling, and
a repeated word in one line no longer shared the cache - the sequential
version got that for free by caching before the next lookup. Rate-limited
replies land in convertWord's catch and quietly degrade to hiragana, so
it is better not to ask that hard: distinct words only, four at a time.
- coerceAtLeast(1) on the cache interval turned a configured 0 into a whole
file rewrite every second, since any chat re-arms the dirty flag. A
non-positive value now falls back to the documented 300 with a warning.
ChannelStorage writes through a temporary file and an atomic move. Bukkit
runs onDisable before cancelling scheduler tasks, so the shutdown save can
overlap a still-pending debounced save, and two truncating writes to
channels.json would interleave into something unparseable. Debouncing made
that window much wider than the old runNow did.
TestLogger now captures log(level, msg, thrown); it only overrode
severe/warning/info, which do not route through each other, so anything
logged with a throwable attached was invisible to every assertion.
The two queue tests fail against the unguarded loop.
Left as review comments: delivery in flight can re-add a reply entry for a
player who just quit. resolveValidTarget filters unreachable targets on
read and the entry is cleared on their next quit, so the cost is a lingering
map entry, not wrong behaviour - and removing it would change what
sendDirectMessage promises, which a test pins.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
Passing ::snapshot to the debounced write meant the three caches were read
from the writer thread while the server thread mutated them. They are
separate maps, so a snapshot could catch a channel already removed from
channelsCache while its membersCache entry still existed - persisting an
orphaned member list - or the reverse, persisting a channel with no members
and therefore no owner. Neither crashes anything, since the readers filter
on channelsCache, but they are wrong state written to channels.json and
carried across restarts.
The snapshot is taken on the mutating thread again and handed over through
a volatile field, so the write still reads the newest state when it
eventually runs rather than the state at queue time. That keeps both
properties: consistent halves, and a batched write that reflects every
change made during the delay.
This does not give back the per-change cost the earlier commit was avoiding,
because copying three maps is not what made saving expensive - the debounce
already coalesces the file write, which is.
Also floors the conversion cache interval at one second. It is now the only
writer besides shutdown, so a non-positive value would both be rejected by
runAtFixedRate and leave the cache unsaved until the server stopped.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
Two regressions from taking the direct message path off the tick thread in
this branch.
A coroutine that threw reported to the JVM default handler, so nothing
reached the plugin log - and since the command had already returned
Success, a failed delivery was invisible to the player and the operator
alike. Before the change the exception propagated out of execute() into
Brigadier. PluginCoroutineScope now carries a CoroutineExceptionHandler,
which every caller that dispatches and returns depends on.
Launching a coroutine per message also dropped the ordering the synchronous
version had. Dispatchers.Default is a pool, so a cached romaji conversion
finishing in microseconds could overtake an uncached one sent before it:
the sender sees their second message first, and on the cross-server path
the relay to Velocity inverts too, so the recipient does as well. With
Japanese conversion on, mixing cached and uncached words is ordinary, not
an edge case.
A mutex would not have fixed it - two launched coroutines reach the lock in
whatever order the pool starts them. PerPlayerWorkQueue instead gives each
player a channel with a single consumer, so submission order is decided on
the calling thread and preserved, while different players stay independent.
The queue is dropped when a player quits; work already queued still runs.
The ordering test fails against the previous launch-per-message code.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
/tell and /reply ran romaji conversion inline. Brigadier executors run on
the main thread, and convertWithRomaji wrapped a Google IME call in
runBlocking with a one-second timeout, so a single direct message could
hold the tick thread for up to a second - twenty ticks - whenever the
words were not already cached. Conversion defaults to on for players once
the feature is enabled, so this was the ordinary path, not an edge case.
The conversion chain is suspending now, and the two commands dispatch
delivery to the plugin scope instead of running it inline. Nothing in that
chain touches world state: it sends chat components and plays client-side
sounds, both of which Paper already accepts off-thread, and which this
plugin already does from AsyncChatEvent.
AsyncChatEvent keeps a blocking bridge, renamed convertWithRomajiBlocking
so the choice is visible at the call site. That handler has to decide
whether to cancel the event and what body to set before it returns, and it
is already off the tick thread.
The commands take their scope as a constructor parameter so tests can
choose one. The new test uses StandardTestDispatcher to pin the property
that matters: execute() returns before delivery has run at all.
Co-Authored-By: Claude <noreply@anthropic.com>
|
|
Every channel mutation built a full snapshot - copying the channel map,
every member list, and stringifying a UUID per active player - and then
queued a task that pretty-printed and rewrote all of channels.json. There
was no debounce, unlike the settings storage, so a fifty-player join wave
meant fifty snapshots and fifty whole-file writes. Joins and quits both go
through it via setPlayerChannel, and so does the self-healing branch of
getPlayerChannelContext, which sits on the channel-chat message path.
ChannelStorage now debounces like the other two storages, and takes the
snapshot as a supplier so it is built once when the write runs rather than
once per queued change. saveToStorage and saveToDisk had the same six-line
snapshot construction; that is now one private function.
The trade-off is the same one the settings storage already makes: a crash
within the debounce window loses the last few seconds of channel state.
Shutdown still writes synchronously.
Co-Authored-By: Claude <noreply@anthropic.com>
|
