DataPacket no longer keeps its own serializer

since a while ago, we're anyway just discarding the internal buffer anyway when the packet is repeatedly encoded, so this doesn't serve any advantage anymore.
We do need a system to be able to reuse encoded packet buffers, but right now we're not reusing them anyway.
This commit is contained in:
Dylan K. Taylor 2021-04-09 15:37:58 +01:00
parent 19f536d68a
commit 0312b62c8a
No known key found for this signature in database
GPG Key ID: 8927471A91CAFD3D
7 changed files with 29 additions and 50 deletions

View File

@ -73,6 +73,7 @@ use pocketmine\network\mcpe\protocol\PlayerListPacket;
use pocketmine\network\mcpe\protocol\PlayStatusPacket; use pocketmine\network\mcpe\protocol\PlayStatusPacket;
use pocketmine\network\mcpe\protocol\RemoveActorPacket; use pocketmine\network\mcpe\protocol\RemoveActorPacket;
use pocketmine\network\mcpe\protocol\serializer\PacketBatch; use pocketmine\network\mcpe\protocol\serializer\PacketBatch;
use pocketmine\network\mcpe\protocol\serializer\PacketSerializer;
use pocketmine\network\mcpe\protocol\ServerboundPacket; use pocketmine\network\mcpe\protocol\ServerboundPacket;
use pocketmine\network\mcpe\protocol\ServerToClientHandshakePacket; use pocketmine\network\mcpe\protocol\ServerToClientHandshakePacket;
use pocketmine\network\mcpe\protocol\SetActorDataPacket; use pocketmine\network\mcpe\protocol\SetActorDataPacket;
@ -355,11 +356,11 @@ class NetworkSession{
} }
try{ try{
foreach($stream->getPackets($this->packetPool, 500) as $packet){ foreach($stream->getPackets($this->packetPool, 500) as [$packet, $buffer]){
try{ try{
$this->handleDataPacket($packet); $this->handleDataPacket($packet, $buffer);
}catch(PacketHandlingException $e){ }catch(PacketHandlingException $e){
$this->logger->debug($packet->getName() . ": " . base64_encode($packet->getSerializer()->getBuffer())); $this->logger->debug($packet->getName() . ": " . base64_encode($buffer));
throw PacketHandlingException::wrap($e, "Error processing " . $packet->getName()); throw PacketHandlingException::wrap($e, "Error processing " . $packet->getName());
} }
} }
@ -372,10 +373,10 @@ class NetworkSession{
/** /**
* @throws PacketHandlingException * @throws PacketHandlingException
*/ */
public function handleDataPacket(Packet $packet) : void{ public function handleDataPacket(Packet $packet, string $buffer) : void{
if(!($packet instanceof ServerboundPacket)){ if(!($packet instanceof ServerboundPacket)){
if($packet instanceof GarbageServerboundPacket){ if($packet instanceof GarbageServerboundPacket){
$this->logger->debug("Garbage serverbound " . $packet->getName() . ": " . base64_encode($packet->getSerializer()->getBuffer())); $this->logger->debug("Garbage serverbound " . $packet->getName() . ": " . base64_encode($buffer));
return; return;
} }
throw new PacketHandlingException("Unexpected non-serverbound packet"); throw new PacketHandlingException("Unexpected non-serverbound packet");
@ -385,12 +386,12 @@ class NetworkSession{
$timings->startTiming(); $timings->startTiming();
try{ try{
$stream = new PacketSerializer($buffer);
try{ try{
$packet->decode(); $packet->decode($stream);
}catch(PacketDecodeException $e){ }catch(PacketDecodeException $e){
throw PacketHandlingException::wrap($e); throw PacketHandlingException::wrap($e);
} }
$stream = $packet->getSerializer();
if(!$stream->feof()){ if(!$stream->feof()){
$remains = substr($stream->getBuffer(), $stream->getOffset()); $remains = substr($stream->getBuffer(), $stream->getOffset());
$this->logger->debug("Still " . strlen($remains) . " bytes unread in " . $packet->getName() . ": " . bin2hex($remains)); $this->logger->debug("Still " . strlen($remains) . " bytes unread in " . $packet->getName() . ": " . bin2hex($remains));

View File

@ -44,21 +44,6 @@ abstract class DataPacket implements Packet{
/** @var int */ /** @var int */
public $recipientSubId = 0; public $recipientSubId = 0;
/** @var PacketSerializer */
private $buf;
public function __construct(){
$this->buf = new PacketSerializer();
}
public function getSerializer() : PacketSerializer{
return $this->buf;
}
public function setSerializer(PacketSerializer $serializer) : void{
$this->buf = $serializer;
}
public function pid() : int{ public function pid() : int{
return $this::NETWORK_ID; return $this::NETWORK_ID;
} }
@ -74,11 +59,10 @@ abstract class DataPacket implements Packet{
/** /**
* @throws PacketDecodeException * @throws PacketDecodeException
*/ */
final public function decode() : void{ final public function decode(PacketSerializer $in) : void{
$this->buf->rewind();
try{ try{
$this->decodeHeader($this->buf); $this->decodeHeader($in);
$this->decodePayload($this->buf); $this->decodePayload($in);
}catch(BinaryDataException | PacketDecodeException $e){ }catch(BinaryDataException | PacketDecodeException $e){
throw PacketDecodeException::wrap($e, $this->getName()); throw PacketDecodeException::wrap($e, $this->getName());
} }
@ -108,10 +92,9 @@ abstract class DataPacket implements Packet{
*/ */
abstract protected function decodePayload(PacketSerializer $in) : void; abstract protected function decodePayload(PacketSerializer $in) : void;
final public function encode() : void{ final public function encode(PacketSerializer $out) : void{
$this->buf = new PacketSerializer(); $this->encodeHeader($out);
$this->encodeHeader($this->buf); $this->encodePayload($out);
$this->encodePayload($this->buf);
} }
protected function encodeHeader(PacketSerializer $out) : void{ protected function encodeHeader(PacketSerializer $out) : void{

View File

@ -27,10 +27,6 @@ use pocketmine\network\mcpe\protocol\serializer\PacketSerializer;
interface Packet{ interface Packet{
public function getSerializer() : PacketSerializer;
public function setSerializer(PacketSerializer $serializer) : void;
public function pid() : int; public function pid() : int;
public function getName() : string; public function getName() : string;
@ -40,9 +36,9 @@ interface Packet{
/** /**
* @throws PacketDecodeException * @throws PacketDecodeException
*/ */
public function decode() : void; public function decode(PacketSerializer $in) : void;
public function encode() : void; public function encode(PacketSerializer $out) : void;
/** /**
* Performs handling for this packet. Usually you'll want an appropriately named method in the session handler for * Performs handling for this packet. Usually you'll want an appropriately named method in the session handler for

View File

@ -23,7 +23,7 @@ declare(strict_types=1);
namespace pocketmine\network\mcpe\protocol; namespace pocketmine\network\mcpe\protocol;
use pocketmine\network\mcpe\protocol\serializer\PacketSerializer; use pocketmine\utils\Binary;
use pocketmine\utils\BinaryDataException; use pocketmine\utils\BinaryDataException;
class PacketPool{ class PacketPool{
@ -216,10 +216,7 @@ class PacketPool{
* @throws BinaryDataException * @throws BinaryDataException
*/ */
public function getPacket(string $buffer) : Packet{ public function getPacket(string $buffer) : Packet{
$serializer = new PacketSerializer($buffer); $offset = 0;
$pk = $this->getPacketById($serializer->getUnsignedVarInt() & DataPacket::PID_MASK); return $this->getPacketById(Binary::readUnsignedVarInt($buffer, $offset) & DataPacket::PID_MASK);
$pk->setSerializer($serializer);
return $pk;
} }
} }

View File

@ -39,14 +39,15 @@ class PacketBatch{
/** /**
* @return \Generator|Packet[] * @return \Generator|Packet[]
* @phpstan-return \Generator<int, Packet, void, void> * @phpstan-return \Generator<int, array{Packet, string}, void, void>
* @throws PacketDecodeException * @throws PacketDecodeException
*/ */
public function getPackets(PacketPool $packetPool, int $max) : \Generator{ public function getPackets(PacketPool $packetPool, int $max) : \Generator{
$serializer = new PacketSerializer($this->buffer); $serializer = new PacketSerializer($this->buffer);
for($c = 0; $c < $max and !$serializer->feof(); ++$c){ for($c = 0; $c < $max and !$serializer->feof(); ++$c){
try{ try{
yield $c => $packetPool->getPacket($serializer->getString()); $buffer = $serializer->getString();
yield $c => [$packetPool->getPacket($buffer), $buffer];
}catch(BinaryDataException $e){ }catch(BinaryDataException $e){
throw new PacketDecodeException("Error decoding packet $c of batch: " . $e->getMessage(), 0, $e); throw new PacketDecodeException("Error decoding packet $c of batch: " . $e->getMessage(), 0, $e);
} }
@ -66,8 +67,9 @@ class PacketBatch{
public static function fromPackets(Packet ...$packets) : self{ public static function fromPackets(Packet ...$packets) : self{
$serializer = new PacketSerializer(); $serializer = new PacketSerializer();
foreach($packets as $packet){ foreach($packets as $packet){
$packet->encode(); $subSerializer = new PacketSerializer();
$serializer->putString($packet->getSerializer()->getBuffer()); $packet->encode($subSerializer);
$serializer->putString($subSerializer->getBuffer());
} }
return new self($serializer->getBuffer()); return new self($serializer->getBuffer());
} }

View File

@ -32,11 +32,11 @@ class DataPacketTest extends TestCase{
$pk = new TestPacket(); $pk = new TestPacket();
$pk->senderSubId = 3; $pk->senderSubId = 3;
$pk->recipientSubId = 2; $pk->recipientSubId = 2;
$pk->encode(); $serializer = new PacketSerializer();
$pk->encode($serializer);
$pk2 = new TestPacket(); $pk2 = new TestPacket();
$pk2->setSerializer(new PacketSerializer($pk->getSerializer()->getBuffer())); $pk2->decode(new PacketSerializer($serializer->getBuffer()));
$pk2->decode();
self::assertSame($pk2->senderSubId, 3); self::assertSame($pk2->senderSubId, 3);
self::assertSame($pk2->recipientSubId, 2); self::assertSame($pk2->recipientSubId, 2);
} }

View File

@ -43,6 +43,6 @@ class LoginPacketTest extends TestCase{
$pk = PacketPool::getInstance()->getPacket($stream->getBuffer()); $pk = PacketPool::getInstance()->getPacket($stream->getBuffer());
$this->expectException(PacketDecodeException::class); $this->expectException(PacketDecodeException::class);
$pk->decode(); //bang $pk->decode(new PacketSerializer($stream->getBuffer())); //bang
} }
} }