Firebombing of commands for the first time in 10 years #6837

dktapps · 2025-10-12T02:34:50Z

This PR implements a command overloading system. Partially inspired by @alvin0319's prior (abandoned) work on #3871, but with deeper integration.

Relevant issues

Closes Command overloading, & automatic command argument parsing & validation system #6518
Closes Client-side command hints #6517 (well, not entirely, but basic support is here)

Highlights

No more nasty switch-cases based on the number of args like we famously had in the /tp command
No more checking if(isset($args[$offset])) - your callback simply won't be called if the number of arguments is wrong or the arguments can't be parsed the way you asked for
Different operations of commands can be implemented in different functions, instead of putting them all into one giant if/elseif/else or a switch
Very basic client side command hints are now supported
Command usage messages are now built from overload parameter infos, instead of needing to be manually written
Commands may have multiple usage messages (one per overload)
/me, /say, /tell now handle quoted inputs correctly - previously /say hello "some user" would broadcast hello some user, but now broadcasts exactly what you typed, i.e. hello "some user"

How it works

Commands register one or more callbacks, along with parameter description information.
- The callback can accept native types of the appropriate args. For example, if you want an int, you can accept int $param
- The number of required args is inferred from the callback given by reflection. For example, if you have an optional string parameter, you can do ?string $param = null and check for null to see if the parameter was specified or not.
- Parameter descriptions are required along with the callable, to tell the system about stuff like:
  - Translatable name of the parameter, used in usage messages and client side suggestions
  - Accepted values, min/max etc
  - How to convert the raw string into the type you want (e.g. for parsing an arg into an Item object; see MappedParameter, and how it's used in commands like /enchant, /effect
  - Literals - these are provided as raw strings instead of Parameter objects - these aren't given to the callback, but must be typed exactly in the position specified (e.g. a subcommand name); see how these are used for /effect <target> clear, /time set <timestamp> etc
- While not easily statically analysed, the command callback will be verified upon registration using reflection against the provided parameter infos.
  - Your command callback must accept at least the types indicated by your parameter infos.
  - You can also accept wider (contravariant) types in your callback's parameters (e.g. you can have IntRangeParameter("xp") whose value is accepted by int $xp, ?int $xp, int|string $xp, but not string $xp)
  - As mentioned above, literals are not passed to the callback.
When a command string is received, Command->executeOverloaded() is called
Each overload is tried one by one.
- The first one that succeeds in parsing the whole command string with no left over tokens is used to run the command. (This is possibly not optimal, but it's the simplest way to do this for now.)
- If no overloads can parse all the args successfully, the usages of all overloads are displayed to the user and no overload is called

Gotchas

No conflict resolution is done. If two overloads could both accept the user's input, the first one registered will be called currently. This may be changed in the future.
Literal matching is done by bruteforce search. This might be slow for subcommands. This is an active area of research.

Follow-up

Implement support for composite parameters like x y z (which are actually 3x RelativeFloat parameters)
Implement support for fixed enums
Implement support for dynamic (soft) enums (this will be non-trivial, because there'll need to be a way to sync updates to clients)
Add translation keys for various parameter names to pocketmine/locale-data
Implement ambiguous overload detection (when a command line could be accepted by multiple overloads, we currently just use the first one)
Explore auto-inferring Parameter infos from a provided callback instead of having them explicitly specified
- This would be fine when all the info needed to parse the parameter is contained in the type, but wouldn't work for stuff like int/float bounds, dynamic enums, or mapped parameters
- Attributes like #[Bounds(0, 100)] int $param could be explored, but this would only work for simple stuff known ahead of time
Implement support for command selectors (Command target selectors #6519)
Get rid of plugin.yml commands, which are hella dated, awful, and can't take advantage of this cool new system (Scrap plugin.yml commands #6506)

API changes

Command->__construct() now requires a non-empty list in array $overloads
The following methods are added:
- Command->executeOverloaded() - called by SimpleCommandMap
- Command->getOverloads()
- Command->getUsages() - returns usages for all overloads the given CommandSender has permissions for
- Command::fetchPermittedPlayerTarget() - similar to VanillaCommand->fetchPermittedPlayerTarget(), but now accessible to all commands instead of just VanillaCommand inheritors
- CommandStringHelper::parseQuoteAwareSingle() - parses a "quoted string \"like this\"" into quoted string "like this" - similar to parseQuoteAware() but only parses 1 arg instead of the entire command
The following methods are removed:
- Command->getPermissions() - permissions are now defined per-overload instead of for the whole command
- Command->setPermissions()
- Command->testPermission()
- Command->testPermissionSilent()
- Command->getUsage() - command usages are now derived from overloads, and there may be more than one per command
- Command->setUsage()
The following classes are removed:
- VanillaCommand - no longer served any purpose since it was all about manual arg parsing
The following classes are added:
- LegacyCommand - provides a similar API to the old-style Command for a quick & dirty upgrade path for old code if you don't want to switch to the new system immediately (this is also still used by ClosureCommand, FormattedCommandAlias and PluginCommand for the time being)
  - LegacyCommand will use your old-style hardcoded usage message if provided, but you should consider switching to the new system. This is only provided to ease upgrading and won't stick around forever.
- Parameter and various subclasses in pocketmine\command\overload package
- CommandOverload - accepts parameter info and callbacks to be given to Command->__construct()
- ParameterParseException - thrown when a Parameter can't parse a command token
Utils::validateCallableSignature() now accepts Closure|Prototype for both arguments

alvin0319 · 2025-10-12T03:11:05Z

Is it safe to use non-mb_* prefixed methods while parsing? Haven't looked code deeply but I saw a lot of substr and strlen used in code - wouldn't this affect to non-english commands?

dktapps · 2025-10-12T12:03:03Z

It should be fine. We only care about byte lengths, and splittable whitespace chars will never be more than 1 byte.

this was a problem with cmdalias, where just writing cmdalias alone would invoke cmdalias list. In itself this wasn't particularly harmful, but it could've been a problem for other commands like timings.

Since it only compares the exact literal bytes, it's possible to have trailing characters that don't match. For example, /timings ons would crash because the 'on' overload would match, but leave a trailing 's' behind.

we only care if it's in the exact position defined by $offset.

this doesn't support enum or player name auto complete for now since the API has no way to expose that information. however, we can get some of the benefits with basic types xp <level>L is not supported because the stupid fucking client crashes when it's used and I wasted all day trying to debug it. Someone else can fix that if they care.

dktapps · 2025-10-13T19:07:39Z

@inxomnyaa raised concerns about performance when processing subcommands. I'm not sure to what extent this is a problem, but I have seen some plugins create much more complicated command signatures than vanilla. The way that overloading works, "subcommands" can be implemented as overloads using literals in this system, but it currently has to try each one, one at a time, to find a matching overload. This could become a problem if many overloads are present. This is sometimes the case with complex plugin commands, where nested commands blow up to create a large number of overload permutations.

Possibly, it might be advantageous to detect which overloads have the same first N parameters, so that if other overloads have the same leading arguments, we can eliminate multiple overloads at once in some cases without having to reparse the entire command over and over again.

It might also be worth exploring whether we can avoid bruteforce lookups when some or all overloads have a literal as their first parameter(s). This would be a common case for subcommands.

SOF3 · 2025-10-16T02:26:09Z

src/command/Command.php

 use function trim;
-use const PHP_INT_MAX;

 abstract class Command{


Should Command be non-abstract now? There are no abstract functions, and there doesn't seem to be a good reason why we need to force users to create a separate class for each command.

Good catch. Maybe this also makes ClosureCommand obsolete too.

SOF3 · 2025-10-16T02:44:05Z

Optimizing overload selection shouldn't be a blocker for this PR. Plugins that are particularly concerned about huge number of overloads could always fall back to RawParameter to leverage their own optimized parsing algorithm.

dktapps · 2025-10-16T03:12:45Z

Optimizing overload selection shouldn't be a blocker for this PR. Plugins that are particularly concerned about huge number of overloads could always fall back to RawParameter to leverage their own optimized parsing algorithm.

Yeah, but then they'd lose client-side command hints, which we all know is the thing plugin devs really care about, for some reason...

WenoxGB · 2025-10-17T17:47:39Z

any plan to add PlayerArgument ?

Adds a tick() method to populate static::$VALUES with online player names,
quoted if they contain spaces. This improves compatibility with command input
parsing and tab-completion for string-based arguments.

PlayerArgument resolves actual Player instances, not raw name strings. This
approach allows dynamic, formatted suggestions without requiring resolution,
making it suitable for custom argument types that expect player names as strings.

Implementation:

Iterates over Server::getInstance()->getOnlinePlayers()
Quotes names with spaces: "Mr Hacker"
Leaves simple names untouched: Steve, Alex
Uses array_combine() to build a key-value map for suggestion use

dktapps · 2025-10-17T18:13:41Z

I don't know about the tick stuff, that doesn't seem necessary to me, but yes there are plans to expand the system.

I'm currently unsure how to deal with target args because of the .self vs .other permissions for various commands.

Enums, soft (dynamic) enums, and composite types like Vector3 are also not supported yet.

This PR implements the minimum of what was needed to get PM's builtin commands moved over to the new system.

john-bv

Just wanted to leave some of my thoughts on here :)

src/command/defaults/ClearCommand.php

src/command/overload/CommandOverload.php

src/command/overload/Parameter.php

john-bv · 2025-10-20T08:00:34Z

src/network/mcpe/NetworkSession.php

+
+			$overloads = [];
+			foreach($command->getOverloads() as $overload){
+				//TODO: we should only send permissible overloads here


Is there a reason this was left as todo? Does it cause problems with the client or something?

Never got around to it

src/network/mcpe/NetworkSession.php

src/command/Command.php

src/network/mcpe/NetworkSession.php

inxomnyaa · 2025-10-22T18:08:27Z

src/network/mcpe/NetworkSession.php

+							//umm... client only allows suffixes on integer params???
+							//TODO: as of 1.21.111, the client crashes if we try to provide actual suffixes for /xp.
+							//The cause for this seems to be undefined behaviour on the client linked to the order and
+							//number of certain hardcoded enums like GameMode, IntParam etc which we don't provided. I


nitpick, *provide

inxomnyaa

Over all, complex, but i can't say i hate it. Good job

Firebombing of commands for the first time in 10 years

50976c2

dktapps added 6 commits October 12, 2025 14:37

Fixed trailing literals not being required

a314d2d

this was a problem with cmdalias, where just writing cmdalias alone would invoke cmdalias list. In itself this wasn't particularly harmful, but it could've been a problem for other commands like timings.

Fixed bad assumption about literals and subsequent whitespace

d0fca5e

Since it only compares the exact literal bytes, it's possible to have trailing characters that don't match. For example, /timings ons would crash because the 'on' overload would match, but leave a trailing 's' behind.

Don't scan the entire command string looking for literals

1ef5e04

we only care if it's in the exact position defined by $offset.

Clean up

371b991

Update comment about suffixed parameters

835faa4

SOF3 reviewed Oct 16, 2025

View reviewed changes

didn't realise I committed this

37632bc

WenoxGB mentioned this pull request Oct 18, 2025

Command Overloading Plan Update pixelforge-studios-PMMP/SkyblockCore#2

Open

john-bv reviewed Oct 20, 2025

View reviewed changes

dktapps added Category: API Related to the plugin API BC break Breaks API compatibility Type: Enhancement Contributes features or other improvements to PocketMine-MP labels Oct 21, 2025

inxomnyaa reviewed Oct 22, 2025

View reviewed changes

src/network/mcpe/NetworkSession.php Show resolved Hide resolved

inxomnyaa reviewed Oct 22, 2025

View reviewed changes

Uh oh!

Uh oh!

Firebombing of commands for the first time in 10 years #6837

Are you sure you want to change the base?

Firebombing of commands for the first time in 10 years #6837

Uh oh!

Conversation

dktapps commented Oct 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Relevant issues

Highlights

How it works

Gotchas

Follow-up

API changes

Uh oh!

alvin0319 commented Oct 12, 2025

Uh oh!

dktapps commented Oct 12, 2025

Uh oh!

dktapps commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SOF3 Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

dktapps Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

SOF3 commented Oct 16, 2025

Uh oh!

dktapps commented Oct 16, 2025

Uh oh!

WenoxGB commented Oct 17, 2025

Uh oh!

dktapps commented Oct 17, 2025

Uh oh!

john-bv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

john-bv Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

dktapps Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

inxomnyaa Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

inxomnyaa left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

dktapps commented Oct 12, 2025 •

edited

Loading

dktapps commented Oct 13, 2025 •

edited

Loading

inxomnyaa Oct 22, 2025 •

edited

Loading