From a73c54bdd02debf94ecb5f1297d2d12040452252 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 4 May 2020 12:35:13 +0100 Subject: [PATCH] making tile spawn compound cache use CacheableNbt instead of strings --- src/block/tile/Spawnable.php | 17 ++++++------- .../mcpe/handler/InGamePacketHandler.php | 14 ++++------- .../mcpe/protocol/BlockActorDataPacket.php | 25 ++++++++++++++++--- .../mcpe/serializer/ChunkSerializer.php | 2 +- .../phpstan/configs/runtime-type-checks.neon | 5 ++++ 5 files changed, 40 insertions(+), 23 deletions(-) diff --git a/src/block/tile/Spawnable.php b/src/block/tile/Spawnable.php index 3204c8cf5..a4851b04f 100644 --- a/src/block/tile/Spawnable.php +++ b/src/block/tile/Spawnable.php @@ -24,12 +24,15 @@ declare(strict_types=1); namespace pocketmine\block\tile; use pocketmine\nbt\tag\CompoundTag; -use pocketmine\nbt\TreeRoot; use pocketmine\network\mcpe\protocol\serializer\NetworkNbtSerializer; +use pocketmine\network\mcpe\protocol\types\CacheableNbt; use function get_class; abstract class Spawnable extends Tile{ - /** @var string|null */ + /** + * @var CacheableNbt|null + * @phpstan-var CacheableNbt<\pocketmine\nbt\tag\CompoundTag>|null + */ private $spawnCompoundCache = null; /** @var bool */ private $dirty = true; //default dirty, until it's been spawned appropriately on the world @@ -55,15 +58,11 @@ abstract class Spawnable extends Tile{ * Returns encoded NBT (varint, little-endian) used to spawn this tile to clients. Uses cache where possible, * populates cache if it is null. * - * @return string encoded NBT + * @phpstan-return CacheableNbt<\pocketmine\nbt\tag\CompoundTag> */ - final public function getSerializedSpawnCompound() : string{ + final public function getSerializedSpawnCompound() : CacheableNbt{ if($this->spawnCompoundCache === null){ - if(self::$nbtWriter === null){ - self::$nbtWriter = new NetworkNbtSerializer(); - } - - $this->spawnCompoundCache = self::$nbtWriter->write(new TreeRoot($this->getSpawnCompound())); + $this->spawnCompoundCache = new CacheableNbt($this->getSpawnCompound()); } return $this->spawnCompoundCache; diff --git a/src/network/mcpe/handler/InGamePacketHandler.php b/src/network/mcpe/handler/InGamePacketHandler.php index 96355daf7..665a08069 100644 --- a/src/network/mcpe/handler/InGamePacketHandler.php +++ b/src/network/mcpe/handler/InGamePacketHandler.php @@ -36,7 +36,7 @@ use pocketmine\item\VanillaItems; use pocketmine\item\WritableBook; use pocketmine\item\WrittenBook; use pocketmine\math\Vector3; -use pocketmine\nbt\NbtDataException; +use pocketmine\nbt\tag\CompoundTag; use pocketmine\nbt\tag\StringTag; use pocketmine\network\BadPacketException; use pocketmine\network\mcpe\convert\SkinAdapterSingleton; @@ -72,7 +72,6 @@ use pocketmine\network\mcpe\protocol\PlayerHotbarPacket; use pocketmine\network\mcpe\protocol\PlayerInputPacket; use pocketmine\network\mcpe\protocol\PlayerSkinPacket; use pocketmine\network\mcpe\protocol\RequestChunkRadiusPacket; -use pocketmine\network\mcpe\protocol\serializer\NetworkNbtSerializer; use pocketmine\network\mcpe\protocol\ServerSettingsRequestPacket; use pocketmine\network\mcpe\protocol\SetPlayerGameTypePacket; use pocketmine\network\mcpe\protocol\ShowCreditsPacket; @@ -87,6 +86,7 @@ use pocketmine\network\mcpe\protocol\types\inventory\ReleaseItemTransactionData; use pocketmine\network\mcpe\protocol\types\inventory\UseItemOnEntityTransactionData; use pocketmine\network\mcpe\protocol\types\inventory\UseItemTransactionData; use pocketmine\player\Player; +use pocketmine\utils\AssumptionFailedError; use function array_push; use function base64_encode; use function count; @@ -554,12 +554,8 @@ class InGamePacketHandler extends PacketHandler{ } $block = $this->player->getLocation()->getWorldNonNull()->getBlock($pos); - try{ - $offset = 0; - $nbt = (new NetworkNbtSerializer())->read($packet->namedtag, $offset, 512)->mustGetCompoundTag(); - }catch(NbtDataException $e){ - throw BadPacketException::wrap($e); - } + $nbt = $packet->namedtag->getRoot(); + if(!($nbt instanceof CompoundTag)) throw new AssumptionFailedError("PHPStan should ensure this is a CompoundTag"); //for phpstorm's benefit if($block instanceof Sign){ if($nbt->hasTag("Text", StringTag::class)){ @@ -580,7 +576,7 @@ class InGamePacketHandler extends PacketHandler{ return true; } - $this->session->getLogger()->debug("Invalid sign update data: " . base64_encode($packet->namedtag)); + $this->session->getLogger()->debug("Invalid sign update data: " . base64_encode($packet->namedtag->getEncodedNbt())); } return false; diff --git a/src/network/mcpe/protocol/BlockActorDataPacket.php b/src/network/mcpe/protocol/BlockActorDataPacket.php index 8c9bfaf69..54ac3fc8b 100644 --- a/src/network/mcpe/protocol/BlockActorDataPacket.php +++ b/src/network/mcpe/protocol/BlockActorDataPacket.php @@ -25,7 +25,10 @@ namespace pocketmine\network\mcpe\protocol; #include +use pocketmine\nbt\NbtDataException; use pocketmine\network\mcpe\protocol\serializer\NetworkBinaryStream; +use pocketmine\network\mcpe\protocol\serializer\NetworkNbtSerializer; +use pocketmine\network\mcpe\protocol\types\CacheableNbt; class BlockActorDataPacket extends DataPacket implements ClientboundPacket, ServerboundPacket{ public const NETWORK_ID = ProtocolInfo::BLOCK_ACTOR_DATA_PACKET; @@ -36,10 +39,16 @@ class BlockActorDataPacket extends DataPacket implements ClientboundPacket, Serv public $y; /** @var int */ public $z; - /** @var string */ + /** + * @var CacheableNbt + * @phpstan-var CacheableNbt<\pocketmine\nbt\tag\CompoundTag> + */ public $namedtag; - public static function create(int $x, int $y, int $z, string $nbt) : self{ + /** + * @phpstan-param CacheableNbt<\pocketmine\nbt\tag\CompoundTag> $nbt + */ + public static function create(int $x, int $y, int $z, CacheableNbt $nbt) : self{ $result = new self; [$result->x, $result->y, $result->z] = [$x, $y, $z]; $result->namedtag = $nbt; @@ -48,12 +57,20 @@ class BlockActorDataPacket extends DataPacket implements ClientboundPacket, Serv protected function decodePayload(NetworkBinaryStream $in) : void{ $in->getBlockPosition($this->x, $this->y, $this->z); - $this->namedtag = $in->getRemaining(); + try{ + $offset = $in->getOffset(); + $this->namedtag = new CacheableNbt( + (new NetworkNbtSerializer())->read($this->getBinaryStream()->getBuffer(), $offset, 512)->mustGetCompoundTag() + ); + $in->setOffset($offset); + }catch(NbtDataException $e){ + throw PacketDecodeException::wrap($e, "Failed decoding block actor NBT"); + } } protected function encodePayload(NetworkBinaryStream $out) : void{ $out->putBlockPosition($this->x, $this->y, $this->z); - $out->put($this->namedtag); + $out->put($this->namedtag->getEncodedNbt()); } public function handle(PacketHandlerInterface $handler) : bool{ diff --git a/src/network/mcpe/serializer/ChunkSerializer.php b/src/network/mcpe/serializer/ChunkSerializer.php index 9b7a4a9bc..94ce8e89c 100644 --- a/src/network/mcpe/serializer/ChunkSerializer.php +++ b/src/network/mcpe/serializer/ChunkSerializer.php @@ -90,7 +90,7 @@ final class ChunkSerializer{ $stream = new BinaryStream(); foreach($chunk->getTiles() as $tile){ if($tile instanceof Spawnable){ - $stream->put($tile->getSerializedSpawnCompound()); + $stream->put($tile->getSerializedSpawnCompound()->getEncodedNbt()); } } diff --git a/tests/phpstan/configs/runtime-type-checks.neon b/tests/phpstan/configs/runtime-type-checks.neon index b0579c90d..de922a304 100644 --- a/tests/phpstan/configs/runtime-type-checks.neon +++ b/tests/phpstan/configs/runtime-type-checks.neon @@ -30,6 +30,11 @@ parameters: count: 3 path: ../../../src/item/Item.php + - + message: "#^Instanceof between pocketmine\\\\nbt\\\\tag\\\\CompoundTag and pocketmine\\\\nbt\\\\tag\\\\CompoundTag will always evaluate to true\\.$#" + count: 1 + path: ../../../src/network/mcpe/handler/InGamePacketHandler.php + - message: "#^Call to function is_array\\(\\) with array\\ will always evaluate to true\\.$#" count: 1