Implemented a better method for detecting permission recalculation

this allows anyone to listen to permissions being recalculated, which is useful for stuff like broadcast channel subscriptions.

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{

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[]
 	 */

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();
 	}
 }

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[]
 	 */

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 {
+	use PermissibleDelegateTrait;
-		recalculatePermissions as private delegateRecalculatePermissions;
-	}
 
 	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->recheckBroadcastPermissions();
-			$this->server->subscribeToBroadcastChannel(Server::BROADCAST_CHANNEL_USERS, $this);
+		$this->getPermissionRecalculationCallbacks()->add(function() : void{
-		}
+			$this->recheckBroadcastPermissions();
-		if($this->hasPermission(Server::BROADCAST_CHANNEL_ADMINISTRATIVE)){
+		});
-			$this->server->subscribeToBroadcastChannel(Server::BROADCAST_CHANNEL_ADMINISTRATIVE, $this);
-		}
 
 		$ev = new PlayerJoinEvent($this,
 			new TranslationContainer(TextFormat::YELLOW . "%multiplayer.player.joined", [

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