ItemBlock: reference blocks directly (take 2)

This was first attempted in f64dc01bd1c14ff3f79bd6c18d0c337dbc0e87e0, but reverted, since I hadn't considered how to handle stripping state data from blocks.

This removes the abusable API RuntimeBlockStateRegistry::fromTypeId() and related methods. These were only used to allow ItemBlocks to magically start referencing other blocks if the blocks were overridden by a plugin, but this was never a well-supported use-case anyway.

Instead of relying on RuntimeBlockStateRegistry, we remember the state that the block had during its constructor, and use that to normalize the non-item properties for asItem().

closes #5609

src/block/Anvil.php

@@ -51,7 +51,7 @@ class Anvil extends Transparent implements Fallable{
 
 	private int $damage = self::UNDAMAGED;
 
-	protected function describeType(RuntimeDataDescriber $w) : void{
+	public function describeType(RuntimeDataDescriber $w) : void{
 		$w->boundedInt(2, self::UNDAMAGED, self::VERY_DAMAGED, $this->damage);
 	}
 

src/block/Block.php

@@ -76,6 +76,8 @@ class Block{
 	 */
 	private static array $stateDataBits = [];
 
+	private int $defaultStateData;
+
 	/**
 	 * @param string $name English name of the block type (TODO: implement translations)
 	 */
@@ -84,6 +86,8 @@ class Block{
 		$this->fallbackName = $name;
 		$this->typeInfo = $typeInfo;
 		$this->position = new Position(0, 0, 0, null);
+
+		$this->defaultStateData = $this->computeStateData();
 	}
 
 	public function __clone(){
@@ -174,7 +178,9 @@ class Block{
 	 * Type information such as colour, wood type, etc. is preserved.
 	 */
 	public function asItem() : Item{
-		return new ItemBlock($this);
+		$normalized = clone $this;
+		$normalized->decodeStateData($this->defaultStateData);
+		return new ItemBlock($normalized);
 	}
 
 	final public function getRequiredTypeDataBits() : int{
@@ -211,6 +217,17 @@ class Block{
 		}
 	}
 
+	private function decodeStateData(int $data) : void{
+		$stateBits = $this->getRequiredStateDataBits();
+		$reader = new RuntimeDataReader($stateBits, $data);
+
+		$this->describeState($reader);
+		$readBits = $reader->getOffset();
+		if($stateBits !== $readBits){
+			throw new \LogicException(get_class($this) . ": Exactly $stateBits bits of state data were provided, but $readBits were read");
+		}
+	}
+
 	/**
 	 * @internal
 	 */
@@ -219,12 +236,7 @@ class Block{
 		$stateBits = $this->getRequiredStateDataBits();
 		$reader = new RuntimeDataReader($typeBits + $stateBits, $data);
 		$this->decodeTypeData($reader->readInt($typeBits));
-
-		$this->describeState($reader);
-		$readBits = $reader->getOffset() - $typeBits;
-		if($stateBits !== $readBits){
-			throw new \LogicException(get_class($this) . ": Exactly $stateBits bits of state data were provided, but $readBits were read");
-		}
+		$this->decodeStateData($reader->readInt($stateBits));
 	}
 
 	/**
@@ -243,6 +255,19 @@ class Block{
 		return $writer->getValue();
 	}
 
+	private function computeStateData() : int{
+		$stateBits = $this->getRequiredStateDataBits();
+		$writer = new RuntimeDataWriter($stateBits);
+
+		$this->describeState($writer);
+		$writtenBits = $writer->getOffset();
+		if($stateBits !== $writtenBits){
+			throw new \LogicException(get_class($this) . ": Exactly $stateBits bits of state data were expected, but $writtenBits were written");
+		}
+
+		return $writer->getValue();
+	}
+
 	/**
 	 * @internal
 	 */
@@ -251,12 +276,7 @@ class Block{
 		$stateBits = $this->getRequiredStateDataBits();
 		$writer = new RuntimeDataWriter($typeBits + $stateBits);
 		$writer->writeInt($typeBits, $this->computeTypeData());
-
-		$this->describeState($writer);
-		$writtenBits = $writer->getOffset() - $typeBits;
-		if($stateBits !== $writtenBits){
-			throw new \LogicException(get_class($this) . ": Exactly $stateBits bits of state data were expected, but $writtenBits were written");
-		}
+		$writer->writeInt($stateBits, $this->computeStateData());
 
 		return $writer->getValue();
 	}
@@ -269,7 +289,7 @@ class Block{
 	 * The method implementation must NOT use conditional logic to determine which properties are written. It must
 	 * always write the same properties in the same order, regardless of the current state of the block.
 	 */
-	protected function describeType(RuntimeDataDescriber $w) : void{
+	public function describeType(RuntimeDataDescriber $w) : void{
 		//NOOP
 	}
 

src/block/Dirt.php

@@ -45,7 +45,7 @@ class Dirt extends Opaque{
 		parent::__construct($idInfo, $name, $typeInfo);
 	}
 
-	protected function describeType(RuntimeDataDescriber $w) : void{
+	public function describeType(RuntimeDataDescriber $w) : void{
 		$w->dirtType($this->dirtType);
 	}
 

src/block/Froglight.php

@@ -35,7 +35,7 @@ final class Froglight extends SimplePillar{
 		parent::__construct($idInfo, $name, $typeInfo);
 	}
 
-	protected function describeType(RuntimeDataDescriber $w) : void{
+	public function describeType(RuntimeDataDescriber $w) : void{
 		$w->froglightType($this->froglightType);
 	}
 

src/block/Light.php

@@ -34,7 +34,7 @@ final class Light extends Flowable{
 
 	private int $level = self::MAX_LIGHT_LEVEL;
 
-	protected function describeType(RuntimeDataDescriber $w) : void{
+	public function describeType(RuntimeDataDescriber $w) : void{
 		$w->boundedInt(4, self::MIN_LIGHT_LEVEL, self::MAX_LIGHT_LEVEL, $this->level);
 	}
 

src/block/NetherVines.php

@@ -52,7 +52,7 @@ class NetherVines extends Flowable{
 		parent::__construct($idInfo, $name, $typeInfo);
 	}
 
-	public function describeState(RuntimeDataDescriber $w) : void{
+	protected function describeState(RuntimeDataDescriber $w) : void{
 		$w->boundedInt(5, 0, self::MAX_AGE, $this->age);
 	}
 

src/block/RedMushroomBlock.php

@@ -36,7 +36,7 @@ class RedMushroomBlock extends Opaque{
 		parent::__construct($idInfo, $name, $typeInfo);
 	}
 
-	protected function describeType(RuntimeDataDescriber $w) : void{
+	public function describeType(RuntimeDataDescriber $w) : void{
 		//these blocks always drop as all-cap, but may exist in other forms in the inventory (particularly creative),
 		//so this information needs to be kept in the type info
 		$w->mushroomBlockType($this->mushroomBlockType);

src/block/RuntimeBlockStateRegistry.php

@@ -151,18 +151,6 @@ class RuntimeBlockStateRegistry{
 		}
 	}
 
-	/**
-	 * @internal
-	 * Returns the default state of the block type associated with the given type ID.
-	 */
-	public function fromTypeId(int $typeId) : Block{
-		if(isset($this->typeIndex[$typeId])){
-			return clone $this->typeIndex[$typeId];
-		}
-
-		throw new \InvalidArgumentException("Block ID $typeId is not registered");
-	}
-
 	public function fromStateId(int $stateId) : Block{
 		if($stateId < 0){
 			throw new \InvalidArgumentException("Block state ID cannot be negative");
@@ -178,22 +166,6 @@ class RuntimeBlockStateRegistry{
 		return $block;
 	}
 
-	/**
-	 * Returns whether a specified block state is already registered in the block factory.
-	 */
-	public function isRegistered(int $typeId) : bool{
-		$b = $this->typeIndex[$typeId] ?? null;
-		return $b !== null && !($b instanceof UnknownBlock);
-	}
-
-	/**
-	 * @return Block[]
-	 * @phpstan-return array<int, Block>
-	 */
-	public function getAllKnownTypes() : array{
-		return $this->typeIndex;
-	}
-
 	/**
 	 * @return Block[]
 	 */

src/block/Skull.php

@@ -49,7 +49,7 @@ class Skull extends Flowable{
 		parent::__construct($idInfo, $name, $typeInfo);
 	}
 
-	protected function describeType(RuntimeDataDescriber $w) : void{
+	public function describeType(RuntimeDataDescriber $w) : void{
 		$w->skullType($this->skullType);
 	}
 

src/block/Slab.php

@@ -37,8 +37,8 @@ class Slab extends Transparent{
 	protected SlabType $slabType;
 
 	public function __construct(BlockIdentifier $idInfo, string $name, BlockTypeInfo $typeInfo){
-		parent::__construct($idInfo, $name . " Slab", $typeInfo);
 		$this->slabType = SlabType::BOTTOM();
+		parent::__construct($idInfo, $name . " Slab", $typeInfo);
 	}
 
 	protected function describeState(RuntimeDataDescriber $w) : void{

src/block/Sponge.php

@@ -28,7 +28,7 @@ use pocketmine\data\runtime\RuntimeDataDescriber;
 class Sponge extends Opaque{
 	protected bool $wet = false;
 
-	protected function describeType(RuntimeDataDescriber $w) : void{
+	public function describeType(RuntimeDataDescriber $w) : void{
 		$w->bool($this->wet);
 	}
 

src/block/TNT.php

@@ -45,7 +45,7 @@ class TNT extends Opaque{
 	protected bool $unstable = false; //TODO: Usage unclear, seems to be a weird hack in vanilla
 	protected bool $worksUnderwater = false;
 
-	protected function describeType(RuntimeDataDescriber $w) : void{
+	public function describeType(RuntimeDataDescriber $w) : void{
 		$w->bool($this->worksUnderwater);
 	}
 

src/block/UnknownBlock.php

@@ -38,7 +38,7 @@ class UnknownBlock extends Transparent{
 		$this->stateData = $stateData;
 	}
 
-	protected function describeType(RuntimeDataDescriber $w) : void{
+	public function describeType(RuntimeDataDescriber $w) : void{
 		//use type instead of state, so we don't lose any information like colour
 		//this might be an improperly registered plugin block
 		$w->int(Block::INTERNAL_STATE_DATA_BITS, $this->stateData);

src/block/Wood.php

@@ -38,7 +38,7 @@ class Wood extends Opaque{
 
 	private bool $stripped = false;
 
-	protected function describeType(RuntimeDataDescriber $w) : void{
+	public function describeType(RuntimeDataDescriber $w) : void{
 		$w->bool($this->stripped);
 	}
 

src/block/utils/ColoredTrait.php

@@ -31,7 +31,7 @@ trait ColoredTrait{
 	private $color;
 
 	/** @see Block::describeType() */
-	protected function describeType(RuntimeDataDescriber $w) : void{
+	public function describeType(RuntimeDataDescriber $w) : void{
 		$w->dyeColor($this->color);
 	}
 

src/block/utils/CopperTrait.php

@@ -44,7 +44,7 @@ trait CopperTrait{
 		parent::__construct($identifier, $name, $typeInfo);
 	}
 
-	protected function describeType(RuntimeDataDescriber $w) : void{
+	public function describeType(RuntimeDataDescriber $w) : void{
 		$w->copperOxidation($this->oxidation);
 		$w->bool($this->waxed);
 	}

src/block/utils/CoralTypeTrait.php

@@ -31,7 +31,7 @@ trait CoralTypeTrait{
 	protected bool $dead = false;
 
 	/** @see Block::describeType() */
-	protected function describeType(RuntimeDataDescriber $w) : void{
+	public function describeType(RuntimeDataDescriber $w) : void{
 		$w->coralType($this->coralType);
 		$w->bool($this->dead);
 	}

src/item/ItemBlock.php

@@ -35,47 +35,29 @@ use pocketmine\data\runtime\RuntimeDataDescriber;
  * just place wheat crops when used on the ground).
  */
 final class ItemBlock extends Item{
-	private int $blockTypeId;
-	private int $blockTypeData;
-
-	private int $fuelTime;
-	private bool $fireProof;
-	private int $maxStackSize;
-
-	public function __construct(Block $block){
+	public function __construct(
+		private Block $block
+	){
 		parent::__construct(ItemIdentifier::fromBlock($block), $block->getName());
-		$this->blockTypeId = $block->getTypeId();
-		$this->blockTypeData = $block->computeTypeData();
-
-		$this->fuelTime = $block->getFuelTime();
-		$this->fireProof = $block->isFireProofAsItem();
-		$this->maxStackSize = $block->getMaxStackSize();
 	}
 
 	protected function describeType(RuntimeDataDescriber $w) : void{
-		$w->int(Block::INTERNAL_STATE_DATA_BITS, $this->blockTypeData);
+		$this->block->describeType($w);
 	}
 
 	public function getBlock(?int $clickedFace = null) : Block{
-		//TODO: HACKY MESS, CLEAN IT UP
-		$factory = RuntimeBlockStateRegistry::getInstance();
-		if(!$factory->isRegistered($this->blockTypeId)){
-			return VanillaBlocks::AIR();
-		}
-		$blockType = $factory->fromTypeId($this->blockTypeId);
-		$blockType->decodeTypeData($this->blockTypeData);
-		return $blockType;
+		return clone $this->block;
 	}
 
 	public function getFuelTime() : int{
-		return $this->fuelTime;
+		return $this->block->getFuelTime();
 	}
 
 	public function isFireProof() : bool{
-		return $this->fireProof;
+		return $this->block->isFireProofAsItem();
 	}
 
 	public function getMaxStackSize() : int{
-		return $this->maxStackSize;
+		return $this->block->getMaxStackSize();
 	}
 }