Skip to content

feat(commands): interactive /progress card, with a glyph-safe Bedrock mode - #117

Open
BharathASL wants to merge 3 commits into
mainfrom
feat/progress-card
Open

BharathASL wants to merge 3 commits into
mainfrom
feat/progress-card

Conversation

@BharathASL

Copy link
Copy Markdown
Contributor

Description

Implements the /progress card (#3) and addresses the first of the five follow-ups filed in #115.

The card. /progress draws 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 highlighted NEXT STEP line. /asr progress reaches the same code — plugin.yml has 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 STEP line and the per-row detail both come from IdleReminderRules.nextStep / IdleReminderRules.outstanding, which is the logic the idle reminder already uses for {NEXT_STEP}. nextStep returned a flat String describing 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 as outstanding(EligibilityResult) and nextStep now 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, so ProgressCardTest asserts 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-card mode.

progress-card.simple-card takes AUTO (default), ALWAYS or NEVER. AUTO draws the plain-ASCII card for a Bedrock client and the full one for a Java client. ALWAYS and NEVER are 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 as new 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.withinBedrockGlyphs defines 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.Glyphs renders 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.yml and reproduced verbatim, so a card over a display name containing an emoji contains that emoji. What SIMPLE guarantees 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 inspect builds its report — ProgressionManager#evaluate reads statistics and advancement progress, both region-owned. The configuration snapshot is read once and carried into the task, including on the /asr progress path 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:

  • No new permission node. /asr progress is gated on antispeedrun.progress, the node the standalone /progress already 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. CommandGrammarTest asserts the reuse.
  • Nothing renamed. No command, subcommand or node changes name, so §2's children: shim and §4b's frozen-token rule do not come into play.
  • PROGRESS is the first Subcommand gated outside antispeedrun.admin. CommandGrammarTest used to assert that the admin tree and the set of nodes /asr enforces are the same set, and that the enum's every node starts with antispeedrun.admin.. Both now partition on a new Subcommand#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 is progress and that antispeedrun.progress is 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#advance called onFailure.accept(thrown) from the catch block, outside the guarded region. If the reporter threw, advance propagated, IdleReminderEngine#tick never 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 in IdleReminderRulesTest cover 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

  • "Renders cleanly across Java & Bedrock (Geyser) clients" — left unticked, and R-21 is why. Nothing in this change or in CI has looked at a Bedrock client. What is true and tested is the replacement criterion: the simple card emits no character outside a defined set, and AUTO selects 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 true default, the three marks, the gold NEXT STEP line, the simple-card mode, and the character-set assertion.

Left for the #5 agent

plugin.yml's journeybook command block, antispeedrun.book, its "returns as a delegate when task #5 lands" comment lines, and every JourneyBook* file are untouched. No book constant was added to Subcommand, so the usage: 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

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation / Hygiene update

Checklist

  • My code follows the code style of this project.
  • I have verified that all unit tests pass locally (./gradlew test). ./gradlew --no-daemon clean build is 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 with JAVA_HOME pointed at a Temurin 21.
  • I have added tests that prove my fix is effective or that my feature works. New ProgressCardTest (rows, next step, the glyph-set assertions and its negative control, style selection); extended CommandGrammarTest and IdleReminderRulesTest.
  • I have updated documentation / CHANGELOG as appropriate. The README already documented /progress and /asr progress as though they existed; that is now true rather than aspirational, so it needed no edit.
  • My commits follow Conventional Commits.
  • Every commit is signed off (git commit -s), per section 5 of CONTRIBUTING.md.

… poll

A throwing onFailure escaped advance, so the engine never stored the
stamped state and the once-a-second loop #110 closed reopened. (#115)

Signed-off-by: Bharath Sendhurpandi <bharathasl74185@gmail.com>
… 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Task 2.1.2: Interactive /progress MiniMessage Card

1 participant