Block: Separate encoding of type and state data

the terminology of this needs improvement, but...

the basic concept here is that 'type' data will persist on an itemstack, while 'state' data will not.

Type data consists of things like:
- Colour
- Coral type
- Wet/dry (sponges)
- Live/dead (coral)
- Wood type

State data consists of things like:
- Facing
- Axis
- Powered/unpowered
- Open/closed

In the past, with the old system, this information was separated by way of getStateBitmask(). This solution was fraught with problems, but achieved the basic goal: removing unwanted block properties from items.

src/block/Anvil.php

@@ -51,15 +51,23 @@ class Anvil extends Transparent implements Fallable{
 		return $this->damage << 2;
 	}
 
-	public function getRequiredStateDataBits() : int{ return 4; }
+	public function getRequiredTypeDataBits() : int{ return 2; }
+
+	protected function decodeType(BlockDataReader $r) : void{
+		$this->setDamage($r->readBoundedInt(2, self::UNDAMAGED, self::VERY_DAMAGED));
+	}
+
+	protected function encodeType(BlockDataWriter $w) : void{
+		$w->writeInt(2, $this->getDamage());
+	}
+
+	public function getRequiredStateDataBits() : int{ return 2; }
 
 	protected function decodeState(BlockDataReader $r) : void{
-		$this->setDamage($r->readBoundedInt(2, self::UNDAMAGED, self::VERY_DAMAGED));
 		$this->setFacing($r->readHorizontalFacing());
 	}
 
 	protected function encodeState(BlockDataWriter $w) : void{
-		$w->writeInt(2, $this->getDamage());
 		$w->writeHorizontalFacing($this->getFacing());
 	}
 

src/block/BaseBanner.php

@@ -55,16 +55,6 @@ abstract class BaseBanner extends Transparent{
 		parent::__construct($idInfo, $name, $breakInfo);
 	}
 
