Extract a Packet interface from DataPacket

this is in preparation for clientbound/serverbound packet separation. I did this already on another branch, but the changeset was dependent on a massive refactor to split apart packets and binarystream which i'm still not fully happy with.

src/pocketmine/Player.php

@@ -103,7 +103,6 @@ use pocketmine\network\mcpe\protocol\AvailableCommandsPacket;
 use pocketmine\network\mcpe\protocol\BlockEntityDataPacket;
 use pocketmine\network\mcpe\protocol\BookEditPacket;
 use pocketmine\network\mcpe\protocol\ChunkRadiusUpdatedPacket;
-use pocketmine\network\mcpe\protocol\DataPacket;
 use pocketmine\network\mcpe\protocol\EntityEventPacket;
 use pocketmine\network\mcpe\protocol\ItemFrameDropItemPacket;
 use pocketmine\network\mcpe\protocol\LevelEventPacket;
@@ -113,6 +112,7 @@ use pocketmine\network\mcpe\protocol\MobEffectPacket;
 use pocketmine\network\mcpe\protocol\ModalFormRequestPacket;
 use pocketmine\network\mcpe\protocol\MovePlayerPacket;
 use pocketmine\network\mcpe\protocol\NetworkChunkPublisherUpdatePacket;
+use pocketmine\network\mcpe\protocol\Packet;
 use pocketmine\network\mcpe\protocol\SetPlayerGameTypePacket;
 use pocketmine\network\mcpe\protocol\SetSpawnPositionPacket;
 use pocketmine\network\mcpe\protocol\SetTitlePacket;
@@ -2546,12 +2546,12 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
 	}
 
 	/**
-	 * @param DataPacket $packet
+	 * @param Packet $packet
-	 * @param bool       $immediate
+	 * @param bool   $immediate
 	 *
 	 * @return bool
 	 */
