Skip to content

Fix command usage message ignoring color tags#8623

Open
mvanhorn wants to merge 2 commits intoSkriptLang:masterfrom
mvanhorn:fix/usage-color-support-8616
Open

Fix command usage message ignoring color tags#8623
mvanhorn wants to merge 2 commits intoSkriptLang:masterfrom
mvanhorn:fix/usage-color-support-8616

Conversation

@mvanhorn
Copy link
Copy Markdown

@mvanhorn mvanhorn commented May 3, 2026

Description

Fixes #8616. When a Skript command defines a usage: value with color tags (e.g. <red>Test), the "incorrect usage" message shown to players runs through CommandSender.sendMessage(String), which does not parse Skript's Adventure-style tags. The cooldown-message path at ScriptCommand.java:296 already converts to a Component via TextComponentParser, so colors render correctly there.

Reproduction (from the issue):

command test <integer>:
    usage: <red>Test
    cooldown: 10 seconds
    cooldown message: <red>Test &aTest
    trigger:
        broadcast arg-1

/test foo (wrong arg type) prints the literal <red>Test. /test 1 then immediately again shows the cooldown message in red as expected.

Change

  • CommandUsage.getUsageComponent(Event) — new method that resolves the usage VariableString and runs the result through TextComponentParser.instance().parseSafe(...), returning an Adventure Component.
  • ScriptCommand.java:340 — use usage.getUsageComponent(event) so sender.sendMessage(...) takes the Adventure overload that honors <red> and &c tags.

bukkitCommand.setUsage(usage.getUsage()) (line 245) is left unchanged: Bukkit's PluginCommand.setUsage takes a plain String and is wired into the Bukkit registry, not into our send-to-player path.

The help-topic output at line 378 (ChatColor.GOLD + "Usage" + ChatColor.RESET + ": " + usage.getUsage()) is intentionally out of scope. It mixes Bukkit ChatColor legacy codes with the usage string and would require a separate Component-prefix refactor; the user's report was specifically about the invalid-argument feedback path.

Validation

  • ./gradlew compileJava
  • ./gradlew compileTestJava
  • Type-check confirms sender.sendMessage(Component) resolves on Paper / Bukkit's Adventure-aware CommandSender.

The runtime color render itself can only be exercised on a Minecraft server, which is what Skript's .sk test harness already does for the cooldown path. Happy to add a runtime .sk regression for usage: if reviewers prefer.

AI assistance disclosure

Per .github/CONTRIBUTING.md, disclosing AI assistance:

This PR was written with assistance from Claude (Anthropic). The bug analysis (cooldown vs. usage code paths in ScriptCommand) and the patch were reviewed before submission.

Fixes #8616

The "incorrect usage" message printed when a player runs a command with
the wrong argument types went through CommandSender.sendMessage(String),
which does not understand Skript's <red>-style color tags (those are
Adventure / TextComponentParser syntax, not Bukkit & codes). Cooldown
messages already round-trip through TextComponentParser via their
Component-converted expression, which is why they render correctly.

Add CommandUsage.getUsageComponent(Event) that parses the resolved usage
string with TextComponentParser, and use it in ScriptCommand's
invalid-argument path. Bukkit's PluginCommand.setUsage(String) is left
unchanged because the Bukkit registry takes plain text.

Fixes SkriptLang#8616
@mvanhorn mvanhorn requested review from a team and sovdeeth as code owners May 3, 2026 12:05
@mvanhorn mvanhorn requested review from Burbulinis and removed request for a team May 3, 2026 12:05
Comment thread src/main/java/ch/njol/skript/command/CommandUsage.java Outdated
@skriptlang-automation skriptlang-automation Bot added the needs reviews A PR that needs additional reviews label May 5, 2026
Per @ShaneBeee and @sovdeeth review: command usage strings are 100%
Skripter-controlled, so parseSafe() restricts unsafe color tags from
ever working in usage entries. Switch to TextComponentParser#parse()
to allow the full tag set, matching the precedent in SkriptUpdater /
SkriptLogger / Skript admin output paths. Updated the Javadoc to
document why unsafe parsing is appropriate for server-controlled text.

Verified locally with `./gradlew compileJava` (BUILD SUCCESSFUL).
@mvanhorn
Copy link
Copy Markdown
Author

mvanhorn commented May 5, 2026

Switched to parse() in 156b2b0 per @ShaneBeee and @sovdeeth -- since command usage is 100% Skripter-controlled and unsafe tags can't otherwise be used in the entry, the safe-only restriction was over-tight. Updated the Javadoc to document why unsafe parsing is appropriate here. Verified locally with ./gradlew compileJava (BUILD SUCCESSFUL on Java 17).

@sovdeeth sovdeeth added the bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. label May 5, 2026
@APickledWalrus
Copy link
Copy Markdown
Member

You'll need to update this to target dev/patch rather than master (should be able to change at the top)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. needs reviews A PR that needs additional reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Command usage message doesn't support colors

4 participants