AvailableCommandsPacket: Clean up internals

this is still disgusting, but it's a little more bearable now.

src/pocketmine/network/mcpe/protocol/AvailableCommandsPacket.php

@@ -31,11 +31,6 @@ use pocketmine\network\mcpe\protocol\types\CommandData;
 use pocketmine\network\mcpe\protocol\types\CommandEnum;
 use pocketmine\network\mcpe\protocol\types\CommandParameter;
 use pocketmine\utils\BinaryDataException;
-use function array_flip;
-use function array_keys;
-use function array_map;
-use function array_search;
-use function array_values;
 use function count;
 use function dechex;
 
@@ -85,30 +80,6 @@ class AvailableCommandsPacket extends DataPacket implements ClientboundPacket{
 	 */
 	public const ARG_FLAG_POSTFIX = 0x1000000;
 
-	/**
-	 * @var string[]
-	 * A list of every single enum value for every single command in the packet, including alias names.
-	 */
-	public $enumValues = [];
-	/** @var int */
-	private $enumValuesCount = 0;
-
-	/**
-	 * @var string[]
-	 * A list of argument postfixes. Used for the /xp command's <int>L.
-	 */
-	public $postfixes = [];
-
-	/**
-	 * @var CommandEnum[]
-	 * List of command enums, from command aliases to argument enums.
-	 */
-	public $enums = [];
-	/**
-	 * @var int[] string => int map of enum name to index
-	 */
-	private $enumMap = [];
-
 	/**
 	 * @var CommandData[]
 	 * List of command data, including name, description, alias indexes and parameters.
@@ -123,20 +94,26 @@ class AvailableCommandsPacket extends DataPacket implements ClientboundPacket{
 	public $softEnums = [];
 
 	protected function decodePayload() : void{
-		for($i = 0, $this->enumValuesCount = $this->getUnsignedVarInt(); $i < $this->enumValuesCount; ++$i){
+		/** @var string[] $enumValues */
-			$this->enumValues[] = $this->getString();
+		$enumValues = [];
+		for($i = 0, $enumValuesCount = $this->getUnsignedVarInt(); $i < $enumValuesCount; ++$i){
+			$enumValues[] = $this->getString();
+		}
+
+		/** @var string[] $postfixes */
+		$postfixes = [];
+		for($i = 0, $count = $this->getUnsignedVarInt(); $i < $count; ++$i){
+			$postfixes[] = $this->getString();
+		}
+
+		/** @var CommandEnum[] $enums */
+		$enums = [];
+		for($i = 0, $count = $this->getUnsignedVarInt(); $i < $count; ++$i){
+			$enums[] = $this->getEnum($enumValues);
 		}
 
 		for($i = 0, $count = $this->getUnsignedVarInt(); $i < $count; ++$i){
-			$this->postfixes[] = $this->getString();
+			$this->commandData[] = $this->getCommandData($enums, $postfixes);
-		}
-
-		for($i = 0, $count = $this->getUnsignedVarInt(); $i < $count; ++$i){
-			$this->enums[] = $this->getEnum();
-		}
-
-		for($i = 0, $count = $this->getUnsignedVarInt(); $i < $count; ++$i){
-			$this->commandData[] = $this->getCommandData();
 		}
 
 		for($i = 0, $count = $this->getUnsignedVarInt(); $i < $count; ++$i){
@@ -145,21 +122,25 @@ class AvailableCommandsPacket extends DataPacket implements ClientboundPacket{
 	}
 
 	/**
+	 * @param string[] $enumValueList
+	 *
 	 * @return CommandEnum
 	 * @throws BadPacketException
 	 * @throws BinaryDataException
 	 */
-	protected function getEnum() : CommandEnum{
+	protected function getEnum(array $enumValueList) : CommandEnum{
 		$retval = new CommandEnum();
 		$retval->enumName = $this->getString();
 
+		$listSize = count($enumValueList);
+
 		for($i = 0, $count = $this->getUnsignedVarInt(); $i < $count; ++$i){
-			$index = $this->getEnumValueIndex();
+			$index = $this->getEnumValueIndex($listSize);
-			if(!isset($this->enumValues[$index])){
+			if(!isset($enumValueList[$index])){
 				throw new BadPacketException("Invalid enum value index $index");
 			}
 			//Get the enum value from the initial pile of mess
-			$retval->enumValues[] = $this->enumValues[$index];
+			$retval->enumValues[] = $enumValueList[$index];
 		}
 
 		return $retval;
@@ -181,17 +162,21 @@ class AvailableCommandsPacket extends DataPacket implements ClientboundPacket{
 		return $retval;
 	}
 
-	protected function putEnum(CommandEnum $enum) : void{
+	/**
+	 * @param CommandEnum $enum
+	 * @param string[]    $enumValueMap
+	 */
+	protected function putEnum(CommandEnum $enum, array $enumValueMap) : void{
 		$this->putString($enum->enumName);
 
 		$this->putUnsignedVarInt(count($enum->enumValues));
+		$listSize = count($enumValueMap);
 		foreach($enum->enumValues as $value){
-			//Dumb bruteforce search. I hate this packet.
+			$index = $enumValueMap[$value] ?? -1;
-			$index = array_search($value, $this->enumValues, true);
+			if($index === -1){
-			if($index === false){
 				throw new \InvalidStateException("Enum value '$value' not found");
 			}
-			$this->putEnumValueIndex($index);
+			$this->putEnumValueIndex($index, $listSize);
 		}
 	}
 
@@ -205,23 +190,25 @@ class AvailableCommandsPacket extends DataPacket implements ClientboundPacket{
 	}
 
 	/**
+	 * @param int $valueCount
+	 *
 	 * @return int
 	 * @throws BinaryDataException
 	 */
-	protected function getEnumValueIndex() : int{
+	protected function getEnumValueIndex(int $valueCount) : int{
-		if($this->enumValuesCount < 256){
+		if($valueCount < 256){
 			return $this->getByte();
-		}elseif($this->enumValuesCount < 65536){
+		}elseif($valueCount < 65536){
 			return $this->getLShort();
 		}else{
 			return $this->getLInt();
 		}
 	}
 
-	protected function putEnumValueIndex(int $index) : void{
+	protected function putEnumValueIndex(int $index, int $valueCount) : void{
-		if($this->enumValuesCount < 256){
+		if($valueCount < 256){
 			$this->putByte($index);
-		}elseif($this->enumValuesCount < 65536){
+		}elseif($valueCount < 65536){
 			$this->putLShort($index);
 		}else{
 			$this->putLInt($index);
@@ -229,17 +216,20 @@ class AvailableCommandsPacket extends DataPacket implements ClientboundPacket{
 	}
 
 	/**
+	 * @param CommandEnum[] $enums
+	 * @param string[]      $postfixes
+	 *
 	 * @return CommandData
 	 * @throws BadPacketException
 	 * @throws BinaryDataException
 	 */
-	protected function getCommandData() : CommandData{
+	protected function getCommandData(array $enums, array $postfixes) : CommandData{
 		$retval = new CommandData();
 		$retval->commandName = $this->getString();
 		$retval->commandDescription = $this->getString();
 		$retval->flags = $this->getByte();
 		$retval->permission = $this->getByte();
-		$retval->aliases = $this->enums[$this->getLInt()] ?? null;
+		$retval->aliases = $enums[$this->getLInt()] ?? null;
 
 		for($overloadIndex = 0, $overloadCount = $this->getUnsignedVarInt(); $overloadIndex < $overloadCount; ++$overloadIndex){
 			for($paramIndex = 0, $paramCount = $this->getUnsignedVarInt(); $paramIndex < $paramCount; ++$paramIndex){
@@ -251,13 +241,13 @@ class AvailableCommandsPacket extends DataPacket implements ClientboundPacket{
 
 				if($parameter->paramType & self::ARG_FLAG_ENUM){
 					$index = ($parameter->paramType & 0xffff);
-					$parameter->enum = $this->enums[$index] ?? null;
+					$parameter->enum = $enums[$index] ?? null;
 					if($parameter->enum === null){
 						throw new BadPacketException("deserializing $retval->commandName parameter $parameter->paramName: expected enum at $index, but got none");
 					}
 				}elseif($parameter->paramType & self::ARG_FLAG_POSTFIX){
 					$index = ($parameter->paramType & 0xffff);
-					$parameter->postfix = $this->postfixes[$index] ?? null;
+					$parameter->postfix = $postfixes[$index] ?? null;
 					if($parameter->postfix === null){
 						throw new BadPacketException("deserializing $retval->commandName parameter $parameter->paramName: expected postfix at $index, but got none");
 					}
@@ -272,14 +262,19 @@ class AvailableCommandsPacket extends DataPacket implements ClientboundPacket{
 		return $retval;
 	}
 
-	protected function putCommandData(CommandData $data) : void{
+	/**
+	 * @param CommandData $data
+	 * @param int[]       $enumIndexes string enum name -> int index
+	 * @param int[]       $postfixIndexes
+	 */
+	protected function putCommandData(CommandData $data, array $enumIndexes, array $postfixIndexes) : void{
 		$this->putString($data->commandName);
 		$this->putString($data->commandDescription);
 		$this->putByte($data->flags);
 		$this->putByte($data->permission);
 
 		if($data->aliases !== null){
-			$this->putLInt($this->enumMap[$data->aliases->enumName] ?? -1);
+			$this->putLInt($enumIndexes[$data->aliases->enumName] ?? -1);
 		}else{
 			$this->putLInt(-1);
 		}
@@ -292,10 +287,10 @@ class AvailableCommandsPacket extends DataPacket implements ClientboundPacket{
 				$this->putString($parameter->paramName);
 
 				if($parameter->enum !== null){
-					$type = self::ARG_FLAG_ENUM | self::ARG_FLAG_VALID | ($this->enumMap[$parameter->enum->enumName] ?? -1);
+					$type = self::ARG_FLAG_ENUM | self::ARG_FLAG_VALID | ($enumIndexes[$parameter->enum->enumName] ?? -1);
 				}elseif($parameter->postfix !== null){
-					$key = array_search($parameter->postfix, $this->postfixes, true);
+					$key = $postfixIndexes[$parameter->postfix] ?? -1;
-					if($key === false){
+					if($key === -1){
 						throw new \InvalidStateException("Postfix '$parameter->postfix' not in postfixes array");
 					}
 					$type = self::ARG_FLAG_POSTFIX | $key;
@@ -310,7 +305,7 @@ class AvailableCommandsPacket extends DataPacket implements ClientboundPacket{
 		}
 	}
 
-	private function argTypeToString(int $argtype) : string{
+	private function argTypeToString(int $argtype, array $postfixes) : string{
 		if($argtype & self::ARG_FLAG_VALID){
 			if($argtype & self::ARG_FLAG_ENUM){
 				return "stringenum (" . ($argtype & 0xffff) . ")";
@@ -339,7 +334,7 @@ class AvailableCommandsPacket extends DataPacket implements ClientboundPacket{
 					return "command";
 			}
 		}elseif($argtype & self::ARG_FLAG_POSTFIX){
-			$postfix = $this->postfixes[$argtype & 0xffff];
+			$postfix = $postfixes[$argtype & 0xffff];
 
 			return "int (postfix $postfix)";
 		}else{
@@ -350,15 +345,22 @@ class AvailableCommandsPacket extends DataPacket implements ClientboundPacket{
 	}
 
 	protected function encodePayload() : void{
-		$enumValuesMap = [];
+		/** @var int[] $enumValueIndexes */
-		$postfixesMap = [];
+		$enumValueIndexes = [];
-		$enumMap = [];
+		/** @var int[] $postfixIndexes */
+		$postfixIndexes = [];
+		/** @var int[] $enumIndexes */
+		$enumIndexes = [];
+		/** @var CommandEnum[] $enums */
+		$enums = [];
 		foreach($this->commandData as $commandData){
 			if($commandData->aliases !== null){
-				$enumMap[$commandData->aliases->enumName] = $commandData->aliases;
+				if(!isset($enumIndexes[$commandData->aliases->enumName])){
+					$enums[$enumIndexes[$commandData->aliases->enumName] = count($enumIndexes)] = $commandData->aliases;
+				}
 
 				foreach($commandData->aliases->enumValues as $str){
-					$enumValuesMap[$str] = true;
+					$enumValueIndexes[$str] = $enumValueIndexes[$str] ?? count($enumValueIndexes); //latest index
 				}
 			}
 
@@ -369,41 +371,39 @@ class AvailableCommandsPacket extends DataPacket implements ClientboundPacket{
 				 */
 				foreach($overload as $parameter){
 					if($parameter->enum !== null){
-						$enumMap[$parameter->enum->enumName] = $parameter->enum;
+						if(!isset($enumIndexes[$parameter->enum->enumName])){
+							$enums[$enumIndexes[$parameter->enum->enumName] = count($enumIndexes)] = $parameter->enum;
+						}
 						foreach($parameter->enum->enumValues as $str){
-							$enumValuesMap[$str] = true;
+							$enumValueIndexes[$str] = $enumValueIndexes[$str] ?? count($enumValueIndexes);
 						}
 					}
 
 					if($parameter->postfix !== null){
-						$postfixesMap[$parameter->postfix] = true;
+						$postfixIndexes[$parameter->postfix] = $postfixIndexes[$parameter->postfix] ?? count($postfixIndexes);
 					}
 				}
 			}
 		}
 
-		$this->enumValues = array_map('\strval', array_keys($enumValuesMap)); //stupid PHP key casting D:
+		$this->putUnsignedVarInt(count($enumValueIndexes));
-		$this->putUnsignedVarInt($this->enumValuesCount = count($this->enumValues));
+		foreach($enumValueIndexes as $enumValue => $index){
-		foreach($this->enumValues as $enumValue){
+			$this->putString((string) $enumValue); //stupid PHP key casting D:
-			$this->putString($enumValue);
 		}
 
-		$this->postfixes = array_map('\strval', array_keys($postfixesMap));
+		$this->putUnsignedVarInt(count($postfixIndexes));
-		$this->putUnsignedVarInt(count($this->postfixes));
+		foreach($postfixIndexes as $postfix => $index){
-		foreach($this->postfixes as $postfix){
+			$this->putString((string) $postfix); //stupid PHP key casting D:
-			$this->putString($postfix);
 		}
 
-		$this->enums = array_values($enumMap);
+		$this->putUnsignedVarInt(count($enums));
-		$this->enumMap = array_flip(array_keys($enumMap));
+		foreach($enums as $enum){
-		$this->putUnsignedVarInt(count($this->enums));
+			$this->putEnum($enum, $enumValueIndexes);
-		foreach($this->enums as $enum){
-			$this->putEnum($enum);
 		}
 
 		$this->putUnsignedVarInt(count($this->commandData));
 		foreach($this->commandData as $data){
-			$this->putCommandData($data);
+			$this->putCommandData($data, $enumIndexes, $postfixIndexes);
 		}
 
 		$this->putUnsignedVarInt(count($this->softEnums));