Remove nonsensical code from block and item serializers

At the time when I wrote this code, I was still in the head space of the kind of ID hijacking that PM4 plugins do to override built-in blocks.

However, this kind of internal ID reuse makes no sense in PM5, since said IDs are only used in the core itself at runtime to identify types and states.

Even if we were to allow overriding block deserializers, overriding serializers makes no sense whatsoever, since the original block would continue to exist and be accessible.
There is a case to be made to allow overriding the deserializer, but not for the serializer.
This commit is contained in:
Dylan K. Taylor 2023-05-20 18:55:36 +01:00
parent 1cb7846f7c
commit 5afeeb8f89
No known key found for this signature in database
GPG Key ID: 8927471A91CAFD3D
2 changed files with 26 additions and 41 deletions

View File

@ -169,7 +169,6 @@ use pocketmine\data\bedrock\block\convert\BlockStateWriter as Writer;
use pocketmine\math\Axis;
use pocketmine\math\Facing;
use pocketmine\utils\AssumptionFailedError;
use function class_parents;
use function get_class;
final class BlockObjectToStateSerializer implements BlockStateSerializer{
@ -177,8 +176,8 @@ final class BlockObjectToStateSerializer implements BlockStateSerializer{
* These callables actually accept Block, but for the sake of type completeness, it has to be never, since we can't
* describe the bottom type of a type hierarchy only containing Block.
*
* @var \Closure[][]
* @phpstan-var array<int, array<class-string, \Closure(never) : Writer>>
* @var \Closure[]
* @phpstan-var array<int, \Closure(never) : Writer>
*/
private array $serializers = [];
@ -212,7 +211,7 @@ final class BlockObjectToStateSerializer implements BlockStateSerializer{
if(isset($this->serializers[$block->getTypeId()])){
throw new \InvalidArgumentException("Block type ID " . $block->getTypeId() . " already has a serializer registered");
}
$this->serializers[$block->getTypeId()][get_class($block)] = $serializer;
$this->serializers[$block->getTypeId()] = $serializer;
}
public function mapSimple(Block $block, string $id) : void{
@ -240,21 +239,16 @@ final class BlockObjectToStateSerializer implements BlockStateSerializer{
public function serializeBlock(Block $blockState) : BlockStateData{
$typeId = $blockState->getTypeId();
$locatedSerializer = $this->serializers[$typeId][get_class($blockState)] ?? null;
if($locatedSerializer === null){
foreach(class_parents($blockState) as $parent){
if(isset($this->serializers[$typeId][$parent])){
$locatedSerializer = $this->serializers[$typeId][$parent];
break;
}
}
}
$locatedSerializer = $this->serializers[$typeId] ?? null;
if($locatedSerializer === null){
throw new BlockStateSerializeException("No serializer registered for " . get_class($blockState) . " with type ID $typeId");
}
/**
* TODO: there is no guarantee that this type actually matches that of $blockState - a plugin may have stolen
* the type ID of the block (which never makes sense, even in a world where overriding block types is a thing).
* In the future we'll need some way to guarantee that type IDs are never reused (perhaps spl_object_id()?)
*
* @var \Closure $serializer
* @phpstan-var \Closure(TBlockType) : Writer $serializer
*/

View File

@ -33,7 +33,6 @@ use pocketmine\item\Item;
use pocketmine\item\ItemBlock;
use pocketmine\item\VanillaItems as Items;
use pocketmine\utils\AssumptionFailedError;
use function class_parents;
use function get_class;
final class ItemSerializer{
@ -41,14 +40,14 @@ final class ItemSerializer{
* These callables actually accept Item, but for the sake of type completeness, it has to be never, since we can't
* describe the bottom type of a type hierarchy only containing Item.
*
* @var \Closure[][]
* @phpstan-var array<int, array<class-string, \Closure(never) : Data>>
* @var \Closure[]
* @phpstan-var array<int, \Closure(never) : Data>
*/
private array $itemSerializers = [];
/**
* @var \Closure[][]
* @phpstan-var array<int, array<class-string, \Closure(never) : Data>>
* @phpstan-var array<int, \Closure(never) : Data>
*/
private array $blockItemSerializers = [];
@ -69,7 +68,7 @@ final class ItemSerializer{
if(isset($this->itemSerializers[$index])){
throw new \InvalidArgumentException("Item type ID " . $index . " already has a serializer registered");
}
$this->itemSerializers[$index][get_class($item)] = $serializer;
$this->itemSerializers[$index] = $serializer;
}
/**
@ -82,7 +81,7 @@ final class ItemSerializer{
if(isset($this->blockItemSerializers[$index])){
throw new AssumptionFailedError("Registering the same blockitem twice!");
}
$this->blockItemSerializers[$index][get_class($block)] = $serializer;
$this->blockItemSerializers[$index] = $serializer;
}
/**
@ -100,21 +99,16 @@ final class ItemSerializer{
}else{
$index = $item->getTypeId();
$locatedSerializer = $this->itemSerializers[$index][get_class($item)] ?? null;
if($locatedSerializer === null){
foreach(class_parents($item) as $parent){
if(isset($this->itemSerializers[$index][$parent])){
$locatedSerializer = $this->itemSerializers[$index][$parent];
break;
}
}
}
$locatedSerializer = $this->itemSerializers[$index] ?? null;
if($locatedSerializer === null){
throw new ItemTypeSerializeException("No serializer registered for " . get_class($item) . " ($index) " . $item->getName());
}
/**
* TODO: there is no guarantee that this type actually matches that of $item - a plugin may have stolen
* the type ID of the item (which never makes sense, even in a world where overriding item types is a thing).
* In the future we'll need some way to guarantee that type IDs are never reused (perhaps spl_object_id()?)
*
* @var \Closure $serializer
* @phpstan-var \Closure(TItemType) : Data $serializer
*/
@ -156,18 +150,15 @@ final class ItemSerializer{
private function serializeBlockItem(Block $block) : Data{
$index = $block->getTypeId();
$locatedSerializer = $this->blockItemSerializers[$index][get_class($block)] ?? null;
if($locatedSerializer === null){
foreach(class_parents($block) as $parent){
if(isset($this->blockItemSerializers[$index][$parent])){
$locatedSerializer = $this->blockItemSerializers[$index][$parent];
break;
}
}
}
$locatedSerializer = $this->blockItemSerializers[$index] ?? null;
if($locatedSerializer !== null){
/** @phpstan-var \Closure(TBlockType) : Data $serializer */
/**
* TODO: there is no guarantee that this type actually matches that of $block - a plugin may have stolen
* the type ID of the block (which never makes sense, even in a world where overriding block types is a thing).
* In the future we'll need some way to guarantee that type IDs are never reused (perhaps spl_object_id()?)
*
* @phpstan-var \Closure(TBlockType) : Data $serializer
*/
$serializer = $locatedSerializer;
$data = $serializer($block);
}else{