-	public function sendDataPacket(DataPacket $packet, bool $immediate = false) : bool{
+	public function sendDataPacket(Packet $packet, bool $immediate = false) : bool{
 		if(!$this->isConnected()){
 			return false;
 		}
@@ -2568,11 +2568,11 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{
 	 * @deprecated This is a proxy for sendDataPacket() and will be removed in the next major release.
 	 * @see Player::sendDataPacket()
 	 *
-	 * @param DataPacket $packet
+	 * @param Packet $packet
 	 *
 	 * @return bool
 	 */
-	public function dataPacket(DataPacket $packet) : bool{
+	public function dataPacket(Packet $packet) : bool{
 		return $this->sendDataPacket($packet, false);
 	}
 

src/pocketmine/Server.php

@@ -75,7 +75,7 @@ use pocketmine\network\mcpe\NetworkCipher;
 use pocketmine\network\mcpe\NetworkCompression;
 use pocketmine\network\mcpe\NetworkSession;
 use pocketmine\network\mcpe\PacketStream;
-use pocketmine\network\mcpe\protocol\DataPacket;
+use pocketmine\network\mcpe\protocol\Packet;
 use pocketmine\network\mcpe\protocol\PlayerListPacket;
 use pocketmine\network\mcpe\protocol\ProtocolInfo;
 use pocketmine\network\mcpe\protocol\types\PlayerListEntry;
@@ -1535,18 +1535,18 @@ class Server{
 	/**
 	 * Broadcasts a Minecraft packet to a list of players
 	 *
-	 * @param Player[]   $players
+	 * @param Player[] $players
-	 * @param DataPacket $packet
+	 * @param Packet   $packet
 	 *
 	 * @return bool
 	 */
-	public function broadcastPacket(array $players, DataPacket $packet) : bool{
+	public function broadcastPacket(array $players, Packet $packet) : bool{
 		return $this->broadcastPackets($players, [$packet]);
 	}
 
 	/**
 	 * @param Player[]     $players
-	 * @param DataPacket[] $packets
+	 * @param Packet[] $packets
 	 *
 	 * @return bool
 	 */

src/pocketmine/event/server/DataPacketBroadcastEvent.php

@@ -25,7 +25,7 @@ namespace pocketmine\event\server;
 
 use pocketmine\event\Cancellable;
 use pocketmine\event\CancellableTrait;
-use pocketmine\network\mcpe\protocol\DataPacket;
+use pocketmine\network\mcpe\protocol\Packet;
 use pocketmine\Player;
 
 /**
@@ -36,12 +36,12 @@ class DataPacketBroadcastEvent extends ServerEvent implements Cancellable{
 
 	/** @var Player[] */
 	private $players;
-	/** @var DataPacket[] */
+	/** @var Packet[] */
 	private $packets;
 
 	/**
-	 * @param Player[]     $players
+	 * @param Player[] $players
-	 * @param DataPacket[] $packets
+	 * @param Packet[] $packets
 	 */
 	public function __construct(array $players, array $packets){
 		$this->players = $players;
@@ -63,14 +63,14 @@ class DataPacketBroadcastEvent extends ServerEvent implements Cancellable{
 	}
 
 	/**
-	 * @return DataPacket[]
+	 * @return Packet[]
 	 */
 	public function getPackets() : array{
 		return $this->packets;
 	}
 
 	/**
-	 * @param DataPacket[] $packets
+	 * @param Packet[] $packets
 	 */
 	public function setPackets(array $packets) : void{
 		$this->packets = $packets;

src/pocketmine/event/server/DataPacketReceiveEvent.php

@@ -25,30 +25,30 @@ namespace pocketmine\event\server;
 
 use pocketmine\event\Cancellable;
 use pocketmine\event\CancellableTrait;
-use pocketmine\network\mcpe\protocol\DataPacket;
+use pocketmine\network\mcpe\protocol\Packet;
 use pocketmine\Player;
 
 class DataPacketReceiveEvent extends ServerEvent implements Cancellable{
 	use CancellableTrait;
 
-	/** @var DataPacket */
+	/** @var Packet */
 	private $packet;
 	/** @var Player */
 	private $player;
 
 	/**
-	 * @param Player     $player
+	 * @param Player $player
-	 * @param DataPacket $packet
+	 * @param Packet $packet
 	 */
-	public function __construct(Player $player, DataPacket $packet){
+	public function __construct(Player $player, Packet $packet){
 		$this->packet = $packet;
 		$this->player = $player;
 	}
 
 	/**
-	 * @return DataPacket
+	 * @return Packet
 	 */
-	public function getPacket() : DataPacket{
+	public function getPacket() : Packet{
 		return $this->packet;
 	}
 

src/pocketmine/event/server/DataPacketSendEvent.php

@@ -25,30 +25,30 @@ namespace pocketmine\event\server;
 
 use pocketmine\event\Cancellable;
 use pocketmine\event\CancellableTrait;
-use pocketmine\network\mcpe\protocol\DataPacket;
+use pocketmine\network\mcpe\protocol\Packet;
 use pocketmine\Player;
 
 class DataPacketSendEvent extends ServerEvent implements Cancellable{
 	use CancellableTrait;
 
-	/** @var DataPacket */
+	/** @var Packet */
 	private $packet;
 	/** @var Player */
 	private $player;
 
 	/**
-	 * @param Player     $player
+	 * @param Player $player
-	 * @param DataPacket $packet
+	 * @param Packet $packet
 	 */
-	public function __construct(Player $player, DataPacket $packet){
+	public function __construct(Player $player, Packet $packet){
 		$this->packet = $packet;
 		$this->player = $player;
 	}
 
 	/**
-	 * @return DataPacket
+	 * @return Packet
 	 */
-	public function getPacket() : DataPacket{
+	public function getPacket() : Packet{
 		return $this->packet;
 	}
 

src/pocketmine/level/Level.php

@@ -73,9 +73,9 @@ use pocketmine\nbt\tag\StringTag;
 use pocketmine\network\mcpe\ChunkRequestTask;
 use pocketmine\network\mcpe\CompressBatchPromise;
 use pocketmine\network\mcpe\protocol\AddEntityPacket;
-use pocketmine\network\mcpe\protocol\DataPacket;
 use pocketmine\network\mcpe\protocol\LevelEventPacket;
 use pocketmine\network\mcpe\protocol\LevelSoundEventPacket;
+use pocketmine\network\mcpe\protocol\Packet;
 use pocketmine\network\mcpe\protocol\SetDifficultyPacket;
 use pocketmine\network\mcpe\protocol\SetTimePacket;
 use pocketmine\network\mcpe\protocol\UpdateBlockPacket;
@@ -180,9 +180,9 @@ class Level implements ChunkManager, Metadatable{
 	/** @var Player[][] */
 	private $playerLoaders = [];
 
-	/** @var DataPacket[][] */
+	/** @var Packet[][] */
 	private $chunkPackets = [];
-	/** @var DataPacket[] */
+	/** @var Packet[] */
 	private $globalPackets = [];
 
 	/** @var float[] */
@@ -595,14 +595,14 @@ class Level implements ChunkManager, Metadatable{
 	}
 
 	/**
-	 * Queues a DataPacket to be sent to all players using the chunk at the specified X/Z coordinates at the end of the
+	 * Queues a packet to be sent to all players using the chunk at the specified X/Z coordinates at the end of the
 	 * current tick.
 	 *
-	 * @param int        $chunkX
+	 * @param int    $chunkX
-	 * @param int        $chunkZ
+	 * @param int    $chunkZ
-	 * @param DataPacket $packet
+	 * @param Packet $packet
 	 */
-	public function addChunkPacket(int $chunkX, int $chunkZ, DataPacket $packet){
+	public function addChunkPacket(int $chunkX, int $chunkZ, Packet $packet){
 		if(!isset($this->chunkPackets[$index = Level::chunkHash($chunkX, $chunkZ)])){
 			$this->chunkPackets[$index] = [$packet];
 		}else{
@@ -613,19 +613,19 @@ class Level implements ChunkManager, Metadatable{
 	/**
 	 * Broadcasts a packet to every player who has the target position within their view distance.
 	 *
-	 * @param Vector3    $pos
+	 * @param Vector3 $pos
-	 * @param DataPacket $packet
+	 * @param Packet  $packet
 	 */
-	public function broadcastPacketToViewers(Vector3 $pos, DataPacket $packet) : void{
+	public function broadcastPacketToViewers(Vector3 $pos, Packet $packet) : void{
 		$this->addChunkPacket($pos->getFloorX() >> 4, $pos->getFloorZ() >> 4, $packet);
 	}
 
 	/**
 	 * Broadcasts a packet to every player in the level.
 	 *
-	 * @param DataPacket $packet
+	 * @param Packet $packet
 	 */
-	public function broadcastGlobalPacket(DataPacket $packet) : void{
+	public function broadcastGlobalPacket(Packet $packet) : void{
 		$this->globalPackets[] = $packet;
 	}
 

src/pocketmine/level/particle/Particle.php

@@ -24,7 +24,7 @@ declare(strict_types=1);
 namespace pocketmine\level\particle;
 
 use pocketmine\math\Vector3;
-use pocketmine\network\mcpe\protocol\DataPacket;
+use pocketmine\network\mcpe\protocol\Packet;
 
 abstract class Particle{
 
@@ -86,7 +86,7 @@ abstract class Particle{
 	/**
 	 * @param Vector3 $pos
 	 *
-	 * @return DataPacket|DataPacket[]
+	 * @return Packet|Packet[]
 	 */
 	abstract public function encode(Vector3 $pos);
 

src/pocketmine/level/sound/Sound.php

@@ -24,14 +24,14 @@ declare(strict_types=1);
 namespace pocketmine\level\sound;
 
 use pocketmine\math\Vector3;
-use pocketmine\network\mcpe\protocol\DataPacket;
+use pocketmine\network\mcpe\protocol\Packet;
 
 abstract class Sound{
 
 	/**
 	 * @param Vector3 $pos
 	 *
-	 * @return DataPacket|DataPacket[]
+	 * @return Packet|Packet[]
 	 */
 	abstract public function encode(Vector3 $pos);
 

src/pocketmine/network/NetworkInterface.php

@@ -39,7 +39,7 @@ interface NetworkInterface{
 	public function start() : void;
 
 	/**
-	 * Sends a DataPacket to the interface, returns an unique identifier for the packet if $needACK is true
+	 * Sends a packet to the interface, returns an unique identifier for the packet if $needACK is true
 	 *
 	 * @param NetworkSession $session
 	 * @param string         $payload

src/pocketmine/network/mcpe/NetworkSession.php

@@ -34,8 +34,8 @@ use pocketmine\network\mcpe\handler\PreSpawnSessionHandler;
 use pocketmine\network\mcpe\handler\ResourcePacksSessionHandler;
 use pocketmine\network\mcpe\handler\SessionHandler;
 use pocketmine\network\mcpe\handler\SimpleSessionHandler;
-use pocketmine\network\mcpe\protocol\DataPacket;
 use pocketmine\network\mcpe\protocol\DisconnectPacket;
+use pocketmine\network\mcpe\protocol\Packet;
 use pocketmine\network\mcpe\protocol\PacketPool;
 use pocketmine\network\mcpe\protocol\PlayStatusPacket;
 use pocketmine\network\mcpe\protocol\ServerToClientHandshakePacket;
@@ -209,11 +209,11 @@ class NetworkSession{
 	}
 
 	/**
-	 * @param DataPacket $packet
+	 * @param Packet $packet
 	 *
 	 * @throws BadPacketException
 	 */
-	public function handleDataPacket(DataPacket $packet) : void{
+	public function handleDataPacket(Packet $packet) : void{
 		$timings = Timings::getReceiveDataPacketTimings($packet);
 		$timings->startTiming();
 
@@ -237,7 +237,7 @@ class NetworkSession{
 		$timings->stopTiming();
 	}
 
-	public function sendDataPacket(DataPacket $packet, bool $immediate = false) : bool{
+	public function sendDataPacket(Packet $packet, bool $immediate = false) : bool{
 		$timings = Timings::getSendDataPacketTimings($packet);
 		$timings->startTiming();
 		try{
@@ -260,9 +260,9 @@ class NetworkSession{
 
 	/**
 	 * @internal
-	 * @param DataPacket $packet
+	 * @param Packet $packet
 	 */
-	public function addToSendBuffer(DataPacket $packet) : void{
+	public function addToSendBuffer(Packet $packet) : void{
 		$timings = Timings::getSendDataPacketTimings($packet);
 		$timings->startTiming();
 		try{

src/pocketmine/network/mcpe/PacketStream.php

@@ -23,19 +23,19 @@ declare(strict_types=1);
 
 namespace pocketmine\network\mcpe;
 
-use pocketmine\network\mcpe\protocol\DataPacket;
+use pocketmine\network\mcpe\protocol\Packet;
 use pocketmine\network\mcpe\protocol\PacketPool;
 
 class PacketStream extends NetworkBinaryStream{
 
-	public function putPacket(DataPacket $packet) : void{
+	public function putPacket(Packet $packet) : void{
-		if(!$packet->isEncoded){
+		if(!$packet->isEncoded()){
 			$packet->encode();
 		}
 		$this->putString($packet->getBuffer());
 	}
 
-	public function getPacket() : DataPacket{
+	public function getPacket() : Packet{
 		return PacketPool::getPacket($this->getString());
 	}
 }

src/pocketmine/network/mcpe/protocol/DataPacket.php

@@ -36,12 +36,12 @@ use function is_object;
 use function is_string;
 use function method_exists;
 
-abstract class DataPacket extends NetworkBinaryStream{
+abstract class DataPacket extends NetworkBinaryStream implements Packet{
 
 	public const NETWORK_ID = 0;
 
 	/** @var bool */
-	public $isEncoded = false;
+	private $isEncoded = false;
 
 	/** @var int */
 	public $senderSubId = 0;
@@ -108,6 +108,10 @@ abstract class DataPacket extends NetworkBinaryStream{
 		$this->isEncoded = true;
 	}
 
+	final public function isEncoded() : bool{
+		return $this->isEncoded;
+	}
+
 	protected function encodeHeader() : void{
 		$this->putUnsignedVarInt(static::NETWORK_ID);
 	}

src/pocketmine/network/mcpe/protocol/Packet.php

@@ -0,0 +1,82 @@
+<?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\network\mcpe\protocol;
+
+use pocketmine\network\BadPacketException;
+use pocketmine\network\mcpe\handler\SessionHandler;
+
+interface Packet{
+
+	public function setOffset(int $offset) : void;
+
+	public function setBuffer(string $buffer = "", int $offset = 0);
+
+	public function getOffset() : int;
+
+	public function getBuffer() : string;
+
+	/**
+	 * Returns whether the offset has reached the end of the buffer.
+	 * @return bool
+	 */
+	public function feof() : bool;
+
+	public function pid() : int;
+
+	public function getName() : string;
+
+	public function canBeSentBeforeLogin() : bool;
+
+	/**
+	 * Returns whether the packet may legally have unread bytes left in the buffer.
+	 * @return bool
+	 */
+	public function mayHaveUnreadBytes() : bool;
+
+	/**
+	 * @throws BadPacketException
+	 */
+	public function decode() : void;
+
+	public function encode() : void;
+
+	public function isEncoded() : bool;
+
+	/**
+	 * Performs handling for this packet. Usually you'll want an appropriately named method in the session handler for
+	 * this.
+	 *
+	 * This method returns a bool to indicate whether the packet was handled or not. If the packet was unhandled, a
+	 * debug message will be logged with a hexdump of the packet.
+	 *
+	 * Typically this method returns the return value of the handler in the supplied SessionHandler. See other packets
+	 * for examples how to implement this.
+	 *
+	 * @param SessionHandler $handler
+	 *
+	 * @return bool true if the packet was handled successfully, false if not.
+	 * @throws BadPacketException if broken data was found in the packet
+	 */
+	public function handle(SessionHandler $handler) : bool;
+}

src/pocketmine/network/mcpe/protocol/PacketPool.php

@@ -28,7 +28,7 @@ use pocketmine\utils\Binary;
 use pocketmine\utils\BinaryDataException;
 
 class PacketPool{
-	/** @var \SplFixedArray<DataPacket> */
+	/** @var \SplFixedArray<Packet> */
 	protected static $pool = null;
 
 	public static function init() : void{
@@ -158,28 +158,28 @@ class PacketPool{
 	}
 
 	/**
-	 * @param DataPacket $packet
+	 * @param Packet $packet
 	 */
-	public static function registerPacket(DataPacket $packet) : void{
+	public static function registerPacket(Packet $packet) : void{
 		static::$pool[$packet->pid()] = clone $packet;
 	}
 
 	/**
 	 * @param int $pid
 	 *
-	 * @return DataPacket
+	 * @return Packet
 	 */
-	public static function getPacketById(int $pid) : DataPacket{
+	public static function getPacketById(int $pid) : Packet{
 		return isset(static::$pool[$pid]) ? clone static::$pool[$pid] : new UnknownPacket();
 	}
 
 	/**
 	 * @param string $buffer
 	 *
-	 * @return DataPacket
+	 * @return Packet
 	 * @throws BadPacketException
 	 */
-	public static function getPacket(string $buffer) : DataPacket{
+	public static function getPacket(string $buffer) : Packet{
 		$offset = 0;
 		try{
 			$pk = static::getPacketById(Binary::readUnsignedVarInt($buffer, $offset));

src/pocketmine/timings/Timings.php

@@ -24,7 +24,7 @@ declare(strict_types=1);
 namespace pocketmine\timings;
 
 use pocketmine\entity\Entity;
-use pocketmine\network\mcpe\protocol\DataPacket;
+use pocketmine\network\mcpe\protocol\Packet;
 use pocketmine\Player;
 use pocketmine\scheduler\TaskHandler;
 use pocketmine\tile\Tile;
@@ -212,31 +212,33 @@ abstract class Timings{
 	}
 
 	/**
-	 * @param DataPacket $pk
+	 * @param Packet $pk
 	 *
 	 * @return TimingsHandler
 	 */
-	public static function getReceiveDataPacketTimings(DataPacket $pk) : TimingsHandler{
+	public static function getReceiveDataPacketTimings(Packet $pk) : TimingsHandler{
-		if(!isset(self::$packetReceiveTimingMap[$pk::NETWORK_ID])){
+		$pid = $pk->pid();
+		if(!isset(self::$packetReceiveTimingMap[$pid])){
 			$pkName = (new \ReflectionClass($pk))->getShortName();
-			self::$packetReceiveTimingMap[$pk::NETWORK_ID] = new TimingsHandler("** receivePacket - " . $pkName . " [0x" . dechex($pk::NETWORK_ID) . "]", self::$playerNetworkReceiveTimer);
+			self::$packetReceiveTimingMap[$pid] = new TimingsHandler("** receivePacket - " . $pkName . " [0x" . dechex($pid) . "]", self::$playerNetworkReceiveTimer);
 		}
 
-		return self::$packetReceiveTimingMap[$pk::NETWORK_ID];
+		return self::$packetReceiveTimingMap[$pid];
 	}
 
 
 	/**
-	 * @param DataPacket $pk
+	 * @param Packet $pk
 	 *
 	 * @return TimingsHandler
 	 */
-	public static function getSendDataPacketTimings(DataPacket $pk) : TimingsHandler{
+	public static function getSendDataPacketTimings(Packet $pk) : TimingsHandler{
-		if(!isset(self::$packetSendTimingMap[$pk::NETWORK_ID])){
+		$pid = $pk->pid();
+		if(!isset(self::$packetSendTimingMap[$pid])){
 			$pkName = (new \ReflectionClass($pk))->getShortName();
-			self::$packetSendTimingMap[$pk::NETWORK_ID] = new TimingsHandler("** sendPacket - " . $pkName . " [0x" . dechex($pk::NETWORK_ID) . "]", self::$playerNetworkSendTimer);
+			self::$packetSendTimingMap[$pid] = new TimingsHandler("** sendPacket - " . $pkName . " [0x" . dechex($pid) . "]", self::$playerNetworkSendTimer);
 		}
 
-		return self::$packetSendTimingMap[$pk::NETWORK_ID];
+		return self::$packetSendTimingMap[$pid];
 	}
 }