PermissibleBase: compute permission diff, do not fire callbacks when diff is empty

this fixes AvailableCommandsPacket getting sent twice when adding a PermissionAttachment.
This commit is contained in:
Dylan K. Taylor 2020-12-02 01:09:29 +00:00
parent 6720e658bd
commit 0634426c26
5 changed files with 42 additions and 10 deletions

View File

@ -70,7 +70,7 @@ interface Permissible{
/**
* @return Set|\Closure[]
* @phpstan-return Set<\Closure() : void>
* @phpstan-return Set<\Closure(array<string, bool> $changedPermissionsOldValues) : void>
*/
public function getPermissionRecalculationCallbacks() : Set;

View File

@ -27,6 +27,7 @@ use Ds\Set;
use pocketmine\plugin\Plugin;
use pocketmine\plugin\PluginException;
use pocketmine\timings\Timings;
use function count;
use function spl_object_id;
class PermissibleBase implements Permissible{
@ -46,7 +47,7 @@ class PermissibleBase implements Permissible{
/**
* @var Set|\Closure[]
* @phpstan-var Set<\Closure() : void>
* @phpstan-var Set<\Closure(array<string, bool> $changedPermissionsOldValues) : void>
*/
private $permissionRecalculationCallbacks;
@ -133,6 +134,7 @@ class PermissibleBase implements Permissible{
$permManager = PermissionManager::getInstance();
$permManager->unsubscribeFromAllPermissions($this);
$oldPermissions = $this->permissions;
$this->permissions = [];
foreach($this->rootPermissions as $name => $isGranted){
@ -149,10 +151,32 @@ class PermissibleBase implements Permissible{
$this->calculateChildPermissions($attachment->getPermissions(), false, $attachment);
}
foreach($this->permissionRecalculationCallbacks as $closure){
//TODO: provide a diff of permissions
$closure();
}
$diff = [];
Timings::$permissibleCalculationDiffTimer->time(function() use ($oldPermissions, &$diff) : void{
foreach($this->permissions as $permissionAttachmentInfo){
$name = $permissionAttachmentInfo->getPermission();
if(!isset($oldPermissions[$name])){
$diff[$name] = false;
}elseif($oldPermissions[$name]->getValue() !== $permissionAttachmentInfo->getValue()){
//permission was previously unset OR the value of the permission changed
//we don't care who assigned the permission, only that the result is different
$diff[$name] = $oldPermissions[$name]->getValue();
}
unset($oldPermissions[$name]);
}
//oldPermissions now only contains permissions that are no longer set after recalculation
foreach($oldPermissions as $permissionAttachmentInfo){
$diff[$permissionAttachmentInfo->getPermission()] = $permissionAttachmentInfo->getValue();
}
});
Timings::$permissibleCalculationCallbackTimer->time(function() use ($diff) : void{
if(count($diff) > 0){
foreach($this->permissionRecalculationCallbacks as $closure){
$closure($diff);
}
}
});
Timings::$permissibleCalculationTimer->stopTiming();
}
@ -176,7 +200,7 @@ class PermissibleBase implements Permissible{
/**
* @return \Closure[]|Set
* @phpstan-return Set<\Closure() : void>
* @phpstan-return Set<\Closure(array<string, bool> $changedPermissionsOldValues) : void>
*/
public function getPermissionRecalculationCallbacks() : Set{ return $this->permissionRecalculationCallbacks; }

View File

@ -73,7 +73,7 @@ trait PermissibleDelegateTrait{
/**
* @return Set|\Closure[]
* @phpstan-return Set<\Closure() : void>
* @phpstan-return Set<\Closure(array<string, bool> $changedPermissionsOldValues) : void>
*/
public function getPermissionRecalculationCallbacks() : Set{
return $this->perm->getPermissionRecalculationCallbacks();

View File

@ -765,8 +765,10 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{
}
$this->spawned = true;
$this->recheckBroadcastPermissions();
$this->getPermissionRecalculationCallbacks()->add(function() : void{
$this->recheckBroadcastPermissions();
$this->getPermissionRecalculationCallbacks()->add(function(array $changedPermissionsOldValues) : void{
if(isset($changedPermissionsOldValues[Server::BROADCAST_CHANNEL_ADMINISTRATIVE]) || isset($changedPermissionsOldValues[Server::BROADCAST_CHANNEL_USERS])){
$this->recheckBroadcastPermissions();
}
});
$ev = new PlayerJoinEvent($this,

View File

@ -79,6 +79,10 @@ abstract class Timings{
public static $generationCallbackTimer;
/** @var TimingsHandler */
public static $permissibleCalculationTimer;
/** @var TimingsHandler */
public static $permissibleCalculationDiffTimer;
/** @var TimingsHandler */
public static $permissibleCalculationCallbackTimer;
/** @var TimingsHandler */
public static $entityMoveTimer;
@ -156,6 +160,8 @@ abstract class Timings{
self::$populationTimer = new TimingsHandler("World Population");
self::$generationCallbackTimer = new TimingsHandler("World Generation Callback");
self::$permissibleCalculationTimer = new TimingsHandler("Permissible Calculation");
self::$permissibleCalculationDiffTimer = new TimingsHandler(self::INCLUDED_BY_OTHER_TIMINGS_PREFIX . "Permissible Calculation - Diff", self::$permissibleCalculationTimer);
self::$permissibleCalculationCallbackTimer = new TimingsHandler(self::INCLUDED_BY_OTHER_TIMINGS_PREFIX . "Permissible Calculation - Callbacks", self::$permissibleCalculationTimer);
self::$syncPlayerDataLoad = new TimingsHandler("Player Data Load");
self::$syncPlayerDataSave = new TimingsHandler("Player Data Save");