Rework command alias handling, step 1

composer.json

@@ -42,7 +42,7 @@
       "pocketmine/callback-validator": "dev-rewrite",
       "pocketmine/color": "^0.3.0",
       "pocketmine/errorhandler": "^0.7.0",
-      "pocketmine/locale-data": "~2.26.0",
+      "pocketmine/locale-data": "~2.27.0",
       "pocketmine/log": "^0.4.0",
       "pocketmine/math": "dev-major-next as 1.0.0",
       "pocketmine/nbt": "~1.2.0",

src/Server.php

@@ -680,11 +680,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;
-		}
+		$command = $this->commandMap->getCommand($name);
+		return $command instanceof PluginOwned ? $command : null;
 	}
 
 	public function getNameBans() : BanList{

src/command/ClosureCommand.php

@@ -38,19 +38,19 @@ final class ClosureCommand extends Command{
 	 * @phpstan-param Execute $execute
 	 */
 	public function __construct(
+		string $namespace,
 		string $name,
 		\Closure $execute,
 		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($namespace, $name, $description, $usageMessage);
 		$this->setPermissions($permissions);
 	}
 

src/command/Command.php

@@ -33,51 +33,36 @@
 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 function strtolower;
+use function trim;
 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;
+	private readonly string $namespace;
+	private readonly string $name;
 
 	/** @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(
+		string $namespace,
+		string $name,
+		private Translatable|string $description = "",
+		private Translatable|string|null $usageMessage = null
+	){
+		if($namespace === ""){
+			throw new \InvalidArgumentException("Command namespace cannot be empty (set it to, for example, your plugin's name)");
+		}
+		if($name === ""){
+			throw new \InvalidArgumentException("Command name cannot be empty");
+		}
+		$this->namespace = strtolower(trim($namespace));
+		//TODO: case handling inconsistency preserved from old code
+		$this->name = trim($name);
 	}
 
 	/**
@@ -89,10 +74,25 @@ public function __construct(string $name, Translatable|string $description = "",
 	 */
 	abstract public function execute(CommandSender $sender, string $commandLabel, array $args);
 
-	public function getName() : string{
+	final public function getNamespace() : string{
+		return $this->namespace;
+	}
+
+	/**
+	 * Returns the local identifier of the command (without namespace or leading slash).
+	 * This cannot be changed after creation.
+	 */
+	final public function getName() : string{
 		return $this->name;
 	}
 
+	/**
+	 * Returns the globally unique ID for the command. This typically looks like namespace:name
+	 */
+	final public function getId() : string{
+		return "$this->namespace:$this->name";
+	}
+
 	/**
 	 * @return string[]
 	 */
@@ -117,12 +117,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 +148,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 +156,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 +168,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 +189,4 @@ public static function broadcastCommandMessage(CommandSender $source, Translatab
 			}
 		}
 	}
-
-	public function __toString() : string{
-		return $this->name;
-	}
 }