ItemTranslator now operates directly using Item, rather than using item ID/meta + ItemFactory

in almost all cases where ItemTranslator is used, an Item already exists, so it doesn't make any sense to make ItemTranslator go and create another item instance just to pass to ‰ItemSerializer.

src/entity/animation/ConsumingItemAnimation.php

@@ -37,7 +37,7 @@ final class ConsumingItemAnimation implements Animation{
 	){}
 
 	public function encode() : array{
-		[$netId, $netData] = ItemTranslator::getInstance()->toNetworkId($this->item->getId(), $this->item->getMeta());
+		[$netId, $netData] = ItemTranslator::getInstance()->toNetworkId($this->item);
 		return [
 			//TODO: need to check the data values
 			ActorEventPacket::create($this->human->getId(), ActorEvent::EATING_ITEM, ($netId << 16) | $netData)

src/network/mcpe/cache/CraftingDataCache.php

@@ -27,6 +27,7 @@ use pocketmine\crafting\CraftingManager;
 use pocketmine\crafting\FurnaceType;
 use pocketmine\crafting\ShapelessRecipeType;
 use pocketmine\item\Item;
+use pocketmine\item\ItemFactory;
 use pocketmine\network\mcpe\convert\ItemTranslator;
 use pocketmine\network\mcpe\convert\TypeConverter;
 use pocketmine\network\mcpe\protocol\CraftingDataPacket;
@@ -135,7 +136,7 @@ final class CraftingDataCache{
 				default => throw new AssumptionFailedError("Unreachable"),
 			};
 			foreach($manager->getFurnaceRecipeManager($furnaceType)->getAll() as $recipe){
-				$input = $converter->coreItemStackToNet($recipe->getInput());
+				$input = $converter->coreItemStackToRecipeIngredient($recipe->getInput());
 				$recipesWithTypeIds[] = new ProtocolFurnaceRecipe(
 					CraftingDataPacket::ENTRY_FURNACE_DATA,
 					$input->getId(),
@@ -167,9 +168,9 @@ final class CraftingDataCache{
 		$itemTranslator = ItemTranslator::getInstance();
 		foreach($manager->getPotionContainerChangeRecipes() as $recipes){
 			foreach($recipes as $recipe){
-				$input = $itemTranslator->toNetworkId($recipe->getInputItemId(), 0);
+				$input = $itemTranslator->toNetworkId(ItemFactory::getInstance()->get($recipe->getInputItemId(), 0));
-				$ingredient = $itemTranslator->toNetworkId($recipe->getIngredient()->getId(), 0);
+				$ingredient = $itemTranslator->toNetworkId($recipe->getIngredient());
-				$output = $itemTranslator->toNetworkId($recipe->getOutputItemId(), 0);
+				$output = $itemTranslator->toNetworkId(ItemFactory::getInstance()->get($recipe->getOutputItemId(), 0));
 				$potionContainerChangeRecipes[] = new ProtocolPotionContainerChangeRecipe(
 					$input[0],
 					$ingredient[0],

src/network/mcpe/convert/ItemTranslator.php

@@ -27,7 +27,7 @@ use pocketmine\data\bedrock\item\ItemDeserializer;
 use pocketmine\data\bedrock\item\ItemSerializer;
 use pocketmine\data\bedrock\item\ItemTypeSerializeException;
 use pocketmine\data\bedrock\item\SavedItemData;
-use pocketmine\item\ItemFactory;
+use pocketmine\item\Item;
 use pocketmine\network\mcpe\protocol\serializer\ItemTypeDictionary;
 use pocketmine\utils\AssumptionFailedError;
 use pocketmine\utils\SingletonTrait;
@@ -54,16 +54,24 @@ final class ItemTranslator{
 	 * @return int[]|null
 	 * @phpstan-return array{int, int, int}|null
 	 */
-	public function toNetworkIdQuiet(int $internalId, int $internalMeta) : ?array{
+	public function toNetworkIdQuiet(Item $item) : ?array{
-		//TODO: we should probably come up with a cache for this
-
 		try{
-			$itemData = $this->itemSerializer->serialize(ItemFactory::getInstance()->get($internalId, $internalMeta));
+			return $this->toNetworkId($item);
 		}catch(ItemTypeSerializeException){
-			//TODO: this will swallow any serializer error; this is not ideal, but it should be OK since unit tests
-			//should cover this
 			return null;
 		}
+	}
+
+	/**
+	 * @return int[]
+	 * @phpstan-return array{int, int, int}
+	 *
+	 * @throws ItemTypeSerializeException
+	 */
+	public function toNetworkId(Item $item) : array{
+		//TODO: we should probably come up with a cache for this
+
+		$itemData = $this->itemSerializer->serialize($item);
 
 		$numericId = $this->dictionary->fromStringId($itemData->getName());
 		$blockStateData = $itemData->getBlock();
@@ -81,40 +89,15 @@ final class ItemTranslator{
 	}
 
 	/**
-	 * @return int[]
-	 * @phpstan-return array{int, int, int}
-	 */
-	public function toNetworkId(int $internalId, int $internalMeta) : array{
-		return $this->toNetworkIdQuiet($internalId, $internalMeta) ??
-			throw new \InvalidArgumentException("Unmapped ID/metadata combination $internalId:$internalMeta");
-	}
-
-	/**
-	 * @return int[]
-	 * @phpstan-return array{int, int}
 	 * @throws TypeConversionException
 	 */
-	public function fromNetworkId(int $networkId, int $networkMeta, int $networkBlockRuntimeId) : array{
+	public function fromNetworkId(int $networkId, int $networkMeta, int $networkBlockRuntimeId) : Item{
 		$stringId = $this->dictionary->fromIntId($networkId);
 
 		$blockStateData = $networkBlockRuntimeId !== self::NO_BLOCK_RUNTIME_ID ?
 			RuntimeBlockMapping::getInstance()->getBlockStateDictionary()->getDataFromStateId($networkBlockRuntimeId) :
 			null;
 
-		$item = $this->itemDeserializer->deserialize(new SavedItemData($stringId, $networkMeta, $blockStateData));
+		return $this->itemDeserializer->deserialize(new SavedItemData($stringId, $networkMeta, $blockStateData));
-		return [$item->getId(), $item->getMeta()];
-	}
-
-	/**
-	 * @return int[]
-	 * @phpstan-return array{int, int}
-	 * @throws TypeConversionException
-	 */
-	public function fromNetworkIdWithWildcardHandling(int $networkId, int $networkMeta) : array{
-		if($networkMeta !== 0x7fff){
-			return $this->fromNetworkId($networkId, $networkMeta, 0);
-		}
-		[$id, ] = $this->fromNetworkId($networkId, 0, 0);
-		return [$id, -1];
 	}
 }

src/network/mcpe/convert/TypeConverter.php

@@ -27,6 +27,7 @@ use pocketmine\block\inventory\CraftingTableInventory;
 use pocketmine\block\inventory\EnchantInventory;
 use pocketmine\block\inventory\LoomInventory;
 use pocketmine\block\inventory\StonecutterInventory;
+use pocketmine\block\VanillaBlocks;
 use pocketmine\inventory\transaction\action\CreateItemAction;
 use pocketmine\inventory\transaction\action\DestroyItemAction;
 use pocketmine\inventory\transaction\action\DropItemAction;
@@ -60,6 +61,8 @@ class TypeConverter{
 	private const PM_ID_TAG = "___Id___";
 	private const PM_META_TAG = "___Meta___";
 
+	private const RECIPE_INPUT_WILDCARD_META = 0x7fff;
+
 	private int $shieldRuntimeId;
 
 	public function __construct(){
@@ -116,10 +119,10 @@ class TypeConverter{
 			return new RecipeIngredient(0, 0, 0);
 		}
 		if($itemStack->hasAnyDamageValue()){
-			[$id, ] = ItemTranslator::getInstance()->toNetworkId($itemStack->getId(), 0);
+			[$id, ] = ItemTranslator::getInstance()->toNetworkId(ItemFactory::getInstance()->get($itemStack->getId()));
-			$meta = 0x7fff;
+			$meta = self::RECIPE_INPUT_WILDCARD_META;
 		}else{
-			[$id, $meta] = ItemTranslator::getInstance()->toNetworkId($itemStack->getId(), $itemStack->getMeta());
+			[$id, $meta] = ItemTranslator::getInstance()->toNetworkId($itemStack);
 		}
 		return new RecipeIngredient($id, $meta, $itemStack->getCount());
 	}
@@ -128,8 +131,17 @@ class TypeConverter{
 		if($ingredient->getId() === 0){
 			return VanillaItems::AIR();
 		}
-		[$id, $meta] = ItemTranslator::getInstance()->fromNetworkIdWithWildcardHandling($ingredient->getId(), $ingredient->getMeta());
+
-		return ItemFactory::getInstance()->get($id, $meta, $ingredient->getCount());
+		//TODO: this won't be handled properly for blockitems because a block runtimeID is expected rather than a meta value
+
+		if($ingredient->getMeta() === self::RECIPE_INPUT_WILDCARD_META){
+			$idItem = ItemTranslator::getInstance()->fromNetworkId($ingredient->getId(), 0, 0);
+			$result = ItemFactory::getInstance()->get($idItem->getId(), -1);
+		}else{
+			$result = ItemTranslator::getInstance()->fromNetworkId($ingredient->getId(), $ingredient->getMeta(), 0);
+		}
+		$result->setCount($ingredient->getCount());
+		return $result;
 	}
 
 	public function coreItemStackToNet(Item $itemStack) : ItemStack{
@@ -141,11 +153,11 @@ class TypeConverter{
 			$nbt = clone $itemStack->getNamedTag();
 		}
 
-		$idMeta = ItemTranslator::getInstance()->toNetworkIdQuiet($itemStack->getId(), $itemStack->getMeta());
+		$idMeta = ItemTranslator::getInstance()->toNetworkIdQuiet($itemStack);
 		if($idMeta === null){
 			//Display unmapped items as INFO_UPDATE, but stick something in their NBT to make sure they don't stack with
 			//other unmapped items.
-			[$id, $meta, $blockRuntimeId] = ItemTranslator::getInstance()->toNetworkId(ItemIds::INFO_UPDATE, 0);
+			[$id, $meta, $blockRuntimeId] = ItemTranslator::getInstance()->toNetworkId(VanillaBlocks::INFO_UPDATE()->asItem());
 			if($nbt === null){
 				$nbt = new CompoundTag();
 			}
@@ -188,12 +200,13 @@ class TypeConverter{
 		}
 		$compound = $itemStack->getNbt();
 
-		[$id, $meta] = ItemTranslator::getInstance()->fromNetworkId($itemStack->getId(), $itemStack->getMeta(), $itemStack->getBlockRuntimeId());
+		$itemResult = ItemTranslator::getInstance()->fromNetworkId($itemStack->getId(), $itemStack->getMeta(), $itemStack->getBlockRuntimeId());
 
 		if($compound !== null){
 			$compound = clone $compound;
 
-			if($id === ItemIds::INFO_UPDATE && $meta === 0){
+			$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);
@@ -214,23 +227,26 @@ class TypeConverter{
 			if($compound->count() === 0){
 				$compound = null;
 			}
-		}
+			if($meta !== null){
 				if($meta < 0 || $meta >= 0x7fff){ //this meta value may have been restored from the NBT
 					throw new TypeConversionException("Item meta must be in range 0 ... " . 0x7fff . " (received $meta)");
 				}
+				$itemResult = ItemFactory::getInstance()->get($id ?? $itemResult->getId(), $meta);
+			}
+		}
 
+		$itemResult->setCount($itemStack->getCount());
+		if($compound !== null){
 			try{
-			return ItemFactory::getInstance()->get(
+				$itemResult->setNamedTag($compound);
-				$id,
-				$meta,
-				$itemStack->getCount(),
-				$compound
-			);
 			}catch(NbtException $e){
 				throw TypeConversionException::wrap($e, "Bad itemstack NBT data");
 			}
 		}
 
+		return $itemResult;
+	}
+
 	/**
 	 * @throws TypeConversionException
 	 */