The plugin has been updated and now supports Pocketmine API 5 #8
The plugin has been updated and now supports Pocketmine API 5 #8MEMOxiiii wants to merge 24 commits into
Conversation
Add interceptor for legacy auth format in Login packets.
- Cast mixed args to string in TransferCommand and ServerCommand - Add unpack() false checks in PacketSerializer - Cast config values to proper types in Portal.php - Fix json_decode/json_encode type safety in login interceptor - Remove incorrect @var UUID docblock
Commando is vendored as source files, not a phar
…scope - Upgrade paroxity/pmmp-phpstan-action from 4.2.0 (PM4) to 5.12.0 (PM5) - Only analyze paroxity/portal code, not vendored Commando/SimplePacketHandler
- Add SimplePacketHandler to scanDirectories - Ignore LoginPacket::\ (exists in newer BedrockProtocol, not in action's phar) - Ignore always-false negation from socket_create return type - Remove unmatched iterable type ignore pattern
Clarified proxy connection instructions in README.
There was a problem hiding this comment.
Pull request overview
This PR updates PortalPM to PocketMine-MP API 5 (v2.0.0), including refactors to the socket thread + command layer, and vendor-includes dependencies (Commando + SimplePacketHandler) to support packet interception and command registration in PM5.
Changes:
- Ported plugin metadata and runtime integrations to PMMP API 5 (TickSleeper notifier changes, threading primitives, command refactors).
- Added packet interception to adapt legacy proxy login auth format to PM5’s expected format.
- Vendored Commando + SimplePacketHandler sources and introduced a compatibility
PacketSerializer.
Reviewed changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pocketmine/network/mcpe/protocol/serializer/PacketSerializer.php | Adds a compatibility serializer implementation used by the plugin’s TCP protocol layer |
| src/paroxity/portal/thread/SocketThread.php | Updates thread IPC and sleeper notification integration for PM5 |
| src/paroxity/portal/Portal.php | Updates enable/disable lifecycle, TickSleeper notifier usage, and adds LoginPacket interception |
| src/paroxity/portal/command/TransferCommand.php | Refactors command args + self-transfer support; injects Portal via constructor |
| src/paroxity/portal/command/ServersCommand.php | Refactors to constructor property injection (Portal $portal) |
| src/paroxity/portal/command/ServerCommand.php | Refactors args handling and Portal injection; removes TargetPlayerArgument usage |
| src/muqsit/simplepackethandler/utils/Utils.php | Adds closure signature parsing + packet PID resolution utilities |
| src/muqsit/simplepackethandler/SimplePacketHandler.php | Adds factory for interceptor/monitor creation tied to PacketPool |
| src/muqsit/simplepackethandler/monitor/PacketMonitorListener.php | Adds MONITOR-level packet monitoring via PMMP events |
| src/muqsit/simplepackethandler/monitor/PacketMonitor.php | Wraps monitor listener implementation |
| src/muqsit/simplepackethandler/monitor/IPacketMonitor.php | Defines monitor interface for incoming/outgoing hooks |
| src/muqsit/simplepackethandler/interceptor/PacketInterceptorListener.php | Adds packet interception with cancellable handlers and selective rebroadcast |
| src/muqsit/simplepackethandler/interceptor/PacketInterceptor.php | Wraps interceptor listener implementation |
| src/muqsit/simplepackethandler/interceptor/IPacketInterceptor.php | Defines interceptor interface for incoming/outgoing interception |
| src/CortexPE/Commando/traits/IArgumentable.php | Adds Commando argumentable interface (vendored) |
| src/CortexPE/Commando/traits/ArgumentableTrait.php | Adds argument registration/parsing trait (vendored) |
| src/CortexPE/Commando/store/SoftEnumStore.php | Adds soft-enum registry + broadcast helper (vendored) |
| src/CortexPE/Commando/PacketHooker.php | Updates command packet hooking using SimplePacketHandler (vendored) |
| src/CortexPE/Commando/IRunnable.php | Adds IRunnable interface used by constraints (vendored) |
| src/CortexPE/Commando/exception/InvalidErrorCode.php | Adds Commando exception type (vendored) |
| src/CortexPE/Commando/exception/HookAlreadyRegistered.php | Adds Commando exception type (vendored) |
| src/CortexPE/Commando/exception/CommandoException.php | Adds base Commando exception (vendored) |
| src/CortexPE/Commando/exception/ArgumentOrderException.php | Adds Commando exception type (vendored) |
| src/CortexPE/Commando/constraint/InGameRequiredConstraint.php | Adds in-game constraint (vendored) |
| src/CortexPE/Commando/constraint/ConsoleRequiredConstraint.php | Adds console-only constraint (vendored) |
| src/CortexPE/Commando/constraint/BaseConstraint.php | Adds base constraint type (vendored) |
| src/CortexPE/Commando/BaseSubCommand.php | Adds BaseSubCommand implementation (vendored) |
| src/CortexPE/Commando/BaseCommand.php | Adds BaseCommand implementation (vendored) |
| src/CortexPE/Commando/args/Vector3Argument.php | Adds vector argument parsing (vendored) |
| src/CortexPE/Commando/args/TextArgument.php | Adds rawtext argument (vendored) |
| src/CortexPE/Commando/args/StringEnumArgument.php | Adds string-enum argument base (vendored) |
| src/CortexPE/Commando/args/RawStringArgument.php | Adds string argument base (vendored) |
| src/CortexPE/Commando/args/IntegerArgument.php | Adds integer argument (vendored) |
| src/CortexPE/Commando/args/FloatArgument.php | Adds float argument (vendored) |
| src/CortexPE/Commando/args/BooleanArgument.php | Adds boolean argument (vendored) |
| src/CortexPE/Commando/args/BlockPositionArgument.php | Adds block-position argument (vendored) |
| src/CortexPE/Commando/args/BaseArgument.php | Adds BaseArgument implementation (vendored) |
| README.md | Rewrites documentation for PM5 + Go proxy + usage/config/permissions |
| plugin.yml | Bumps API to 5.0.0 and version to 2.0.0 |
| phpstan.neon.dist | Updates PHPStan scope/scan dirs and adds new ignore rules |
| .poggit.yml | Removes Commando virion dependency (now vendored) |
| .github/workflows/main.yml | Updates actions/checkout and PHPStan action version; removes virion download step |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| namespace pocketmine\network\mcpe\protocol\serializer; | ||
|
|
||
| use Ramsey\Uuid\Uuid; | ||
| use Ramsey\Uuid\UuidInterface; | ||
| use function ord; | ||
| use function pack; | ||
| use function strlen; | ||
| use function strrev; | ||
| use function substr; | ||
| use function unpack; | ||
|
|
||
| /** | ||
| * Compatibility serializer for plugins that rely on the old PacketSerializer API. | ||
| * This implements only the methods needed by PortalPM packet encoding/decoding. | ||
| */ | ||
| class PacketSerializer | ||
| { |
There was a problem hiding this comment.
This file defines pocketmine\network\mcpe\protocol\serializer\PacketSerializer, which is a core PocketMine-MP class name in PMMP 5.x. Shipping a plugin class under the pocketmine\ namespace risks a fatal class redeclaration or unpredictable autoloading collisions with the server core. Prefer moving this compatibility serializer under the plugin’s own namespace (e.g. paroxity\portal\protocol\PacketSerializer) and update imports accordingly.
| private function read(int $length): string | ||
| { | ||
| $value = substr($this->buffer, $this->offset, $length); | ||
| $this->offset += strlen($value); | ||
| return $value; | ||
| } |
There was a problem hiding this comment.
read() silently returns fewer bytes than requested when the buffer is exhausted. Callers like getByte() then call ord(""), which throws at runtime, and unpack() calls may emit warnings/return false inconsistently. read() should verify that $length bytes are available and throw a descriptive exception (and advance the offset consistently) when attempting to read past the end of the buffer.
| private function getUnsignedVarInt(): int | ||
| { | ||
| $value = 0; | ||
| $shift = 0; | ||
|
|
||
| while(true) { | ||
| $byte = $this->getByte(); | ||
| $value |= (($byte & 0x7f) << $shift); | ||
| if(($byte & 0x80) === 0) { | ||
| return $value; | ||
| } | ||
| $shift += 7; | ||
| } |
There was a problem hiding this comment.
getUnsignedVarInt() has no maximum length/shift guard. A malformed stream (e.g., missing a terminator byte with 0x80 cleared) can cause an unbounded loop and/or shift overflow. Add a maximum bytes/shift limit for unsigned varints (e.g., 5 bytes / 35 bits for 32-bit values, or an explicit cap matching your protocol) and throw when the limit is exceeded or when the buffer ends unexpectedly.
| $read = @socket_read($socket, $length); | ||
| if ($read !== false) { | ||
| $this->receiveBuffer[] = $read; | ||
| $this->notifier->wakeupSleeper(); | ||
| $notifier->wakeupSleeper(); | ||
| } | ||
| } elseif ($read === "") { |
There was a problem hiding this comment.
The socket is non-blocking, but this code assumes socket_read($socket, $length) returns the full payload in one call. TCP reads can return partial data; pushing incomplete payloads into $receiveBuffer will corrupt packet decoding. Consider buffering and looping until $length bytes are read (and similarly ensure the 4-byte length header is fully read before decoding).
| $interceptor = SimplePacketHandler::createInterceptor($this, EventPriority::LOWEST); | ||
| $interceptor->interceptIncoming(function(LoginPacket $packet, NetworkSession $session) : bool { | ||
| $json = json_decode($packet->authInfoJson, false); | ||
| if ($json instanceof \stdClass && isset($json->chain) && !isset($json->AuthenticationType)) { | ||
| $encoded = json_encode([ | ||
| "AuthenticationType" => 2, | ||
| "Certificate" => $packet->authInfoJson, | ||
| "Token" => "" | ||
| ]); | ||
| if($encoded !== false){ | ||
| $packet->authInfoJson = $encoded; | ||
| } | ||
| } |
There was a problem hiding this comment.
This interceptor reads/writes $packet->authInfoJson, but LoginPacket in PocketMine-MP 5.x does not declare this property (your PHPStan config also ignores an “undefined property” error). At runtime this will create a dynamic property and won’t affect the actual serialized login data, so the auth conversion likely won’t work. Update this to modify the correct LoginPacket fields used by PM5 (e.g. the chain/client JWT fields) or use the packet’s public API for auth data rather than an undeclared property.
| final public function execute(CommandSender $sender, string $commandLabel, array $args){ | ||
| $this->currentSender = $sender; | ||
| if(!$this->testPermission($sender)){ | ||
| return; | ||
| } |
There was a problem hiding this comment.
BaseCommand::execute() is missing a return type and doesn’t return a boolean. In PocketMine-MP 5.x, pocketmine\command\Command::execute() returns bool, so this override is likely signature-incompatible (fatal error on load) and/or returns null where the caller expects bool. Add the correct : bool return type and ensure every branch returns true/false consistently (e.g. false on permission/parse failures, true when handled).
| $coords = explode(" ", $testString); | ||
| if(count($coords) === 3) { | ||
| foreach($coords as $coord) { | ||
| if(!$this->isValidCoordinate($coord, $sender instanceof Vector3)) { |
There was a problem hiding this comment.
canParse() passes $sender instanceof Vector3 as the $locatable flag. $sender is a CommandSender and will never be a Vector3, so locatable coordinates like ~ ~ ~ will be rejected even when the sender is an Entity (which parse() actually supports). This should likely check $sender instanceof Entity (or $sender instanceof Position depending on intended behavior).
| if(!$this->isValidCoordinate($coord, $sender instanceof Vector3)) { | |
| if(!$this->isValidCoordinate($coord, $sender instanceof Entity)) { |
- Add legacyAuth bool field to RegisterServerPacket (encode/decode) - Set legacyAuth=true when registering (PocketMine uses legacy auth)
V2.0.0