From 9bb3c93285d8589cde855b08318e32514eff78c4 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sat, 27 Oct 2018 15:26:01 +0100 Subject: [PATCH] Remove network-serialized item NBT from API layer, item NBT is now retained for the lifetime of the stack --- src/pocketmine/inventory/BaseInventory.php | 12 +-- src/pocketmine/inventory/ShapedRecipe.php | 2 +- src/pocketmine/inventory/ShapelessRecipe.php | 4 +- .../transaction/CraftingTransaction.php | 2 +- src/pocketmine/item/Item.php | 89 ++++++------------- src/pocketmine/item/ItemFactory.php | 20 ++--- .../network/mcpe/NetworkBinaryStream.php | 28 ++++-- 7 files changed, 66 insertions(+), 91 deletions(-) diff --git a/src/pocketmine/inventory/BaseInventory.php b/src/pocketmine/inventory/BaseInventory.php index 27c0fd6c9..cc335e6de 100644 --- a/src/pocketmine/inventory/BaseInventory.php +++ b/src/pocketmine/inventory/BaseInventory.php @@ -181,7 +181,7 @@ abstract class BaseInventory implements Inventory{ public function contains(Item $item) : bool{ $count = max(1, $item->getCount()); $checkDamage = !$item->hasAnyDamageValue(); - $checkTags = $item->hasCompoundTag(); + $checkTags = $item->hasNamedTag(); foreach($this->getContents() as $i){ if($item->equals($i, $checkDamage, $checkTags)){ $count -= $i->getCount(); @@ -197,7 +197,7 @@ abstract class BaseInventory implements Inventory{ public function all(Item $item) : array{ $slots = []; $checkDamage = !$item->hasAnyDamageValue(); - $checkTags = $item->hasCompoundTag(); + $checkTags = $item->hasNamedTag(); foreach($this->getContents() as $index => $i){ if($item->equals($i, $checkDamage, $checkTags)){ $slots[$index] = $i; @@ -209,7 +209,7 @@ abstract class BaseInventory implements Inventory{ public function remove(Item $item) : void{ $checkDamage = !$item->hasAnyDamageValue(); - $checkTags = $item->hasCompoundTag(); + $checkTags = $item->hasNamedTag(); foreach($this->getContents() as $index => $i){ if($item->equals($i, $checkDamage, $checkTags)){ @@ -221,7 +221,7 @@ abstract class BaseInventory implements Inventory{ public function first(Item $item, bool $exact = false) : int{ $count = $exact ? $item->getCount() : max(1, $item->getCount()); $checkDamage = $exact || !$item->hasAnyDamageValue(); - $checkTags = $exact || $item->hasCompoundTag(); + $checkTags = $exact || $item->hasNamedTag(); foreach($this->getContents() as $index => $i){ if($item->equals($i, $checkDamage, $checkTags) and ($i->getCount() === $count or (!$exact and $i->getCount() > $count))){ @@ -249,7 +249,7 @@ abstract class BaseInventory implements Inventory{ public function canAddItem(Item $item) : bool{ $item = clone $item; $checkDamage = !$item->hasAnyDamageValue(); - $checkTags = $item->hasCompoundTag(); + $checkTags = $item->hasNamedTag(); for($i = 0, $size = $this->getSize(); $i < $size; ++$i){ $slot = $this->getItem($i); if($item->equals($slot, $checkDamage, $checkTags)){ @@ -342,7 +342,7 @@ abstract class BaseInventory implements Inventory{ } foreach($itemSlots as $index => $slot){ - if($slot->equals($item, !$slot->hasAnyDamageValue(), $slot->hasCompoundTag())){ + if($slot->equals($item, !$slot->hasAnyDamageValue(), $slot->hasNamedTag())){ $amount = min($item->getCount(), $slot->getCount()); $slot->setCount($slot->getCount() - $amount); $item->setCount($item->getCount() - $amount); diff --git a/src/pocketmine/inventory/ShapedRecipe.php b/src/pocketmine/inventory/ShapedRecipe.php index b15778ea9..ae787c080 100644 --- a/src/pocketmine/inventory/ShapedRecipe.php +++ b/src/pocketmine/inventory/ShapedRecipe.php @@ -197,7 +197,7 @@ class ShapedRecipe implements CraftingRecipe{ $given = $grid->getIngredient($reverse ? $this->width - $x - 1 : $x, $y); $required = $this->getIngredient($x, $y); - if(!$required->equals($given, !$required->hasAnyDamageValue(), $required->hasCompoundTag()) or $required->getCount() > $given->getCount()){ + if(!$required->equals($given, !$required->hasAnyDamageValue(), $required->hasNamedTag()) or $required->getCount() > $given->getCount()){ return false; } } diff --git a/src/pocketmine/inventory/ShapelessRecipe.php b/src/pocketmine/inventory/ShapelessRecipe.php index a8dbcb007..7db3d3c13 100644 --- a/src/pocketmine/inventory/ShapelessRecipe.php +++ b/src/pocketmine/inventory/ShapelessRecipe.php @@ -81,7 +81,7 @@ class ShapelessRecipe implements CraftingRecipe{ if($item->getCount() <= 0){ break; } - if($ingredient->equals($item, !$item->hasAnyDamageValue(), $item->hasCompoundTag())){ + if($ingredient->equals($item, !$item->hasAnyDamageValue(), $item->hasNamedTag())){ unset($this->ingredients[$index]); $item->pop(); } @@ -124,7 +124,7 @@ class ShapelessRecipe implements CraftingRecipe{ foreach($this->ingredients as $needItem){ foreach($input as $j => $haveItem){ - if($haveItem->equals($needItem, !$needItem->hasAnyDamageValue(), $needItem->hasCompoundTag()) and $haveItem->getCount() >= $needItem->getCount()){ + if($haveItem->equals($needItem, !$needItem->hasAnyDamageValue(), $needItem->hasNamedTag()) and $haveItem->getCount() >= $needItem->getCount()){ unset($input[$j]); continue 2; } diff --git a/src/pocketmine/inventory/transaction/CraftingTransaction.php b/src/pocketmine/inventory/transaction/CraftingTransaction.php index de7fec6fa..0ef31b375 100644 --- a/src/pocketmine/inventory/transaction/CraftingTransaction.php +++ b/src/pocketmine/inventory/transaction/CraftingTransaction.php @@ -69,7 +69,7 @@ class CraftingTransaction extends InventoryTransaction{ $haveCount = 0; foreach($txItems as $j => $txItem){ - if($txItem->equals($recipeItem, !$wildcards or !$recipeItem->hasAnyDamageValue(), !$wildcards or $recipeItem->hasCompoundTag())){ + if($txItem->equals($recipeItem, !$wildcards or !$recipeItem->hasAnyDamageValue(), !$wildcards or $recipeItem->hasNamedTag())){ $haveCount += $txItem->getCount(); unset($txItems[$j]); } diff --git a/src/pocketmine/item/Item.php b/src/pocketmine/item/Item.php index e92da3194..2df5a4dd1 100644 --- a/src/pocketmine/item/Item.php +++ b/src/pocketmine/item/Item.php @@ -81,14 +81,14 @@ class Item implements ItemIds, \JsonSerializable{ * * This function redirects to {@link ItemFactory#get}. * - * @param int $id - * @param int $meta - * @param int $count - * @param CompoundTag|string $tags + * @param int $id + * @param int $meta + * @param int $count + * @param CompoundTag $tags * * @return Item */ - public static function get(int $id, int $meta = 0, int $count = 1, $tags = "") : Item{ + public static function get(int $id, int $meta = 0, int $count = 1, ?CompoundTag $tags = null) : Item{ return ItemFactory::get($id, $meta, $count, $tags); } @@ -170,10 +170,8 @@ class Item implements ItemIds, \JsonSerializable{ protected $id; /** @var int */ protected $meta; - /** @var string */ - private $tags = ""; /** @var CompoundTag|null */ - private $cachedNBT = null; + private $nbt = null; /** @var int */ protected $count = 1; /** @var string */ @@ -199,40 +197,6 @@ class Item implements ItemIds, \JsonSerializable{ $this->name = $name; } - /** - * Sets the Item's NBT - * - * @param CompoundTag|string $tags - * - * @return Item - */ - public function setCompoundTag($tags) : Item{ - if($tags instanceof CompoundTag){ - $this->setNamedTag($tags); - }else{ - $this->tags = (string) $tags; - $this->cachedNBT = null; - } - - return $this; - } - - /** - * Returns the serialized NBT of the Item - * @return string - */ - public function getCompoundTag() : string{ - return $this->tags; - } - - /** - * Returns whether this Item has a non-empty NBT. - * @return bool - */ - public function hasCompoundTag() : bool{ - return $this->tags !== ""; - } - /** * @return bool */ @@ -537,6 +501,14 @@ class Item implements ItemIds, \JsonSerializable{ $this->setNamedTag($tag); } + /** + * Returns whether this Item has a non-empty NBT. + * @return bool + */ + public function hasNamedTag() : bool{ + return $this->nbt !== null and $this->nbt->count() > 0; + } + /** * Returns a tree of Tag objects representing the Item's NBT. If the item does not have any NBT, an empty CompoundTag * object is returned to allow the caller to manipulate and apply back to the item. @@ -544,11 +516,7 @@ class Item implements ItemIds, \JsonSerializable{ * @return CompoundTag */ public function getNamedTag() : CompoundTag{ - if(!$this->hasCompoundTag() and $this->cachedNBT === null){ - $this->cachedNBT = new CompoundTag(); - } - - return $this->cachedNBT ?? ($this->cachedNBT = self::parseCompoundTag($this->tags)); + return $this->nbt ?? ($this->nbt = new CompoundTag()); } /** @@ -563,8 +531,7 @@ class Item implements ItemIds, \JsonSerializable{ return $this->clearNamedTag(); } - $this->cachedNBT = $tag; - $this->tags = self::writeCompoundTag($tag); + $this->nbt = clone $tag; return $this; } @@ -574,7 +541,8 @@ class Item implements ItemIds, \JsonSerializable{ * @return Item */ public function clearNamedTag() : Item{ - return $this->setCompoundTag(""); + $this->nbt = null; + return $this; } /** @@ -817,9 +785,7 @@ class Item implements ItemIds, \JsonSerializable{ final public function equals(Item $item, bool $checkDamage = true, bool $checkCompound = true) : bool{ if($this->id === $item->getId() and (!$checkDamage or $this->getDamage() === $item->getDamage())){ if($checkCompound){ - if($item->getCompoundTag() === $this->getCompoundTag()){ - return true; - }elseif($this->hasCompoundTag() and $item->hasCompoundTag()){ + if($this->hasNamedTag() and $item->hasNamedTag()){ //Serialized NBT didn't match, check the cached object tree. return $this->getNamedTag()->equals($item->getNamedTag()); } @@ -846,7 +812,7 @@ class Item implements ItemIds, \JsonSerializable{ * @return string */ final public function __toString() : string{ - return "Item " . $this->name . " (" . $this->id . ":" . ($this->hasAnyDamageValue() ? "?" : $this->getDamage()) . ")x" . $this->count . ($this->hasCompoundTag() ? " tags:0x" . bin2hex($this->getCompoundTag()) : ""); + return "Item " . $this->name . " (" . $this->id . ":" . ($this->hasAnyDamageValue() ? "?" : $this->getDamage()) . ")x" . $this->count . ($this->hasNamedTag() ? " tags:0x" . self::writeCompoundTag($this->nbt) : ""); } /** @@ -867,8 +833,8 @@ class Item implements ItemIds, \JsonSerializable{ $data["count"] = $this->getCount(); } - if($this->hasCompoundTag()){ - $data["nbt_b64"] = base64_encode($this->getCompoundTag()); + if($this->hasNamedTag()){ + $data["nbt_b64"] = base64_encode(self::writeCompoundTag($this->getNamedTag())); } return $data; @@ -893,10 +859,7 @@ class Item implements ItemIds, \JsonSerializable{ $nbt = base64_decode($data["nbt_b64"], true); } return ItemFactory::get( - (int) $data["id"], - (int) ($data["damage"] ?? 0), - (int) ($data["count"] ?? 1), - (string) $nbt + (int) $data["id"], (int) ($data["damage"] ?? 0), (int) ($data["count"] ?? 1), $nbt !== "" ? self::parseCompoundTag($nbt) : null ); } @@ -915,7 +878,7 @@ class Item implements ItemIds, \JsonSerializable{ new ShortTag("Damage", $this->getDamage()) ]); - if($this->hasCompoundTag()){ + if($this->hasNamedTag()){ $itemNBT = clone $this->getNamedTag(); $itemNBT->setName("tag"); $result->setTag($itemNBT); @@ -970,6 +933,8 @@ class Item implements ItemIds, \JsonSerializable{ } public function __clone(){ - $this->cachedNBT = null; + if($this->nbt !== null){ + $this->nbt = clone $this->nbt; + } } } diff --git a/src/pocketmine/item/ItemFactory.php b/src/pocketmine/item/ItemFactory.php index 67023c599..2c2622951 100644 --- a/src/pocketmine/item/ItemFactory.php +++ b/src/pocketmine/item/ItemFactory.php @@ -324,19 +324,15 @@ class ItemFactory{ /** * Returns an instance of the Item with the specified id, meta, count and NBT. * - * @param int $id - * @param int $meta - * @param int $count - * @param CompoundTag|string $tags + * @param int $id + * @param int $meta + * @param int $count + * @param CompoundTag $tags * * @return Item - * @throws \TypeError + * @throws \InvalidArgumentException */ - public static function get(int $id, int $meta = 0, int $count = 1, $tags = "") : Item{ - if(!is_string($tags) and !($tags instanceof CompoundTag)){ - throw new \TypeError("`tags` argument must be a string or CompoundTag instance, " . (is_object($tags) ? "instance of " . get_class($tags) : gettype($tags)) . " given"); - } - + public static function get(int $id, int $meta = 0, int $count = 1, ?CompoundTag $tags = null) : Item{ /** @var Item $item */ $item = null; if($meta !== -1){ @@ -357,7 +353,9 @@ class ItemFactory{ } $item->setCount($count); - $item->setCompoundTag($tags); + if($tags !== null){ + $item->setNamedTag($tags); + } return $item; } diff --git a/src/pocketmine/network/mcpe/NetworkBinaryStream.php b/src/pocketmine/network/mcpe/NetworkBinaryStream.php index 8cdfd68c9..d857d9720 100644 --- a/src/pocketmine/network/mcpe/NetworkBinaryStream.php +++ b/src/pocketmine/network/mcpe/NetworkBinaryStream.php @@ -30,12 +30,15 @@ use pocketmine\entity\Entity; use pocketmine\item\Item; use pocketmine\item\ItemFactory; use pocketmine\math\Vector3; +use pocketmine\nbt\LittleEndianNBTStream; use pocketmine\network\mcpe\protocol\types\CommandOriginData; use pocketmine\network\mcpe\protocol\types\EntityLink; use pocketmine\utils\BinaryStream; use pocketmine\utils\UUID; class NetworkBinaryStream extends BinaryStream{ + /** @var LittleEndianNBTStream */ + private static $itemNbtSerializer = null; public function getString() : string{ return $this->get($this->getUnsignedVarInt()); @@ -77,10 +80,12 @@ class NetworkBinaryStream extends BinaryStream{ $cnt = $auxValue & 0xff; $nbtLen = $this->getLShort(); - $nbt = ""; - + $compound = null; if($nbtLen > 0){ - $nbt = $this->get($nbtLen); + if(self::$itemNbtSerializer === null){ + self::$itemNbtSerializer = new LittleEndianNBTStream(); + } + $compound = self::$itemNbtSerializer->read($this->get($nbtLen)); } //TODO @@ -93,7 +98,7 @@ class NetworkBinaryStream extends BinaryStream{ $this->getString(); } - return ItemFactory::get($id, $data, $cnt, $nbt); + return ItemFactory::get($id, $data, $cnt, $compound); } @@ -108,10 +113,17 @@ class NetworkBinaryStream extends BinaryStream{ $auxValue = (($item->getDamage() & 0x7fff) << 8) | $item->getCount(); $this->putVarInt($auxValue); - $nbt = $item->getCompoundTag(); - $nbtLen = strlen($nbt); - if($nbtLen > 32767){ - throw new \InvalidArgumentException("NBT encoded length must be < 32768, got $nbtLen bytes"); + $nbt = ""; + $nbtLen = 0; + if($item->hasNamedTag()){ + if(self::$itemNbtSerializer === null){ + self::$itemNbtSerializer = new LittleEndianNBTStream(); + } + $nbt = self::$itemNbtSerializer->write($item->getNamedTag()); + $nbtLen = strlen($nbt); + if($nbtLen > 32767){ + throw new \InvalidArgumentException("NBT encoded length must be < 32768, got $nbtLen bytes"); + } } $this->putLShort($nbtLen);