From 8f48fe48561f1902f25c532f64ee9c3c594698fb Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 13 Nov 2024 15:35:41 +0000 Subject: [PATCH] Fully separate hierarchies for sync & async events there's no way to combine these without causing type compatibility issues for one side or the other. we might be able to use traits to reduce duplication, but the separation here seems to be necessary. --- src/event/AsyncEvent.php | 5 +- src/event/AsyncHandlerList.php | 88 +++++++++++++++++++ ...stener.php => AsyncRegisteredListener.php} | 36 +++++--- src/event/HandlerList.php | 20 +---- src/event/HandlerListManager.php | 80 +++++++++++++---- src/plugin/PluginManager.php | 9 +- 6 files changed, 185 insertions(+), 53 deletions(-) create mode 100644 src/event/AsyncHandlerList.php rename src/event/{RegisteredAsyncListener.php => AsyncRegisteredListener.php} (70%) diff --git a/src/event/AsyncEvent.php b/src/event/AsyncEvent.php index 123376024..24c97224c 100644 --- a/src/event/AsyncEvent.php +++ b/src/event/AsyncEvent.php @@ -28,7 +28,6 @@ use pocketmine\promise\PromiseResolver; use pocketmine\timings\Timings; use pocketmine\utils\ObjectSet; use function array_shift; -use function assert; use function count; /** @@ -99,14 +98,13 @@ abstract class AsyncEvent{ * @phpstan-return Promise */ private function callPriority(int $priority) : Promise{ - $handlers = HandlerListManager::global()->getListFor(static::class)->getListenersByPriority($priority); + $handlers = HandlerListManager::global()->getAsyncListFor(static::class)->getListenersByPriority($priority); /** @phpstan-var PromiseResolver $resolver */ $resolver = new PromiseResolver(); $nonConcurrentHandlers = []; foreach($handlers as $registration){ - assert($registration instanceof RegisteredAsyncListener); if($registration->canBeCalledConcurrently()){ $result = $registration->callAsync($this); if($result !== null) { @@ -127,7 +125,6 @@ abstract class AsyncEvent{ }else{ $this->waitForPromises()->onCompletion(function() use (&$nonConcurrentHandlers, $testResolve){ $handler = array_shift($nonConcurrentHandlers); - assert($handler instanceof RegisteredAsyncListener); $result = $handler->callAsync($this); if($result !== null) { $this->promises->add($result); diff --git a/src/event/AsyncHandlerList.php b/src/event/AsyncHandlerList.php new file mode 100644 index 000000000..a2e238afb --- /dev/null +++ b/src/event/AsyncHandlerList.php @@ -0,0 +1,88 @@ + $class + */ + public function __construct( + private string $class, + private ?AsyncHandlerList $parentList, + ){} + + public function register(AsyncRegisteredListener $listener) : void{ + if(isset($this->handlerSlots[$listener->getPriority()][spl_object_id($listener)])){ + throw new \InvalidArgumentException("This listener is already registered to priority {$listener->getPriority()} of event {$this->class}"); + } + $this->handlerSlots[$listener->getPriority()][spl_object_id($listener)] = $listener; + } + + /** + * @param AsyncRegisteredListener[] $listeners + */ + public function registerAll(array $listeners) : void{ + foreach($listeners as $listener){ + $this->register($listener); + } + } + + public function unregister(AsyncRegisteredListener|Plugin|Listener $object) : void{ + //TODO: Not loving the duplication here + if($object instanceof Plugin || $object instanceof Listener){ + foreach($this->handlerSlots as $priority => $list){ + foreach($list as $hash => $listener){ + if(($object instanceof Plugin && $listener->getPlugin() === $object) + || ($object instanceof Listener && (new \ReflectionFunction($listener->getHandler()))->getClosureThis() === $object) //this doesn't even need to be a listener :D + ){ + unset($this->handlerSlots[$priority][$hash]); + } + } + } + }else{ + unset($this->handlerSlots[$object->getPriority()][spl_object_id($object)]); + } + } + + public function clear() : void{ + $this->handlerSlots = []; + } + + /** + * @return AsyncRegisteredListener[] + */ + public function getListenersByPriority(int $priority) : array{ + return $this->handlerSlots[$priority] ?? []; + } + + public function getParent() : ?AsyncHandlerList{ + return $this->parentList; + } +} diff --git a/src/event/RegisteredAsyncListener.php b/src/event/AsyncRegisteredListener.php similarity index 70% rename from src/event/RegisteredAsyncListener.php rename to src/event/AsyncRegisteredListener.php index 6a0413d99..1d0f65b71 100644 --- a/src/event/RegisteredAsyncListener.php +++ b/src/event/AsyncRegisteredListener.php @@ -26,28 +26,35 @@ namespace pocketmine\event; use pocketmine\plugin\Plugin; use pocketmine\promise\Promise; use pocketmine\timings\TimingsHandler; +use function in_array; -class RegisteredAsyncListener extends RegisteredListener{ +class AsyncRegisteredListener{ /** * @phpstan-param \Closure(AsyncEvent) : Promise $handler */ public function __construct( - protected \Closure $handler, - int $priority, - Plugin $plugin, - bool $handleCancelled, + private \Closure $handler, + private int $priority, + private Plugin $plugin, + private bool $handleCancelled, private bool $exclusiveCall, - protected TimingsHandler $timings + private TimingsHandler $timings ){ - parent::__construct($handler, $priority, $plugin, $handleCancelled, $timings); + if(!in_array($priority, EventPriority::ALL, true)){ + throw new \InvalidArgumentException("Invalid priority number $priority"); + } } - public function canBeCalledConcurrently() : bool{ - return !$this->exclusiveCall; + public function getHandler() : \Closure{ + return $this->handler; } - public function callEvent(Event $event) : void{ - throw new \BadMethodCallException("Cannot call async event synchronously, use callAsync() instead"); + public function getPlugin() : Plugin{ + return $this->plugin; + } + + public function getPriority() : int{ + return $this->priority; } /** @@ -64,4 +71,11 @@ class RegisteredAsyncListener extends RegisteredListener{ $this->timings->stopTiming(); } } + + public function isHandlingCancelled() : bool{ + return $this->handleCancelled; + } + public function canBeCalledConcurrently() : bool{ + return !$this->exclusiveCall; + } } diff --git a/src/event/HandlerList.php b/src/event/HandlerList.php index 89629e140..2072cd522 100644 --- a/src/event/HandlerList.php +++ b/src/event/HandlerList.php @@ -25,7 +25,6 @@ namespace pocketmine\event; use pocketmine\plugin\Plugin; use function array_merge; -use function array_merge_recursive; use function krsort; use function spl_object_id; use const SORT_NUMERIC; @@ -38,7 +37,7 @@ class HandlerList{ private array $affectedHandlerCaches = []; /** - * @phpstan-param class-string $class + * @phpstan-param class-string $class */ public function __construct( private string $class, @@ -127,23 +126,12 @@ class HandlerList{ $handlerLists[] = $currentList; } - $listeners = []; - $asyncListeners = []; - $exclusiveAsyncListeners = []; + $listenersByPriority = []; foreach($handlerLists as $currentList){ - foreach($currentList->handlerSlots as $priority => $listenersToSort){ - foreach($listenersToSort as $listener){ - if(!$listener instanceof RegisteredAsyncListener){ - $listeners[$priority][] = $listener; - }elseif(!$listener->canBeCalledConcurrently()){ - $asyncListeners[$priority][] = $listener; - }else{ - $exclusiveAsyncListeners[$priority][] = $listener; - } - } + foreach($currentList->handlerSlots as $priority => $listeners){ + $listenersByPriority[$priority] = array_merge($listenersByPriority[$priority] ?? [], $listeners); } } - $listenersByPriority = array_merge_recursive($listeners, $asyncListeners, $exclusiveAsyncListeners); //TODO: why on earth do the priorities have higher values for lower priority? krsort($listenersByPriority, SORT_NUMERIC); diff --git a/src/event/HandlerListManager.php b/src/event/HandlerListManager.php index 4fc9ac305..aedf065a3 100644 --- a/src/event/HandlerListManager.php +++ b/src/event/HandlerListManager.php @@ -35,24 +35,37 @@ class HandlerListManager{ } /** @var HandlerList[] classname => HandlerList */ - private array $allLists = []; + private array $allSyncLists = []; /** * @var RegisteredListenerCache[] event class name => cache * @phpstan-var array, RegisteredListenerCache> */ - private array $handlerCaches = []; + private array $syncHandlerCaches = []; + + /** @var AsyncHandlerList[] classname => AsyncHandlerList */ + private array $allAsyncLists = []; /** * Unregisters all the listeners * If a Plugin or Listener is passed, all the listeners with that object will be removed */ - public function unregisterAll(RegisteredListener|Plugin|Listener|null $object = null) : void{ - if($object instanceof Listener || $object instanceof Plugin || $object instanceof RegisteredListener){ - foreach($this->allLists as $h){ - $h->unregister($object); + public function unregisterAll(RegisteredListener|AsyncRegisteredListener|Plugin|Listener|null $object = null) : void{ + if($object !== null){ + if(!$object instanceof AsyncRegisteredListener){ + foreach($this->allSyncLists as $h){ + $h->unregister($object); + } + } + if(!$object instanceof RegisteredListener){ + foreach($this->allAsyncLists as $h){ + $h->unregister($object); + } } }else{ - foreach($this->allLists as $h){ + foreach($this->allSyncLists as $h){ + $h->clear(); + } + foreach($this->allAsyncLists as $h){ $h->clear(); } } @@ -67,9 +80,10 @@ class HandlerListManager{ } /** - * @phpstan-param \ReflectionClass $class + * @phpstan-template TEvent of Event|AsyncEvent + * @phpstan-param \ReflectionClass $class * - * @phpstan-return \ReflectionClass|null + * @phpstan-return \ReflectionClass|null */ private static function resolveNearestHandleableParent(\ReflectionClass $class) : ?\ReflectionClass{ for($parent = $class->getParentClass(); $parent !== false; $parent = $parent->getParentClass()){ @@ -86,26 +100,28 @@ class HandlerListManager{ * * Calling this method also lazily initializes the $classMap inheritance tree of handler lists. * - * @phpstan-template TEvent of Event|AsyncEvent - * @phpstan-param class-string $event + * @phpstan-param class-string $event * * @throws \ReflectionException * @throws \InvalidArgumentException */ public function getListFor(string $event) : HandlerList{ - if(isset($this->allLists[$event])){ - return $this->allLists[$event]; + if(isset($this->allSyncLists[$event])){ + return $this->allSyncLists[$event]; } $class = new \ReflectionClass($event); + if(!$class->isSubclassOf(Event::class)){ + throw new \InvalidArgumentException("Cannot get sync handler list for async event"); + } if(!self::isValidClass($class)){ throw new \InvalidArgumentException("Event must be non-abstract or have the @allowHandle annotation"); } $parent = self::resolveNearestHandleableParent($class); $cache = new RegisteredListenerCache(); - $this->handlerCaches[$event] = $cache; - return $this->allLists[$event] = new HandlerList( + $this->syncHandlerCaches[$event] = $cache; + return $this->allSyncLists[$event] = new HandlerList( $event, parentList: $parent !== null ? $this->getListFor($parent->getName()) : null, handlerCache: $cache @@ -113,12 +129,40 @@ class HandlerListManager{ } /** - * @phpstan-param class-string $event + * Returns the HandlerList for async listeners that explicitly handle this event. + * + * @phpstan-param class-string $event + * + * @throws \ReflectionException + * @throws \InvalidArgumentException + */ + public function getAsyncListFor(string $event) : AsyncHandlerList{ + if(isset($this->allAsyncLists[$event])){ + return $this->allAsyncLists[$event]; + } + + $class = new \ReflectionClass($event); + if(!$class->isSubclassOf(AsyncEvent::class)){ + throw new \InvalidArgumentException("Cannot get async handler list for sync event"); + } + if(!self::isValidClass($class)){ + throw new \InvalidArgumentException("Event must be non-abstract or have the @allowHandle annotation"); + } + + $parent = self::resolveNearestHandleableParent($class); + return $this->allAsyncLists[$event] = new AsyncHandlerList( + $event, + parentList: $parent !== null ? $this->getAsyncListFor($parent->getName()) : null, + ); + } + + /** + * @phpstan-param class-string $event * * @return RegisteredListener[] */ public function getHandlersFor(string $event) : array{ - $cache = $this->handlerCaches[$event] ?? null; + $cache = $this->syncHandlerCaches[$event] ?? null; //getListFor() will populate the cache for the next call return $cache?->list ?? $this->getListFor($event)->getListenerList(); } @@ -127,6 +171,6 @@ class HandlerListManager{ * @return HandlerList[] */ public function getAll() : array{ - return $this->allLists; + return $this->allSyncLists; } } diff --git a/src/plugin/PluginManager.php b/src/plugin/PluginManager.php index ca126ab4c..8c99e9f40 100644 --- a/src/plugin/PluginManager.php +++ b/src/plugin/PluginManager.php @@ -32,7 +32,7 @@ use pocketmine\event\Listener; use pocketmine\event\ListenerMethodTags; use pocketmine\event\plugin\PluginDisableEvent; use pocketmine\event\plugin\PluginEnableEvent; -use pocketmine\event\RegisteredAsyncListener; +use pocketmine\event\AsyncRegisteredListener; use pocketmine\event\RegisteredListener; use pocketmine\lang\KnownTranslationFactory; use pocketmine\permission\DefaultPermissions; @@ -702,7 +702,8 @@ class PluginManager{ * * @throws \ReflectionException */ - public function registerAsyncEvent(string $event, \Closure $handler, int $priority, Plugin $plugin, bool $handleCancelled = false, bool $exclusiveCall = false) : RegisteredAsyncListener{ + public function registerAsyncEvent(string $event, \Closure $handler, int $priority, Plugin $plugin, bool $handleCancelled = false, bool $exclusiveCall = false) : AsyncRegisteredListener{ + //TODO: Not loving the code duplication here if(!is_subclass_of($event, AsyncEvent::class)){ throw new PluginException($event . " is not an AsyncEvent"); } @@ -715,8 +716,8 @@ class PluginManager{ $timings = Timings::getEventHandlerTimings($event, $handlerName, $plugin->getDescription()->getFullName()); - $registeredListener = new RegisteredAsyncListener($handler, $priority, $plugin, $handleCancelled, $exclusiveCall, $timings); - HandlerListManager::global()->getListFor($event)->register($registeredListener); + $registeredListener = new AsyncRegisteredListener($handler, $priority, $plugin, $handleCancelled, $exclusiveCall, $timings); + HandlerListManager::global()->getAsyncListFor($event)->register($registeredListener); return $registeredListener; }