Declare command metadata and permission in plugin.yml, and stop /fly falling through to usage - #11
Conversation
…h to usage Adds a permissions block to plugin.yml declaring FlyCommand.fly with default op, plus description and usage metadata for the Fly command, so the manifest matches what USER_GUIDE.md, COMMANDS.md and Main.java already claim. Returning true from the permission-denied branch of onCommand is part of the same change rather than a separate one: with a usage string now declared, falling through to the trailing return false would newly print that usage line underneath the plugin's own permission alert. Adds @OverRide to onCommand, matching onEnable and onDisable. Closes #5 Closes #6 Closes #7 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Self-reviewPosted as a plain comment rather than a formal review: this is the authoring session's own rubric pass, not independent review. External anchorThe mechanical consistency checks are the anchor for this repository — there is no build system, and both workflows are checkout-only. They were run on the branch head and are green; their output is quoted in the PR description. Rubric
Findings folded in from the diff (no inline anchors used)
VerdictThe manifest half of this change is verified. The two source changes are not, and cannot be by anything in this repository — see #9. Merge is therefore requested only after the on-server recipe in the PR description has been run by a maintainer with a Spigot or Paper server. This review comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener). drafted by Claude on behalf of Daniel Stephenson |
Summary
permissions:block is added toplugin.yml, declaringFlyCommand.flywithdefault: opand the description already published inUSER_GUIDE.md. The node is checked byMain.javaand documented in two files, but was declared nowhere.description:andusage:are added under theFlycommand key, with the description taken verbatim fromCOMMANDS.md.return true;is added to the permission-denied branch ofonCommand, so the plugin's own alert is the only message shown to a player lacking the permission.@Overrideis added toonCommand, matchingonEnableandonDisablein the same class.## [Unreleased]inCHANGELOG.md.Why these three issues are batched
The manifest change and the return-value change are coupled and should not be separated. Before this branch, the permission-denied branch fell through to the trailing
return false, butPluginCommand.executeonly sends a usage message when the command's usage string is non-empty — and nousage:was declared, so nothing was in fact printed after the alert. Declaring ausage:for issue #5 is precisely what would have made issue #6's defect start being observed by players. Landing #5 alone would therefore have introduced the double message that #6 describes.This refines what issue #6 states about the current observable behaviour: the second line it describes is latent rather than already visible. The defect in the return value is real either way, and the fix is unchanged.
Validation
Mechanical checks (run on the branch head; output quoted)
Manifest and source agreement:
main:equals<package>.<class>and matches the file's path underme/.Every permission checked by the source is declared and documented:
The declared
default: opmatches theoppublished in theUSER_GUIDE.mdtable.Every handled label has a manifest key:
The manifest parses, and parses to the intended structure:
Regression evidence for the manifest defect
The manifest was reverted to
origin/mainand the permission check was re-run. It reported the defect —plugin.ymlproduced no match while both documentation files continued to name the node:The manifest change was then restored and the check came back clean. The assertion is therefore specific enough to observe the defect, not a false negative.
What is NOT verified here
The two
Main.javachanges are behavioural and compile-time respectively, and nothing in this repository can compile or run the plugin. There is no build system, and both GitHub Actions workflows contain a singleactions/checkout@v4step with norun:steps — a green check confirms only that the repository could be cloned. That gap is tracked in #9.Both source changes are therefore recorded UNVERIFIED and require a human with a server before release.
On-server reproduction recipe
A Spigot or Paper server at API 1.13 or later is required.
README.md, and place it inplugins/. Start the server./flyis registered./help flyshould now show the declared description rather than nothing.FlyCommand.fly, and run/fly.Alert: Permission 'FlyCommand.fly' required.return true;is removed while the newusage:is kept: that alert followed by a second line,/fly.FlyCommand.flyto that same non-operator account and run/flytwice, confirming flight is enabled and then disabled./fly, confirming toggling still works —default: opis intended to codify the pre-existing fallback rather than change it, and that is the assumption most worth checking on a real server./flyfrom the server console, confirmingAlert: Can't be used by console.is shown and no error is logged.CommandExecutor.onCommand.Issues deferred this cycle
USER_GUIDE.mdandCHANGELOG.mdtreatment. It is a good candidate for the next cycle.plugin.yml'smain:and to the project-structure entry in.github/copilot-instructions.md, and maintainer authorization is requested on that issue before any of it is started.Closes #5
Closes #6
Closes #7
This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).
drafted by Claude on behalf of Daniel Stephenson