Remove item NBT runtime usage, marginalize to serialize/deserialize

this is a more tame version of my initial attempt to firehose item NBT. It marginalizes the use of item NBT to the places where it's needed on interfaces, leaving the internals clean to operate on whatever they like.

src/pocketmine/item/Armor.php

@@ -30,6 +30,7 @@ use pocketmine\inventory\ArmorInventory;
 use pocketmine\item\enchantment\Enchantment;
 use pocketmine\item\enchantment\ProtectionEnchantment;
 use pocketmine\math\Vector3;
+use pocketmine\nbt\tag\CompoundTag;
 use pocketmine\nbt\tag\IntTag;
 use pocketmine\player\Player;
 use pocketmine\utils\Binary;
@@ -44,6 +45,9 @@ abstract class Armor extends Durable{
 	/** @var ArmorTypeInfo */
 	private $armorInfo;
 
+	/** @var Color|null */
+	protected $customColor = null;
+
 	public function __construct(int $id, int $variant, string $name, ArmorTypeInfo $info){
 		parent::__construct($id, $variant, $name);
 		$this->armorInfo = $info;
@@ -72,11 +76,7 @@ abstract class Armor extends Durable{
 	 * @return Color|null
 	 */
 	public function getCustomColor() : ?Color{
-		if($this->getNamedTag()->hasTag(self::TAG_CUSTOM_COLOR, IntTag::class)){
+		return $this->customColor;
-			return Color::fromARGB(Binary::unsignInt($this->getNamedTag()->getInt(self::TAG_CUSTOM_COLOR)));
-		}
-
-		return null;
 	}
 
 	/**
@@ -87,7 +87,7 @@ abstract class Armor extends Durable{
 	 * @return $this
 	 */
 	public function setCustomColor(Color $color) : self{
-		$this->getNamedTag()->setInt(self::TAG_CUSTOM_COLOR, Binary::signInt($color->toARGB()));
+		$this->customColor = $color;
 		return $this;
 	}
 
@@ -137,4 +137,20 @@ abstract class Armor extends Durable{
 		$player->getArmorInventory()->setItem($this->getArmorSlot(), $this->pop());
 		return ItemUseResult::SUCCESS();
 	}
+
+	protected function deserializeCompoundTag(CompoundTag $tag) : void{
+		parent::deserializeCompoundTag($tag);
+		if($tag->hasTag(self::TAG_CUSTOM_COLOR, IntTag::class)){
+			$this->customColor = Color::fromARGB(Binary::unsignInt($tag->getInt(self::TAG_CUSTOM_COLOR)));
+		}else{
+			$this->customColor = null;
+		}
+	}
+
+	protected function serializeCompoundTag(CompoundTag $tag) : void{
+		parent::serializeCompoundTag($tag);
+		$this->customColor !== null ?
+			$tag->setInt(self::TAG_CUSTOM_COLOR, Binary::signInt($this->customColor->toARGB())) :
+			$tag->removeTag(self::TAG_CUSTOM_COLOR);
+	}
 }

src/pocketmine/item/Banner.php

@@ -40,9 +40,14 @@ class Banner extends Item{
 	/** @var DyeColor */
 	private $color;
 
+	/** @var BannerPattern[]|Deque */
+	private $patterns;
+
 	public function __construct(int $id, int $variant, string $name, DyeColor $color){
 		parent::__construct($id, $variant, $name);
 		$this->color = $color;
+
+		$this->patterns = new Deque();
 	}
 
 	/**
@@ -64,15 +69,7 @@ class Banner extends Item{
 	 * @return Deque|BannerPattern[]
 	 */
 	public function getPatterns() : Deque{
-		$result = new Deque();
+		return $this->patterns;
-		$tag = $this->getNamedTag()->getListTag(self::TAG_PATTERNS);
-		if($tag !== null){
-			/** @var CompoundTag $t */
-			foreach($tag as $t){
-				$result->push(new BannerPattern($t->getString(self::TAG_PATTERN_NAME), DyeColor::fromMagicNumber($t->getInt(self::TAG_PATTERN_COLOR), true)));
-			}
-		}
-		return $result;
 	}
 
 	/**
@@ -81,19 +78,51 @@ class Banner extends Item{
 	 * @return $this
 	 */
 	public function setPatterns(Deque $patterns) : self{
-		$tag = new ListTag();
+		$this->patterns = $patterns;
-		/** @var BannerPattern $pattern */
-		foreach($patterns as $pattern){
-			$tag->push(CompoundTag::create()
-				->setString(self::TAG_PATTERN_NAME, $pattern->getId())
-				->setInt(self::TAG_PATTERN_COLOR, $pattern->getColor()->getInvertedMagicNumber())
-			);
-		}
-		$this->getNamedTag()->setTag(self::TAG_PATTERNS, $tag);
 		return $this;
 	}
 
 	public function getFuelTime() : int{
 		return 300;
 	}
+
+	protected function deserializeCompoundTag(CompoundTag $tag) : void{
+		parent::deserializeCompoundTag($tag);
+
+		$this->patterns = new Deque();
+
+		$patterns = $tag->getListTag(self::TAG_PATTERNS);
+		if($patterns !== null){
+			/** @var CompoundTag $t */
+			foreach($patterns as $t){
+				$this->patterns->push(new BannerPattern($t->getString(self::TAG_PATTERN_NAME), DyeColor::fromMagicNumber($t->getInt(self::TAG_PATTERN_COLOR), true)));
+			}
+		}
+	}
+
+	protected function serializeCompoundTag(CompoundTag $tag) : void{
+		parent::serializeCompoundTag($tag);
+
+		if(!$this->patterns->isEmpty()){
+			$patterns = new ListTag();
+			/** @var BannerPattern $pattern */
+			foreach($this->patterns as $pattern){
+				$patterns->push(CompoundTag::create()
+					->setString(self::TAG_PATTERN_NAME, $pattern->getId())
+					->setInt(self::TAG_PATTERN_COLOR, $pattern->getColor()->getInvertedMagicNumber())
+				);
+			}
+
+
+			$tag->setTag(self::TAG_PATTERNS, $patterns);
+		}else{
+			$tag->removeTag(self::TAG_PATTERNS);
+		}
+	}
+
+	public function __clone(){
+		parent::__clone();
+		//we don't need to duplicate the individual patterns because they are immutable
+		$this->patterns = $this->patterns->copy();
+	}
 }

src/pocketmine/item/Durable.php

@@ -24,6 +24,7 @@ declare(strict_types=1);
 namespace pocketmine\item;
 
 use pocketmine\item\enchantment\Enchantment;
+use pocketmine\nbt\tag\CompoundTag;
 use function lcg_value;
 use function min;
 
@@ -31,6 +32,8 @@ abstract class Durable extends Item{
 
 	/** @var int */
 	protected $damage = 0;
+	/** @var bool */
+	private $unbreakable = false;
 
 	public function getMeta() : int{
 		return $this->damage;
@@ -41,7 +44,7 @@ abstract class Durable extends Item{
 	 * @return bool
 	 */
 	public function isUnbreakable() : bool{
-		return $this->getNamedTag()->getByte("Unbreakable", 0) !== 0;
+		return $this->unbreakable;
 	}
 
 	/**
@@ -52,7 +55,7 @@ abstract class Durable extends Item{
 	 * @return $this
 	 */
 	public function setUnbreakable(bool $value = true) : self{
-		$this->getNamedTag()->setByte("Unbreakable", $value ? 1 : 0);
+		$this->unbreakable = $value;
 		return $this;
 	}
 
@@ -128,4 +131,14 @@ abstract class Durable extends Item{
 	public function isBroken() : bool{
 		return $this->damage >= $this->getMaxDurability();
 	}
+
+	protected function deserializeCompoundTag(CompoundTag $tag) : void{
+		parent::deserializeCompoundTag($tag);
+		$this->unbreakable = $tag->getByte("Unbreakable", 0) !== 0;
+	}
+
+	protected function serializeCompoundTag(CompoundTag $tag) : void{
+		parent::serializeCompoundTag($tag);
+		$this->unbreakable ? $tag->setByte("Unbreakable", 1) : $tag->removeTag("Unbreakable");
+	}
 }

src/pocketmine/item/Item.php

@@ -26,6 +26,8 @@ declare(strict_types=1);
  */
 namespace pocketmine\item;
 
+use function array_map;
+use Ds\Set;
 use pocketmine\block\Block;
 use pocketmine\block\BlockBreakInfo;
 use pocketmine\block\BlockToolType;
@@ -44,11 +46,12 @@ use pocketmine\nbt\tag\StringTag;
 use pocketmine\nbt\TreeRoot;
 use pocketmine\player\Player;
 use pocketmine\utils\Binary;
-use function array_map;
 use function base64_decode;
 use function base64_encode;
 use function get_class;
+use function gettype;
 use function hex2bin;
+use function is_string;
 
 class Item implements ItemIds, \JsonSerializable{
 	public const TAG_ENCH = "ench";
@@ -69,6 +72,25 @@ class Item implements ItemIds, \JsonSerializable{
 	/** @var string */
 	protected $name;
 
+	//TODO: this stuff should be moved to itemstack properties, not mushed in with type properties
+
+	/** @var EnchantmentInstance[] */
+	protected $enchantments = [];
+	/** @var string */
+	protected $customName = "";
+	/** @var string[] */
+	protected $lore = [];
+	/**
+	 * TODO: this needs to die in a fire
+	 * @var CompoundTag|null
+	 */
+	protected $blockEntityTag = null;
+
+	/** @var Set|string[] */
+	protected $canPlaceOn;
+	/** @var Set|string[] */
+	protected $canDestroy;
+
 	/**
 	 * Constructs a new Item type. This constructor should ONLY be used when constructing a new item TYPE to register
 	 * into the index.
@@ -87,17 +109,20 @@ class Item implements ItemIds, \JsonSerializable{
 		$this->id = $id;
 		$this->meta = $variant !== -1 ? $variant & 0x7FFF : -1;
 		$this->name = $name;
+
+		$this->canPlaceOn = new Set();
+		$this->canDestroy = new Set();
 	}
 
 	/**
 	 * @return bool
 	 */
 	public function hasCustomBlockData() : bool{
-		return $this->getNamedTag()->hasTag(self::TAG_BLOCK_ENTITY_TAG, CompoundTag::class);
+		return $this->blockEntityTag !== null;
 	}
 
 	public function clearCustomBlockData(){
-		$this->getNamedTag()->removeTag(self::TAG_BLOCK_ENTITY_TAG);
+		$this->blockEntityTag = null;
 		return $this;
 	}
 
@@ -107,7 +132,8 @@ class Item implements ItemIds, \JsonSerializable{
 	 * @return Item
 	 */
 	public function setCustomBlockData(CompoundTag $compound) : Item{
-		$this->getNamedTag()->setTag(self::TAG_BLOCK_ENTITY_TAG, clone $compound);
+		$this->blockEntityTag = clone $compound;
+
 		return $this;
 	}
 
@@ -115,14 +141,14 @@ class Item implements ItemIds, \JsonSerializable{
 	 * @return CompoundTag|null
 	 */
 	public function getCustomBlockData() : ?CompoundTag{
-		return $this->getNamedTag()->getCompoundTag(self::TAG_BLOCK_ENTITY_TAG);
+		return $this->blockEntityTag;
 	}
 
 	/**
 	 * @return bool
 	 */
 	public function hasEnchantments() : bool{
-		return $this->getNamedTag()->hasTag(self::TAG_ENCH, ListTag::class);
+		return !empty($this->enchantments);
 	}
 
 	/**
@@ -132,20 +158,8 @@ class Item implements ItemIds, \JsonSerializable{
 	 * @return bool
 	 */
 	public function hasEnchantment(Enchantment $enchantment, int $level = -1) : bool{
-		$ench = $this->getNamedTag()->getListTag(self::TAG_ENCH);
-		if(!($ench instanceof ListTag)){
-			return false;
-		}
 		$id = $enchantment->getId();
-
+		return isset($this->enchantments[$id]) and ($level === -1 or $this->enchantments[$id]->getLevel() === $level);
-		/** @var CompoundTag $entry */
-		foreach($ench as $entry){
-			if($entry->getShort("id") === $id and ($level === -1 or $entry->getShort("lvl") === $level)){
-				return true;
-			}
-		}
-
-		return false;
 	}
 
 	/**
@@ -154,23 +168,7 @@ class Item implements ItemIds, \JsonSerializable{
 	 * @return EnchantmentInstance|null
 	 */
 	public function getEnchantment(Enchantment $enchantment) : ?EnchantmentInstance{
-		$ench = $this->getNamedTag()->getListTag(self::TAG_ENCH);
+		return $this->enchantments[$enchantment->getId()] ?? null;
-		if(!($ench instanceof ListTag)){
-			return null;
-		}
-
-		$id = $enchantment->getId();
-		/** @var CompoundTag $entry */
-		foreach($ench as $entry){
-			if($entry->getShort("id") === $id){
-				$e = Enchantment::get($entry->getShort("id"));
-				if($e !== null){
-					return new EnchantmentInstance($e, $entry->getShort("lvl"));
-				}
-			}
-		}
-
-		return null;
 	}
 
 	/**
@@ -180,18 +178,9 @@ class Item implements ItemIds, \JsonSerializable{
 	 * @return Item
 	 */
 	public function removeEnchantment(Enchantment $enchantment, int $level = -1) : Item{
-		$ench = $this->getNamedTag()->getListTag(self::TAG_ENCH);
+		$instance = $this->getEnchantment($enchantment);
-		if(!($ench instanceof ListTag)){
+		if($instance !== null and ($level === -1 or $instance->getLevel() === $level)){
-			return $this;
+			unset($this->enchantments[$enchantment->getId()]);
-		}
-
-		$id = $enchantment->getId();
-		/** @var CompoundTag $entry */
-		foreach($ench as $k => $entry){
-			if($entry->getShort("id") === $id and ($level === -1 or $entry->getShort("lvl") === $level)){
-				$ench->remove($k);
-				break;
-			}
 		}
 
 		return $this;
@@ -201,8 +190,7 @@ class Item implements ItemIds, \JsonSerializable{
 	 * @return Item
 	 */
 	public function removeEnchantments() : Item{
-		$this->getNamedTag()->removeTag(self::TAG_ENCH);
+		$this->enchantments = [];
-
 		return $this;
 	}
 
@@ -212,34 +200,7 @@ class Item implements ItemIds, \JsonSerializable{
 	 * @return Item
 	 */
 	public function addEnchantment(EnchantmentInstance $enchantment) : Item{
-		$found = false;
+		$this->enchantments[$enchantment->getId()] = $enchantment;
-
-		$ench = $this->getNamedTag()->getListTag(self::TAG_ENCH);
-		if(!($ench instanceof ListTag)){
-			$ench = new ListTag([], NBT::TAG_Compound);
-		}else{
-			/** @var CompoundTag $entry */
-			foreach($ench as $k => $entry){
-				if($entry->getShort("id") === $enchantment->getId()){
-					$ench->set($k, CompoundTag::create()
-						->setShort("id", $enchantment->getId())
-						->setShort("lvl", $enchantment->getLevel())
-					);
-					$found = true;
-					break;
-				}
-			}
-		}
-
-		if(!$found){
-			$ench->push(CompoundTag::create()
-				->setShort("id", $enchantment->getId())
-				->setShort("lvl", $enchantment->getLevel())
-			);
-		}
-
-		$this->getNamedTag()->setTag(self::TAG_ENCH, $ench);
-
 		return $this;
 	}
 
@@ -247,21 +208,7 @@ class Item implements ItemIds, \JsonSerializable{
 	 * @return EnchantmentInstance[]
 	 */
 	public function getEnchantments() : array{
-		/** @var EnchantmentInstance[] $enchantments */
+		return $this->enchantments;
-		$enchantments = [];
-
-		$ench = $this->getNamedTag()->getListTag(self::TAG_ENCH);
-		if($ench instanceof ListTag){
-			/** @var CompoundTag $entry */
-			foreach($ench as $entry){
-				$e = Enchantment::get($entry->getShort("id"));
-				if($e !== null){
-					$enchantments[] = new EnchantmentInstance($e, $entry->getShort("lvl"));
-				}
-			}
-		}
-
-		return $enchantments;
 	}
 
 	/**
@@ -273,42 +220,21 @@ class Item implements ItemIds, \JsonSerializable{
 	 * @return int
 	 */
 	public function getEnchantmentLevel(Enchantment $enchantment) : int{
-		$ench = $this->getNamedTag()->getListTag(self::TAG_ENCH);
+		return ($instance = $this->getEnchantment($enchantment)) !== null ? $instance->getLevel() : 0;
-		if($ench !== null){
-			/** @var CompoundTag $entry */
-			$enchantmentId = $enchantment->getId();
-			foreach($ench as $entry){
-				if($entry->getShort("id") === $enchantmentId){
-					return $entry->getShort("lvl");
-				}
-			}
-		}
-
-		return 0;
 	}
 
 	/**
 	 * @return bool
 	 */
 	public function hasCustomName() : bool{
-		$display = $this->getNamedTag()->getCompoundTag(self::TAG_DISPLAY);
+		return $this->customName !== "";
-		if($display instanceof CompoundTag){
-			return $display->hasTag(self::TAG_DISPLAY_NAME);
-		}
-
-		return false;
 	}
 
 	/**
 	 * @return string
 	 */
 	public function getCustomName() : string{
-		$display = $this->getNamedTag()->getCompoundTag(self::TAG_DISPLAY);
+		return $this->customName;
-		if($display instanceof CompoundTag){
-			return $display->getString(self::TAG_DISPLAY_NAME, "");
-		}
-
-		return "";
 	}
 
 	/**
@@ -317,19 +243,8 @@ class Item implements ItemIds, \JsonSerializable{
 	 * @return Item
 	 */
 	public function setCustomName(string $name) : Item{
-		if($name === ""){
+		//TODO: encoding might need to be checked here
-			$this->clearCustomName();
+		$this->customName = $name;
-		}
-
-		/** @var CompoundTag $display */
-		$display = $this->getNamedTag()->getCompoundTag(self::TAG_DISPLAY);
-		if($display === null){
-			$display = new CompoundTag();
-		}
-
-		$display->setString(self::TAG_DISPLAY_NAME, $name);
-		$this->getNamedTag()->setTag(self::TAG_DISPLAY, $display);
-
 		return $this;
 	}
 
@@ -337,17 +252,7 @@ class Item implements ItemIds, \JsonSerializable{
 	 * @return Item
 	 */
 	public function clearCustomName() : Item{
-		$display = $this->getNamedTag()->getCompoundTag(self::TAG_DISPLAY);
+		$this->setCustomName("");
-		if($display instanceof CompoundTag){
-			$display->removeTag(self::TAG_DISPLAY_NAME);
-
-			if($display->getCount() === 0){
-				$this->getNamedTag()->removeTag(self::TAG_DISPLAY);
-			}else{
-				$this->getNamedTag()->setTag(self::TAG_DISPLAY, $display);
-			}
-		}
-
 		return $this;
 	}
 
@@ -355,12 +260,7 @@ class Item implements ItemIds, \JsonSerializable{
 	 * @return string[]
 	 */
 	public function getLore() : array{
-		$display = $this->getNamedTag()->getCompoundTag(self::TAG_DISPLAY);
+		return $this->lore;
-		if($display instanceof CompoundTag and ($lore = $display->getListTag(self::TAG_DISPLAY_LORE)) !== null){
-			return $lore->getAllValues();
-		}
-
-		return [];
 	}
 
 	/**
@@ -369,18 +269,41 @@ class Item implements ItemIds, \JsonSerializable{
 	 * @return Item
 	 */
 	public function setLore(array $lines) : Item{
-		$display = $this->getNamedTag()->getCompoundTag(self::TAG_DISPLAY);
+		foreach($lines as $line){
-		if(!($display instanceof CompoundTag)){
+			if(!is_string($line)){
-			$display = new CompoundTag();
+				throw new \TypeError("Expected string[], but found " . gettype($line) . " in given array");
+			}
+		}
+		$this->lore = $lines;
+		return $this;
 	}
 
-		$display->setTag(self::TAG_DISPLAY_LORE, new ListTag(array_map(function(string $str) : StringTag{
+	/**
-			return new StringTag($str);
+	 * @return Set|string[]
-		}, $lines), NBT::TAG_String));
+	 */
+	public function getCanPlaceOn() : Set{
+		return $this->canPlaceOn;
+	}
 
-		$this->getNamedTag()->setTag(self::TAG_DISPLAY, $display);
+	/**
+	 * @param Set|string[] $canPlaceOn
+	 */
+	public function setCanPlaceOn(Set $canPlaceOn) : void{
+		$this->canPlaceOn = $canPlaceOn;
+	}
 
-		return $this;
+	/**
+	 * @return Set|string[]
+	 */
+	public function getCanDestroy() : Set{
+		return $this->canDestroy;
+	}
+
+	/**
+	 * @param Set|string[] $canDestroy
+	 */
+	public function setCanDestroy(Set $canDestroy) : void{
+		$this->canDestroy = $canDestroy;
 	}
 
 	/**
@@ -388,7 +311,7 @@ class Item implements ItemIds, \JsonSerializable{
 	 * @return bool
 	 */
 	public function hasNamedTag() : bool{
-		return $this->nbt !== null and $this->nbt->count() > 0;
+		return $this->getNamedTag()->count() > 0;
 	}
 
 	/**
@@ -398,7 +321,11 @@ class Item implements ItemIds, \JsonSerializable{
 	 * @return CompoundTag
 	 */
 	public function getNamedTag() : CompoundTag{
-		return $this->nbt ?? ($this->nbt = new CompoundTag());
+		if($this->nbt === null){
+			$this->nbt = new CompoundTag();
+		}
+		$this->serializeCompoundTag($this->nbt);
+		return $this->nbt;
 	}
 
 	/**
@@ -414,6 +341,7 @@ class Item implements ItemIds, \JsonSerializable{
 		}
 
 		$this->nbt = clone $tag;
+		$this->deserializeCompoundTag($this->nbt);
 
 		return $this;
 	}
@@ -423,10 +351,121 @@ class Item implements ItemIds, \JsonSerializable{
 	 * @return Item
 	 */
 	public function clearNamedTag() : Item{
-		$this->nbt = null;
+		$this->nbt = new CompoundTag();
+		$this->deserializeCompoundTag($this->nbt);
 		return $this;
 	}
 
+	protected function deserializeCompoundTag(CompoundTag $tag) : void{
+		$this->customName = "";
+		$this->lore = [];
+
+		$display = $tag->getCompoundTag(self::TAG_DISPLAY);
+		if($display !== null){
+			$this->customName = $display->getString(self::TAG_DISPLAY_NAME, $this->customName, true);
+			$lore = $tag->getListTag(self::TAG_DISPLAY_LORE);
+			if($lore !== null and $lore->getTagType() === NBT::TAG_String){
+				/** @var StringTag $t */
+				foreach($lore as $t){
+					$this->lore[] = $t->getValue();
+				}
+			}
+		}
+
+		$this->removeEnchantments();
+		$enchantments = $tag->getListTag(self::TAG_ENCH);
+		if($enchantments !== null and $enchantments->getTagType() === NBT::TAG_Compound){
+			/** @var CompoundTag $enchantment */
+			foreach($enchantments as $enchantment){
+				$magicNumber = $enchantment->getShort("id", 0, true);
+				$level = $enchantment->getShort("lvl", 0, true);
+				if($magicNumber <= 0 or $level <= 0){
+					continue;
+				}
+				$type = Enchantment::get($magicNumber);
+				if($type !== null){
+					$this->addEnchantment(new EnchantmentInstance($type, $level));
+				}
+			}
+		}
+
+		$this->blockEntityTag = $tag->getCompoundTag(self::TAG_BLOCK_ENTITY_TAG);
+
+		$this->canPlaceOn = new Set();
+		$canPlaceOn = $tag->getListTag("CanPlaceOn");
+		if($canPlaceOn !== null){
+			/** @var StringTag $tag */
+			foreach($canPlaceOn as $entry){
+				$this->canPlaceOn->add($entry->getValue());
+			}
+		}
+		$this->canDestroy = new Set();
+		$canDestroy = $tag->getListTag("CanDestroy");
+		if($canDestroy !== null){
+			/** @var StringTag $entry */
+			foreach($canDestroy as $entry){
+				$this->canDestroy->add($entry->getValue());
+			}
+		}
+	}
+
+	protected function serializeCompoundTag(CompoundTag $tag) : void{
+		$display = $tag->getCompoundTag(self::TAG_DISPLAY) ?? new CompoundTag();
+
+		$this->hasCustomName() ?
+			$display->setString(self::TAG_DISPLAY_NAME, $this->getCustomName()) :
+			$display->removeTag(self::TAG_DISPLAY);
+
+		if(!empty($this->lore)){
+			$loreTag = new ListTag();
+			foreach($this->lore as $line){
+				$loreTag->push(new StringTag($line));
+			}
+			$display->setTag(self::TAG_DISPLAY_LORE, $loreTag);
+		}else{
+			$display->removeTag(self::TAG_DISPLAY_LORE);
+		}
+		$display->count() > 0 ?
+			$tag->setTag(self::TAG_DISPLAY, $display) :
+			$tag->removeTag(self::TAG_DISPLAY);
+
+		if($this->hasEnchantments()){
+			$ench = new ListTag();
+			foreach($this->getEnchantments() as $enchantmentInstance){
+				$ench->push(CompoundTag::create()
+					->setShort("id", $enchantmentInstance->getType()->getId())
+					->setShort("lvl", $enchantmentInstance->getLevel())
+				);
+			}
+			$tag->setTag(self::TAG_ENCH, $ench);
+		}else{
+			$tag->removeTag(self::TAG_ENCH);
+		}
+
+		$this->hasCustomBlockData() ?
+			$tag->setTag(self::TAG_BLOCK_ENTITY_TAG, clone $this->getCustomBlockData()) :
+			$tag->removeTag(self::TAG_BLOCK_ENTITY_TAG);
+
+		if(!$this->canPlaceOn->isEmpty()){
+			$canPlaceOn = new ListTag();
+			foreach($this->canPlaceOn as $item){
+				$canPlaceOn->push(new StringTag($item));
+			}
+			$tag->setTag("CanPlaceOn", $canPlaceOn);
+		}else{
+			$tag->removeTag("CanPlaceOn");
+		}
+		if(!$this->canDestroy->isEmpty()){
+			$canDestroy = new ListTag();
+			foreach($this->canDestroy as $item){
+				$canDestroy->push(new StringTag($item));
+			}
+			$tag->setTag("CanDestroy", $canDestroy);
+		}else{
+			$tag->removeTag("CanDestroy");
+		}
+	}
+
 	/**
 	 * @return int
 	 */
@@ -667,11 +706,10 @@ 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->getMeta() === $item->getMeta())){
 			if($checkCompound){
-				if($this->hasNamedTag() and $item->hasNamedTag()){ //both items have NBT
+				$tag1 = $this->getNamedTag();
-					return $this->getNamedTag()->equals($item->getNamedTag());
+				$tag2 = $item->getNamedTag();
-				}
 
-				return (!$this->hasNamedTag() and !$item->hasNamedTag()); //both items must have no NBT
+				return ($tag1 === null and $tag2 === null) or ($tag1 !== null and $tag2 !== null and $tag1->equals($tag2));
 			}else{
 				return true;
 			}
@@ -695,7 +733,7 @@ class Item implements ItemIds, \JsonSerializable{
 	 * @return string
 	 */
 	final public function __toString() : string{
-		return "Item " . $this->name . " (" . $this->id . ":" . ($this->hasAnyDamageValue() ? "?" : $this->getMeta()) . ")x" . $this->count . ($this->hasNamedTag() ? " tags:" . base64_encode((new LittleEndianNbtSerializer())->write(new TreeRoot($this->nbt))) : "");
+		return "Item " . $this->name . " (" . $this->id . ":" . ($this->hasAnyDamageValue() ? "?" : $this->getMeta()) . ")x" . $this->count . (($tag = $this->getNamedTag()) !== null ? " tags:0x" . base64_encode((new LittleEndianNbtSerializer())->write(new TreeRoot($tag))) : "");
 	}
 
 	/**
@@ -716,8 +754,8 @@ class Item implements ItemIds, \JsonSerializable{
 			$data["count"] = $this->getCount();
 		}
 
-		if($this->hasNamedTag()){
+		if(($tag = $this->getNamedTag()) !== null){
-			$data["nbt_b64"] = base64_encode((new LittleEndianNbtSerializer())->write(new TreeRoot($this->getNamedTag())));
+			$data["nbt_b64"] = base64_encode((new LittleEndianNbtSerializer())->write(new TreeRoot($tag)));
 		}
 
 		return $data;
@@ -761,8 +799,8 @@ class Item implements ItemIds, \JsonSerializable{
 			->setByte("Count", Binary::signByte($this->count))
 			->setShort("Damage", $this->getMeta());
 
-		if($this->hasNamedTag()){
+		if(($itemNBT = $this->getNamedTag()) !== null){
-			$result->setTag("tag", clone $this->getNamedTag());
+			$result->setTag("tag", $itemNBT);
 		}
 
 		if($slot !== -1){
@@ -814,5 +852,11 @@ class Item implements ItemIds, \JsonSerializable{
 		if($this->nbt !== null){
 			$this->nbt = clone $this->nbt;
 		}
+		if($this->blockEntityTag !== null){
+			$this->blockEntityTag = clone $this->blockEntityTag;
+		}
+		$this->canPlaceOn = $this->canPlaceOn->copy();
+		$this->canDestroy = $this->canDestroy->copy();
+		$this->enchantments = array_map(function(EnchantmentInstance $i){ return clone $i; }, $this->enchantments);
 	}
 }

src/pocketmine/item/WritableBookBase.php

@@ -23,16 +23,23 @@ declare(strict_types=1);
 
 namespace pocketmine\item;
 
-use pocketmine\nbt\NBT;
+use Ds\Deque;
 use pocketmine\nbt\tag\CompoundTag;
 use pocketmine\nbt\tag\ListTag;
 
 abstract class WritableBookBase extends Item{
-
 	public const TAG_PAGES = "pages"; //TAG_List<TAG_Compound>
 	public const TAG_PAGE_TEXT = "text"; //TAG_String
 	public const TAG_PAGE_PHOTONAME = "photoname"; //TAG_String - TODO
 
+	/** @var WritableBookPage[]|Deque */
+	private $pages;
+
+	public function __construct(int $id, int $variant, string $name){
+		parent::__construct($id, $variant, $name);
+		$this->pages = new Deque();
+	}
+
 	/**
 	 * Returns whether the given page exists in this book.
 	 *
@@ -41,7 +48,7 @@ abstract class WritableBookBase extends Item{
 	 * @return bool
 	 */
 	public function pageExists(int $pageId) : bool{
-		return $this->getPagesTag()->isset($pageId);
+		return isset($this->pages[$pageId]);
 	}
 
 	/**
@@ -49,20 +56,11 @@ abstract class WritableBookBase extends Item{
 	 *
 	 * @param int $pageId
 	 *
-	 * @return string|null
+	 * @return string
+	 * @throws \OutOfRangeException if requesting a nonexisting page
 	 */
-	public function getPageText(int $pageId) : ?string{
+	public function getPageText(int $pageId) : string{
-		$pages = $this->getNamedTag()->getListTag(self::TAG_PAGES);
+		return $this->pages[$pageId]->getText();
-		if($pages === null){
-			return null;
-		}
-
-		$page = $pages->get($pageId);
-		if($page instanceof CompoundTag){
-			return $page->getString(self::TAG_PAGE_TEXT, "");
-		}
-
-		return null;
 	}
 
 	/**
@@ -78,14 +76,7 @@ abstract class WritableBookBase extends Item{
 			$this->addPage($pageId);
 		}
 
-		/** @var CompoundTag[]|ListTag $pagesTag */
+		$this->pages->set($pageId, new WritableBookPage($pageText));
-		$pagesTag = $this->getPagesTag();
-		/** @var CompoundTag $page */
-		$page = $pagesTag->get($pageId);
-		$page->setString(self::TAG_PAGE_TEXT, $pageText);
-
-		$this->getNamedTag()->setTag(self::TAG_PAGES, $pagesTag);
-
 		return $this;
 	}
 
@@ -102,16 +93,9 @@ abstract class WritableBookBase extends Item{
 			throw new \InvalidArgumentException("Page number \"$pageId\" is out of range");
 		}
 
-		$pagesTag = $this->getPagesTag();
+		for($current = $this->pages->count(); $current <= $pageId; $current++){
-
+			$this->pages->push(new WritableBookPage(""));
-		for($current = $pagesTag->count(); $current <= $pageId; $current++){
-			$pagesTag->push(CompoundTag::create()
-				->setString(self::TAG_PAGE_TEXT, "")
-				->setString(self::TAG_PAGE_PHOTONAME, "")
-			);
 		}
-
-		$this->getNamedTag()->setTag(self::TAG_PAGES, $pagesTag);
 		return $this;
 	}
 
@@ -123,9 +107,7 @@ abstract class WritableBookBase extends Item{
 	 * @return $this
 	 */
 	public function deletePage(int $pageId) : self{
-		$pagesTag = $this->getPagesTag();
+		$this->pages->remove($pageId);
-		$pagesTag->remove($pageId);
-
 		return $this;
 	}
 
@@ -138,15 +120,7 @@ abstract class WritableBookBase extends Item{
 	 * @return $this
 	 */
 	public function insertPage(int $pageId, string $pageText = "") : self{
-		$pagesTag = $this->getPagesTag();
+		$this->pages->insert($pageId, new WritableBookPage($pageText));
-
-		$pagesTag->insert($pageId, CompoundTag::create()
-			->setString(self::TAG_PAGE_TEXT, $pageText)
-			->setString(self::TAG_PAGE_PHOTONAME, "")
-		);
-
-		$this->getNamedTag()->setTag(self::TAG_PAGES, $pagesTag);
-
 		return $this;
 	}
 
@@ -157,12 +131,9 @@ abstract class WritableBookBase extends Item{
 	 * @param int $pageId2
 	 *
 	 * @return bool indicating success
+	 * @throws \OutOfRangeException if either of the pages does not exist
 	 */
 	public function swapPages(int $pageId1, int $pageId2) : bool{
-		if(!$this->pageExists($pageId1) or !$this->pageExists($pageId2)){
-			return false;
-		}
-
 		$pageContents1 = $this->getPageText($pageId1);
 		$pageContents2 = $this->getPageText($pageId2);
 		$this->setPageText($pageId1, $pageContents2);
@@ -178,30 +149,51 @@ abstract class WritableBookBase extends Item{
 	/**
 	 * Returns an array containing all pages of this book.
 	 *
-	 * @return CompoundTag[]
+	 * @return WritableBookPage[]|Deque
 	 */
-	public function getPages() : array{
+	public function getPages() : Deque{
-		$pages = $this->getNamedTag()->getListTag(self::TAG_PAGES);
+		return $this->pages;
-		if($pages === null){
-			return [];
-		}
-
-		return $pages->getValue();
-	}
-
-	protected function getPagesTag() : ListTag{
-		return $this->getNamedTag()->getListTag(self::TAG_PAGES) ?? new ListTag([], NBT::TAG_Compound);
 	}
 
 	/**
-	 * @param CompoundTag[] $pages
+	 * @param WritableBookPage[]|Deque $pages
-	 *
-	 * @return $this
 	 */
-	public function setPages(array $pages) : self{
+	public function setPages(Deque $pages) : void{
-		$nbt = $this->getNamedTag();
+		$this->pages = $pages;
-		$nbt->setTag(self::TAG_PAGES, new ListTag($pages, NBT::TAG_Compound));
+	}
-		$this->setNamedTag($nbt);
+
-		return $this;
+	protected function deserializeCompoundTag(CompoundTag $tag) : void{
+		parent::deserializeCompoundTag($tag);
+		$this->pages = new Deque();
+
+		$pages = $tag->getListTag(self::TAG_PAGES);
+		if($pages !== null){
+			/** @var CompoundTag $page */
+			foreach($pages as $page){
+				$this->pages->push(new WritableBookPage($page->getString(self::TAG_PAGE_TEXT), $page->getString(self::TAG_PAGE_PHOTONAME, "")));
+			}
+		}
+	}
+
+	protected function serializeCompoundTag(CompoundTag $tag) : void{
+		parent::serializeCompoundTag($tag);
+		if(!$this->pages->isEmpty()){
+			$pages = new ListTag();
+			foreach($this->pages as $page){
+				$pages->push(CompoundTag::create()
+					->setString(self::TAG_PAGE_TEXT, $page->getText())
+					->setString(self::TAG_PAGE_PHOTONAME, $page->getPhotoName())
+				);
+			}
+			$tag->setTag(self::TAG_PAGES, $pages);
+		}else{
+			$tag->removeTag(self::TAG_PAGES);
+		}
+	}
+
+	public function __clone(){
+		parent::__clone();
+		//no need to deep-copy each page, the objects are immutable
+		$this->pages = $this->pages->copy();
 	}
 }

src/pocketmine/item/WritableBookPage.php

@@ -0,0 +1,52 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+*/
+
+declare(strict_types=1);
+
+namespace pocketmine\item;
+
+class WritableBookPage{
+
+	/** @var string */
+	private $text;
+	/** @var string */
+	private $photoName;
+
+	public function __construct(string $text, string $photoName = ""){
+		//TODO: data validation, encoding checks
+		$this->text = $text;
+		$this->photoName = $photoName;
+	}
+
+	/**
+	 * @return string
+	 */
+	public function getText() : string{
+		return $this->text;
+	}
+
+	/**
+	 * @return string
+	 */
+	public function getPhotoName() : string{
+		return $this->photoName;
+	}
+}

src/pocketmine/item/WrittenBook.php

@@ -23,6 +23,8 @@ declare(strict_types=1);
 
 namespace pocketmine\item;
 
+use pocketmine\nbt\tag\CompoundTag;
+
 class WrittenBook extends WritableBookBase{
 
 	public const GENERATION_ORIGINAL = 0;
@@ -34,6 +36,13 @@ class WrittenBook extends WritableBookBase{
 	public const TAG_AUTHOR = "author"; //TAG_String
 	public const TAG_TITLE = "title"; //TAG_String
 
+	/** @var int */
+	private $generation = self::GENERATION_ORIGINAL;
+	/** @var string */
+	private $author = "";
+	/** @var string */
+	private $title = "";
+
 	public function getMaxStackSize() : int{
 		return 16;
 	}
@@ -45,7 +54,7 @@ class WrittenBook extends WritableBookBase{
 	 * @return int
 	 */
 	public function getGeneration() : int{
-		return $this->getNamedTag()->getInt(self::TAG_GENERATION, self::GENERATION_ORIGINAL);
+		return $this->generation;
 	}
 
 	/**
@@ -59,9 +68,8 @@ class WrittenBook extends WritableBookBase{
 		if($generation < 0 or $generation > 3){
 			throw new \InvalidArgumentException("Generation \"$generation\" is out of range");
 		}
-		$namedTag = $this->getNamedTag();
+
-		$namedTag->setInt(self::TAG_GENERATION, $generation);
+		$this->generation = $generation;
-		$this->setNamedTag($namedTag);
 		return $this;
 	}
 
@@ -73,7 +81,7 @@ class WrittenBook extends WritableBookBase{
 	 * @return string
 	 */
 	public function getAuthor() : string{
-		return $this->getNamedTag()->getString(self::TAG_AUTHOR, "");
+		return $this->author;
 	}
 
 	/**
@@ -84,9 +92,7 @@ class WrittenBook extends WritableBookBase{
 	 * @return $this
 	 */
 	public function setAuthor(string $authorName) : self{
-		$namedTag = $this->getNamedTag();
+		$this->author = $authorName;
-		$namedTag->setString(self::TAG_AUTHOR, $authorName);
-		$this->setNamedTag($namedTag);
 		return $this;
 	}
 
@@ -96,7 +102,7 @@ class WrittenBook extends WritableBookBase{
 	 * @return string
 	 */
 	public function getTitle() : string{
-		return $this->getNamedTag()->getString(self::TAG_TITLE, "");
+		return $this->title;
 	}
 
 	/**
@@ -107,9 +113,21 @@ class WrittenBook extends WritableBookBase{
 	 * @return $this
 	 */
 	public function setTitle(string $title) : self{
-		$namedTag = $this->getNamedTag();
+		$this->title = $title;
-		$namedTag->setString(self::TAG_TITLE, $title);
-		$this->setNamedTag($namedTag);
 		return $this;
 	}
+
+	protected function deserializeCompoundTag(CompoundTag $tag) : void{
+		parent::deserializeCompoundTag($tag);
+		$this->generation = $tag->getInt(self::TAG_GENERATION, $this->generation);
+		$this->author = $tag->getString(self::TAG_AUTHOR, $this->author);
+		$this->title = $tag->getString(self::TAG_TITLE, $this->title);
+	}
+
+	protected function serializeCompoundTag(CompoundTag $tag) : void{
+		parent::serializeCompoundTag($tag);
+		$tag->setInt(self::TAG_GENERATION, $this->generation);
+		$tag->setString(self::TAG_AUTHOR, $this->author);
+		$tag->setString(self::TAG_TITLE, $this->title);
+	}
 }

src/pocketmine/world/World.php

@@ -51,8 +51,6 @@ use pocketmine\item\ItemFactory;
 use pocketmine\item\ItemUseResult;
 use pocketmine\math\AxisAlignedBB;
 use pocketmine\math\Vector3;
-use pocketmine\nbt\tag\ListTag;
-use pocketmine\nbt\tag\StringTag;
 use pocketmine\network\mcpe\protocol\BlockActorDataPacket;
 use pocketmine\network\mcpe\protocol\ClientboundPacket;
 use pocketmine\network\mcpe\protocol\LevelEventPacket;
@@ -1611,19 +1609,14 @@ class World implements ChunkManager{
 			}
 
 			if($player->isAdventure(true) and !$ev->isCancelled()){
-				$tag = $item->getNamedTag()->getListTag("CanDestroy");
 				$canBreak = false;
-				if($tag instanceof ListTag){
+				foreach($item->getCanDestroy() as $v){
-					foreach($tag as $v){
+					$entry = ItemFactory::fromString($v);
-						if($v instanceof StringTag){
-							$entry = ItemFactory::fromString($v->getValue());
 					if($entry->getBlock()->isSameType($target)){
 						$canBreak = true;
 						break;
 					}
 				}
-					}
-				}
 
 				$ev->setCancelled(!$canBreak);
 			}
@@ -1759,18 +1752,13 @@ class World implements ChunkManager{
 			$ev = new BlockPlaceEvent($player, $hand, $blockReplace, $blockClicked, $item);
 			if($player->isAdventure(true) and !$ev->isCancelled()){
 				$canPlace = false;
-				$tag = $item->getNamedTag()->getListTag("CanPlaceOn");
+				foreach($item->getCanPlaceOn() as $v){
-				if($tag instanceof ListTag){
+					$entry = ItemFactory::fromString($v);
-					foreach($tag as $v){
-						if($v instanceof StringTag){
-							$entry = ItemFactory::fromString($v->getValue());
 					if($entry->getBlock()->isSameType($blockClicked)){
 						$canPlace = true;
 						break;
 					}
 				}
-					}
-				}
 
 				$ev->setCancelled(!$canPlace);
 			}