From 13bb1c26fb8bc496d13f092393137c221ff3048f Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 6 Jun 2022 20:51:26 +0100 Subject: [PATCH] ItemTranslator now operates directly using Item, rather than using item ID/meta + ItemFactory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../animation/ConsumingItemAnimation.php | 2 +- src/network/mcpe/cache/CraftingDataCache.php | 9 +-- src/network/mcpe/convert/ItemTranslator.php | 51 ++++++---------- src/network/mcpe/convert/TypeConverter.php | 58 ++++++++++++------- 4 files changed, 60 insertions(+), 60 deletions(-) diff --git a/src/entity/animation/ConsumingItemAnimation.php b/src/entity/animation/ConsumingItemAnimation.php index 83ff1d5e7..60639ea6b 100644 --- a/src/entity/animation/ConsumingItemAnimation.php +++ b/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) diff --git a/src/network/mcpe/cache/CraftingDataCache.php b/src/network/mcpe/cache/CraftingDataCache.php index 1f09525d8..a8a881cff 100644 --- a/src/network/mcpe/cache/CraftingDataCache.php +++ b/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); - $ingredient = $itemTranslator->toNetworkId($recipe->getIngredient()->getId(), 0); - $output = $itemTranslator->toNetworkId($recipe->getOutputItemId(), 0); + $input = $itemTranslator->toNetworkId(ItemFactory::getInstance()->get($recipe->getInputItemId(), 0)); + $ingredient = $itemTranslator->toNetworkId($recipe->getIngredient()); + $output = $itemTranslator->toNetworkId(ItemFactory::getInstance()->get($recipe->getOutputItemId(), 0)); $potionContainerChangeRecipes[] = new ProtocolPotionContainerChangeRecipe( $input[0], $ingredient[0], diff --git a/src/network/mcpe/convert/ItemTranslator.php b/src/network/mcpe/convert/ItemTranslator.php index 6bd890333..09cbac106 100644 --- a/src/network/mcpe/convert/ItemTranslator.php +++ b/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{ - //TODO: we should probably come up with a cache for this - + public function toNetworkIdQuiet(Item $item) : ?array{ 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 [$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]; + return $this->itemDeserializer->deserialize(new SavedItemData($stringId, $networkMeta, $blockStateData)); } } diff --git a/src/network/mcpe/convert/TypeConverter.php b/src/network/mcpe/convert/TypeConverter.php index 8d4fd63bd..6272ff4df 100644 --- a/src/network/mcpe/convert/TypeConverter.php +++ b/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); - $meta = 0x7fff; + [$id, ] = ItemTranslator::getInstance()->toNetworkId(ItemFactory::getInstance()->get($itemStack->getId())); + $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,21 +227,24 @@ class TypeConverter{ if($compound->count() === 0){ $compound = 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)"); + 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); + } } - try{ - return ItemFactory::getInstance()->get( - $id, - $meta, - $itemStack->getCount(), - $compound - ); - }catch(NbtException $e){ - throw TypeConversionException::wrap($e, "Bad itemstack NBT data"); + $itemResult->setCount($itemStack->getCount()); + if($compound !== null){ + try{ + $itemResult->setNamedTag($compound); + }catch(NbtException $e){ + throw TypeConversionException::wrap($e, "Bad itemstack NBT data"); + } } + + return $itemResult; } /**