feat(commands): interactive /progress card, with a glyph-safe Bedrock mode - #117
Open
BharathASL wants to merge 3 commits into
Open
BharathASL wants to merge 3 commits into
BharathASL wants to merge 3 commits into
Conversation
… mode Adds /progress and the /asr progress delegate plugin.yml had promised, plus progress-card.simple-card for clients without the full glyph set. Signed-off-by: Bharath Sendhurpandi <bharathasl74185@gmail.com>
Signed-off-by: Bharath Sendhurpandi <bharathasl74185@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Implements the
/progresscard (#3) and addresses the first of the five follow-ups filed in #115.The card.
/progressdraws a player their own progression: every dimension gate in configured order, marked completed, in-progress or locked, with what each is still waiting on, and a highlightedNEXT STEPline./asr progressreaches the same code —plugin.ymlhas been advertising that token since before it existed, which #73 had to withdraw, and this is the delegate that line was promising.The next step is computed once, not twice. The
NEXT STEPline and the per-row detail both come fromIdleReminderRules.nextStep/IdleReminderRules.outstanding, which is the logic the idle reminder already uses for{NEXT_STEP}.nextStepreturned a flatStringdescribing only the first outstanding milestone, which is the wrong shape for a card that names every one, so rather than writing a second copy here the clause-building half was pulled out asoutstanding(EligibilityResult)andnextStepnow calls it. A card and a reminder that disagreed about what a player owes a gate would be a real bug and a confusing one, soProgressCardTestasserts the card's line is byte-for-byte the string the reminder would have said.R-21, and what actually got built. The audit was right that "renders cleanly across Java & Bedrock (Geyser) clients" is not something CI can check. Both halves of its suggested resolution are taken: there is no box frame at all — the card is colour, two-space indentation and a status mark — and there is a
simple-cardmode.progress-card.simple-cardtakesAUTO(default),ALWAYSorNEVER.AUTOdraws the plain-ASCII card for a Bedrock client and the full one for a Java client.ALWAYSandNEVERare the escape hatches for an operator whose answer differs from the plugin's — a resource pack, or detection that reads their setup wrong.Bedrock detection needs no new dependency. Floodgate stays a
softdepend; nothing compiles against it. Two conditions have to hold: Floodgate is actually enabled on this server, and the player's id has zero high bits, which is what Floodgate produces because it builds a Bedrock id asnew UUID(0, xuid)while a Java player's id is a version 4 UUID whose version nibble lives in the high half. The plugin-enabled check is what keeps the bit test from being a heuristic looking for a reason to be wrong on a server with no Bedrock players at all.The character-set criterion.
ProgressCardRenderer.withinBedrockGlyphsdefines the set as printable ASCII, deliberately narrower than Bedrock's real font: a conservative set is checkable and a generous one is an argument about which Geyser version and which resource pack.ProgressCardTest.Glyphsrenders every simple card through MiniMessage to plain text and asserts every character of the result is inside it, over each of the card's four shapes. There is a negative control asserting the full card does leave the set, so the positive assertion cannot pass by the two modes quietly becoming the same thing.One thing the criterion is not, stated in the code and asserted: it is about the plugin's own furniture, not the operator's strings. Milestone display names and advancement keys are read from
config.ymland reproduced verbatim, so a card over a display name containing an emoji contains that emoji. WhatSIMPLEguarantees is that the card adds nothing of its own outside the set. Everything interpolated is escaped first, so none of it is parsed as MiniMessage.Threading. The card is built on the viewer's own
EntityScheduler, exactly as/asr inspectbuilds its report —ProgressionManager#evaluatereads statistics and advancement progress, both region-owned. The configuration snapshot is read once and carried into the task, including on the/asr progresspath where the dispatcher has already read it.Naming (AGENTS.md §5,
brand/COMMAND_NAMING.md). Read before touching either surface. Three things fall out of it and are worth stating explicitly:/asr progressis gated onantispeedrun.progress, the node the standalone/progressalready uses. §2 is about node changes breaking silently; a second node for the same capability under a second spelling of the same command is the same failure with extra steps — an operator would have to discover and grant it before something that already worked kept working.CommandGrammarTestasserts the reuse.children:shim and §4b's frozen-token rule do not come into play.PROGRESSis the firstSubcommandgated outsideantispeedrun.admin.CommandGrammarTestused to assert that the admin tree and the set of nodes/asrenforces are the same set, and that the enum's every node starts withantispeedrun.admin.. Both now partition on a newSubcommand#administrative()rather than being weakened: the admin tree is still exactly the admin subcommands, uniqueness is still checked over every constant, and a new test asserts that the only non-administrative one isprogressand thatantispeedrun.progressis not reachable from the admin tree. Folding it in would have made a player's view of their own progression an operator privilege depending on which spelling they typed.#115, item 1 only.
IdleReminderRules#advancecalledonFailure.accept(thrown)from the catch block, outside the guarded region. If the reporter threw,advancepropagated,IdleReminderEngine#ticknever stored the stamped state, and the once-a-second loop #110 closed reopened. The call is now guarded; there is nowhere to report a failure of the reporting channel to, so it is dropped, which the code says in as many words. Two tests inIdleReminderRulesTestcover it: one that a throwing reporter still yields a stamped state, and one running the engine's loop for 45 simulated seconds with both the delivery and the report failing, asserting one attempt rather than forty-six and that the cooldown is still a cooldown.Items 2 to 5 of #115 are out of scope and untouched. #115 stays open carrying them.
Acceptance criteria I could not honestly tick
AUTOselects it for Floodgate players. Whether Geyser then renders that set the way anyone hopes is a visual claim, and it is the one the audit said not to make.Everything else on #3 is implemented and covered: the permission and its
truedefault, the three marks, the goldNEXT STEPline, thesimple-cardmode, and the character-set assertion.Left for the #5 agent
plugin.yml'sjourneybookcommand block,antispeedrun.book, its "returns as a delegate when task #5 lands" comment lines, and everyJourneyBook*file are untouched. Nobookconstant was added toSubcommand, so theusage:line reads/asr <reload|profile|unlock|bypass|inspect|progress>and #5 extends it the same way this change did.Related Issues
Closes #3
Item 1 of #115 is addressed here. That issue stays open, and is deliberately not auto-closed, because its remaining four items are untouched.
Type of Change
Checklist
./gradlew test)../gradlew --no-daemon clean buildis green: 492 tests, 0 failures, 0 errors, 0 skipped. Note for anyone who hits the toolchain trap recorded elsewhere — Gradle 9.7.1 starts fine on the installed JDK 26; what fails is the Java 21 toolchain, so this was run withJAVA_HOMEpointed at a Temurin 21.ProgressCardTest(rows, next step, the glyph-set assertions and its negative control, style selection); extendedCommandGrammarTestandIdleReminderRulesTest./progressand/asr progressas though they existed; that is now true rather than aspirational, so it needed no edit.git commit -s), per section 5 of CONTRIBUTING.md.