Merge pull request #6416 from pmmp/blockstate-schema-generator-improv…

…ements

Blockstate schema generator improvements

src/data/bedrock/block/upgrade/BlockStateUpgradeSchema.php

@@ -23,6 +23,7 @@
 
 namespace pocketmine\data\bedrock\block\upgrade;
 
+use pocketmine\data\bedrock\block\upgrade\BlockStateUpgradeSchemaFlattenInfo as FlattenInfo;
 use pocketmine\data\bedrock\block\upgrade\BlockStateUpgradeSchemaValueRemap as ValueRemap;
 use pocketmine\nbt\tag\Tag;
 use function count;
@@ -58,6 +59,12 @@ final class BlockStateUpgradeSchema{
 	 */
 	public array $remappedPropertyValues = [];
 
+	/**
+	 * @var FlattenInfo[]
+	 * @phpstan-var array<string, FlattenInfo>
+	 */
+	public array $flattenedProperties = [];
+
 	/**
 	 * @var BlockStateUpgradeSchemaBlockRemap[][]
 	 * @phpstan-var array<string, list<BlockStateUpgradeSchemaBlockRemap>>
@@ -93,6 +100,7 @@ public function isEmpty() : bool{
 			$this->removedProperties,
 			$this->renamedProperties,
 			$this->remappedPropertyValues,
+			$this->flattenedProperties,
 			$this->remappedStates,
 		] as $list){
 			if(count($list) !== 0){

src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaBlockRemap.php

@@ -40,16 +40,16 @@ final class BlockStateUpgradeSchemaBlockRemap{
 	 */
 	public function __construct(
 		public array $oldState,
-		public string|BlockStateUpgradeSchemaFlattenedName $newName,
+		public string|BlockStateUpgradeSchemaFlattenInfo $newName,
 		public array $newState,
 		public array $copiedState
 	){}
 
 	public function equals(self $that) : bool{
 		$sameName = $this->newName === $that->newName ||
 			(
-				$this->newName instanceof BlockStateUpgradeSchemaFlattenedName &&
-				$that->newName instanceof BlockStateUpgradeSchemaFlattenedName &&
+				$this->newName instanceof BlockStateUpgradeSchemaFlattenInfo &&
+				$that->newName instanceof BlockStateUpgradeSchemaFlattenInfo &&
 				$this->newName->equals($that->newName)
 			);
 		if(!$sameName){

src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaFlattenedName.php → src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaFlattenInfo.php

@@ -23,20 +23,25 @@
 
 namespace pocketmine\data\bedrock\block\upgrade;
 
+use pocketmine\nbt\tag\ByteTag;
+use pocketmine\nbt\tag\IntTag;
+use pocketmine\nbt\tag\StringTag;
 use function ksort;
 use const SORT_STRING;
 
-final class BlockStateUpgradeSchemaFlattenedName{
+final class BlockStateUpgradeSchemaFlattenInfo{
 
 	/**
 	 * @param string[] $flattenedValueRemaps
 	 * @phpstan-param array<string, string> $flattenedValueRemaps
+	 * @phpstan-param ?class-string<ByteTag|IntTag|StringTag> $flattenedPropertyType
 	 */
 	public function __construct(
 		public string $prefix,
 		public string $flattenedProperty,
 		public string $suffix,
-		public array $flattenedValueRemaps
+		public array $flattenedValueRemaps,
+		public ?string $flattenedPropertyType = null
 	){
 		ksort($this->flattenedValueRemaps, SORT_STRING);
 	}
@@ -45,6 +50,7 @@ public function equals(self $that) : bool{
 		return $this->prefix === $that->prefix &&
 			$this->flattenedProperty === $that->flattenedProperty &&
 			$this->suffix === $that->suffix &&
-			$this->flattenedValueRemaps === $that->flattenedValueRemaps;
+			$this->flattenedValueRemaps === $that->flattenedValueRemaps &&
+			$this->flattenedPropertyType === $that->flattenedPropertyType;
 	}
 }

src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaUtils.php

@@ -25,7 +25,7 @@
 
 use pocketmine\data\bedrock\block\upgrade\model\BlockStateUpgradeSchemaModel;
 use pocketmine\data\bedrock\block\upgrade\model\BlockStateUpgradeSchemaModelBlockRemap;
-use pocketmine\data\bedrock\block\upgrade\model\BlockStateUpgradeSchemaModelFlattenedName;
+use pocketmine\data\bedrock\block\upgrade\model\BlockStateUpgradeSchemaModelFlattenInfo;
 use pocketmine\data\bedrock\block\upgrade\model\BlockStateUpgradeSchemaModelTag;
 use pocketmine\data\bedrock\block\upgrade\model\BlockStateUpgradeSchemaModelValueRemap;
 use pocketmine\nbt\tag\ByteTag;
@@ -155,20 +155,24 @@ public static function fromJsonModel(BlockStateUpgradeSchemaModel $model, int $s
 			}
 		}
 
+		foreach(Utils::stringifyKeys($model->flattenedProperties ?? []) as $blockName => $flattenRule){
+			$result->flattenedProperties[$blockName] = self::jsonModelToFlattenRule($flattenRule);
+		}
+
 		foreach(Utils::stringifyKeys($model->remappedStates ?? []) as $oldBlockName => $remaps){
 			foreach($remaps as $remap){
-				if(isset($remap->newName) === isset($remap->newFlattenedName)){
+				if(isset($remap->newName)){
+					$remapName = $remap->newName;
+				}elseif(isset($remap->newFlattenedName)){
+					$flattenRule = $remap->newFlattenedName;
+					$remapName = self::jsonModelToFlattenRule($flattenRule);
+				}else{
 					throw new \UnexpectedValueException("Expected exactly one of 'newName' or 'newFlattenedName' properties to be set");
 				}
 
 				$result->remappedStates[$oldBlockName][] = new BlockStateUpgradeSchemaBlockRemap(
 					array_map(fn(BlockStateUpgradeSchemaModelTag $tag) => self::jsonModelToTag($tag), $remap->oldState ?? []),
-					$remap->newName ?? new BlockStateUpgradeSchemaFlattenedName(
-						$remap->newFlattenedName->prefix,
-						$remap->newFlattenedName->flattenedProperty,
-						$remap->newFlattenedName->suffix,
-						$remap->newFlattenedName->flattenedValueRemaps ?? [],
-					),
+					$remapName,
 					array_map(fn(BlockStateUpgradeSchemaModelTag $tag) => self::jsonModelToTag($tag), $remap->newState ?? []),
 					$remap->copiedState ?? []
 				);
@@ -254,6 +258,36 @@ private static function buildRemappedValuesIndex(BlockStateUpgradeSchema $schema
 		$model->remappedPropertyValues = $modelDedupMapping;
 	}
 
+	private static function flattenRuleToJsonModel(BlockStateUpgradeSchemaFlattenInfo $flattenRule) : BlockStateUpgradeSchemaModelFlattenInfo{
+		return new BlockStateUpgradeSchemaModelFlattenInfo(
+			$flattenRule->prefix,
+			$flattenRule->flattenedProperty,
+			$flattenRule->suffix,
+			$flattenRule->flattenedValueRemaps,
+			match($flattenRule->flattenedPropertyType){
+				StringTag::class => null, //omit for TAG_String, as this is the common case
+				ByteTag::class => "byte",
+				IntTag::class => "int",
+				default => throw new \LogicException("Unexpected tag type " . $flattenRule->flattenedPropertyType . " in flattened property type")
+			}
+		);
+	}
+
+	private static function jsonModelToFlattenRule(BlockStateUpgradeSchemaModelFlattenInfo $flattenRule) : BlockStateUpgradeSchemaFlattenInfo{
+		return new BlockStateUpgradeSchemaFlattenInfo(
+			$flattenRule->prefix,
+			$flattenRule->flattenedProperty,
+			$flattenRule->suffix,
+			$flattenRule->flattenedValueRemaps ?? [],
+			match ($flattenRule->flattenedPropertyType) {
+				"string", null => StringTag::class,
+				"int" => IntTag::class,
+				"byte" => ByteTag::class,
+				default => throw new \UnexpectedValueException("Unexpected flattened property type $flattenRule->flattenedPropertyType, expected 'string', 'int' or 'byte'")
+			}
+		);
+	}
+
 	public static function toJsonModel(BlockStateUpgradeSchema $schema) : BlockStateUpgradeSchemaModel{
 		$result = new BlockStateUpgradeSchemaModel();
 		$result->maxVersionMajor = $schema->maxVersionMajor;
@@ -292,19 +326,19 @@ public static function toJsonModel(BlockStateUpgradeSchema $schema) : BlockState
 
 		self::buildRemappedValuesIndex($schema, $result);
 
+		foreach(Utils::stringifyKeys($schema->flattenedProperties) as $blockName => $flattenRule){
+			$result->flattenedProperties[$blockName] = self::flattenRuleToJsonModel($flattenRule);
+		}
+		if(isset($result->flattenedProperties)){
+			ksort($result->flattenedProperties);
+		}
+
 		foreach(Utils::stringifyKeys($schema->remappedStates) as $oldBlockName => $remaps){
 			$keyedRemaps = [];
 			foreach($remaps as $remap){
 				$modelRemap = new BlockStateUpgradeSchemaModelBlockRemap(
 					array_map(fn(Tag $tag) => self::tagToJsonModel($tag), $remap->oldState),
-					is_string($remap->newName) ?
-						$remap->newName :
-						new BlockStateUpgradeSchemaModelFlattenedName(
-							$remap->newName->prefix,
-							$remap->newName->flattenedProperty,
-							$remap->newName->suffix,
-							$remap->newName->flattenedValueRemaps
-						),
+					is_string($remap->newName) ? $remap->newName : self::flattenRuleToJsonModel($remap->newName),
 					array_map(fn(Tag $tag) => self::tagToJsonModel($tag), $remap->newState),
 					$remap->copiedState
 				);

src/data/bedrock/block/upgrade/BlockStateUpgrader.php

@@ -24,10 +24,14 @@
 namespace pocketmine\data\bedrock\block\upgrade;
 
 use pocketmine\data\bedrock\block\BlockStateData;
+use pocketmine\nbt\tag\ByteTag;
+use pocketmine\nbt\tag\IntTag;
 use pocketmine\nbt\tag\StringTag;
 use pocketmine\nbt\tag\Tag;
+use pocketmine\utils\AssumptionFailedError;
 use pocketmine\utils\Utils;
 use function count;
+use function get_class;
 use function is_string;
 use function ksort;
 use function max;
@@ -79,6 +83,8 @@ public function upgrade(BlockStateData $blockStateData) : BlockStateData{
 			 * version doesn't tell us which of the schemas have already been applied.
 			 * If there's only one schema for a version (the norm), we can safely assume it's already been applied if
 			 * the version is the same, and skip over it.
+			 * TODO: this causes issues when testing isolated schemas since there will only be one schema for a version.
+			 * The second check should be disabled for that case.
 			 */
 			if($version > $resultVersion || (count($schemaList) === 1 && $version === $resultVersion)){
 				continue;
@@ -104,10 +110,21 @@ private function applySchema(BlockStateUpgradeSchema $schema, BlockStateData $bl
 		}
 
 		$oldName = $blockStateData->getName();
-		$newName = $schema->renamedIds[$oldName] ?? null;
+		$states = $blockStateData->getStates();
+
+		if(isset($schema->renamedIds[$oldName]) && isset($schema->flattenedProperties[$oldName])){
+			//TODO: this probably ought to be validated when the schema is constructed
+			throw new AssumptionFailedError("Both renamedIds and flattenedProperties are set for the same block ID \"$oldName\" - don't know what to do");
+		}
+		if(isset($schema->renamedIds[$oldName])){
+			$newName = $schema->renamedIds[$oldName] ?? null;
+		}elseif(isset($schema->flattenedProperties[$oldName])){
+			[$newName, $states] = $this->applyPropertyFlattened($schema->flattenedProperties[$oldName], $oldName, $states);
+		}else{
+			$newName = null;
+		}
 
 		$stateChanges = 0;
-		$states = $blockStateData->getStates();
 
 		$states = $this->applyPropertyAdded($schema, $oldName, $states, $stateChanges);
 		$states = $this->applyPropertyRemoved($schema, $oldName, $states, $stateChanges);
@@ -140,15 +157,8 @@ private function applyStateRemapped(BlockStateUpgradeSchema $schema, BlockStateD
 				if(is_string($remap->newName)){
 					$newName = $remap->newName;
 				}else{
-					$flattenedValue = $oldState[$remap->newName->flattenedProperty] ?? null;
-					if($flattenedValue instanceof StringTag){
-						$embedValue = $remap->newName->flattenedValueRemaps[$flattenedValue->getValue()] ?? $flattenedValue->getValue();
-						$newName = sprintf("%s%s%s", $remap->newName->prefix, $embedValue, $remap->newName->suffix);
-						unset($oldState[$remap->newName->flattenedProperty]);
-					}else{
-						//flattened property is not a TAG_String, so this transformation is not applicable
-						continue;
-					}
+					//discard flatten modifications to state - the remap newState and copiedState will take care of it
+					[$newName, ] = $this->applyPropertyFlattened($remap->newName, $oldName, $oldState);
 				}
 
 				$newState = $remap->newState;
@@ -266,4 +276,32 @@ private function applyPropertyValueChanged(BlockStateUpgradeSchema $schema, stri
 
 		return $states;
 	}
+
+	/**
+	 * @param Tag[] $states
+	 * @phpstan-param array<string, Tag> $states
+	 *
+	 * @return (string|Tag[])[]
+	 * @phpstan-return array{0: string, 1: array<string, Tag>}
+	 */
+	private function applyPropertyFlattened(BlockStateUpgradeSchemaFlattenInfo $flattenInfo, string $oldName, array $states) : array{
+		$flattenedValue = $states[$flattenInfo->flattenedProperty] ?? null;
+		$expectedType = $flattenInfo->flattenedPropertyType;
+		if(!$flattenedValue instanceof $expectedType){
+			//flattened property is not of the expected type, so this transformation is not applicable
+			return [$oldName, $states];
+		}
+		$embedKey = match(get_class($flattenedValue)){
+			StringTag::class => $flattenedValue->getValue(),
+			ByteTag::class => (string) $flattenedValue->getValue(),
+			IntTag::class => (string) $flattenedValue->getValue(),
+			//flattenedPropertyType is always one of these three types, but PHPStan doesn't know that
+			default => throw new AssumptionFailedError("flattenedPropertyType should be one of these three types, but have " . get_class($flattenedValue)),
+		};
+		$embedValue = $flattenInfo->flattenedValueRemaps[$embedKey] ?? $embedKey;
+		$newName = sprintf("%s%s%s", $flattenInfo->prefix, $embedValue, $flattenInfo->suffix);
+		unset($states[$flattenInfo->flattenedProperty]);
+
+		return [$newName, $states];
+	}
 }

src/data/bedrock/block/upgrade/model/BlockStateUpgradeSchemaModel.php

@@ -75,6 +75,12 @@ final class BlockStateUpgradeSchemaModel implements \JsonSerializable{
 	 */
 	public array $remappedPropertyValuesIndex;
 
+	/**
+	 * @var BlockStateUpgradeSchemaModelFlattenInfo[]
+	 * @phpstan-var array<string, BlockStateUpgradeSchemaModelFlattenInfo>
+	 */
+	public array $flattenedProperties;
+
 	/**
 	 * @var BlockStateUpgradeSchemaModelBlockRemap[][]
 	 * @phpstan-var array<string, list<BlockStateUpgradeSchemaModelBlockRemap>>

src/data/bedrock/block/upgrade/model/BlockStateUpgradeSchemaModelBlockRemap.php

@@ -43,7 +43,7 @@ final class BlockStateUpgradeSchemaModelBlockRemap{
 	 * Either this or newName must be present
 	 * Due to technical limitations of jsonmapper, we can't use a union type here
 	 */
-	public BlockStateUpgradeSchemaModelFlattenedName $newFlattenedName;
+	public BlockStateUpgradeSchemaModelFlattenInfo $newFlattenedName;
 
 	/**
 	 * @var BlockStateUpgradeSchemaModelTag[]|null
@@ -67,9 +67,9 @@ final class BlockStateUpgradeSchemaModelBlockRemap{
 	 * @phpstan-param array<string, BlockStateUpgradeSchemaModelTag> $newState
 	 * @phpstan-param list<string> $copiedState
 	 */
-	public function __construct(array $oldState, string|BlockStateUpgradeSchemaModelFlattenedName $newNameRule, array $newState, array $copiedState){
+	public function __construct(array $oldState, string|BlockStateUpgradeSchemaModelFlattenInfo $newNameRule, array $newState, array $copiedState){
 		$this->oldState = count($oldState) === 0 ? null : $oldState;
-		if($newNameRule instanceof BlockStateUpgradeSchemaModelFlattenedName){
+		if($newNameRule instanceof BlockStateUpgradeSchemaModelFlattenInfo){
 			$this->newFlattenedName = $newNameRule;
 		}else{
 			$this->newName = $newNameRule;

src/data/bedrock/block/upgrade/model/BlockStateUpgradeSchemaModelFlattenedName.php → src/data/bedrock/block/upgrade/model/BlockStateUpgradeSchemaModelFlattenInfo.php

@@ -25,12 +25,13 @@
 
 use function count;
 
-final class BlockStateUpgradeSchemaModelFlattenedName implements \JsonSerializable{
+final class BlockStateUpgradeSchemaModelFlattenInfo implements \JsonSerializable{
 
 	/** @required */
 	public string $prefix;
 	/** @required */
 	public string $flattenedProperty;
+	public ?string $flattenedPropertyType = null;
 	/** @required */
 	public string $suffix;
 	/**
@@ -43,11 +44,12 @@ final class BlockStateUpgradeSchemaModelFlattenedName implements \JsonSerializab
 	 * @param string[] $flattenedValueRemaps
 	 * @phpstan-param array<string, string> $flattenedValueRemaps
 	 */
-	public function __construct(string $prefix, string $flattenedProperty, string $suffix, array $flattenedValueRemaps){
+	public function __construct(string $prefix, string $flattenedProperty, string $suffix, array $flattenedValueRemaps, ?string $flattenedPropertyType = null){
 		$this->prefix = $prefix;
 		$this->flattenedProperty = $flattenedProperty;
 		$this->suffix = $suffix;
 		$this->flattenedValueRemaps = $flattenedValueRemaps;
+		$this->flattenedPropertyType = $flattenedPropertyType;
 	}
 
 	/**
@@ -58,6 +60,9 @@ public function jsonSerialize() : array{
 		if(count($this->flattenedValueRemaps) === 0){
 			unset($result["flattenedValueRemaps"]);
 		}
+		if($this->flattenedPropertyType === null){
+			unset($result["flattenedPropertyType"]);
+		}
 		return $result;
 	}
 }

tests/phpunit/data/bedrock/block/upgrade/BlockStateUpgraderTest.php

@@ -24,8 +24,10 @@
 namespace pocketmine\data\bedrock\block\upgrade;
 
 use PHPUnit\Framework\TestCase;
+use pocketmine\block\Block;
 use pocketmine\data\bedrock\block\BlockStateData;
 use pocketmine\nbt\tag\IntTag;
+use pocketmine\nbt\tag\StringTag;
 use const PHP_INT_MAX;
 
 class BlockStateUpgraderTest extends TestCase{
@@ -210,6 +212,23 @@ public function testRemapAndRenameProperty(\Closure $getStateData, ?int $valueAf
 		self::assertSame($upgradedStateData->getState(self::TEST_PROPERTY_2)?->getValue(), $valueAfter);
 	}
 
+	public function testFlattenProperty() : void{
+		$schema = $this->getNewSchema();
+		$schema->flattenedProperties[self::TEST_BLOCK] = new BlockStateUpgradeSchemaFlattenInfo(
+			"minecraft:",
+			"test",
+			"_suffix",
+			[],
+			StringTag::class
+		);
+
+		$stateData = new BlockStateData(self::TEST_BLOCK, ["test" => new StringTag("value1")], 0);
+		$upgradedStateData = $this->upgrade($stateData, fn() => $stateData);
+
+		self::assertSame("minecraft:value1_suffix", $upgradedStateData->getName());
+		self::assertEmpty($upgradedStateData->getStates());
+	}
+
 	/**
 	 * @phpstan-return \Generator<int, array{int, int, bool, int}, void, void>
 	 */