From bdb5845cecc777ded4e37071a9ae976a8dcbe0d1 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 5 Aug 2024 22:33:30 +0100 Subject: [PATCH 01/17] Allow name flattening rules where multiple old values map to the same new ID this allows more compaction in certain cases, such as tallgrass recently. instead of blacklisting any mapping which reuses the same flattened infix, we select the flatten property which produces the smallest number of distinct rules, which produces the most compact schema possible. this change also permits potentially flattening other types of properties such as for corals (live/dead and type), although only one may be selected at a time. --- tools/generate-blockstate-upgrade-schema.php | 30 +++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/tools/generate-blockstate-upgrade-schema.php b/tools/generate-blockstate-upgrade-schema.php index 54984d459..299d97d05 100644 --- a/tools/generate-blockstate-upgrade-schema.php +++ b/tools/generate-blockstate-upgrade-schema.php @@ -42,6 +42,7 @@ use function array_key_last; use function array_keys; use function array_map; use function array_shift; +use function array_unique; use function array_values; use function count; use function dirname; @@ -423,6 +424,10 @@ function processRemappedStates(array $upgradeTable) : array{ $filter = $pair->old->getStates(); foreach($unchangedStatesByNewName[$pair->new->getName()] as $unchangedPropertyName){ + if($unchangedPropertyName === $propertyName){ + $notFlattenedProperties[$propertyName] = true; + continue 2; + } unset($filter[$unchangedPropertyName]); } unset($filter[$propertyName]); @@ -436,26 +441,31 @@ function processRemappedStates(array $upgradeTable) : array{ $notFlattenedProperties[$propertyName] = true; continue; } - foreach(Utils::stringifyKeys($valuesToIds) as $otherRawValue => $otherNewId){ - if($otherRawValue === $rawValue){ - continue; - } - if($otherNewId === $pair->new->getName()){ - //this old value maps to the same new ID as another old value - bad candidate for flattening - $notFlattenedProperties[$propertyName] = true; - continue 2; - } - } } $candidateFlattenedValues[$propertyName][$rawFilter][$rawValue] = $pair->new->getName(); } } + foreach(Utils::stringifyKeys($candidateFlattenedValues) as $propertyName => $filters){ + foreach($filters as $valuesToIds){ + if(count(array_unique($valuesToIds)) === 1){ + //this property doesn't influence the new ID + $notFlattenedProperties[$propertyName] = true; + continue 2; + } + } + } foreach(Utils::stringifyKeys($notFlattenedProperties) as $propertyName => $_){ unset($candidateFlattenedValues[$propertyName]); } $flattenedProperties = buildFlattenPropertyRules($candidateFlattenedValues); $flattenProperty = array_key_first($flattenedProperties); + //Properties with fewer rules take up less space for the same result + foreach(Utils::stringifyKeys($flattenedProperties) as $propertyName => $rules){ + if(count($rules) < count($flattenedProperties[$flattenProperty])){ + $flattenProperty = $propertyName; + } + } $list = []; From be2437ac6e6cd851f0cd282f54983cb2e9fcba6f Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 5 Aug 2024 22:38:02 +0100 Subject: [PATCH 02/17] Support for flattening TAG_Byte and TAG_Int properties this allows optimisation in upcoming versions. --- .../BlockStateUpgradeSchemaFlattenedName.php | 10 ++++- .../upgrade/BlockStateUpgradeSchemaUtils.php | 33 ++++++++++---- .../block/upgrade/BlockStateUpgrader.php | 23 +++++++--- ...ckStateUpgradeSchemaModelFlattenedName.php | 7 ++- tools/generate-blockstate-upgrade-schema.php | 45 ++++++++++++++++--- 5 files changed, 94 insertions(+), 24 deletions(-) diff --git a/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaFlattenedName.php b/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaFlattenedName.php index 1c95dd9c7..e2f7f798d 100644 --- a/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaFlattenedName.php +++ b/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaFlattenedName.php @@ -23,6 +23,9 @@ declare(strict_types=1); 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; @@ -31,12 +34,14 @@ final class BlockStateUpgradeSchemaFlattenedName{ /** * @param string[] $flattenedValueRemaps * @phpstan-param array $flattenedValueRemaps + * @phpstan-param class-string|class-string|class-string|null $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 @@ final class BlockStateUpgradeSchemaFlattenedName{ 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; } } diff --git a/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaUtils.php b/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaUtils.php index 832631490..d66b7e68c 100644 --- a/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaUtils.php +++ b/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaUtils.php @@ -157,18 +157,29 @@ final class BlockStateUpgradeSchemaUtils{ 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 = new BlockStateUpgradeSchemaFlattenedName( + $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'") + } + ); + }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 ?? [] ); @@ -303,7 +314,13 @@ final class BlockStateUpgradeSchemaUtils{ $remap->newName->prefix, $remap->newName->flattenedProperty, $remap->newName->suffix, - $remap->newName->flattenedValueRemaps + $remap->newName->flattenedValueRemaps, + match($remap->newName->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 " . $remap->newName->flattenedPropertyType . " in flattened property type") + } ), array_map(fn(Tag $tag) => self::tagToJsonModel($tag), $remap->newState), $remap->copiedState diff --git a/src/data/bedrock/block/upgrade/BlockStateUpgrader.php b/src/data/bedrock/block/upgrade/BlockStateUpgrader.php index 4a305d8bc..ddb1e1359 100644 --- a/src/data/bedrock/block/upgrade/BlockStateUpgrader.php +++ b/src/data/bedrock/block/upgrade/BlockStateUpgrader.php @@ -24,10 +24,14 @@ declare(strict_types=1); 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; @@ -141,14 +145,21 @@ final class BlockStateUpgrader{ $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 + $expectedType = $remap->newName->flattenedPropertyType; + if(!$flattenedValue instanceof $expectedType){ + //flattened property is not of the expected type, so this transformation is not applicable continue; } + $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 = $remap->newName->flattenedValueRemaps[$embedKey] ?? $embedKey; + $newName = sprintf("%s%s%s", $remap->newName->prefix, $embedValue, $remap->newName->suffix); + unset($oldState[$remap->newName->flattenedProperty]); } $newState = $remap->newState; diff --git a/src/data/bedrock/block/upgrade/model/BlockStateUpgradeSchemaModelFlattenedName.php b/src/data/bedrock/block/upgrade/model/BlockStateUpgradeSchemaModelFlattenedName.php index 001192f47..6d03bbc12 100644 --- a/src/data/bedrock/block/upgrade/model/BlockStateUpgradeSchemaModelFlattenedName.php +++ b/src/data/bedrock/block/upgrade/model/BlockStateUpgradeSchemaModelFlattenedName.php @@ -31,6 +31,7 @@ final class BlockStateUpgradeSchemaModelFlattenedName implements \JsonSerializab 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 $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 @@ final class BlockStateUpgradeSchemaModelFlattenedName implements \JsonSerializab if(count($this->flattenedValueRemaps) === 0){ unset($result["flattenedValueRemaps"]); } + if($this->flattenedPropertyType === null){ + unset($result["flattenedPropertyType"]); + } return $result; } } diff --git a/tools/generate-blockstate-upgrade-schema.php b/tools/generate-blockstate-upgrade-schema.php index 299d97d05..d2d0a8c41 100644 --- a/tools/generate-blockstate-upgrade-schema.php +++ b/tools/generate-blockstate-upgrade-schema.php @@ -30,6 +30,8 @@ use pocketmine\data\bedrock\block\upgrade\BlockStateUpgradeSchemaFlattenedName; use pocketmine\data\bedrock\block\upgrade\BlockStateUpgradeSchemaUtils; use pocketmine\data\bedrock\block\upgrade\BlockStateUpgradeSchemaValueRemap; use pocketmine\nbt\LittleEndianNbtSerializer; +use pocketmine\nbt\tag\ByteTag; +use pocketmine\nbt\tag\IntTag; use pocketmine\nbt\tag\StringTag; use pocketmine\nbt\tag\Tag; use pocketmine\nbt\TreeRoot; @@ -48,7 +50,10 @@ use function count; use function dirname; use function file_put_contents; use function fwrite; +use function get_class; +use function get_debug_type; use function implode; +use function is_numeric; use function json_encode; use function ksort; use function min; @@ -312,11 +317,13 @@ function findCommonSuffix(array $strings) : string{ /** * @param string[][][] $candidateFlattenedValues * @phpstan-param array>> $candidateFlattenedValues + * @param string[] $candidateFlattenPropertyTypes + * @phpstan-param array|class-string|class-string> $candidateFlattenPropertyTypes * * @return BlockStateUpgradeSchemaFlattenedName[][] * @phpstan-return array> */ -function buildFlattenPropertyRules(array $candidateFlattenedValues) : array{ +function buildFlattenPropertyRules(array $candidateFlattenedValues, array $candidateFlattenPropertyTypes) : array{ $flattenPropertyRules = []; foreach(Utils::stringifyKeys($candidateFlattenedValues) as $propertyName => $filters){ foreach(Utils::stringifyKeys($filters) as $filter => $valueToId){ @@ -340,11 +347,26 @@ function buildFlattenPropertyRules(array $candidateFlattenedValues) : array{ } } + $allNumeric = true; + if(count($valueMap) > 0){ + foreach(Utils::stringifyKeys($valueMap) as $value => $newValue){ + if(!is_numeric($value)){ + $allNumeric = false; + break; + } + } + if($allNumeric){ + //add a dummy key to force the JSON to be an object and not a list + $valueMap["dummy"] = "map_not_list"; + } + } + $flattenPropertyRules[$propertyName][$filter] = new BlockStateUpgradeSchemaFlattenedName( $idPrefix, $propertyName, $idSuffix, - $valueMap + $valueMap, + $candidateFlattenPropertyTypes[$propertyName], ); } } @@ -407,16 +429,25 @@ function processRemappedStates(array $upgradeTable) : array{ $notFlattenedProperties = []; $candidateFlattenedValues = []; + $candidateFlattenedPropertyTypes = []; foreach($upgradeTable as $pair){ foreach(Utils::stringifyKeys($pair->old->getStates()) as $propertyName => $propertyValue){ if(isset($notFlattenedProperties[$propertyName])){ continue; } - if(!$propertyValue instanceof StringTag){ + if(!$propertyValue instanceof StringTag && !$propertyValue instanceof IntTag && !$propertyValue instanceof ByteTag){ $notFlattenedProperties[$propertyName] = true; continue; } - $rawValue = $propertyValue->getValue(); + $previousType = $candidateFlattenedPropertyTypes[$propertyName] ?? null; + if($previousType !== null && $previousType !== get_class($propertyValue)){ + //mismatched types for the same property name - this has never happened so far, but it's not impossible + $notFlattenedProperties[$propertyName] = true; + continue; + } + $candidateFlattenedPropertyTypes[$propertyName] = get_class($propertyValue); + + $rawValue = (string) $propertyValue->getValue(); if($rawValue === ""){ $notFlattenedProperties[$propertyName] = true; continue; @@ -458,7 +489,7 @@ function processRemappedStates(array $upgradeTable) : array{ unset($candidateFlattenedValues[$propertyName]); } - $flattenedProperties = buildFlattenPropertyRules($candidateFlattenedValues); + $flattenedProperties = buildFlattenPropertyRules($candidateFlattenedValues, $candidateFlattenedPropertyTypes); $flattenProperty = array_key_first($flattenedProperties); //Properties with fewer rules take up less space for the same result foreach(Utils::stringifyKeys($flattenedProperties) as $propertyName => $rules){ @@ -485,8 +516,8 @@ function processRemappedStates(array $upgradeTable) : array{ ksort($cleanedNewState); if($flattenProperty !== null){ $flattenedValue = $cleanedOldState[$flattenProperty] ?? null; - if(!$flattenedValue instanceof StringTag){ - throw new AssumptionFailedError("This should always be a TAG_String ($newName $flattenProperty)"); + if(!$flattenedValue instanceof StringTag && !$flattenedValue instanceof IntTag && !$flattenedValue instanceof ByteTag){ + throw new AssumptionFailedError("Non-flattenable type of tag ($newName $flattenProperty) but have " . get_debug_type($flattenedValue)); } unset($cleanedOldState[$flattenProperty]); } From d0d7a995fb60a493462323208ff2b887003cef44 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 5 Aug 2024 22:38:32 +0100 Subject: [PATCH 03/17] Add a TODO in BlockStateUpgrader this issue can be worked around by adding a dummy schema, but it's a bit clunky. --- src/data/bedrock/block/upgrade/BlockStateUpgrader.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/data/bedrock/block/upgrade/BlockStateUpgrader.php b/src/data/bedrock/block/upgrade/BlockStateUpgrader.php index ddb1e1359..b0612585c 100644 --- a/src/data/bedrock/block/upgrade/BlockStateUpgrader.php +++ b/src/data/bedrock/block/upgrade/BlockStateUpgrader.php @@ -83,6 +83,8 @@ final class BlockStateUpgrader{ * 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; From 2aa64dc15ef7bf0b35fb233e8d5348c258d9034d Mon Sep 17 00:00:00 2001 From: IvanCraft623 Date: Mon, 5 Aug 2024 17:13:23 -0500 Subject: [PATCH 04/17] Simplify phpstan-doc type hint for better readability --- .../block/upgrade/BlockStateUpgradeSchemaFlattenedName.php | 2 +- tools/generate-blockstate-upgrade-schema.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaFlattenedName.php b/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaFlattenedName.php index e2f7f798d..8259f690d 100644 --- a/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaFlattenedName.php +++ b/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaFlattenedName.php @@ -34,7 +34,7 @@ final class BlockStateUpgradeSchemaFlattenedName{ /** * @param string[] $flattenedValueRemaps * @phpstan-param array $flattenedValueRemaps - * @phpstan-param class-string|class-string|class-string|null $flattenedPropertyType + * @phpstan-param ?class-string $flattenedPropertyType */ public function __construct( public string $prefix, diff --git a/tools/generate-blockstate-upgrade-schema.php b/tools/generate-blockstate-upgrade-schema.php index d2d0a8c41..43bcc71f1 100644 --- a/tools/generate-blockstate-upgrade-schema.php +++ b/tools/generate-blockstate-upgrade-schema.php @@ -318,7 +318,7 @@ function findCommonSuffix(array $strings) : string{ * @param string[][][] $candidateFlattenedValues * @phpstan-param array>> $candidateFlattenedValues * @param string[] $candidateFlattenPropertyTypes - * @phpstan-param array|class-string|class-string> $candidateFlattenPropertyTypes + * @phpstan-param array> $candidateFlattenPropertyTypes * * @return BlockStateUpgradeSchemaFlattenedName[][] * @phpstan-return array> From 26761c2b87608afb6b2bbdfa5daa96bda53adcb9 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Fri, 9 Aug 2024 12:37:08 +0100 Subject: [PATCH 05/17] Blockstate schema tool now supports testing schemas as well as generating them this is useful when making changes to the generator, since regenerated schemas can now be tested for validity. This helps to find bugs in the generator. --- ...hp => blockstate-upgrade-schema-utils.php} | 70 +++++++++++++++---- 1 file changed, 55 insertions(+), 15 deletions(-) rename tools/{generate-blockstate-upgrade-schema.php => blockstate-upgrade-schema-utils.php} (90%) diff --git a/tools/generate-blockstate-upgrade-schema.php b/tools/blockstate-upgrade-schema-utils.php similarity index 90% rename from tools/generate-blockstate-upgrade-schema.php rename to tools/blockstate-upgrade-schema-utils.php index 43bcc71f1..c8dafcb67 100644 --- a/tools/generate-blockstate-upgrade-schema.php +++ b/tools/blockstate-upgrade-schema-utils.php @@ -21,9 +21,10 @@ declare(strict_types=1); -namespace pocketmine\tools\generate_blockstate_upgrade_schema; +namespace pocketmine\tools\blockstate_upgrade_schema_utils; use pocketmine\data\bedrock\block\BlockStateData; +use pocketmine\data\bedrock\block\upgrade\BlockStateUpgrader; use pocketmine\data\bedrock\block\upgrade\BlockStateUpgradeSchema; use pocketmine\data\bedrock\block\upgrade\BlockStateUpgradeSchemaBlockRemap; use pocketmine\data\bedrock\block\upgrade\BlockStateUpgradeSchemaFlattenedName; @@ -607,32 +608,71 @@ function generateBlockStateUpgradeSchema(array $upgradeTable) : BlockStateUpgrad return $result; } +/** + * @param BlockStateMapping[][] $upgradeTable + * @phpstan-param array> $upgradeTable + */ +function testBlockStateUpgradeSchema(array $upgradeTable, BlockStateUpgradeSchema $schema) : bool{ + //TODO: HACK! + //the upgrader won't apply the schema if it's the same version and there's only one schema with a matching version + //ID (for performance reasons), which is a problem for testing isolated schemas + //add a dummy schema to bypass this optimization + $dummySchema = new BlockStateUpgradeSchema($schema->maxVersionMajor, $schema->maxVersionMinor, $schema->maxVersionPatch, $schema->maxVersionRevision, $schema->getSchemaId() + 1); + $upgrader = new BlockStateUpgrader([$schema, $dummySchema]); + + foreach($upgradeTable as $mappingsByOldName){ + foreach($mappingsByOldName as $mapping){ + $expectedNewState = $mapping->new; + + $actualNewState = $upgrader->upgrade($mapping->old); + + if(!$expectedNewState->equals($actualNewState)){ + \GlobalLogger::get()->error("Expected: " . $expectedNewState->toNbt()); + \GlobalLogger::get()->error("Actual: " . $actualNewState->toNbt()); + return false; + } + } + } + + return true; +} + /** * @param string[] $argv */ function main(array $argv) : int{ - if(count($argv) !== 3){ - fwrite(STDERR, "Required arguments: input file path, output file path\n"); + if(count($argv) !== 4 || ($argv[1] !== "generate" && $argv[1] !== "test")){ + fwrite(STDERR, "Required arguments: \n"); return 1; } - $input = $argv[1]; - $output = $argv[2]; + $mode = $argv[1]; + $upgradeTableFile = $argv[2]; + $schemaFile = $argv[3]; - $table = loadUpgradeTable($input, false); + $table = loadUpgradeTable($upgradeTableFile, false); ksort($table, SORT_STRING); - $diff = generateBlockStateUpgradeSchema($table); - if($diff->isEmpty()){ - \GlobalLogger::get()->warning("All states appear to be the same! No schema generated."); - return 0; + if($mode === "generate"){ + $diff = generateBlockStateUpgradeSchema($table); + if($diff->isEmpty()){ + \GlobalLogger::get()->warning("All states appear to be the same! No schema generated."); + return 0; + } + file_put_contents( + $schemaFile, + json_encode(BlockStateUpgradeSchemaUtils::toJsonModel($diff), JSON_PRETTY_PRINT) . "\n" + ); + \GlobalLogger::get()->info("Schema file $schemaFile generated successfully."); + }else{ + $schema = BlockStateUpgradeSchemaUtils::loadSchemaFromString(Filesystem::fileGetContents($schemaFile), 0); + if(!testBlockStateUpgradeSchema($table, $schema)){ + \GlobalLogger::get()->error("Schema $schemaFile does not produce the results predicted by $upgradeTableFile"); + return 1; + } + \GlobalLogger::get()->info("Schema $schemaFile is valid according to $upgradeTableFile"); } - file_put_contents( - $output, - json_encode(BlockStateUpgradeSchemaUtils::toJsonModel($diff), JSON_PRETTY_PRINT) . "\n" - ); - \GlobalLogger::get()->info("Schema file $output generated successfully."); return 0; } From 33dc995cc748cd6967c32a65844d88d28df45037 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Fri, 9 Aug 2024 13:12:46 +0100 Subject: [PATCH 06/17] blockstate-upgrade-schema-utils: added a command to update old schemas to a newer format this is useful when the generator was updated with new features & optimisations, to reduce the size and/or improve readability of existing schemas. --- tools/blockstate-upgrade-schema-utils.php | 137 ++++++++++++++++------ 1 file changed, 104 insertions(+), 33 deletions(-) diff --git a/tools/blockstate-upgrade-schema-utils.php b/tools/blockstate-upgrade-schema-utils.php index c8dafcb67..a9069d429 100644 --- a/tools/blockstate-upgrade-schema-utils.php +++ b/tools/blockstate-upgrade-schema-utils.php @@ -36,6 +36,7 @@ use pocketmine\nbt\tag\IntTag; use pocketmine\nbt\tag\StringTag; use pocketmine\nbt\tag\Tag; use pocketmine\nbt\TreeRoot; +use pocketmine\network\mcpe\convert\BlockStateDictionary; use pocketmine\network\mcpe\protocol\serializer\NetworkNbtSerializer; use pocketmine\utils\AssumptionFailedError; use pocketmine\utils\Filesystem; @@ -90,18 +91,18 @@ function encodeProperty(Tag $tag) : string{ } /** + * @param TreeRoot[] $oldNewStateList + * @phpstan-param list $oldNewStateList + * * @return BlockStateMapping[][] * @phpstan-return array> */ -function loadUpgradeTable(string $file, bool $reverse) : array{ - $contents = Filesystem::fileGetContents($file); - $data = (new NetworkNbtSerializer())->readMultiple($contents); - +function buildUpgradeTableFromData(array $oldNewStateList, bool $reverse) : array{ $result = []; - for($i = 0; isset($data[$i]); $i += 2){ - $oldTag = $data[$i]->mustGetCompoundTag(); - $newTag = $data[$i + 1]->mustGetCompoundTag(); + for($i = 0; isset($oldNewStateList[$i]); $i += 2){ + $oldTag = $oldNewStateList[$i]->mustGetCompoundTag(); + $newTag = $oldNewStateList[$i + 1]->mustGetCompoundTag(); $old = BlockStateData::fromNbt($reverse ? $newTag : $oldTag); $new = BlockStateData::fromNbt($reverse ? $oldTag : $newTag); @@ -114,6 +115,17 @@ function loadUpgradeTable(string $file, bool $reverse) : array{ return $result; } +/** + * @return BlockStateMapping[][] + * @phpstan-return array> + */ +function loadUpgradeTableFromFile(string $file, bool $reverse) : array{ + $contents = Filesystem::fileGetContents($file); + $data = (new NetworkNbtSerializer())->readMultiple($contents); + + return buildUpgradeTableFromData($data, $reverse); +} + /** * @param BlockStateData[] $states * @phpstan-param array $states @@ -640,41 +652,100 @@ function testBlockStateUpgradeSchema(array $upgradeTable, BlockStateUpgradeSchem /** * @param string[] $argv */ -function main(array $argv) : int{ - if(count($argv) !== 4 || ($argv[1] !== "generate" && $argv[1] !== "test")){ - fwrite(STDERR, "Required arguments: \n"); - return 1; - } - - $mode = $argv[1]; +function cmdGenerate(array $argv) : int{ $upgradeTableFile = $argv[2]; $schemaFile = $argv[3]; - $table = loadUpgradeTable($upgradeTableFile, false); + $table = loadUpgradeTableFromFile($upgradeTableFile, false); ksort($table, SORT_STRING); - if($mode === "generate"){ - $diff = generateBlockStateUpgradeSchema($table); - if($diff->isEmpty()){ - \GlobalLogger::get()->warning("All states appear to be the same! No schema generated."); - return 0; - } - file_put_contents( - $schemaFile, - json_encode(BlockStateUpgradeSchemaUtils::toJsonModel($diff), JSON_PRETTY_PRINT) . "\n" - ); - \GlobalLogger::get()->info("Schema file $schemaFile generated successfully."); - }else{ - $schema = BlockStateUpgradeSchemaUtils::loadSchemaFromString(Filesystem::fileGetContents($schemaFile), 0); - if(!testBlockStateUpgradeSchema($table, $schema)){ - \GlobalLogger::get()->error("Schema $schemaFile does not produce the results predicted by $upgradeTableFile"); - return 1; - } - \GlobalLogger::get()->info("Schema $schemaFile is valid according to $upgradeTableFile"); + $diff = generateBlockStateUpgradeSchema($table); + if($diff->isEmpty()){ + \GlobalLogger::get()->warning("All states appear to be the same! No schema generated."); + return 0; } + file_put_contents( + $schemaFile, + json_encode(BlockStateUpgradeSchemaUtils::toJsonModel($diff), JSON_PRETTY_PRINT) . "\n" + ); + \GlobalLogger::get()->info("Schema file $schemaFile generated successfully."); + return 0; +} + +/** + * @param string[] $argv + */ +function cmdTest(array $argv) : int{ + $upgradeTableFile = $argv[2]; + $schemaFile = $argv[3]; + + $table = loadUpgradeTableFromFile($upgradeTableFile, false); + + ksort($table, SORT_STRING); + + $schema = BlockStateUpgradeSchemaUtils::loadSchemaFromString(Filesystem::fileGetContents($schemaFile), 0); + if(!testBlockStateUpgradeSchema($table, $schema)){ + \GlobalLogger::get()->error("Schema $schemaFile does not produce the results predicted by $upgradeTableFile"); + return 1; + } + \GlobalLogger::get()->info("Schema $schemaFile is valid according to $upgradeTableFile"); return 0; } +/** + * @param string[] $argv + */ +function cmdUpdate(array $argv) : int{ + [, , $oldSchemaFile, $oldPaletteFile, $newSchemaFile] = $argv; + + $palette = BlockStateDictionary::loadPaletteFromString(Filesystem::fileGetContents($oldPaletteFile)); + $schema = BlockStateUpgradeSchemaUtils::loadSchemaFromString(Filesystem::fileGetContents($oldSchemaFile), 0); + //TODO: HACK! + //the upgrader won't apply the schema if it's the same version and there's only one schema with a matching version + //ID (for performance reasons), which is a problem for testing isolated schemas + //add a dummy schema to bypass this optimization + $dummySchema = new BlockStateUpgradeSchema($schema->maxVersionMajor, $schema->maxVersionMinor, $schema->maxVersionPatch, $schema->maxVersionRevision, $schema->getSchemaId() + 1); + $upgrader = new BlockStateUpgrader([$schema, $dummySchema]); + + $tags = []; + foreach($palette as $stateData){ + $tags[] = new TreeRoot($stateData->toNbt()); + $tags[] = new TreeRoot($upgrader->upgrade($stateData)->toNbt()); + } + + $upgradeTable = buildUpgradeTableFromData($tags, false); + $newSchema = generateBlockStateUpgradeSchema($upgradeTable); + file_put_contents( + $newSchemaFile, + json_encode(BlockStateUpgradeSchemaUtils::toJsonModel($newSchema), JSON_PRETTY_PRINT) . "\n" + ); + \GlobalLogger::get()->info("Schema file $newSchemaFile updated to new format (from $oldSchemaFile) successfully."); + return 0; +} + +/** + * @param string[] $argv + */ +function main(array $argv) : int{ + $options = [ + "generate" => [["palette upgrade table file", "schema output file"], cmdGenerate(...)], + "test" => [["palette upgrade table file", "schema output file"], cmdTest(...)], + "update" => [["schema input file", "old palette file", "updated schema output file"], cmdUpdate(...)] + ]; + + $selected = $argv[1] ?? null; + if($selected === null || !isset($options[$selected])){ + fwrite(STDERR, "Available commands:\n"); + foreach($options as $command => [$args, $callback]){ + fwrite(STDERR, " - $command " . implode(" ", array_map(fn(string $a) => "<$a>", $args)) . "\n"); + } + return 1; + } + + $callback = $options[$selected][1]; + return $callback($argv); +} + exit(main($argv)); From c8f567b0937c406097b9cc26fc26b5cd4c7ea21d Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 17 Oct 2024 20:24:57 +0100 Subject: [PATCH 07/17] Fix missing arg count check --- tools/blockstate-upgrade-schema-utils.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/blockstate-upgrade-schema-utils.php b/tools/blockstate-upgrade-schema-utils.php index a9069d429..cd782b9c4 100644 --- a/tools/blockstate-upgrade-schema-utils.php +++ b/tools/blockstate-upgrade-schema-utils.php @@ -745,6 +745,10 @@ function main(array $argv) : int{ } $callback = $options[$selected][1]; + if(count($argv) !== count($options[$selected][0]) + 2){ + fwrite(STDERR, "Usage: {$argv[0]} $selected " . implode(" ", array_map(fn(string $a) => "<$a>", $options[$selected][0])) . "\n"); + return 1; + } return $callback($argv); } From 7f9e79c83e1130f3cf74c33917a33ff6fd7a074b Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 24 Oct 2024 13:30:24 +0100 Subject: [PATCH 08/17] Automatically test new schemas to ensure they produce the results predicted by the input file --- tools/blockstate-upgrade-schema-utils.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tools/blockstate-upgrade-schema-utils.php b/tools/blockstate-upgrade-schema-utils.php index a779ab9c3..c8dfb0ab2 100644 --- a/tools/blockstate-upgrade-schema-utils.php +++ b/tools/blockstate-upgrade-schema-utils.php @@ -692,6 +692,13 @@ function cmdGenerate(array $argv) : int{ \GlobalLogger::get()->warning("All states appear to be the same! No schema generated."); return 0; } + + if(!testBlockStateUpgradeSchema($table, $diff)){ + \GlobalLogger::get()->error("Generated schema does not produce the results expected by $upgradeTableFile"); + \GlobalLogger::get()->error("This is probably a bug in the schema generation code. Please report this to the developers."); + return 1; + } + file_put_contents( $schemaFile, json_encode(BlockStateUpgradeSchemaUtils::toJsonModel($diff), JSON_PRETTY_PRINT) . "\n" @@ -744,6 +751,13 @@ function cmdUpdate(array $argv) : int{ $upgradeTable = buildUpgradeTableFromData($tags, false); $newSchema = generateBlockStateUpgradeSchema($upgradeTable); + + if(!testBlockStateUpgradeSchema($upgradeTable, $newSchema)){ + \GlobalLogger::get()->error("Updated schema does not produce the expected results!"); + \GlobalLogger::get()->error("This is probably a bug in the schema generation code. Please report this to the developers."); + return 1; + } + file_put_contents( $newSchemaFile, json_encode(BlockStateUpgradeSchemaUtils::toJsonModel($newSchema), JSON_PRETTY_PRINT) . "\n" From acbfb0a3e9a8e2d4be25836d7cd359461c7e4b73 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 24 Oct 2024 13:30:55 +0100 Subject: [PATCH 09/17] Support for updating a batch of schemas using BlockPaletteArchive --- tools/blockstate-upgrade-schema-utils.php | 50 ++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/tools/blockstate-upgrade-schema-utils.php b/tools/blockstate-upgrade-schema-utils.php index c8dfb0ab2..b73954aff 100644 --- a/tools/blockstate-upgrade-schema-utils.php +++ b/tools/blockstate-upgrade-schema-utils.php @@ -41,6 +41,7 @@ use pocketmine\network\mcpe\protocol\serializer\NetworkNbtSerializer; use pocketmine\utils\AssumptionFailedError; use pocketmine\utils\Filesystem; use pocketmine\utils\Utils; +use Symfony\Component\Filesystem\Path; use function array_key_first; use function array_key_last; use function array_keys; @@ -50,15 +51,19 @@ use function array_unique; use function array_values; use function count; use function dirname; +use function file_exists; use function file_put_contents; use function fwrite; use function get_class; use function get_debug_type; use function implode; +use function is_dir; use function is_numeric; use function json_encode; use function ksort; use function min; +use function preg_match; +use function scandir; use function sort; use function strlen; use function strrev; @@ -766,6 +771,48 @@ function cmdUpdate(array $argv) : int{ return 0; } +/** + * @param string[] $argv + */ +function cmdUpdateAll(array $argv) : int{ + $oldPaletteFilenames = [ + '1.9.0' => '1.09.0', + '1.19.50' => '1.19.50.23_beta', + '1.19.60' => '1.19.60.26_beta', + '1.19.70' => '1.19.70.26_beta', + '1.19.80' => '1.19.80.24_beta', + ]; + $schemaDir = $argv[2]; + $paletteArchiveDir = $argv[3]; + + $schemaFileNames = scandir($schemaDir); + if($schemaFileNames === false){ + \GlobalLogger::get()->error("Failed to read schema directory $schemaDir"); + return 1; + } + foreach($schemaFileNames as $file){ + $schemaFile = Path::join($schemaDir, $file); + if(!file_exists($schemaFile) || is_dir($schemaFile)){ + continue; + } + + if(preg_match('/^\d{4}_(.+?)_to_(.+?).json/', $file, $matches) !== 1){ + continue; + } + $oldPaletteFile = Path::join($paletteArchiveDir, ($oldPaletteFilenames[$matches[1]] ?? $matches[1]) . '.nbt'); + + //a bit clunky but it avoids having to make yet another function + //TODO: perhaps in the future we should write the result to a tmpfile until all schemas are updated, + //and then copy the results into place at the end + if(cmdUpdate([$argv[0], "update", $schemaFile, $oldPaletteFile, $schemaFile]) !== 0){ + return 1; + } + } + + \GlobalLogger::get()->info("All schemas updated successfully."); + return 0; +} + /** * @param string[] $argv */ @@ -773,7 +820,8 @@ function main(array $argv) : int{ $options = [ "generate" => [["palette upgrade table file", "schema output file"], cmdGenerate(...)], "test" => [["palette upgrade table file", "schema output file"], cmdTest(...)], - "update" => [["schema input file", "old palette file", "updated schema output file"], cmdUpdate(...)] + "update" => [["schema input file", "old palette file", "updated schema output file"], cmdUpdate(...)], + "update-all" => [["schema folder", "path to BlockPaletteArchive"], cmdUpdateAll(...)] ]; $selected = $argv[1] ?? null; From 22718c4971460fae5b4d98fabff199da77c6acf0 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 24 Oct 2024 16:12:28 +0100 Subject: [PATCH 10/17] Add support for specialized flattenedProperties in schema format --- .../block/upgrade/BlockStateUpgradeSchema.php | 8 + .../BlockStateUpgradeSchemaBlockRemap.php | 6 +- ...=> BlockStateUpgradeSchemaFlattenInfo.php} | 2 +- .../upgrade/BlockStateUpgradeSchemaUtils.php | 71 +++++--- .../block/upgrade/BlockStateUpgrader.php | 62 +++++-- .../model/BlockStateUpgradeSchemaModel.php | 6 + ...BlockStateUpgradeSchemaModelBlockRemap.php | 6 +- ...ockStateUpgradeSchemaModelFlattenInfo.php} | 2 +- .../block/upgrade/BlockStateUpgraderTest.php | 19 ++ tools/blockstate-upgrade-schema-utils.php | 170 +++++++++++++----- 10 files changed, 251 insertions(+), 101 deletions(-) rename src/data/bedrock/block/upgrade/{BlockStateUpgradeSchemaFlattenedName.php => BlockStateUpgradeSchemaFlattenInfo.php} (97%) rename src/data/bedrock/block/upgrade/model/{BlockStateUpgradeSchemaModelFlattenedName.php => BlockStateUpgradeSchemaModelFlattenInfo.php} (95%) diff --git a/src/data/bedrock/block/upgrade/BlockStateUpgradeSchema.php b/src/data/bedrock/block/upgrade/BlockStateUpgradeSchema.php index 6d280ecf7..f8894cfd2 100644 --- a/src/data/bedrock/block/upgrade/BlockStateUpgradeSchema.php +++ b/src/data/bedrock/block/upgrade/BlockStateUpgradeSchema.php @@ -23,6 +23,7 @@ declare(strict_types=1); 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 + */ + public array $flattenedProperties = []; + /** * @var BlockStateUpgradeSchemaBlockRemap[][] * @phpstan-var array> @@ -93,6 +100,7 @@ final class BlockStateUpgradeSchema{ $this->removedProperties, $this->renamedProperties, $this->remappedPropertyValues, + $this->flattenedProperties, $this->remappedStates, ] as $list){ if(count($list) !== 0){ diff --git a/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaBlockRemap.php b/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaBlockRemap.php index 611ad04e2..676afbaf4 100644 --- a/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaBlockRemap.php +++ b/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaBlockRemap.php @@ -40,7 +40,7 @@ final class BlockStateUpgradeSchemaBlockRemap{ */ public function __construct( public array $oldState, - public string|BlockStateUpgradeSchemaFlattenedName $newName, + public string|BlockStateUpgradeSchemaFlattenInfo $newName, public array $newState, public array $copiedState ){} @@ -48,8 +48,8 @@ final class BlockStateUpgradeSchemaBlockRemap{ 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){ diff --git a/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaFlattenedName.php b/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaFlattenInfo.php similarity index 97% rename from src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaFlattenedName.php rename to src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaFlattenInfo.php index 8259f690d..4a14a1291 100644 --- a/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaFlattenedName.php +++ b/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaFlattenInfo.php @@ -29,7 +29,7 @@ use pocketmine\nbt\tag\StringTag; use function ksort; use const SORT_STRING; -final class BlockStateUpgradeSchemaFlattenedName{ +final class BlockStateUpgradeSchemaFlattenInfo{ /** * @param string[] $flattenedValueRemaps diff --git a/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaUtils.php b/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaUtils.php index d66b7e68c..08eba8978 100644 --- a/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaUtils.php +++ b/src/data/bedrock/block/upgrade/BlockStateUpgradeSchemaUtils.php @@ -25,7 +25,7 @@ namespace pocketmine\data\bedrock\block\upgrade; 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,24 +155,17 @@ final class BlockStateUpgradeSchemaUtils{ } } + 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)){ $remapName = $remap->newName; }elseif(isset($remap->newFlattenedName)){ $flattenRule = $remap->newFlattenedName; - $remapName = new BlockStateUpgradeSchemaFlattenedName( - $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'") - } - ); + $remapName = self::jsonModelToFlattenRule($flattenRule); }else{ throw new \UnexpectedValueException("Expected exactly one of 'newName' or 'newFlattenedName' properties to be set"); } @@ -265,6 +258,36 @@ final class BlockStateUpgradeSchemaUtils{ $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; @@ -303,25 +326,19 @@ final class BlockStateUpgradeSchemaUtils{ 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, - match($remap->newName->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 " . $remap->newName->flattenedPropertyType . " in flattened property type") - } - ), + is_string($remap->newName) ? $remap->newName : self::flattenRuleToJsonModel($remap->newName), array_map(fn(Tag $tag) => self::tagToJsonModel($tag), $remap->newState), $remap->copiedState ); diff --git a/src/data/bedrock/block/upgrade/BlockStateUpgrader.php b/src/data/bedrock/block/upgrade/BlockStateUpgrader.php index b0612585c..e95f3c80f 100644 --- a/src/data/bedrock/block/upgrade/BlockStateUpgrader.php +++ b/src/data/bedrock/block/upgrade/BlockStateUpgrader.php @@ -110,10 +110,21 @@ final class BlockStateUpgrader{ } $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); @@ -146,22 +157,9 @@ final class BlockStateUpgrader{ if(is_string($remap->newName)){ $newName = $remap->newName; }else{ - $flattenedValue = $oldState[$remap->newName->flattenedProperty] ?? null; - $expectedType = $remap->newName->flattenedPropertyType; - if(!$flattenedValue instanceof $expectedType){ - //flattened property is not of the expected type, so this transformation is not applicable - continue; - } - $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 = $remap->newName->flattenedValueRemaps[$embedKey] ?? $embedKey; - $newName = sprintf("%s%s%s", $remap->newName->prefix, $embedValue, $remap->newName->suffix); - unset($oldState[$remap->newName->flattenedProperty]); + //yes, overwriting $oldState here is intentional, although we probably don't actually need it anyway + //it shouldn't make any difference unless the flattened property appears in copiedState for some reason + [$newName, $oldState] = $this->applyPropertyFlattened($remap->newName, $oldName, $oldState); } $newState = $remap->newState; @@ -279,4 +277,32 @@ final class BlockStateUpgrader{ return $states; } + + /** + * @param Tag[] $states + * @phpstan-param array $states + * + * @return (string|Tag[])[] + * @phpstan-return array{0: string, 1: array} + */ + 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]; + } } diff --git a/src/data/bedrock/block/upgrade/model/BlockStateUpgradeSchemaModel.php b/src/data/bedrock/block/upgrade/model/BlockStateUpgradeSchemaModel.php index 1a4a14c87..7d91438e4 100644 --- a/src/data/bedrock/block/upgrade/model/BlockStateUpgradeSchemaModel.php +++ b/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 + */ + public array $flattenedProperties; + /** * @var BlockStateUpgradeSchemaModelBlockRemap[][] * @phpstan-var array> diff --git a/src/data/bedrock/block/upgrade/model/BlockStateUpgradeSchemaModelBlockRemap.php b/src/data/bedrock/block/upgrade/model/BlockStateUpgradeSchemaModelBlockRemap.php index 0f518479e..6accf1f02 100644 --- a/src/data/bedrock/block/upgrade/model/BlockStateUpgradeSchemaModelBlockRemap.php +++ b/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 $newState * @phpstan-param list $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; diff --git a/src/data/bedrock/block/upgrade/model/BlockStateUpgradeSchemaModelFlattenedName.php b/src/data/bedrock/block/upgrade/model/BlockStateUpgradeSchemaModelFlattenInfo.php similarity index 95% rename from src/data/bedrock/block/upgrade/model/BlockStateUpgradeSchemaModelFlattenedName.php rename to src/data/bedrock/block/upgrade/model/BlockStateUpgradeSchemaModelFlattenInfo.php index 6d03bbc12..6da590287 100644 --- a/src/data/bedrock/block/upgrade/model/BlockStateUpgradeSchemaModelFlattenedName.php +++ b/src/data/bedrock/block/upgrade/model/BlockStateUpgradeSchemaModelFlattenInfo.php @@ -25,7 +25,7 @@ namespace pocketmine\data\bedrock\block\upgrade\model; use function count; -final class BlockStateUpgradeSchemaModelFlattenedName implements \JsonSerializable{ +final class BlockStateUpgradeSchemaModelFlattenInfo implements \JsonSerializable{ /** @required */ public string $prefix; diff --git a/tests/phpunit/data/bedrock/block/upgrade/BlockStateUpgraderTest.php b/tests/phpunit/data/bedrock/block/upgrade/BlockStateUpgraderTest.php index 4d4d321ec..91afd8ed9 100644 --- a/tests/phpunit/data/bedrock/block/upgrade/BlockStateUpgraderTest.php +++ b/tests/phpunit/data/bedrock/block/upgrade/BlockStateUpgraderTest.php @@ -24,8 +24,10 @@ declare(strict_types=1); 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 @@ class BlockStateUpgraderTest extends TestCase{ 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 */ diff --git a/tools/blockstate-upgrade-schema-utils.php b/tools/blockstate-upgrade-schema-utils.php index b73954aff..e1a5e7c3b 100644 --- a/tools/blockstate-upgrade-schema-utils.php +++ b/tools/blockstate-upgrade-schema-utils.php @@ -27,7 +27,7 @@ use pocketmine\data\bedrock\block\BlockStateData; use pocketmine\data\bedrock\block\upgrade\BlockStateUpgrader; use pocketmine\data\bedrock\block\upgrade\BlockStateUpgradeSchema; use pocketmine\data\bedrock\block\upgrade\BlockStateUpgradeSchemaBlockRemap; -use pocketmine\data\bedrock\block\upgrade\BlockStateUpgradeSchemaFlattenedName; +use pocketmine\data\bedrock\block\upgrade\BlockStateUpgradeSchemaFlattenInfo; use pocketmine\data\bedrock\block\upgrade\BlockStateUpgradeSchemaUtils; use pocketmine\data\bedrock\block\upgrade\BlockStateUpgradeSchemaValueRemap; use pocketmine\nbt\LittleEndianNbtSerializer; @@ -183,6 +183,11 @@ function processStateGroup(string $oldName, array $upgradeTable, BlockStateUpgra $removedProperties = []; $renamedProperties = []; + $uniqueNewIds = []; + foreach($upgradeTable as $pair){ + $uniqueNewIds[$pair->new->getName()] = $pair->new->getName(); + } + foreach(Utils::stringifyKeys($newProperties) as $newPropertyName => $newPropertyValues){ if(count($newPropertyValues) === 1){ $newPropertyValue = $newPropertyValues[array_key_first($newPropertyValues)]; @@ -278,6 +283,61 @@ function processStateGroup(string $oldName, array $upgradeTable, BlockStateUpgra } } + if(count($uniqueNewIds) > 1){ + //detect possible flattening + $flattenedProperty = null; + $flattenedPropertyType = null; + $flattenedPropertyMap = []; + foreach($removedProperties as $removedProperty){ + $valueMap = []; + foreach($upgradeTable as $pair){ + $oldValue = $pair->old->getState($removedProperty); + if($oldValue === null){ + throw new AssumptionFailedError("We already checked that all states had consistent old properties"); + } + //TODO: lots of similar logic to the remappedStates builder below + if(!$oldValue instanceof ByteTag && !$oldValue instanceof IntTag && !$oldValue instanceof StringTag){ + //unknown property type - bad candidate for flattening + continue 2; + } + if($flattenedPropertyType === null){ + $flattenedPropertyType = get_class($oldValue); + }elseif(!$oldValue instanceof $flattenedPropertyType){ + //property type mismatch - bad candidate for flattening + continue 2; + } + + $rawValue = (string) $oldValue->getValue(); + $existingNewId = $valueMap[$rawValue] ?? null; + if($existingNewId !== null && $existingNewId !== $pair->new->getName()){ + //this property value is associated with multiple new IDs - bad candidate for flattening + continue 2; + } + $valueMap[$rawValue] = $pair->new->getName(); + } + + if($flattenedProperty !== null){ + //found multiple candidates for flattening - fallback to remappedStates + return false; + } + //we found a suitable candidate + $flattenedProperty = $removedProperty; + $flattenedPropertyMap = $valueMap; + break; + } + + if($flattenedProperty === null){ + //can't figure out how the new IDs are related to the old states - fallback to remappedStates + return false; + } + if($flattenedPropertyType === null){ + throw new AssumptionFailedError("This should never happen at this point"); + } + + $result->flattenedProperties[$oldName] = buildFlattenPropertyRule($flattenedPropertyMap, $flattenedProperty, $flattenedPropertyType); + unset($removedProperties[$flattenedProperty]); + } + //finally, write the results to the schema if(count($remappedPropertyValues) !== 0){ @@ -332,60 +392,69 @@ function findCommonSuffix(array $strings) : string{ return strrev(findCommonPrefix($reversed)); } +/** + * @param string[] $valueToId + * @phpstan-param array $valueToId + * @phpstan-param class-string $propertyType + */ +function buildFlattenPropertyRule(array $valueToId, string $propertyName, string $propertyType) : BlockStateUpgradeSchemaFlattenInfo{ + $ids = array_values($valueToId); + + //TODO: this is a bit too enthusiastic. For example, when flattening the old "stone", it will see that + //"granite", "andesite", "stone" etc all have "e" as a common suffix, which works, but looks a bit daft. + //This also causes more remaps to be generated than necessary, since some of the values are already + //contained in the new ID. + $idPrefix = findCommonPrefix($ids); + $idSuffix = findCommonSuffix($ids); + if(strlen($idSuffix) < 2){ + $idSuffix = ""; + } + + $valueMap = []; + foreach(Utils::stringifyKeys($valueToId) as $value => $newId){ + $newValue = substr($newId, strlen($idPrefix), $idSuffix !== "" ? -strlen($idSuffix) : null); + if($newValue !== $value){ + $valueMap[$value] = $newValue; + } + } + + $allNumeric = true; + if(count($valueMap) > 0){ + foreach(Utils::stringifyKeys($valueMap) as $value => $newValue){ + if(!is_numeric($value)){ + $allNumeric = false; + break; + } + } + if($allNumeric){ + //add a dummy key to force the JSON to be an object and not a list + $valueMap["dummy"] = "map_not_list"; + } + } + + return new BlockStateUpgradeSchemaFlattenInfo( + $idPrefix, + $propertyName, + $idSuffix, + $valueMap, + $propertyType, + ); +} + /** * @param string[][][] $candidateFlattenedValues * @phpstan-param array>> $candidateFlattenedValues * @param string[] $candidateFlattenPropertyTypes * @phpstan-param array> $candidateFlattenPropertyTypes * - * @return BlockStateUpgradeSchemaFlattenedName[][] - * @phpstan-return array> + * @return BlockStateUpgradeSchemaFlattenInfo[][] + * @phpstan-return array> */ function buildFlattenPropertyRules(array $candidateFlattenedValues, array $candidateFlattenPropertyTypes) : array{ $flattenPropertyRules = []; foreach(Utils::stringifyKeys($candidateFlattenedValues) as $propertyName => $filters){ foreach(Utils::stringifyKeys($filters) as $filter => $valueToId){ - $ids = array_values($valueToId); - - //TODO: this is a bit too enthusiastic. For example, when flattening the old "stone", it will see that - //"granite", "andesite", "stone" etc all have "e" as a common suffix, which works, but looks a bit daft. - //This also causes more remaps to be generated than necessary, since some of the values are already - //contained in the new ID. - $idPrefix = findCommonPrefix($ids); - $idSuffix = findCommonSuffix($ids); - if(strlen($idSuffix) < 2){ - $idSuffix = ""; - } - - $valueMap = []; - foreach(Utils::stringifyKeys($valueToId) as $value => $newId){ - $newValue = substr($newId, strlen($idPrefix), $idSuffix !== "" ? -strlen($idSuffix) : null); - if($newValue !== $value){ - $valueMap[$value] = $newValue; - } - } - - $allNumeric = true; - if(count($valueMap) > 0){ - foreach(Utils::stringifyKeys($valueMap) as $value => $newValue){ - if(!is_numeric($value)){ - $allNumeric = false; - break; - } - } - if($allNumeric){ - //add a dummy key to force the JSON to be an object and not a list - $valueMap["dummy"] = "map_not_list"; - } - } - - $flattenPropertyRules[$propertyName][$filter] = new BlockStateUpgradeSchemaFlattenedName( - $idPrefix, - $propertyName, - $idSuffix, - $valueMap, - $candidateFlattenPropertyTypes[$propertyName], - ); + $flattenPropertyRules[$propertyName][$filter] = buildFlattenPropertyRule($valueToId, $propertyName, $candidateFlattenPropertyTypes[$propertyName]); } } ksort($flattenPropertyRules, SORT_STRING); @@ -642,10 +711,15 @@ function generateBlockStateUpgradeSchema(array $upgradeTable) : BlockStateUpgrad throw new \RuntimeException("States with the same ID should be fully consistent"); } }else{ - //block mapped to multiple different new IDs; we can't guess these, so we just do a plain old remap - //even if some of the states stay under the same ID, the compression techniques used by this function - //implicitly rely on knowing the full set of old states and their new transformations - $result->remappedStates[$oldName] = processRemappedStates($blockStateMappings); + //try processing this as a regular state group first + //if a property was flattened into the ID, the remaining states will normally be consistent + //if not we fall back to remap states and state filters + if(!processStateGroup($oldName, $blockStateMappings, $result)){ + //block mapped to multiple different new IDs; we can't guess these, so we just do a plain old remap + //even if some of the states stay under the same ID, the compression techniques used by this function + //implicitly rely on knowing the full set of old states and their new transformations + $result->remappedStates[$oldName] = processRemappedStates($blockStateMappings); + } } } From d01203d7c4d48be9c7a92dbfa385ac8844ea1841 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 24 Oct 2024 18:50:28 +0100 Subject: [PATCH 11/17] Reduce code duplication --- tools/blockstate-upgrade-schema-utils.php | 81 +++++++++++------------ 1 file changed, 38 insertions(+), 43 deletions(-) diff --git a/tools/blockstate-upgrade-schema-utils.php b/tools/blockstate-upgrade-schema-utils.php index e1a5e7c3b..b7a9a4169 100644 --- a/tools/blockstate-upgrade-schema-utils.php +++ b/tools/blockstate-upgrade-schema-utils.php @@ -295,25 +295,9 @@ function processStateGroup(string $oldName, array $upgradeTable, BlockStateUpgra if($oldValue === null){ throw new AssumptionFailedError("We already checked that all states had consistent old properties"); } - //TODO: lots of similar logic to the remappedStates builder below - if(!$oldValue instanceof ByteTag && !$oldValue instanceof IntTag && !$oldValue instanceof StringTag){ - //unknown property type - bad candidate for flattening + if(!checkFlattenPropertySuitability($oldValue, $flattenedPropertyType, $pair->new->getName(), $valueMap)){ continue 2; } - if($flattenedPropertyType === null){ - $flattenedPropertyType = get_class($oldValue); - }elseif(!$oldValue instanceof $flattenedPropertyType){ - //property type mismatch - bad candidate for flattening - continue 2; - } - - $rawValue = (string) $oldValue->getValue(); - $existingNewId = $valueMap[$rawValue] ?? null; - if($existingNewId !== null && $existingNewId !== $pair->new->getName()){ - //this property value is associated with multiple new IDs - bad candidate for flattening - continue 2; - } - $valueMap[$rawValue] = $pair->new->getName(); } if($flattenedProperty !== null){ @@ -392,6 +376,37 @@ function findCommonSuffix(array $strings) : string{ return strrev(findCommonPrefix($reversed)); } +/** + * @param string[] $valueToIdMap + * @phpstan-param ?class-string $expectedType + * @phpstan-param-out class-string $expectedType + * @phpstan-param array $valueToIdMap + * @phpstan-param-out array $valueToIdMap + */ +function checkFlattenPropertySuitability(Tag $oldValue, ?string &$expectedType, string $actualNewId, array &$valueToIdMap) : bool{ + //TODO: lots of similar logic to the remappedStates builder below + if(!$oldValue instanceof ByteTag && !$oldValue instanceof IntTag && !$oldValue instanceof StringTag){ + //unknown property type - bad candidate for flattening + return false; + } + if($expectedType === null){ + $expectedType = get_class($oldValue); + }elseif(!$oldValue instanceof $expectedType){ + //property type mismatch - bad candidate for flattening + return false; + } + + $rawValue = (string) $oldValue->getValue(); + $existingNewId = $valueToIdMap[$rawValue] ?? null; + if($existingNewId !== null && $existingNewId !== $actualNewId){ + //this property value is associated with multiple new IDs - bad candidate for flattening + return false; + } + $valueToIdMap[$rawValue] = $actualNewId; + + return true; +} + /** * @param string[] $valueToId * @phpstan-param array $valueToId @@ -522,23 +537,6 @@ function processRemappedStates(array $upgradeTable) : array{ if(isset($notFlattenedProperties[$propertyName])){ continue; } - if(!$propertyValue instanceof StringTag && !$propertyValue instanceof IntTag && !$propertyValue instanceof ByteTag){ - $notFlattenedProperties[$propertyName] = true; - continue; - } - $previousType = $candidateFlattenedPropertyTypes[$propertyName] ?? null; - if($previousType !== null && $previousType !== get_class($propertyValue)){ - //mismatched types for the same property name - this has never happened so far, but it's not impossible - $notFlattenedProperties[$propertyName] = true; - continue; - } - $candidateFlattenedPropertyTypes[$propertyName] = get_class($propertyValue); - - $rawValue = (string) $propertyValue->getValue(); - if($rawValue === ""){ - $notFlattenedProperties[$propertyName] = true; - continue; - } $filter = $pair->old->getStates(); foreach($unchangedStatesByNewName[$pair->new->getName()] as $unchangedPropertyName){ @@ -551,16 +549,13 @@ function processRemappedStates(array $upgradeTable) : array{ unset($filter[$propertyName]); $rawFilter = encodeOrderedProperties($filter); - if(isset($candidateFlattenedValues[$propertyName][$rawFilter])){ - $valuesToIds = $candidateFlattenedValues[$propertyName][$rawFilter]; - $existingNewId = $valuesToIds[$rawValue] ?? null; - if($existingNewId !== null && $existingNewId !== $pair->new->getName()){ - //this old value is associated with multiple new IDs - bad candidate for flattening - $notFlattenedProperties[$propertyName] = true; - continue; - } + $candidateFlattenedValues[$propertyName][$rawFilter] ??= []; + $expectedType = $candidateFlattenedPropertyTypes[$propertyName] ?? null; + if(!checkFlattenPropertySuitability($propertyValue, $expectedType, $pair->new->getName(), $candidateFlattenedValues[$propertyName][$rawFilter])){ + $notFlattenedProperties[$propertyName] = true; + continue; } - $candidateFlattenedValues[$propertyName][$rawFilter][$rawValue] = $pair->new->getName(); + $candidateFlattenedPropertyTypes[$propertyName] = $expectedType; } } foreach(Utils::stringifyKeys($candidateFlattenedValues) as $propertyName => $filters){ From c0b74b03419038f34bb25fa0c5dc05440bf86c5b Mon Sep 17 00:00:00 2001 From: "Dylan T." Date: Sun, 3 Nov 2024 14:05:46 +0000 Subject: [PATCH 12/17] Update BlockStateUpgrader.php --- src/data/bedrock/block/upgrade/BlockStateUpgrader.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/data/bedrock/block/upgrade/BlockStateUpgrader.php b/src/data/bedrock/block/upgrade/BlockStateUpgrader.php index e95f3c80f..2dce762b8 100644 --- a/src/data/bedrock/block/upgrade/BlockStateUpgrader.php +++ b/src/data/bedrock/block/upgrade/BlockStateUpgrader.php @@ -157,9 +157,8 @@ final class BlockStateUpgrader{ if(is_string($remap->newName)){ $newName = $remap->newName; }else{ - //yes, overwriting $oldState here is intentional, although we probably don't actually need it anyway - //it shouldn't make any difference unless the flattened property appears in copiedState for some reason - [$newName, $oldState] = $this->applyPropertyFlattened($remap->newName, $oldName, $oldState); + //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; From c63d0ef1b6187ab7bbffb70625d509620c1e3ae7 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sun, 3 Nov 2024 14:39:41 +0000 Subject: [PATCH 13/17] Fix dodgy ignored PHPStan error --- src/network/mcpe/InventoryManager.php | 1 + tests/phpstan/configs/actual-problems.neon | 5 ----- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/network/mcpe/InventoryManager.php b/src/network/mcpe/InventoryManager.php index c0969b61b..e4c303121 100644 --- a/src/network/mcpe/InventoryManager.php +++ b/src/network/mcpe/InventoryManager.php @@ -364,6 +364,7 @@ class InventoryManager{ FurnaceType::FURNACE => WindowTypes::FURNACE, FurnaceType::BLAST_FURNACE => WindowTypes::BLAST_FURNACE, FurnaceType::SMOKER => WindowTypes::SMOKER, + FurnaceType::CAMPFIRE, FurnaceType::SOUL_CAMPFIRE => throw new \LogicException("Campfire inventory cannot be displayed to a player") }, $inv instanceof EnchantInventory => WindowTypes::ENCHANTMENT, $inv instanceof BrewingStandInventory => WindowTypes::BREWING_STAND, diff --git a/tests/phpstan/configs/actual-problems.neon b/tests/phpstan/configs/actual-problems.neon index 2d0e6d398..cc647da80 100644 --- a/tests/phpstan/configs/actual-problems.neon +++ b/tests/phpstan/configs/actual-problems.neon @@ -580,11 +580,6 @@ parameters: count: 1 path: ../../../src/network/mcpe/ChunkRequestTask.php - - - message: "#^Match expression does not handle remaining values\\: pocketmine\\\\crafting\\\\FurnaceType\\:\\:CAMPFIRE\\|pocketmine\\\\crafting\\\\FurnaceType\\:\\:SOUL_CAMPFIRE$#" - count: 1 - path: ../../../src/network/mcpe/InventoryManager.php - - message: "#^Cannot call method doFirstSpawn\\(\\) on pocketmine\\\\player\\\\Player\\|null\\.$#" count: 1 From 72fc1386319f481328d4c9b4f7efe899767ecf81 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sun, 3 Nov 2024 14:43:15 +0000 Subject: [PATCH 14/17] Regenerate PHPStan baselines --- tests/phpstan/configs/actual-problems.neon | 25 ---------------------- tests/phpstan/configs/phpstan-bugs.neon | 14 ++++++------ 2 files changed, 7 insertions(+), 32 deletions(-) diff --git a/tests/phpstan/configs/actual-problems.neon b/tests/phpstan/configs/actual-problems.neon index cc647da80..e778cf004 100644 --- a/tests/phpstan/configs/actual-problems.neon +++ b/tests/phpstan/configs/actual-problems.neon @@ -785,11 +785,6 @@ parameters: count: 1 path: ../../../src/resourcepacks/ZippedResourcePack.php - - - message: "#^Parameter \\#2 \\$length of function fread expects int\\<0, max\\>, int given\\.$#" - count: 1 - path: ../../../src/resourcepacks/ZippedResourcePack.php - - message: "#^Property pocketmine\\\\resourcepacks\\\\ZippedResourcePack\\:\\:\\$fileResource \\(resource\\) does not accept resource\\|false\\.$#" count: 1 @@ -1035,11 +1030,6 @@ parameters: count: 1 path: ../../../src/world/format/io/region/RegionLoader.php - - - message: "#^Parameter \\#2 \\$length of function fread expects int\\<0, max\\>, int given\\.$#" - count: 1 - path: ../../../src/world/format/io/region/RegionLoader.php - - message: "#^Parameter \\#2 \\$size of function ftruncate expects int\\<0, max\\>, int given\\.$#" count: 1 @@ -1190,18 +1180,3 @@ parameters: count: 1 path: ../../../src/world/light/SkyLightUpdate.php - - - message: "#^Parameter \\#1 \\$json of function json_decode expects string, string\\|false given\\.$#" - count: 1 - path: ../../phpunit/block/BlockTest.php - - - - message: "#^Parameter \\#1 \\$json of function json_decode expects string, string\\|false given\\.$#" - count: 1 - path: ../../phpunit/block/regenerate_consistency_check.php - - - - message: "#^Parameter \\#1 \\$logFile of class pocketmine\\\\utils\\\\MainLogger constructor expects string, string\\|false given\\.$#" - count: 1 - path: ../../phpunit/scheduler/AsyncPoolTest.php - diff --git a/tests/phpstan/configs/phpstan-bugs.neon b/tests/phpstan/configs/phpstan-bugs.neon index 0ead377ba..0fc3defda 100644 --- a/tests/phpstan/configs/phpstan-bugs.neon +++ b/tests/phpstan/configs/phpstan-bugs.neon @@ -5,20 +5,20 @@ parameters: count: 1 path: ../../../src/block/CakeWithCandle.php - - - message: "#^Method pocketmine\\\\block\\\\DoubleTallGrass\\:\\:traitGetDropsForIncompatibleTool\\(\\) return type has no value type specified in iterable type array\\.$#" - count: 1 - path: ../../../src/block/DoubleTallGrass.php - - message: "#^Method pocketmine\\\\block\\\\CopperDoor\\:\\:onInteractCopper\\(\\) has parameter \\$returnedItems with no value type specified in iterable type array\\.$#" count: 1 - path: ../../../src/block/utils/CopperTrait.php + path: ../../../src/block/CopperDoor.php - message: "#^Method pocketmine\\\\block\\\\CopperTrapdoor\\:\\:onInteractCopper\\(\\) has parameter \\$returnedItems with no value type specified in iterable type array\\.$#" count: 1 - path: ../../../src/block/utils/CopperTrait.php + path: ../../../src/block/CopperTrapdoor.php + + - + message: "#^Method pocketmine\\\\block\\\\DoubleTallGrass\\:\\:traitGetDropsForIncompatibleTool\\(\\) return type has no value type specified in iterable type array\\.$#" + count: 1 + path: ../../../src/block/DoubleTallGrass.php - message: "#^Call to function assert\\(\\) with false and 'unknown hit type' will always evaluate to false\\.$#" From 84464cde4f6f3506e0b55e136d83592de2ce1fa0 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sun, 3 Nov 2024 14:44:50 +0000 Subject: [PATCH 15/17] Update BedrockBlockUpgradeSchema --- composer.json | 2 +- composer.lock | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/composer.json b/composer.json index 9ecbad32f..ed5bb582f 100644 --- a/composer.json +++ b/composer.json @@ -33,7 +33,7 @@ "composer-runtime-api": "^2.0", "adhocore/json-comment": "~1.2.0", "pocketmine/netresearch-jsonmapper": "~v4.4.999", - "pocketmine/bedrock-block-upgrade-schema": "~4.5.0+bedrock-1.21.40", + "pocketmine/bedrock-block-upgrade-schema": "~5.0.0+bedrock-1.21.40", "pocketmine/bedrock-data": "~2.14.0+bedrock-1.21.40", "pocketmine/bedrock-item-upgrade-schema": "~1.13.0+bedrock-1.21.40", "pocketmine/bedrock-protocol": "~35.0.0+bedrock-1.21.40", diff --git a/composer.lock b/composer.lock index d4cb38ddc..b2f3e7858 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "5c5882370131d2ae3a043819c05e6f9c", + "content-hash": "b2fbf6e7a9d650341dc71fa4dd124681", "packages": [ { "name": "adhocore/json-comment", @@ -127,16 +127,16 @@ }, { "name": "pocketmine/bedrock-block-upgrade-schema", - "version": "4.5.0", + "version": "5.0.0", "source": { "type": "git", "url": "https://github.com/pmmp/BedrockBlockUpgradeSchema.git", - "reference": "7943b894e050d68dd21b5c7fa609827a4e2e30f1" + "reference": "20dd5c11e9915bacea4fe2cf649e1d23697a6e52" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/pmmp/BedrockBlockUpgradeSchema/zipball/7943b894e050d68dd21b5c7fa609827a4e2e30f1", - "reference": "7943b894e050d68dd21b5c7fa609827a4e2e30f1", + "url": "https://api.github.com/repos/pmmp/BedrockBlockUpgradeSchema/zipball/20dd5c11e9915bacea4fe2cf649e1d23697a6e52", + "reference": "20dd5c11e9915bacea4fe2cf649e1d23697a6e52", "shasum": "" }, "type": "library", @@ -147,9 +147,9 @@ "description": "Schemas describing how to upgrade saved block data in older Minecraft: Bedrock Edition world saves", "support": { "issues": "https://github.com/pmmp/BedrockBlockUpgradeSchema/issues", - "source": "https://github.com/pmmp/BedrockBlockUpgradeSchema/tree/4.5.0" + "source": "https://github.com/pmmp/BedrockBlockUpgradeSchema/tree/5.0.0" }, - "time": "2024-10-23T16:15:24+00:00" + "time": "2024-11-03T14:13:50+00:00" }, { "name": "pocketmine/bedrock-data", From 96b12bddc1149d9a8610c6bffc555609790b70e8 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sun, 3 Nov 2024 15:24:43 +0000 Subject: [PATCH 16/17] Prepare 5.21.0 release --- changelogs/5.21.md | 103 ++++++++++++++++++++++++++++++++++++++++++++ src/VersionInfo.php | 4 +- 2 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 changelogs/5.21.md diff --git a/changelogs/5.21.md b/changelogs/5.21.md new file mode 100644 index 000000000..b8131a3c8 --- /dev/null +++ b/changelogs/5.21.md @@ -0,0 +1,103 @@ +# 5.21.0 +Released 3rd November 2024. + +This is a minor feature release, including gameplay features and minor internals improvements. + +**Plugin compatibility:** Plugins for previous 5.x versions will run unchanged on this release, unless they use internal APIs, reflection, or packages like the `pocketmine\network\mcpe` or `pocketmine\data` namespace. +Do not update plugin minimum API versions unless you need new features added in this release. + +**WARNING: If your plugin uses the `pocketmine\network\mcpe` namespace, you're not shielded by API change constraints.** +Consider using the `mcpe-protocol` directive in `plugin.yml` as a constraint if you're using packets directly. + +## Gameplay +- Added the following new blocks: + - Campfire + - Chiseled Copper + - Chiseled Tuff + - Chiseled Tuff Bricks + - Copper Bulb + - Copper Door + - Copper Grate + - Copper Trapdoor + - Polished Tuff, Slabs, Stairs and Walls + - Soul Campfire + - Tuff Bricks, Slabs, Stairs and Walls + - Tuff Slab, Stairs and Walls +- Added the following new types of painting: + - backyard + - baroque + - bouquet + - cavebird + - changing + - cotan + - endboss + - fern + - finding + - humble + - lowmist + - meditative + - orb + - owlemons + - passage + - pond + - prairie_ride + - sunflowers + - tides + - unpacked +- Armor slots are now properly restricted (on the server side) to only contain the appropriate type of armor or headwear. +- Implemented Aqua Affinity enchantment. Since the server doesn't currently enforce any movement restrictions in water, this enchantment works based on client-side behaviour only. + +## API +### `pocketmine\block` +- The following new API methods have been added: + - `public ChiseledBookshelf->getLastInteractedSlot() : ?ChiseledBookshelfSlot` + - `public ChiseledBookshelf->setLastInteractedSlot(?ChiseledBookshelfSlot $lastInteractedSlot) : $this` +- The following new classes have been added: + - `utils\CopperMaterial` - interface implemented by all copper-like blocks with oxidation and waxed properties + - `CopperBulb` + - `CopperDoor` + - `CopperGrate` + - `CopperTrapdoor` + - `SoulCampfire` + - `Campfire` +- The following enums have new cases: + - `utils\BannerPatternType` has new cases `FLOW` and `GUSTER` + +### `pocketmine\crafting` +- The following enums have new cases: + - `FurnaceType` has new cases `CAMPFIRE` and `SOUL_CAMPFIRE` + +### `pocketmine\event` +- The following new classes have been added: + - `block\CampfireCookEvent` - called when a campfire finishes cooking an item + +### `pocketmine\inventory` +- Added support for slot validators, which permit restricting the types of items a player can put into an inventory slot. + - The following new classes have been added: + - `transaction\action\SlotValidator` - interface + - `transaction\action\CallbackSlotValidator` - class allowing a closure to be used for slot content validation + - `SlotValidatedInventory` - implemented by inventories which support the use of slot validators + +### `pocketmine\item` +- The following new API methods have been added: + - `public Item->getCooldownTag() : ?string` - returns the cooldown group this item belongs to, used for ensuring that, for example, different types of goat horns all respect a general horn cooldown +- The following new classes have been added: + - `ItemCooldownTags` - list of cooldown group tags used by PocketMine-MP + +### `pocketmine\world\sound` +- The following new classes have been added + - `CampfireSound` - sound made by campfires while lit + +## Tools +- `tools/blockstate-upgrade-schema-utils.php` (formerly `generate-blockstate-upgrade-schema.php`) has several improvements: + - Support for generating `flattenedProperties` rules as per [BedrockBlockUpgradeSchema 5.0.0](https://github.com/pmmp/BedrockBlockUpgradeSchema/releases/tag/5.0.0) + - Improved criteria for flattened property selection to minimize the amount of rules required + - Several subcommands are now available: + - `generate` - generates a schema from provided data + - `update` - regenerates an existing schema in a newer format + - `update-all` - regenerates a folder of existing schemas in a newer format (useful for updating `BedrockBlockUpgradeSchema` en masse) + - `test` - verifies that a schema produces the results expected by provided data + +## Internals +- Fixed incorrect visibility of `createEntity` in spawn eggs. +- Added support for newer `BedrockBlockUpgradeSchema` in `BlockStateUpgrader`. diff --git a/src/VersionInfo.php b/src/VersionInfo.php index 5de1f5a05..4dc9ea7f9 100644 --- a/src/VersionInfo.php +++ b/src/VersionInfo.php @@ -31,8 +31,8 @@ use function str_repeat; final class VersionInfo{ public const NAME = "PocketMine-MP"; - public const BASE_VERSION = "5.20.2"; - public const IS_DEVELOPMENT_BUILD = true; + public const BASE_VERSION = "5.21.0"; + public const IS_DEVELOPMENT_BUILD = false; public const BUILD_CHANNEL = "stable"; /** From e598364f0695495cbe71ddf0b62f134b51091c6e Mon Sep 17 00:00:00 2001 From: github-actions Date: Sun, 3 Nov 2024 15:30:16 +0000 Subject: [PATCH 17/17] 5.21.1 is next Commit created by: https://github.com/pmmp/RestrictedActions/actions/runs/11652565588 --- src/VersionInfo.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/VersionInfo.php b/src/VersionInfo.php index 4dc9ea7f9..8fe6d32dd 100644 --- a/src/VersionInfo.php +++ b/src/VersionInfo.php @@ -31,8 +31,8 @@ use function str_repeat; final class VersionInfo{ public const NAME = "PocketMine-MP"; - public const BASE_VERSION = "5.21.0"; - public const IS_DEVELOPMENT_BUILD = false; + public const BASE_VERSION = "5.21.1"; + public const IS_DEVELOPMENT_BUILD = true; public const BUILD_CHANNEL = "stable"; /**