Remove network-serialized item NBT from API layer, item NBT is now retained for the lifetime of the stack

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);

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;
 				}
 			}

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;
 				}

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]);
 				}

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         $id
-	 * @param int                $meta
+	 * @param int         $meta
-	 * @param int                $count
+	 * @param int         $count
-	 * @param CompoundTag|string $tags
+	 * @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){
+		return $this->nbt ?? ($this->nbt = new CompoundTag());
-			$this->cachedNBT = new CompoundTag();
-		}
-
-		return $this->cachedNBT ?? ($this->cachedNBT = self::parseCompoundTag($this->tags));
 	}
 
 	/**
@@ -563,8 +531,7 @@ class Item implements ItemIds, \JsonSerializable{
 			return $this->clearNamedTag();
 		}
 
-		$this->cachedNBT = $tag;
+		$this->nbt = clone $tag;
-		$this->tags = self::writeCompoundTag($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()){
+				if($this->hasNamedTag() and $item->hasNamedTag()){
-					return true;
-				}elseif($this->hasCompoundTag() and $item->hasCompoundTag()){
 					//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()){
+		if($this->hasNamedTag()){
-			$data["nbt_b64"] = base64_encode($this->getCompoundTag());
+			$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["id"], (int) ($data["damage"] ?? 0), (int) ($data["count"] ?? 1), $nbt !== "" ? self::parseCompoundTag($nbt) : null
-			(int) ($data["damage"] ?? 0),
-			(int) ($data["count"] ?? 1),
-			(string) $nbt
 		);
 	}
 
@@ -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;
+		}
 	}
 }

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         $id
-	 * @param int                $meta
+	 * @param int         $meta
-	 * @param int                $count
+	 * @param int         $count
-	 * @param CompoundTag|string $tags
+	 * @param CompoundTag $tags
 	 *
 	 * @return Item
-	 * @throws \TypeError
+	 * @throws \InvalidArgumentException
 	 */
-	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{
-		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");
-		}
-
 		/** @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;
 	}
 

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();
+		$nbt = "";
-		$nbtLen = strlen($nbt);
+		$nbtLen = 0;
-		if($nbtLen > 32767){
+		if($item->hasNamedTag()){
-			throw new \InvalidArgumentException("NBT encoded length must be < 32768, got $nbtLen bytes");
+			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);