Rework command alias handling, step 1

src/Server.php

@@ -28,6 +28,7 @@
 namespace pocketmine;
 
 use pocketmine\command\Command;
+use pocketmine\command\CommandMapEntry;
 use pocketmine\command\CommandSender;
 use pocketmine\command\SimpleCommandMap;
 use pocketmine\console\ConsoleCommandSender;
@@ -680,11 +681,8 @@ public function getConfigGroup() : ServerConfigGroup{
 	 * @phpstan-return (Command&PluginOwned)|null
 	 */
 	public function getPluginCommand(string $name){
-		if(($command = $this->commandMap->getCommand($name)) instanceof PluginOwned){
-			return $command;
-		}else{
-			return null;
-		}
+		$entry = $this->commandMap->getEntry($name);
+		return $entry instanceof CommandMapEntry && $entry->command instanceof PluginOwned ? $entry->command : null;
 	}
 
 	public function getNameBans() : BanList{

src/command/ClosureCommand.php

@@ -43,14 +43,13 @@ public function __construct(
 		array $permissions,
 		Translatable|string $description = "",
 		Translatable|string|null $usageMessage = null,
-		array $aliases = []
 	){
 		Utils::validateCallableSignature(
 			fn(CommandSender $sender, Command $command, string $commandLabel, array $args) : mixed => 1,
 			$execute,
 		);
 		$this->execute = $execute;
-		parent::__construct($name, $description, $usageMessage, $aliases);
+		parent::__construct($name, $description, $usageMessage);
 		$this->setPermissions($permissions);
 	}
 

src/command/Command.php

@@ -33,52 +33,21 @@
 use pocketmine\Server;
 use pocketmine\utils\BroadcastLoggerForwarder;
 use pocketmine\utils\TextFormat;
-use function array_values;
 use function explode;
 use function implode;
 use function str_replace;
 use const PHP_INT_MAX;
 
 abstract class Command{
-
-	private string $name;
-
-	private string $nextLabel;
-	private string $label;
-
-	/**
-	 * @var string[]
-	 * @phpstan-var list<string>
-	 */
-	private array $aliases = [];
-
-	/**
-	 * @var string[]
-	 * @phpstan-var list<string>
-	 */
-	private array $activeAliases = [];
-
-	private ?CommandMap $commandMap = null;
-
-	protected Translatable|string $description = "";
-
-	protected Translatable|string $usageMessage;
-
 	/** @var string[] */
 	private array $permission = [];
 	private Translatable|string|null $permissionMessage = null;
 
-	/**
-	 * @param string[] $aliases
-	 * @phpstan-param list<string> $aliases
-	 */
-	public function __construct(string $name, Translatable|string $description = "", Translatable|string|null $usageMessage = null, array $aliases = []){
-		$this->name = $name;
-		$this->setLabel($name);
-		$this->setDescription($description);
-		$this->usageMessage = $usageMessage ?? ("/" . $name);
-		$this->setAliases($aliases);
-	}
+	public function __construct(
+		private string $name,
+		private Translatable|string $description = "",
+		private Translatable|string|null $usageMessage = null
+	){}
 
 	/**
 	 * @param string[] $args
@@ -89,6 +58,10 @@ public function __construct(string $name, Translatable|string $description = "",
 	 */
 	abstract public function execute(CommandSender $sender, string $commandLabel, array $args);
 
+	/**
+	 * Returns the local identifier of the command (without namespace or leading slash).
+	 * This cannot be changed after creation.
+	 */
 	public function getName() : string{
 		return $this->name;
 	}
@@ -117,12 +90,17 @@ public function setPermission(?string $permission) : void{
 		$this->setPermissions($permission === null ? [] : explode(";", $permission, limit: PHP_INT_MAX));
 	}
 
-	public function testPermission(CommandSender $target, ?string $permission = null) : bool{
+	/**
+	 * @param string        $context    usually the command name, but may include extra args if useful (e.g. for subcommands)
+	 * @param CommandSender $target     the target to check the permission for
+	 * @param string|null   $permission the permission to check, if null, will check if the target has any of the command's permissions
+	 */
+	public function testPermission(string $context, CommandSender $target, ?string $permission = null) : bool{
 		if($this->testPermissionSilent($target, $permission)){
 			return true;
 		}
 
-		$message = $this->permissionMessage ?? KnownTranslationFactory::pocketmine_command_error_permission($this->name);
+		$message = $this->permissionMessage ?? KnownTranslationFactory::pocketmine_command_error_permission($context);
 		if($message instanceof Translatable){
 			$target->sendMessage($message->prefix(TextFormat::RED));
 		}elseif($message !== ""){
@@ -143,62 +121,6 @@ public function testPermissionSilent(CommandSender $target, ?string $permission
 		return false;
 	}
 
-	public function getLabel() : string{
-		return $this->label;
-	}
-
-	public function setLabel(string $name) : bool{
-		$this->nextLabel = $name;
-		if(!$this->isRegistered()){
-			$this->label = $name;
-
-			return true;
-		}
-
-		return false;
-	}
-
-	/**
-	 * Registers the command into a Command map
-	 */
-	public function register(CommandMap $commandMap) : bool{
-		if($this->allowChangesFrom($commandMap)){
-			$this->commandMap = $commandMap;
-
-			return true;
-		}
-
-		return false;
-	}
-
-	public function unregister(CommandMap $commandMap) : bool{
-		if($this->allowChangesFrom($commandMap)){
-			$this->commandMap = null;
-			$this->activeAliases = $this->aliases;
-			$this->label = $this->nextLabel;
-
-			return true;
-		}
-
-		return false;
-	}
-
-	private function allowChangesFrom(CommandMap $commandMap) : bool{
-		return $this->commandMap === null || $this->commandMap === $commandMap;
-	}
-
-	public function isRegistered() : bool{
-		return $this->commandMap !== null;
-	}
-
-	/**
-	 * @return string[]
-	 * @phpstan-return list<string>
-	 */
-	public function getAliases() : array{
-		return $this->activeAliases;
-	}
-
 	public function getPermissionMessage() : Translatable|string|null{
 		return $this->permissionMessage;
 	}
@@ -207,22 +129,10 @@ public function getDescription() : Translatable|string{
 		return $this->description;
 	}
 
-	public function getUsage() : Translatable|string{
+	public function getUsage() : Translatable|string|null{
 		return $this->usageMessage;
 	}
 
-	/**
-	 * @param string[] $aliases
-	 * @phpstan-param list<string> $aliases
-	 */
-	public function setAliases(array $aliases) : void{
-		$aliases = array_values($aliases); //because plugins can and will pass crap
-		$this->aliases = $aliases;
-		if(!$this->isRegistered()){
-			$this->activeAliases = $aliases;
-		}
-	}
-
 	public function setDescription(Translatable|string $description) : void{
 		$this->description = $description;
 	}
@@ -231,7 +141,7 @@ public function setPermissionMessage(Translatable|string $permissionMessage) : v
 		$this->permissionMessage = $permissionMessage;
 	}
 
-	public function setUsage(Translatable|string $usage) : void{
+	public function setUsage(Translatable|string|null $usage) : void{
 		$this->usageMessage = $usage;
 	}
 
@@ -252,8 +162,4 @@ public static function broadcastCommandMessage(CommandSender $source, Translatab
 			}
 		}
 	}
-
-	public function __toString() : string{
-		return $this->name;
-	}
 }

src/command/CommandMap.php

@@ -24,18 +24,35 @@
 namespace pocketmine\command;
 
 interface CommandMap{
-
 	/**
-	 * @param Command[] $commands
+	 * Registering a command with (namespace="myplugin", command(name="mycommand"), otherAliases=["myc"]) will bind:
+	 * - /myplugin:mycommand (always works, error thrown if not unique)
+	 * - /mycommand (only works if not conflicted, not required to be unique)
+	 * - /myc (only works if not conflicted, not required to be unique)
+	 *
+	 * If two commands claim the same alias, it will become conflicted, and neither command will be usable with that
+	 * alias unless the alias is explicitly rebound with registerAlias().
+	 * The user will be shown an error when trying to use it, listing all namespaced names (not aliases) of the commands
+	 * bound to it. The user can then use one of the namespaced names to run the command they want.
+	 * Conflicted aliases will not be included in the returned CommandMapEntry.
+	 *
+	 * @param string[] $otherAliases
+	 *
+	 * @phpstan-param list<string> $otherAliases
 	 */
-	public function registerAll(string $fallbackPrefix, array $commands) : void;
-
-	public function register(string $fallbackPrefix, Command $command, ?string $label = null) : bool;
+	public function register(string $namespace, Command $command, array $otherAliases = []) : CommandMapEntry;
 
 	public function dispatch(CommandSender $sender, string $cmdLine) : bool;
 
 	public function clearCommands() : void;
 
-	public function getCommand(string $name) : ?Command;
+	/**
+	 * Returns the command(s) bound to the given name or alias.
+	 * This will return an array if the alias is conflicted (multiple commands bound to it).
+	 *
+	 * @return CommandMapEntry|CommandMapEntry[]|null
+	 * @phpstan-return CommandMapEntry|array<int, CommandMapEntry>|null
+	 */
+	public function getEntry(string $name) : CommandMapEntry|array|null;
 
 }

src/command/CommandMapEntry.php

@@ -0,0 +1,53 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+ */
+
+declare(strict_types=1);
+
+namespace pocketmine\command;
+
+use pocketmine\lang\Translatable;
+
+final class CommandMapEntry{
+
+	/**
+	 * @param string[] $aliases
+	 * @phpstan-param non-empty-list<string> $aliases
+	 */
+	public function __construct(
+		public readonly string $namespace,
+		public readonly Command $command,
+		public readonly array $aliases
+	){}
+
+	public function getNamespacedName() : string{
+		return $this->namespace . ":" . $this->command->getName();
+	}
+
+	public function getPreferredAlias() : string{
+		return $this->aliases[0];
+	}
+
+	public function getUsage() : Translatable|string{
+		//TODO: usage messages ought to use user-specified alias, not command preferred
+		//command-preferred is confusing if the user used a different alias
+		return $this->command->getUsage() ?? "/" . $this->getPreferredAlias();
+	}
+}

src/command/FormattedCommandAlias.php

@@ -50,10 +50,10 @@ class FormattedCommandAlias extends Command{
 	 * @param string[] $formatStrings
 	 */
 	public function __construct(
-		string $alias,
+		string $name,
 		private array $formatStrings
 	){
-		parent::__construct($alias, KnownTranslationFactory::pocketmine_command_userDefined_description());
+		parent::__construct($name, KnownTranslationFactory::pocketmine_command_userDefined_description());
 	}
 
 	public function execute(CommandSender $sender, string $commandLabel, array $args){
@@ -95,18 +95,20 @@ public function execute(CommandSender $sender, string $commandLabel, array $args
 				throw new AssumptionFailedError("This should have been checked before construction");
 			}
 
-			if(($target = $commandMap->getCommand($commandLabel)) !== null){
-				$timings = Timings::getCommandDispatchTimings($target->getLabel());
+			if(($target = $commandMap->getEntry($commandLabel)) instanceof CommandMapEntry){
+
+				$timings = Timings::getCommandDispatchTimings($target->getNamespacedName());
 				$timings->startTiming();
 
 				try{
-					$target->execute($sender, $commandLabel, $commandArgs);
+					$target->command->execute($sender, $commandLabel, $commandArgs);
 				}catch(InvalidCommandSyntaxException $e){
 					$sender->sendMessage($sender->getLanguage()->translate(KnownTranslationFactory::commands_generic_usage($target->getUsage())));
 				}finally{
 					$timings->stopTiming();
 				}
 			}else{
+				//TODO: this seems suspicious - why do we continue alias execution if one of the commands is borked?
 				$sender->sendMessage($sender->getLanguage()->translate(KnownTranslationFactory::pocketmine_command_notFound($commandLabel, "/help")->prefix(TextFormat::RED)));
 
 				//to match the behaviour of SimpleCommandMap::dispatch()