-	public function getRequiredStateDataBits() : int{ return 0; }
-
-	protected function decodeState(BlockDataReader $r) : void{
-		//TODO: we currently purposely don't read or write colour (it's stored on the tile)
-	}
-
-	protected function encodeState(BlockDataWriter $w) : void{
-		//TODO: we currently purposely don't read or write colour (it's stored on the tile)
-	}
-
 	public function readStateFromWorld() : void{
 		parent::readStateFromWorld();
 		$tile = $this->position->getWorld()->getTile($this->position);

src/block/Bed.php

@@ -60,14 +60,12 @@ class Bed extends Transparent{
 		$this->facing = $r->readHorizontalFacing();
 		$this->occupied = $r->readBool();
 		$this->head = $r->readBool();
-		//TODO: we currently purposely don't read or write colour (it's stored on the tile)
 	}
 
 	protected function encodeState(BlockDataWriter $w) : void{
 		$w->writeHorizontalFacing($this->facing);
 		$w->writeBool($this->occupied);
 		$w->writeBool($this->head);
-		//TODO: we currently purposely don't read or write colour (it's stored on the tile)
 	}
 
 	public function readStateFromWorld() : void{

src/block/Block.php

@@ -108,16 +108,31 @@ class Block{
 		return 0;
 	}
 
+	public function getRequiredTypeDataBits() : int{ return 0; }
+
 	public function getRequiredStateDataBits() : int{ return 0; }
 
 	final public function decodeStateData(int $data) : void{
-		$givenBits = $this->getRequiredStateDataBits();
+		$typeBits = $this->getRequiredTypeDataBits();
+		$stateBits = $this->getRequiredStateDataBits();
+		$givenBits = $typeBits + $stateBits;
 		$reader = new BlockDataReader($givenBits, $data);
-		$this->decodeState($reader);
+
+		$this->decodeType($reader);
 		$readBits = $reader->getOffset();
-		if($givenBits !== $readBits){
-			throw new \LogicException("Exactly $givenBits bits of state data were provided, but only $readBits were read");
+		if($typeBits !== $readBits){
+			throw new \LogicException("Exactly $typeBits bits of type data were provided, but $readBits were read");
 		}
+
+		$this->decodeState($reader);
+		$readBits = $reader->getOffset() - $typeBits;
+		if($stateBits !== $readBits){
+			throw new \LogicException("Exactly $stateBits bits of state data were provided, but $readBits were read");
+		}
+	}
+
+	protected function decodeType(BlockDataReader $r) : void{
+		//NOOP
 	}
 
 	protected function decodeState(BlockDataReader $r) : void{
@@ -128,17 +143,30 @@ class Block{
 	 * @internal
 	 */
 	final public function computeStateData() : int{
-		$requiredBits = $this->getRequiredStateDataBits();
+		$typeBits = $this->getRequiredTypeDataBits();
+		$stateBits = $this->getRequiredStateDataBits();
+		$requiredBits = $typeBits + $stateBits;
 		$writer = new BlockDataWriter($requiredBits);
-		$this->encodeState($writer);
 
+		$this->encodeType($writer);
 		$writtenBits = $writer->getOffset();
-		if($requiredBits !== $writtenBits){
-			throw new \LogicException("Exactly $requiredBits bits of state data were expected, but only $writtenBits were written");
+		if($typeBits !== $writtenBits){
+			throw new \LogicException("Exactly $typeBits bits of type data were expected, but $writtenBits were written");
 		}
+
+		$this->encodeState($writer);
+		$writtenBits = $writer->getOffset() - $typeBits;
+		if($stateBits !== $writtenBits){
+			throw new \LogicException("Exactly $stateBits bits of state data were expected, but $writtenBits were written");
+		}
+
 		return $writer->getValue();
 	}
 
+	protected function encodeType(BlockDataWriter $w) : void{
+		//NOOP
+	}
+
 	protected function encodeState(BlockDataWriter $w) : void{
 		//NOOP
 	}

src/block/BlockFactory.php

@@ -864,7 +864,7 @@ class BlockFactory{
 
 		//TODO: this bruteforce approach to discovering all valid states is very inefficient for larger state data sizes
 		//at some point we'll need to find a better way to do this
-		$bits = $block->getRequiredStateDataBits();
+		$bits = $block->getRequiredTypeDataBits() + $block->getRequiredStateDataBits();
 		if($bits > Block::INTERNAL_STATE_DATA_BITS){
 			throw new \InvalidArgumentException("Block state data cannot use more than " . Block::INTERNAL_STATE_DATA_BITS . " bits");
 		}

src/block/Dirt.php

@@ -39,13 +39,13 @@ class Dirt extends Opaque{
 		return $this->coarse ? BlockLegacyMetadata::DIRT_FLAG_COARSE : 0;
 	}
 
-	public function getRequiredStateDataBits() : int{ return 1; }
+	public function getRequiredTypeDataBits() : int{ return 1; }
 
-	protected function decodeState(BlockDataReader $r) : void{
+	protected function decodeType(BlockDataReader $r) : void{
 		$this->coarse = $r->readBool();
 	}
 
-	protected function encodeState(BlockDataWriter $w) : void{
+	protected function encodeType(BlockDataWriter $w) : void{
 		$w->writeBool($this->coarse);
 	}
 

src/block/FloorBanner.php

@@ -33,22 +33,7 @@ use pocketmine\player\Player;
 use pocketmine\world\BlockTransaction;
 
 final class FloorBanner extends BaseBanner{
-	use SignLikeRotationTrait {
-		decodeState as decodeRotation;
-		encodeState as encodeRotation;
-	}
-
-	public function getRequiredStateDataBits() : int{ return 4; }
-
-	protected function decodeState(BlockDataReader $r) : void{
-		parent::decodeState($r);
-		$this->decodeRotation($r);
-	}
-
-	protected function encodeState(BlockDataWriter $w) : void{
-		parent::encodeState($w);
-		$this->encodeRotation($w);
-	}
+	use SignLikeRotationTrait;
 
 	protected function getSupportingFace() : int{
 		return Facing::DOWN;

src/block/FloorCoralFan.php

@@ -50,12 +50,10 @@ final class FloorCoralFan extends BaseCoral{
 	public function getRequiredStateDataBits() : int{ return parent::getRequiredStateDataBits() + 1; }
 
 	protected function decodeState(BlockDataReader $r) : void{
-		parent::decodeState($r);
 		$this->axis = $r->readHorizontalAxis();
 	}
 
 	protected function encodeState(BlockDataWriter $w) : void{
-		parent::encodeState($w);
 		$w->writeHorizontalAxis($this->axis);
 	}
 

src/block/Skull.php

@@ -25,7 +25,9 @@ namespace pocketmine\block;
 
 use pocketmine\block\tile\Skull as TileSkull;
 use pocketmine\block\utils\BlockDataReader;
+use pocketmine\block\utils\BlockDataReaderHelper;
 use pocketmine\block\utils\BlockDataWriter;
+use pocketmine\block\utils\BlockDataWriterHelper;
 use pocketmine\block\utils\InvalidBlockStateException;
 use pocketmine\block\utils\SkullType;
 use pocketmine\item\Item;
@@ -51,6 +53,16 @@ class Skull extends Flowable{
 		parent::__construct($idInfo, $name, $breakInfo);
 	}
 
+	public function getRequiredTypeDataBits() : int{ return 3; }
+
+	protected function decodeType(BlockDataReader $r) : void{
+		$this->skullType = BlockDataReaderHelper::readSkullType($r);
+	}
+
+	protected function encodeType(BlockDataWriter $w) : void{
+		BlockDataWriterHelper::writeSkullType($w, $this->skullType);
+	}
+
 	public function getRequiredStateDataBits() : int{ return 3; }
 
 	protected function decodeState(BlockDataReader $r) : void{

src/block/Sponge.php

@@ -33,13 +33,13 @@ class Sponge extends Opaque{
 		return $this->wet ? BlockLegacyMetadata::SPONGE_FLAG_WET : 0;
 	}
 
-	public function getRequiredStateDataBits() : int{ return 1; }
+	public function getRequiredTypeDataBits() : int{ return 1; }
 
-	protected function decodeState(BlockDataReader $r) : void{
+	protected function decodeType(BlockDataReader $r) : void{
 		$this->wet = $r->readBool();
 	}
 
-	protected function encodeState(BlockDataWriter $w) : void{
+	protected function encodeType(BlockDataWriter $w) : void{
 		$w->writeBool($this->wet);
 	}
 

src/block/TNT.php

@@ -49,16 +49,24 @@ class TNT extends Opaque{
 		return $this->worksUnderwater ? BlockLegacyMetadata::TNT_FLAG_UNDERWATER : 0;
 	}
 
-	public function getRequiredStateDataBits() : int{ return 2; }
+	public function getRequiredTypeDataBits() : int{ return 1; }
+
+	protected function decodeType(BlockDataReader $r) : void{
+		$this->worksUnderwater = $r->readBool();
+	}
+
+	protected function encodeType(BlockDataWriter $w) : void{
+		$w->writeBool($this->worksUnderwater);
+	}
+
+	public function getRequiredStateDataBits() : int{ return 1; }
 
 	protected function decodeState(BlockDataReader $r) : void{
 		$this->unstable = $r->readBool();
-		$this->worksUnderwater = $r->readBool();
 	}
 
 	protected function encodeState(BlockDataWriter $w) : void{
 		$w->writeBool($this->unstable);
-		$w->writeBool($this->worksUnderwater);
 	}
 
 	public function isUnstable() : bool{ return $this->unstable; }

src/block/VanillaBlocks.php

@@ -626,7 +626,7 @@ final class VanillaBlocks{
 		self::register("barrel", $factory->get(Ids::BARREL, 0));
 		self::register("barrier", $factory->get(Ids::BARRIER, 0));
 		self::register("beacon", $factory->get(Ids::BEACON, 0));
-		self::register("bed", $factory->get(Ids::BED, 0));
+		self::register("bed", $factory->get(Ids::BED, 13));
 		self::register("bedrock", $factory->get(Ids::BEDROCK, 0));
 		self::register("beetroots", $factory->get(Ids::BEETROOTS, 0));
 		self::register("bell", $factory->get(Ids::BELL, 1));
@@ -943,7 +943,7 @@ final class VanillaBlocks{
 		self::register("material_reducer", $factory->get(Ids::MATERIAL_REDUCER, 0));
 		self::register("melon", $factory->get(Ids::MELON, 0));
 		self::register("melon_stem", $factory->get(Ids::MELON_STEM, 0));
-		self::register("mob_head", $factory->get(Ids::MOB_HEAD, 2));
+		self::register("mob_head", $factory->get(Ids::MOB_HEAD, 19));
 		self::register("monster_spawner", $factory->get(Ids::MONSTER_SPAWNER, 0));
 		self::register("mossy_cobblestone", $factory->get(Ids::MOSSY_COBBLESTONE, 0));
 		self::register("mossy_cobblestone_slab", $factory->get(Ids::MOSSY_COBBLESTONE_SLAB, 0));

src/block/WallBanner.php

@@ -34,22 +34,7 @@ use pocketmine\player\Player;
 use pocketmine\world\BlockTransaction;
 
 final class WallBanner extends BaseBanner{
-	use HorizontalFacingTrait {
-		decodeState as decodeFacing;
-		encodeState as encodeFacing;
-	}
-
-	public function getRequiredStateDataBits() : int{ return 2; }
-
-	protected function decodeState(BlockDataReader $r) : void{
-		parent::decodeState($r);
-		$this->decodeFacing($r);
-	}
-
-	protected function encodeState(BlockDataWriter $w) : void{
-		parent::encodeState($w);
-		$this->encodeFacing($w);
-	}
+	use HorizontalFacingTrait;
 
 	protected function getSupportingFace() : int{
 		return Facing::opposite($this->facing);

src/block/WallCoralFan.php

@@ -49,12 +49,10 @@ final class WallCoralFan extends BaseCoral{
 	public function getRequiredStateDataBits() : int{ return parent::getRequiredStateDataBits() + 2; }
 
 	protected function decodeState(BlockDataReader $r) : void{
-		parent::decodeState($r);
 		$this->facing = $r->readHorizontalFacing();
 	}
 
 	protected function encodeState(BlockDataWriter $w) : void{
-		parent::encodeState($w);
 		$w->writeHorizontalFacing($this->facing);
 	}
 

src/block/utils/ColoredTrait.php

@@ -37,15 +37,15 @@ trait ColoredTrait{
 		return DyeColorIdMap::getInstance()->toId($this->color);
 	}
 
-	public function getRequiredStateDataBits() : int{ return 4; }
+	public function getRequiredTypeDataBits() : int{ return 4; }
 
-	/** @see Block::decodeState() */
-	protected function decodeState(BlockDataReader $r) : void{
+	/** @see Block::decodeType() */
+	protected function decodeType(BlockDataReader $r) : void{
 		$this->color = BlockDataReaderHelper::readDyeColor($r);
 	}
 
-	/** @see Block::encodeState() */
-	protected function encodeState(BlockDataWriter $w) : void{
+	/** @see Block::encodeType() */
+	protected function encodeType(BlockDataWriter $w) : void{
 		BlockDataWriterHelper::writeDyeColor($w, $this->color);
 	}
 

src/block/utils/CoralTypeTrait.php

@@ -23,18 +23,22 @@ declare(strict_types=1);
 
 namespace pocketmine\block\utils;
 
+use pocketmine\block\Block;
+
 trait CoralTypeTrait{
 	protected CoralType $coralType;
 	protected bool $dead = false;
 
-	public function getRequiredStateDataBits() : int{ return 4; }
+	public function getRequiredTypeDataBits() : int{ return 4; }
 
-	protected function decodeState(BlockDataReader $r) : void{
+	/** @see Block::decodeType() */
+	protected function decodeType(BlockDataReader $r) : void{
 		$this->coralType = BlockDataReaderHelper::readCoralType($r);
 		$this->dead = $r->readBool();
 	}
 
-	protected function encodeState(BlockDataWriter $w) : void{
+	/** @see Block::encodeType() */
+	protected function encodeType(BlockDataWriter $w) : void{
 		BlockDataWriterHelper::writeCoralType($w, $this->coralType);
 		$w->writeBool($this->dead);
 	}

tests/phpunit/data/bedrock/block/convert/BlockSerializerDeserializerTest.php

@@ -24,9 +24,14 @@ declare(strict_types=1);
 namespace pocketmine\data\bedrock\block\convert;
 
 use PHPUnit\Framework\TestCase;
+use pocketmine\block\Bed;
 use pocketmine\block\BlockFactory;
+use pocketmine\block\BlockTypeIds;
+use pocketmine\block\Skull;
+use pocketmine\block\utils\DyeColor;
 use pocketmine\data\bedrock\block\BlockStateDeserializeException;
 use pocketmine\data\bedrock\block\BlockStateSerializeException;
+use pocketmine\block\BaseBanner;
 use function print_r;
 
 final class BlockSerializerDeserializerTest extends TestCase{
@@ -51,6 +56,19 @@ final class BlockSerializerDeserializerTest extends TestCase{
 				self::fail($e->getMessage());
 			}
 
+			//The following are workarounds for differences in blockstate representation in Bedrock vs PM
+			//In these cases, some properties are not stored in the blockstate (but rather in the block entity NBT), but
+			//they do form part of the internal blockstate hash in PM. This leads to inconsistencies when serializing
+			//and deserializing blockstates.
+			if(
+				($block instanceof BaseBanner && $newBlock instanceof BaseBanner) ||
+				($block instanceof Bed && $newBlock instanceof Bed)
+			){
+				$newBlock->setColor($block->getColor());
+			}elseif($block instanceof Skull && $newBlock instanceof Skull){
+				$newBlock->setSkullType($block->getSkullType());
+			}
+
 			self::assertSame($block->getStateId(), $newBlock->getStateId(), "Mismatch of blockstate for " . $block->getName() . ", " . print_r($block, true) . " vs " . print_r($newBlock, true));
 		}
 	}