Break Block's dependence on ItemFactory, and item legacy IDs

let's GOOOOOOOOOOOOOOOOOOOOOOOOOOOOO

src/block/BaseSign.php

@@ -41,9 +41,16 @@ abstract class BaseSign extends Transparent{
 	protected SignText $text;
 	protected ?int $editorEntityRuntimeId = null;
 
-	public function __construct(BlockIdentifier $idInfo, string $name, BlockBreakInfo $breakInfo){
+	/** @var \Closure() : Item */
+	private \Closure $asItemCallback;
+
+	/**
+	 * @param \Closure() : Item $asItemCallback
+	 */
+	public function __construct(BlockIdentifier $idInfo, string $name, BlockBreakInfo $breakInfo, \Closure $asItemCallback){
 		parent::__construct($idInfo, $name, $breakInfo);
 		$this->text = new SignText();
+		$this->asItemCallback = $asItemCallback;
 	}
 
 	public function readStateFromWorld() : void{
@@ -139,4 +146,8 @@ abstract class BaseSign extends Transparent{
 
 		return false;
 	}
+
+	public function asItem() : Item{
+		return ($this->asItemCallback)();
+	}
 }

src/block/Block.php

@@ -34,7 +34,7 @@ use pocketmine\data\runtime\block\BlockDataWriter;
 use pocketmine\entity\Entity;
 use pocketmine\item\enchantment\VanillaEnchantments;
 use pocketmine\item\Item;
-use pocketmine\item\ItemFactory;
+use pocketmine\item\ItemBlock;
 use pocketmine\math\Axis;
 use pocketmine\math\AxisAlignedBB;
 use pocketmine\math\RayTraceResult;
@@ -90,10 +90,7 @@ class Block{
 	}
 
 	public function asItem() : Item{
-		return ItemFactory::getInstance()->get(
+		return new ItemBlock($this);
-			$this->getLegacyItemId(),
-			$this->getLegacyItemMeta()
-		);
 	}
 
 	public function getLegacyItemId() : int{
@@ -112,10 +109,9 @@ class Block{
 
 	public function getRequiredStateDataBits() : int{ return 0; }
 
-	final public function decodeStateData(int $data) : void{
+	final public function decodeTypeData(int $data) : void{
 		$typeBits = $this->getRequiredTypeDataBits();
-		$stateBits = $this->getRequiredStateDataBits();
+		$givenBits = $typeBits;
-		$givenBits = $typeBits + $stateBits;
 		$reader = new BlockDataReader($givenBits, $data);
 
 		$this->decodeType($reader);
@@ -123,6 +119,14 @@ class Block{
 		if($typeBits !== $readBits){
 			throw new \LogicException("Exactly $typeBits bits of type data were provided, but $readBits were read");
 		}
+	}
+
+	final public function decodeStateData(int $data) : void{
+		$typeBits = $this->getRequiredTypeDataBits();
+		$stateBits = $this->getRequiredStateDataBits();
+		$givenBits = $typeBits + $stateBits;
+		$reader = new BlockDataReader($givenBits, $data);
+		$this->decodeTypeData($reader->readInt($typeBits));
 
 		$this->decodeState($reader);
 		$readBits = $reader->getOffset() - $typeBits;
@@ -139,6 +143,20 @@ class Block{
 		//NOOP
 	}
 
+	final public function computeTypeData() : int{
+		$typeBits = $this->getRequiredTypeDataBits();
+		$requiredBits = $typeBits;
+		$writer = new BlockDataWriter($requiredBits);
+
+		$this->encodeType($writer);
+		$writtenBits = $writer->getOffset();
+		if($typeBits !== $writtenBits){
+			throw new \LogicException("Exactly $typeBits bits of type data were expected, but $writtenBits were written");
+		}
+
+		return $writer->getValue();
+	}
+
 	/**
 	 * @internal
 	 */
@@ -147,12 +165,7 @@ class Block{
 		$stateBits = $this->getRequiredStateDataBits();
 		$requiredBits = $typeBits + $stateBits;
 		$writer = new BlockDataWriter($requiredBits);
-
+		$writer->writeInt($typeBits, $this->computeTypeData());
-		$this->encodeType($writer);
-		$writtenBits = $writer->getOffset();
-		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;

src/block/BlockFactory.php

@@ -480,8 +480,9 @@ class BlockFactory{
 			$this->register(new WoodenPressurePlate(BlockLegacyIdHelper::getWoodenPressurePlateIdentifier($treeType), $name . " Pressure Plate", $woodenPressurePlateBreakInfo));
 			$this->register(new WoodenTrapdoor(BlockLegacyIdHelper::getWoodenTrapdoorIdentifier($treeType), $name . " Trapdoor", $woodenDoorBreakInfo));
 
-			$this->register(new FloorSign(BlockLegacyIdHelper::getWoodenFloorSignIdentifier($treeType), $name . " Sign", $signBreakInfo));
+			[$floorSignId, $wallSignId, $signAsItem] = BlockLegacyIdHelper::getWoodenSignInfo($treeType);
-			$this->register(new WallSign(BlockLegacyIdHelper::getWoodenWallSignIdentifier($treeType), $name . " Wall Sign", $signBreakInfo));
+			$this->register(new FloorSign($floorSignId, $name . " Sign", $signBreakInfo, $signAsItem));
+			$this->register(new WallSign($wallSignId, $name . " Wall Sign", $signBreakInfo, $signAsItem));
 		}
 
 		$sandstoneBreakInfo = new BreakInfo(0.8, ToolType::PICKAXE, ToolTier::WOOD()->getHarvestLevel());
@@ -923,6 +924,18 @@ class BlockFactory{
 		return $block;
 	}
 
+	/**
+	 * @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];
+		}
+
+		return null;
+	}
+
 	public function fromFullBlock(int $fullState) : Block{
 		return $this->get($fullState >> Block::INTERNAL_STATE_DATA_BITS, $fullState & Block::INTERNAL_STATE_DATA_MASK);
 	}

src/block/BlockLegacyIdHelper.php

@@ -30,6 +30,7 @@ use pocketmine\block\utils\DyeColor;
 use pocketmine\block\utils\TreeType;
 use pocketmine\data\bedrock\block\BlockLegacyMetadata;
 use pocketmine\item\ItemIds;
+use pocketmine\item\VanillaItems;
 use pocketmine\utils\AssumptionFailedError;
 
 final class BlockLegacyIdHelper{
@@ -130,38 +131,48 @@ final class BlockLegacyIdHelper{
 		}, ItemIds::SAPLING, $treeType->getMagicNumber());
 	}
 
-	public static function getWoodenFloorSignIdentifier(TreeType $treeType) : BID{
+	/**
+	 * @return BID[]|\Closure[]
+	 * @phpstan-return array{BID, BID, \Closure() : \pocketmine\item\Item}
+	 */
+	public static function getWoodenSignInfo(TreeType $treeType) : array{
 		switch($treeType->id()){
 			case TreeType::OAK()->id():
-				return new BID(Ids::OAK_SIGN, ItemIds::SIGN, 0, TileSign::class);
+				return [
+					new BID(Ids::OAK_SIGN, ItemIds::SIGN, 0, TileSign::class),
+					new BID(Ids::OAK_WALL_SIGN, ItemIds::SIGN, 0, TileSign::class),
+					fn() => VanillaItems::OAK_SIGN()
+				];
 			case TreeType::SPRUCE()->id():
-				return new BID(Ids::SPRUCE_SIGN, ItemIds::SPRUCE_SIGN, 0, TileSign::class);
+				return [
+					new BID(Ids::SPRUCE_SIGN, ItemIds::SPRUCE_SIGN, 0, TileSign::class),
+					new BID(Ids::SPRUCE_WALL_SIGN, ItemIds::SPRUCE_SIGN, 0, TileSign::class),
+					fn() => VanillaItems::SPRUCE_SIGN()
+				];
 			case TreeType::BIRCH()->id():
-				return new BID(Ids::BIRCH_SIGN, ItemIds::BIRCH_SIGN, 0, TileSign::class);
+				return [
+					new BID(Ids::BIRCH_SIGN, ItemIds::BIRCH_SIGN, 0, TileSign::class),
+					new BID(Ids::BIRCH_WALL_SIGN, ItemIds::BIRCH_SIGN, 0, TileSign::class),
+					fn() => VanillaItems::BIRCH_SIGN()
+				];
 			case TreeType::JUNGLE()->id():
-				return new BID(Ids::JUNGLE_SIGN, ItemIds::JUNGLE_SIGN, 0, TileSign::class);
+				return [
+					new BID(Ids::JUNGLE_SIGN, ItemIds::JUNGLE_SIGN, 0, TileSign::class),
+					new BID(Ids::JUNGLE_WALL_SIGN, ItemIds::JUNGLE_SIGN, 0, TileSign::class),
+					fn() => VanillaItems::JUNGLE_SIGN()
+				];
 			case TreeType::ACACIA()->id():
-				return new BID(Ids::ACACIA_SIGN, ItemIds::ACACIA_SIGN, 0, TileSign::class);
+				return [
+					new BID(Ids::ACACIA_SIGN, ItemIds::ACACIA_SIGN, 0, TileSign::class),
+					new BID(Ids::ACACIA_WALL_SIGN, ItemIds::ACACIA_SIGN, 0, TileSign::class),
+					fn() => VanillaItems::ACACIA_SIGN()
+				];
 			case TreeType::DARK_OAK()->id():
-				return new BID(Ids::DARK_OAK_SIGN, ItemIds::DARKOAK_SIGN, 0, TileSign::class);
+				return [
-		}
+					new BID(Ids::DARK_OAK_SIGN, ItemIds::DARKOAK_SIGN, 0, TileSign::class),
-		throw new AssumptionFailedError("Switch should cover all wood types");
+					new BID(Ids::DARK_OAK_WALL_SIGN, ItemIds::DARKOAK_SIGN, 0, TileSign::class),
-	}
+					fn() => VanillaItems::DARK_OAK_SIGN()
-
+				];
-	public static function getWoodenWallSignIdentifier(TreeType $treeType) : BID{
-		switch($treeType->id()){
-			case TreeType::OAK()->id():
-				return new BID(Ids::OAK_WALL_SIGN, ItemIds::SIGN, 0, TileSign::class);
-			case TreeType::SPRUCE()->id():
-				return new BID(Ids::SPRUCE_WALL_SIGN, ItemIds::SPRUCE_SIGN, 0, TileSign::class);
-			case TreeType::BIRCH()->id():
-				return new BID(Ids::BIRCH_WALL_SIGN, ItemIds::BIRCH_SIGN, 0, TileSign::class);
-			case TreeType::JUNGLE()->id():
-				return new BID(Ids::JUNGLE_WALL_SIGN, ItemIds::JUNGLE_SIGN, 0, TileSign::class);
-			case TreeType::ACACIA()->id():
-				return new BID(Ids::ACACIA_WALL_SIGN, ItemIds::ACACIA_SIGN, 0, TileSign::class);
-			case TreeType::DARK_OAK()->id():
-				return new BID(Ids::DARK_OAK_WALL_SIGN, ItemIds::DARKOAK_SIGN, 0, TileSign::class);
 		}
 		throw new AssumptionFailedError("Switch should cover all wood types");
 	}

src/block/Tripwire.php

@@ -25,6 +25,8 @@ namespace pocketmine\block;
 
 use pocketmine\data\runtime\block\BlockDataReader;
 use pocketmine\data\runtime\block\BlockDataWriter;
+use pocketmine\item\Item;
+use pocketmine\item\VanillaItems;
 
 class Tripwire extends Flowable{
 	protected bool $triggered = false;
@@ -79,4 +81,8 @@ class Tripwire extends Flowable{
 		$this->disarmed = $disarmed;
 		return $this;
 	}
+
+	public function asItem() : Item{
+		return VanillaItems::STRING();
+	}
 }

src/item/Item.php

@@ -399,7 +399,7 @@ class Item implements \JsonSerializable{
 	}
 
 	public function isNull() : bool{
-		return $this->count <= 0 || $this->getId() === ItemIds::AIR;
+		return $this->count <= 0;
 	}
 
 	/**

src/item/ItemBlock.php

@@ -25,6 +25,7 @@ namespace pocketmine\item;
 
 use pocketmine\block\Block;
 use pocketmine\block\BlockFactory;
+use pocketmine\block\VanillaBlocks;
 
 /**
  * Class used for Items that directly represent blocks, such as stone, dirt, wood etc.
@@ -33,15 +34,23 @@ use pocketmine\block\BlockFactory;
  * just place wheat crops when used on the ground).
  */
 final class ItemBlock extends Item{
-	private int $blockFullId;
+	private int $blockTypeId;
+	private int $blockTypeData;
 
 	public function __construct(Block $block){
 		parent::__construct(ItemIdentifier::fromBlock($block), $block->getName());
-		$this->blockFullId = $block->getStateId();
+		$this->blockTypeId = $block->getTypeId();
+		$this->blockTypeData = $block->computeTypeData();
 	}
 
 	public function getBlock(?int $clickedFace = null) : Block{
-		return BlockFactory::getInstance()->fromFullBlock($this->blockFullId);
+		//TODO: HACKY MESS, CLEAN IT UP
+		$blockType = BlockFactory::getInstance()->fromTypeId($this->blockTypeId);
+		if($blockType === null){
+			return VanillaBlocks::AIR();
+		}
+		$blockType->decodeTypeData($this->blockTypeData);
+		return $blockType;
 	}
 
 	public function getFuelTime() : int{

src/item/ItemIdentifier.php

@@ -47,7 +47,10 @@ class ItemIdentifier{
 	public static function fromBlock(Block $block) : self{
 		//negative item type IDs are treated as block IDs
 		//TODO: maybe an ItemBlockIdentifier is in order?
-		return new self(-$block->getTypeId(), $block->getLegacyItemId(), $block->getLegacyItemMeta());
+		//TODO: this isn't vanilla-compliant, but it'll do for now - we only use the "legacy" item ID/meta for full type
+		//indexing right now, because item type IDs aren't granular enough
+		//this should be removed once that's addressed
+		return new self(-$block->getTypeId(), -$block->getTypeId(), $block->computeTypeData());
 	}
 
 	public function getTypeId() : int{ return $this->typeId; }

src/network/mcpe/convert/TypeConverter.php

@@ -33,19 +33,15 @@ use pocketmine\crafting\ExactRecipeIngredient;
 use pocketmine\crafting\MetaWildcardRecipeIngredient;
 use pocketmine\crafting\RecipeIngredient;
 use pocketmine\data\bedrock\item\BlockItemIdMap;
-use pocketmine\data\SavedDataLoadingException;
 use pocketmine\inventory\transaction\action\CreateItemAction;
 use pocketmine\inventory\transaction\action\DestroyItemAction;
 use pocketmine\inventory\transaction\action\DropItemAction;
 use pocketmine\inventory\transaction\action\InventoryAction;
 use pocketmine\inventory\transaction\action\SlotChangeAction;
 use pocketmine\item\Item;
-use pocketmine\item\ItemFactory;
-use pocketmine\item\ItemIds;
 use pocketmine\item\VanillaItems;
 use pocketmine\nbt\NbtException;
 use pocketmine\nbt\tag\CompoundTag;
-use pocketmine\nbt\tag\IntTag;
 use pocketmine\network\mcpe\InventoryManager;
 use pocketmine\network\mcpe\protocol\types\GameMode as ProtocolGameMode;
 use pocketmine\network\mcpe\protocol\types\inventory\ContainerIds;
@@ -211,28 +207,6 @@ class TypeConverter{
 
 		if($compound !== null){
 			$compound = clone $compound;
-
-			$id = $meta = null;
-			if($itemResult->getId() === ItemIds::INFO_UPDATE && $itemResult->getMeta() === 0){
-				if(($idTag = $compound->getTag(self::PM_ID_TAG)) instanceof IntTag){
-					$id = $idTag->getValue();
-					$compound->removeTag(self::PM_ID_TAG);
-				}
-				if(($metaTag = $compound->getTag(self::PM_META_TAG)) instanceof IntTag){
-					$meta = $metaTag->getValue();
-					$compound->removeTag(self::PM_META_TAG);
-				}
-			}
-			if($compound->count() === 0){
-				$compound = null;
-			}
-			if($meta !== null){
-				try{
-					$itemResult = ItemFactory::getInstance()->get($id ?? $itemResult->getId(), $meta);
-				}catch(SavedDataLoadingException $e){
-					throw new TypeConversionException("Failed loading network item: " . $e->getMessage(), 0, $e);
-				}
-			}
 		}
 
 		$itemResult->setCount($itemStack->getCount());