diff --git a/src/network/mcpe/NetworkSession.php b/src/network/mcpe/NetworkSession.php index 86d3d49a1..6e45c2f7a 100644 --- a/src/network/mcpe/NetworkSession.php +++ b/src/network/mcpe/NetworkSession.php @@ -250,6 +250,16 @@ class NetworkSession{ $effectManager->getEffectAddHooks()->remove($effectAddHook); $effectManager->getEffectRemoveHooks()->remove($effectRemoveHook); }); + + $permissionHooks = $this->player->getPermissionRecalculationCallbacks(); + $permissionHooks->add($permHook = function() : void{ + $this->logger->debug("Syncing available commands and adventure settings due to permission recalculation"); + $this->syncAdventureSettings($this->player); + $this->syncAvailableCommands(); + }); + $this->disposeHooks->add(static function() use ($permissionHooks, $permHook) : void{ + $permissionHooks->remove($permHook); + }); } public function getPlayer() : ?Player{ diff --git a/src/permission/Permissible.php b/src/permission/Permissible.php index 26ba2b1ed..db7cb7431 100644 --- a/src/permission/Permissible.php +++ b/src/permission/Permissible.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace pocketmine\permission; +use Ds\Set; use pocketmine\plugin\Plugin; interface Permissible{ @@ -67,6 +68,12 @@ interface Permissible{ public function recalculatePermissions() : void; + /** + * @return Set|\Closure[] + * @phpstan-return Set<\Closure() : void> + */ + public function getPermissionRecalculationCallbacks() : Set; + /** * @return PermissionAttachmentInfo[] */ diff --git a/src/permission/PermissibleBase.php b/src/permission/PermissibleBase.php index da4ebf513..c144bd852 100644 --- a/src/permission/PermissibleBase.php +++ b/src/permission/PermissibleBase.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace pocketmine\permission; +use Ds\Set; use pocketmine\plugin\Plugin; use pocketmine\plugin\PluginException; use pocketmine\timings\Timings; @@ -46,7 +47,15 @@ class PermissibleBase implements Permissible{ /** @var PermissionAttachmentInfo[] */ private $permissions = []; + /** + * @var Set|\Closure[] + * @phpstan-var Set<\Closure() : void> + */ + private $permissionRecalculationCallbacks; + public function __construct(?Permissible $permissible, bool $isOp){ + $this->permissionRecalculationCallbacks = new Set(); + $this->parent = $permissible; //TODO: we can't setBasePermission here directly due to bad architecture that causes recalculatePermissions to explode @@ -149,6 +158,11 @@ class PermissibleBase implements Permissible{ $this->calculateChildPermissions($attachment->getPermissions(), false, $attachment); } + foreach($this->permissionRecalculationCallbacks as $closure){ + //TODO: provide a diff of permissions + $closure(); + } + Timings::$permissibleCalculationTimer->stopTiming(); } @@ -169,6 +183,12 @@ class PermissibleBase implements Permissible{ } } + /** + * @return \Closure[]|Set + * @phpstan-return Set<\Closure() : void> + */ + public function getPermissionRecalculationCallbacks() : Set{ return $this->permissionRecalculationCallbacks; } + /** * @return PermissionAttachmentInfo[] */ @@ -181,5 +201,6 @@ class PermissibleBase implements Permissible{ $this->permissions = []; //PermissionAttachmentInfo doesn't reference Permissible anymore, but it references PermissionAttachment which does $this->attachments = []; //this might still be a problem if the attachments are still referenced, but we can't do anything about that $this->parent = null; + $this->permissionRecalculationCallbacks->clear(); } } diff --git a/src/permission/PermissibleDelegateTrait.php b/src/permission/PermissibleDelegateTrait.php index 9d1b57d74..d1850f02f 100644 --- a/src/permission/PermissibleDelegateTrait.php +++ b/src/permission/PermissibleDelegateTrait.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace pocketmine\permission; +use Ds\Set; use pocketmine\plugin\Plugin; trait PermissibleDelegateTrait{ @@ -70,6 +71,14 @@ trait PermissibleDelegateTrait{ $this->perm->recalculatePermissions(); } + /** + * @return Set|\Closure[] + * @phpstan-return Set<\Closure() : void> + */ + public function getPermissionRecalculationCallbacks() : Set{ + return $this->perm->getPermissionRecalculationCallbacks(); + } + /** * @return PermissionAttachmentInfo[] */ diff --git a/src/player/Player.php b/src/player/Player.php index fb1e45e33..fca4a5ae1 100644 --- a/src/player/Player.php +++ b/src/player/Player.php @@ -136,9 +136,7 @@ use const PHP_INT_MAX; * Main class that handles networking, recovery, and packet sending to the server part */ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ - use PermissibleDelegateTrait { - recalculatePermissions as private delegateRecalculatePermissions; - } + use PermissibleDelegateTrait; private const MOVES_PER_TICK = 2; private const MOVE_BACKLOG_SIZE = 100 * self::MOVES_PER_TICK; //100 ticks backlog (5 seconds) @@ -524,30 +522,6 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ return $this->isConnected(); } - public function recalculatePermissions() : void{ - $this->server->unsubscribeFromBroadcastChannel(Server::BROADCAST_CHANNEL_USERS, $this); - $this->server->unsubscribeFromBroadcastChannel(Server::BROADCAST_CHANNEL_ADMINISTRATIVE, $this); - - if($this->perm === null){ - return; - } - - $this->delegateRecalculatePermissions(); - - $this->networkSession->syncAdventureSettings($this); - - if($this->spawned){ - if($this->hasPermission(Server::BROADCAST_CHANNEL_USERS)){ - $this->server->subscribeToBroadcastChannel(Server::BROADCAST_CHANNEL_USERS, $this); - } - if($this->hasPermission(Server::BROADCAST_CHANNEL_ADMINISTRATIVE)){ - $this->server->subscribeToBroadcastChannel(Server::BROADCAST_CHANNEL_ADMINISTRATIVE, $this); - } - - $this->networkSession->syncAvailableCommands(); - } - } - public function isConnected() : bool{ return $this->networkSession !== null and $this->networkSession->isConnected(); } @@ -771,6 +745,16 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ Timings::$playerChunkSendTimer->stopTiming(); } + private function recheckBroadcastPermissions() : void{ + foreach([Server::BROADCAST_CHANNEL_USERS, Server::BROADCAST_CHANNEL_ADMINISTRATIVE] as $channel){ + if($this->hasPermission($channel)){ + $this->server->subscribeToBroadcastChannel($channel, $this); + }else{ + $this->server->unsubscribeFromBroadcastChannel($channel, $this); + } + } + } + /** * Called by the network system when the pre-spawn sequence is completed (e.g. after sending spawn chunks). * This fires join events and broadcasts join messages to other online players. @@ -780,12 +764,10 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ return; } $this->spawned = true; - if($this->hasPermission(Server::BROADCAST_CHANNEL_USERS)){ - $this->server->subscribeToBroadcastChannel(Server::BROADCAST_CHANNEL_USERS, $this); - } - if($this->hasPermission(Server::BROADCAST_CHANNEL_ADMINISTRATIVE)){ - $this->server->subscribeToBroadcastChannel(Server::BROADCAST_CHANNEL_ADMINISTRATIVE, $this); - } + $this->recheckBroadcastPermissions(); + $this->getPermissionRecalculationCallbacks()->add(function() : void{ + $this->recheckBroadcastPermissions(); + }); $ev = new PlayerJoinEvent($this, new TranslationContainer(TextFormat::YELLOW . "%multiplayer.player.joined", [ diff --git a/tests/phpstan/configs/l8-baseline.neon b/tests/phpstan/configs/l8-baseline.neon index 00c0c4502..ef7fd7b38 100644 --- a/tests/phpstan/configs/l8-baseline.neon +++ b/tests/phpstan/configs/l8-baseline.neon @@ -120,6 +120,11 @@ parameters: count: 1 path: ../../../src/network/mcpe/NetworkSession.php + - + message: "#^Parameter \\#1 \\$for of method pocketmine\\\\network\\\\mcpe\\\\NetworkSession\\:\\:syncAdventureSettings\\(\\) expects pocketmine\\\\player\\\\Player, pocketmine\\\\player\\\\Player\\|null given\\.$#" + count: 3 + path: ../../../src/network/mcpe/NetworkSession.php + - message: "#^Parameter \\#1 \\$clientPub of class pocketmine\\\\network\\\\mcpe\\\\encryption\\\\PrepareEncryptionTask constructor expects Mdanter\\\\Ecc\\\\Crypto\\\\Key\\\\PublicKeyInterface, Mdanter\\\\Ecc\\\\Crypto\\\\Key\\\\PublicKeyInterface\\|null given\\.$#" count: 1 @@ -155,11 +160,6 @@ parameters: count: 1 path: ../../../src/network/mcpe/NetworkSession.php - - - message: "#^Parameter \\#1 \\$for of method pocketmine\\\\network\\\\mcpe\\\\NetworkSession\\:\\:syncAdventureSettings\\(\\) expects pocketmine\\\\player\\\\Player, pocketmine\\\\player\\\\Player\\|null given\\.$#" - count: 2 - path: ../../../src/network/mcpe/NetworkSession.php - - message: "#^Cannot call method syncAll\\(\\) on pocketmine\\\\network\\\\mcpe\\\\InventoryManager\\|null\\.$#" count: 1 @@ -287,7 +287,7 @@ parameters: - message: "#^Cannot call method syncAdventureSettings\\(\\) on pocketmine\\\\network\\\\mcpe\\\\NetworkSession\\|null\\.$#" - count: 4 + count: 3 path: ../../../src/player/Player.php - @@ -295,11 +295,6 @@ parameters: count: 1 path: ../../../src/player/Player.php - - - message: "#^Cannot call method syncAvailableCommands\\(\\) on pocketmine\\\\network\\\\mcpe\\\\NetworkSession\\|null\\.$#" - count: 1 - path: ../../../src/player/Player.php - - message: "#^Method pocketmine\\\\player\\\\Player\\:\\:getNetworkSession\\(\\) should return pocketmine\\\\network\\\\mcpe\\\\NetworkSession but returns pocketmine\\\\network\\\\mcpe\\\\NetworkSession\\|null\\.$#" count: 1