From 5fe57a8f6f8c919b6d911394b31416f79d37d694 Mon Sep 17 00:00:00 2001 From: ShockedPlot7560 Date: Sat, 14 Oct 2023 21:36:44 +0200 Subject: [PATCH 01/47] temporaly add Promise::all code came from #6015 and will be subject to probable changes --- src/promise/Promise.php | 53 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/src/promise/Promise.php b/src/promise/Promise.php index e7188197c7d..aae08159f19 100644 --- a/src/promise/Promise.php +++ b/src/promise/Promise.php @@ -23,10 +23,11 @@ namespace pocketmine\promise; +use function count; use function spl_object_id; /** - * @phpstan-template TValue + * @phpstan-template-covariant TValue */ final class Promise{ /** @@ -52,4 +53,54 @@ public function onCompletion(\Closure $onSuccess, \Closure $onFailure) : void{ public function isResolved() : bool{ return $this->shared->resolved; } + + /** + * Returns a promise that will resolve only once all the Promises in + * `$promises` have resolved. The resolution value of the returned promise + * will be an array containing the resolution values of each Promises in + * `$promises` indexed by the respective Promises' array keys. + * + * @template TPromiseValue + * @phpstan-param Promise[] $promises + * + * @phpstan-return Promise> + */ + public static function all(array $promises) : Promise { + /** @phpstan-var PromiseResolver> $resolver */ + $resolver = new PromiseResolver(); + $values = []; + $toResolve = count($promises); + $continue = true; + + foreach($promises as $key => $promise){ + $values[$key] = null; + + $promise->onCompletion( + function(mixed $value) use ($resolver, $key, &$toResolve, &$continue, &$values) : void{ + $values[$key] = $value; + + if(--$toResolve === 0 && $continue){ + $resolver->resolve($values); + } + }, + function() use ($resolver, &$continue) : void{ + if($continue){ + $continue = false; + $resolver->reject(); + } + } + ); + + if(!$continue){ + break; + } + } + + if($toResolve === 0){ + $continue = false; + $resolver->resolve($values); + } + + return $resolver->getPromise(); + } } From a84fc2b9019dc6172a6429bc9a2daef3bbe478ad Mon Sep 17 00:00:00 2001 From: ShockedPlot7560 Date: Sat, 14 Oct 2023 23:13:35 +0200 Subject: [PATCH 02/47] introduce AsyncEvent and ::callAsync() An asynchronous event is one that allows the addition of promises to be resolved before being completed. This implementation integrates priority levels, allowing you to wait for all promises added to one level before moving on to the next. This is made possible by separating the event call logic into several functions, which can then be integrated into AsyncEventTrait::callAsync() --- src/event/AsyncEvent.php | 53 ++++++++++++++ src/event/AsyncEventTrait.php | 84 ++++++++++++++++++++++ src/event/Event.php | 17 ++++- tests/phpstan/configs/actual-problems.neon | 5 ++ 4 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 src/event/AsyncEvent.php create mode 100644 src/event/AsyncEventTrait.php diff --git a/src/event/AsyncEvent.php b/src/event/AsyncEvent.php new file mode 100644 index 00000000000..3902a96ff7b --- /dev/null +++ b/src/event/AsyncEvent.php @@ -0,0 +1,53 @@ + $promise + */ + public function addPromise(Promise $promise) : void; + + /** + * Be prudent, calling an event asynchronously can produce unexpected results. + * During the execution of the event, the server, the player and the event context may have changed state. + * + * @phpstan-param ObjectSet> $promiseSet + * + * @phpstan-return Promise + */ + public static function callAsync(AsyncEvent&Event $event, ObjectSet $promiseSet) : Promise; +} diff --git a/src/event/AsyncEventTrait.php b/src/event/AsyncEventTrait.php new file mode 100644 index 00000000000..b907ba5a304 --- /dev/null +++ b/src/event/AsyncEventTrait.php @@ -0,0 +1,84 @@ +> */ + private ObjectSet $promises; + + /** + * @phpstan-param ObjectSet>|null $promises + */ + private function initializePromises(?ObjectSet &$promises) : void{ + $promises ??= new ObjectSet(); + $this->promises = $promises; + } + + public function addPromise(Promise $promise) : void{ + if(!isset($this->promises)){ + throw new \RuntimeException("Cannot add promises, be sure to initialize the promises set in the constructor"); + } + $this->promises->add($promise); + } + + final public static function callAsync(AsyncEvent&Event $event, ObjectSet $promiseSet) : Promise{ + $event->checkMaxDepthCall(); + + /** @phpstan-var PromiseResolver $globalResolver */ + $globalResolver = new PromiseResolver(); + + $callable = function(int $priority) use ($event, $promiseSet) : Promise{ + $handlers = HandlerListManager::global()->getListFor(static::class)->getListenersByPriority($priority); + $event->callHandlers($handlers); + + $array = $promiseSet->toArray(); + $promiseSet->clear(); + + return Promise::all($array); + }; + + $priorities = EventPriority::ALL; + $testResolve = function () use (&$testResolve, &$priorities, $callable, $globalResolver){ + if(count($priorities) === 0){ + $globalResolver->resolve(null); + }else{ + $callable(array_shift($priorities))->onCompletion(function() use ($testResolve) : void{ + $testResolve(); + }, function () use ($globalResolver) { + $globalResolver->reject(); + }); + } + }; + + $testResolve(); + + return $globalResolver->getPromise(); + } +} diff --git a/src/event/Event.php b/src/event/Event.php index 21b8ae36a5d..6334c16b201 100644 --- a/src/event/Event.php +++ b/src/event/Event.php @@ -47,15 +47,30 @@ final public function getEventName() : string{ * @throws \RuntimeException if event call recursion reaches the max depth limit */ public function call() : void{ + $this->checkMaxDepthCall(); + $this->callHandlers(null); + } + + /** + * @internal used by AsyncEventTrait and Event + */ + final protected function checkMaxDepthCall() : void{ if(self::$eventCallDepth >= self::MAX_EVENT_CALL_DEPTH){ //this exception will be caught by the parent event call if all else fails throw new \RuntimeException("Recursive event call detected (reached max depth of " . self::MAX_EVENT_CALL_DEPTH . " calls)"); } + } + /** + * @param RegisteredListener[]|null $handlers + * + * @internal used by AsyncEventTrait and Event + */ + final protected function callHandlers(?array $handlers) : void{ $timings = Timings::getEventTimings($this); $timings->startTiming(); - $handlers = HandlerListManager::global()->getHandlersFor(static::class); + $handlers = $handlers ?? HandlerListManager::global()->getHandlersFor(static::class); ++self::$eventCallDepth; try{ diff --git a/tests/phpstan/configs/actual-problems.neon b/tests/phpstan/configs/actual-problems.neon index 070858e8042..4278cc3a81d 100644 --- a/tests/phpstan/configs/actual-problems.neon +++ b/tests/phpstan/configs/actual-problems.neon @@ -1205,3 +1205,8 @@ parameters: count: 1 path: ../../phpunit/scheduler/AsyncPoolTest.php + - + message: "#^Right side of && is always true\\.$#" + count: 1 + path: ../../../src/promise/Promise.php + From 7a4b9a036705548c4cf78674e03b2942da53e3fd Mon Sep 17 00:00:00 2001 From: ShockedPlot7560 Date: Sun, 22 Oct 2023 15:01:11 +0200 Subject: [PATCH 03/47] events: asynchandler is defined by their return type and event type If the event is async and the handler return Promise, it will be handle as an async event. However, if the event is async and the handler return smth different than Promise, it will be handle synchronously. An async handler can specify if it wishes to be called with no concurrent handlers with the tag @noConcurrentCall --- src/event/AsyncEvent.php | 12 +-- src/event/AsyncEventDelegate.php | 131 ++++++++++++++++++++++++++ src/event/AsyncEventTrait.php | 56 ++--------- src/event/Event.php | 27 ++---- src/event/HandlerList.php | 19 +++- src/event/ListenerMethodTags.php | 1 + src/event/RegisteredAsyncListener.php | 58 ++++++++++++ src/event/player/PlayerChatEvent.php | 5 +- src/player/Player.php | 12 ++- src/plugin/PluginManager.php | 49 +++++++++- 10 files changed, 281 insertions(+), 89 deletions(-) create mode 100644 src/event/AsyncEventDelegate.php create mode 100644 src/event/RegisteredAsyncListener.php diff --git a/src/event/AsyncEvent.php b/src/event/AsyncEvent.php index 3902a96ff7b..9f1784b9029 100644 --- a/src/event/AsyncEvent.php +++ b/src/event/AsyncEvent.php @@ -24,7 +24,6 @@ namespace pocketmine\event; use pocketmine\promise\Promise; -use pocketmine\utils\ObjectSet; /** * This interface is implemented by an Event subclass if and only if it can be called asynchronously. @@ -34,20 +33,11 @@ * When all the promises of a priority level have been resolved, the next priority level is called. */ interface AsyncEvent{ - /** - * Add a promise to the set of promises that will be awaited before the next priority level is called. - * - * @phpstan-param Promise $promise - */ - public function addPromise(Promise $promise) : void; - /** * Be prudent, calling an event asynchronously can produce unexpected results. * During the execution of the event, the server, the player and the event context may have changed state. * - * @phpstan-param ObjectSet> $promiseSet - * * @phpstan-return Promise */ - public static function callAsync(AsyncEvent&Event $event, ObjectSet $promiseSet) : Promise; + public function callAsync() : Promise; } diff --git a/src/event/AsyncEventDelegate.php b/src/event/AsyncEventDelegate.php new file mode 100644 index 00000000000..1811e349acd --- /dev/null +++ b/src/event/AsyncEventDelegate.php @@ -0,0 +1,131 @@ +> $promises */ + private ObjectSet $promises; + + public function __construct( + private AsyncEvent&Event $event + ){ + $this->promises = new ObjectSet(); + } + + /** + * @phpstan-return Promise + */ + public function callAsync() : Promise { + $this->promises->clear(); + return $this->callDepth($this->callAsyncDepth(...)); + } + + /** + * @phpstan-return Promise + */ + private function callAsyncDepth() : Promise{ + /** @phpstan-var PromiseResolver $globalResolver */ + $globalResolver = new PromiseResolver(); + + $priorities = EventPriority::ALL; + $testResolve = function () use (&$testResolve, &$priorities, $globalResolver){ + if(count($priorities) === 0){ + $globalResolver->resolve(""); // TODO: see #6110 + }else{ + $this->callPriority(array_shift($priorities))->onCompletion(function() use ($testResolve) : void{ + $testResolve(); + }, function () use ($globalResolver) { + $globalResolver->reject(); + }); + } + }; + + $testResolve(); + + return $globalResolver->getPromise(); + } + + /** + * @phpstan-return Promise + */ + private function callPriority(int $priority) : Promise { + $handlers = HandlerListManager::global()->getListFor($this->event::class)->getListenersByPriority($priority); + + /** @phpstan-var PromiseResolver $resolver */ + $resolver = new PromiseResolver(); + + $nonConcurrentHandlers = []; + foreach($handlers as $registration){ + if($registration instanceof RegisteredAsyncListener){ + if($registration->canBeCallConcurrently()){ + $this->promises->add($registration->callAsync($this->event)); + }else{ + $nonConcurrentHandlers[] = $registration; + } + }else{ + $registration->callEvent($this->event); + } + } + + $testResolve = function() use (&$nonConcurrentHandlers, &$testResolve, $resolver){ + if(count($nonConcurrentHandlers) === 0){ + $this->waitForPromises()->onCompletion(function() use ($resolver){ + $resolver->resolve(""); // TODO: see #6110 + }, function() use ($resolver){ + $resolver->reject(); + }); + }else{ + $this->waitForPromises()->onCompletion(function() use (&$nonConcurrentHandlers, $testResolve) { + $handler = array_shift($nonConcurrentHandlers); + if($handler instanceof RegisteredAsyncListener){ + $this->promises->add($handler->callAsync($this->event)); + } + $testResolve(); + }, function() use ($resolver) { + $resolver->reject(); + }); + } + }; + + $testResolve(); + + return $resolver->getPromise(); + } + + /** + * @phpstan-return Promise + */ + private function waitForPromises() : Promise { + $array = $this->promises->toArray(); + $this->promises->clear(); + + return Promise::all($array); + } +} diff --git a/src/event/AsyncEventTrait.php b/src/event/AsyncEventTrait.php index b907ba5a304..5fd12aca028 100644 --- a/src/event/AsyncEventTrait.php +++ b/src/event/AsyncEventTrait.php @@ -24,61 +24,17 @@ namespace pocketmine\event; use pocketmine\promise\Promise; -use pocketmine\promise\PromiseResolver; -use pocketmine\utils\ObjectSet; -use function array_shift; -use function count; trait AsyncEventTrait { - /** @phpstan-var ObjectSet> */ - private ObjectSet $promises; + private AsyncEventDelegate $delegate; /** - * @phpstan-param ObjectSet>|null $promises + * @phpstan-return Promise */ - private function initializePromises(?ObjectSet &$promises) : void{ - $promises ??= new ObjectSet(); - $this->promises = $promises; - } - - public function addPromise(Promise $promise) : void{ - if(!isset($this->promises)){ - throw new \RuntimeException("Cannot add promises, be sure to initialize the promises set in the constructor"); + final public function callAsync() : Promise{ + if(!isset($this->delegate)){ + $this->delegate = new AsyncEventDelegate($this); } - $this->promises->add($promise); - } - - final public static function callAsync(AsyncEvent&Event $event, ObjectSet $promiseSet) : Promise{ - $event->checkMaxDepthCall(); - - /** @phpstan-var PromiseResolver $globalResolver */ - $globalResolver = new PromiseResolver(); - - $callable = function(int $priority) use ($event, $promiseSet) : Promise{ - $handlers = HandlerListManager::global()->getListFor(static::class)->getListenersByPriority($priority); - $event->callHandlers($handlers); - - $array = $promiseSet->toArray(); - $promiseSet->clear(); - - return Promise::all($array); - }; - - $priorities = EventPriority::ALL; - $testResolve = function () use (&$testResolve, &$priorities, $callable, $globalResolver){ - if(count($priorities) === 0){ - $globalResolver->resolve(null); - }else{ - $callable(array_shift($priorities))->onCompletion(function() use ($testResolve) : void{ - $testResolve(); - }, function () use ($globalResolver) { - $globalResolver->reject(); - }); - } - }; - - $testResolve(); - - return $globalResolver->getPromise(); + return $this->delegate->callAsync(); } } diff --git a/src/event/Event.php b/src/event/Event.php index 6334c16b201..6ba39031a3b 100644 --- a/src/event/Event.php +++ b/src/event/Event.php @@ -47,36 +47,27 @@ final public function getEventName() : string{ * @throws \RuntimeException if event call recursion reaches the max depth limit */ public function call() : void{ - $this->checkMaxDepthCall(); - $this->callHandlers(null); + $this->callDepth(function() { + $handlers = HandlerListManager::global()->getHandlersFor(static::class); + + foreach($handlers as $registration){ + $registration->callEvent($this); + } + }); } - /** - * @internal used by AsyncEventTrait and Event - */ - final protected function checkMaxDepthCall() : void{ + final protected function callDepth(\Closure $closure) : mixed{ if(self::$eventCallDepth >= self::MAX_EVENT_CALL_DEPTH){ //this exception will be caught by the parent event call if all else fails throw new \RuntimeException("Recursive event call detected (reached max depth of " . self::MAX_EVENT_CALL_DEPTH . " calls)"); } - } - /** - * @param RegisteredListener[]|null $handlers - * - * @internal used by AsyncEventTrait and Event - */ - final protected function callHandlers(?array $handlers) : void{ $timings = Timings::getEventTimings($this); $timings->startTiming(); - $handlers = $handlers ?? HandlerListManager::global()->getHandlersFor(static::class); - ++self::$eventCallDepth; try{ - foreach($handlers as $registration){ - $registration->callEvent($this); - } + return $closure(); }finally{ --self::$eventCallDepth; $timings->stopTiming(); diff --git a/src/event/HandlerList.php b/src/event/HandlerList.php index 74eedf3a4a2..b99a128542d 100644 --- a/src/event/HandlerList.php +++ b/src/event/HandlerList.php @@ -24,9 +24,11 @@ namespace pocketmine\event; use pocketmine\plugin\Plugin; +use function array_filter; use function array_merge; use function krsort; use function spl_object_id; +use function usort; use const SORT_NUMERIC; class HandlerList{ @@ -128,11 +130,26 @@ public function getListenerList() : array{ } $listenersByPriority = []; + $asyncListenersByPriority = []; foreach($handlerLists as $currentList){ foreach($currentList->handlerSlots as $priority => $listeners){ - $listenersByPriority[$priority] = array_merge($listenersByPriority[$priority] ?? [], $listeners); + $syncListeners = array_filter($listeners, static function(RegisteredListener $listener) : bool{ return !($listener instanceof RegisteredAsyncListener); }); + $listenersByPriority[$priority] = array_merge($listenersByPriority[$priority] ?? [], $syncListeners); + + $asyncListeners = array_filter($listeners, static function(RegisteredListener $listener) : bool{ return $listener instanceof RegisteredAsyncListener; }); + $asyncListenersByPriority[$priority] = array_merge($asyncListenersByPriority[$priority] ?? [], $asyncListeners); } } + foreach($asyncListenersByPriority as $priority => $asyncListeners){ + usort($asyncListeners, static function(RegisteredAsyncListener $a, RegisteredAsyncListener $b) : int{ + if($a->canBeCallConcurrently()){ + return $b->canBeCallConcurrently() ? 0 : -1; + }else{ + return $b->canBeCallConcurrently() ? -1 : 0; + } + }); + $listenersByPriority[$priority] = array_merge($listenersByPriority[$priority] ?? [], $asyncListeners); + } //TODO: why on earth do the priorities have higher values for lower priority? krsort($listenersByPriority, SORT_NUMERIC); diff --git a/src/event/ListenerMethodTags.php b/src/event/ListenerMethodTags.php index cb932ce27ac..0941fb94a98 100644 --- a/src/event/ListenerMethodTags.php +++ b/src/event/ListenerMethodTags.php @@ -31,4 +31,5 @@ final class ListenerMethodTags{ public const HANDLE_CANCELLED = "handleCancelled"; public const NOT_HANDLER = "notHandler"; public const PRIORITY = "priority"; + public const NO_CONCURRENT_CALL = "noConcurrentCall"; } diff --git a/src/event/RegisteredAsyncListener.php b/src/event/RegisteredAsyncListener.php new file mode 100644 index 00000000000..e5bbde84b21 --- /dev/null +++ b/src/event/RegisteredAsyncListener.php @@ -0,0 +1,58 @@ +returnPromise = $handler($event); + if(!$this->returnPromise instanceof Promise){ + throw new \TypeError("Async event handler must return a Promise"); + } + }; + parent::__construct($handler, $priority, $plugin, $handleCancelled, $timings); + } + + public function canBeCallConcurrently() : bool{ + return !$this->noConcurrentCall; + } + + public function callAsync(AsyncEvent&Event $event) : Promise{ + $this->callEvent($event); + return $this->returnPromise; + } +} diff --git a/src/event/player/PlayerChatEvent.php b/src/event/player/PlayerChatEvent.php index 1cdbb49fb80..87151bc2383 100644 --- a/src/event/player/PlayerChatEvent.php +++ b/src/event/player/PlayerChatEvent.php @@ -24,6 +24,8 @@ namespace pocketmine\event\player; use pocketmine\command\CommandSender; +use pocketmine\event\AsyncEvent; +use pocketmine\event\AsyncEventTrait; use pocketmine\event\Cancellable; use pocketmine\event\CancellableTrait; use pocketmine\player\chat\ChatFormatter; @@ -33,8 +35,9 @@ /** * Called when a player chats something */ -class PlayerChatEvent extends PlayerEvent implements Cancellable{ +class PlayerChatEvent extends PlayerEvent implements Cancellable, AsyncEvent{ use CancellableTrait; + use AsyncEventTrait; /** * @param CommandSender[] $recipients diff --git a/src/player/Player.php b/src/player/Player.php index 0afa0bffe36..9969ec8be4b 100644 --- a/src/player/Player.php +++ b/src/player/Player.php @@ -1513,10 +1513,14 @@ public function chat(string $message) : bool{ Timings::$playerCommand->stopTiming(); }else{ $ev = new PlayerChatEvent($this, $messagePart, $this->server->getBroadcastChannelSubscribers(Server::BROADCAST_CHANNEL_USERS), new StandardChatFormatter()); - $ev->call(); - if(!$ev->isCancelled()){ - $this->server->broadcastMessage($ev->getFormatter()->format($ev->getPlayer()->getDisplayName(), $ev->getMessage()), $ev->getRecipients()); - } + $ev->callAsync() + ->onCompletion(function() use ($ev) { + if(!$ev->isCancelled()){ + $this->server->broadcastMessage($ev->getFormatter()->format($ev->getPlayer()->getDisplayName(), $ev->getMessage()), $ev->getRecipients()); + } + }, function (){ + }); + } } } diff --git a/src/plugin/PluginManager.php b/src/plugin/PluginManager.php index 67ca8cc374e..f918d916a68 100644 --- a/src/plugin/PluginManager.php +++ b/src/plugin/PluginManager.php @@ -23,6 +23,7 @@ namespace pocketmine\plugin; +use pocketmine\event\AsyncEvent; use pocketmine\event\Cancellable; use pocketmine\event\Event; use pocketmine\event\EventPriority; @@ -31,11 +32,13 @@ use pocketmine\event\ListenerMethodTags; use pocketmine\event\plugin\PluginDisableEvent; use pocketmine\event\plugin\PluginEnableEvent; +use pocketmine\event\RegisteredAsyncListener; use pocketmine\event\RegisteredListener; use pocketmine\lang\KnownTranslationFactory; use pocketmine\permission\DefaultPermissions; use pocketmine\permission\PermissionManager; use pocketmine\permission\PermissionParser; +use pocketmine\promise\Promise; use pocketmine\Server; use pocketmine\timings\Timings; use pocketmine\utils\AssumptionFailedError; @@ -626,8 +629,29 @@ public function registerEvents(Listener $listener, Plugin $plugin) : void{ throw new PluginException("Event handler " . Utils::getNiceClosureName($handlerClosure) . "() declares invalid @" . ListenerMethodTags::HANDLE_CANCELLED . " value \"" . $tags[ListenerMethodTags::HANDLE_CANCELLED] . "\""); } } + $noConcurrentCall = false; + if(isset($tags[ListenerMethodTags::NO_CONCURRENT_CALL])){ + if(!is_a($eventClass, AsyncEvent::class, true)){ + throw new PluginException(sprintf( + "Event handler %s() declares @%s for non-async event of type %s", + Utils::getNiceClosureName($handlerClosure), + ListenerMethodTags::NO_CONCURRENT_CALL, + $eventClass + )); + } + switch(strtolower($tags[ListenerMethodTags::NO_CONCURRENT_CALL])){ + case "true": + case "": + $noConcurrentCall = true; + break; + case "false": + break; + default: + throw new PluginException("Event handler " . Utils::getNiceClosureName($handlerClosure) . "() declares invalid @" . ListenerMethodTags::NO_CONCURRENT_CALL . " value \"" . $tags[ListenerMethodTags::NO_CONCURRENT_CALL] . "\""); + } + } - $this->registerEvent($eventClass, $handlerClosure, $priority, $plugin, $handleCancelled); + $this->registerEvent($eventClass, $handlerClosure, $priority, $plugin, $handleCancelled, $noConcurrentCall); } } @@ -636,11 +660,11 @@ public function registerEvents(Listener $listener, Plugin $plugin) : void{ * * @phpstan-template TEvent of Event * @phpstan-param class-string $event - * @phpstan-param \Closure(TEvent) : void $handler + * @phpstan-param (\Closure(TEvent) : void)|(\Closure(AsyncEvent&TEvent) : Promise) $handler * * @throws \ReflectionException */ - public function registerEvent(string $event, \Closure $handler, int $priority, Plugin $plugin, bool $handleCancelled = false) : RegisteredListener{ + public function registerEvent(string $event, \Closure $handler, int $priority, Plugin $plugin, bool $handleCancelled = false, bool $noConcurrentCall = false) : RegisteredListener{ if(!is_subclass_of($event, Event::class)){ throw new PluginException($event . " is not an Event"); } @@ -653,8 +677,25 @@ public function registerEvent(string $event, \Closure $handler, int $priority, P $timings = Timings::getEventHandlerTimings($event, $handlerName, $plugin->getDescription()->getFullName()); - $registeredListener = new RegisteredListener($handler, $priority, $plugin, $handleCancelled, $timings); + if(is_subclass_of($event, AsyncEvent::class) && $this->canHandleAsyncEvent($handler)){ + $registeredListener = new RegisteredAsyncListener($handler, $priority, $plugin, $handleCancelled, $noConcurrentCall, $timings); + }else{ + $registeredListener = new RegisteredListener($handler, $priority, $plugin, $handleCancelled, $timings); + } HandlerListManager::global()->getListFor($event)->register($registeredListener); return $registeredListener; } + + /** + * Check if the given handler return type is async-compatible (equal to Promise) + * + * @phpstan-template TEvent of Event + * @phpstan-param (\Closure(TEvent) : void)|(\Closure(AsyncEvent&TEvent) : Promise) $handler + */ + private function canHandleAsyncEvent(\Closure $handler) : bool { + $reflection = new \ReflectionFunction($handler); + $return = $reflection->getReturnType(); + + return $return instanceof \ReflectionNamedType && $return->getName() === Promise::class; + } } From 9b2b92ac1faeab03d18992baabf67689cdc47cd4 Mon Sep 17 00:00:00 2001 From: ShockedPlot7560 Date: Sun, 22 Oct 2023 15:05:06 +0200 Subject: [PATCH 04/47] oops, remove test code --- src/event/player/PlayerChatEvent.php | 5 +---- src/player/Player.php | 11 ++++------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/event/player/PlayerChatEvent.php b/src/event/player/PlayerChatEvent.php index 87151bc2383..1cdbb49fb80 100644 --- a/src/event/player/PlayerChatEvent.php +++ b/src/event/player/PlayerChatEvent.php @@ -24,8 +24,6 @@ namespace pocketmine\event\player; use pocketmine\command\CommandSender; -use pocketmine\event\AsyncEvent; -use pocketmine\event\AsyncEventTrait; use pocketmine\event\Cancellable; use pocketmine\event\CancellableTrait; use pocketmine\player\chat\ChatFormatter; @@ -35,9 +33,8 @@ /** * Called when a player chats something */ -class PlayerChatEvent extends PlayerEvent implements Cancellable, AsyncEvent{ +class PlayerChatEvent extends PlayerEvent implements Cancellable{ use CancellableTrait; - use AsyncEventTrait; /** * @param CommandSender[] $recipients diff --git a/src/player/Player.php b/src/player/Player.php index 9969ec8be4b..c9af9bb94b6 100644 --- a/src/player/Player.php +++ b/src/player/Player.php @@ -1513,13 +1513,10 @@ public function chat(string $message) : bool{ Timings::$playerCommand->stopTiming(); }else{ $ev = new PlayerChatEvent($this, $messagePart, $this->server->getBroadcastChannelSubscribers(Server::BROADCAST_CHANNEL_USERS), new StandardChatFormatter()); - $ev->callAsync() - ->onCompletion(function() use ($ev) { - if(!$ev->isCancelled()){ - $this->server->broadcastMessage($ev->getFormatter()->format($ev->getPlayer()->getDisplayName(), $ev->getMessage()), $ev->getRecipients()); - } - }, function (){ - }); + $ev->call(); + if(!$ev->isCancelled()){ + $this->server->broadcastMessage($ev->getFormatter()->format($ev->getPlayer()->getDisplayName(), $ev->getMessage()), $ev->getRecipients()); + } } } From b78ff0041811abed22e17f33c463373b34855771 Mon Sep 17 00:00:00 2001 From: ShockedPlot7560 Date: Sun, 22 Oct 2023 15:06:17 +0200 Subject: [PATCH 05/47] fix style --- src/event/AsyncEventDelegate.php | 8 ++++---- src/event/Event.php | 2 +- src/event/RegisteredAsyncListener.php | 2 +- src/plugin/PluginManager.php | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/event/AsyncEventDelegate.php b/src/event/AsyncEventDelegate.php index 1811e349acd..661ae8f3ac3 100644 --- a/src/event/AsyncEventDelegate.php +++ b/src/event/AsyncEventDelegate.php @@ -42,7 +42,7 @@ public function __construct( /** * @phpstan-return Promise */ - public function callAsync() : Promise { + public function callAsync() : Promise{ $this->promises->clear(); return $this->callDepth($this->callAsyncDepth(...)); } @@ -75,7 +75,7 @@ private function callAsyncDepth() : Promise{ /** * @phpstan-return Promise */ - private function callPriority(int $priority) : Promise { + private function callPriority(int $priority) : Promise{ $handlers = HandlerListManager::global()->getListFor($this->event::class)->getListenersByPriority($priority); /** @phpstan-var PromiseResolver $resolver */ @@ -102,7 +102,7 @@ private function callPriority(int $priority) : Promise { $resolver->reject(); }); }else{ - $this->waitForPromises()->onCompletion(function() use (&$nonConcurrentHandlers, $testResolve) { + $this->waitForPromises()->onCompletion(function() use (&$nonConcurrentHandlers, $testResolve){ $handler = array_shift($nonConcurrentHandlers); if($handler instanceof RegisteredAsyncListener){ $this->promises->add($handler->callAsync($this->event)); @@ -122,7 +122,7 @@ private function callPriority(int $priority) : Promise { /** * @phpstan-return Promise */ - private function waitForPromises() : Promise { + private function waitForPromises() : Promise{ $array = $this->promises->toArray(); $this->promises->clear(); diff --git a/src/event/Event.php b/src/event/Event.php index 6ba39031a3b..660290a6087 100644 --- a/src/event/Event.php +++ b/src/event/Event.php @@ -47,7 +47,7 @@ final public function getEventName() : string{ * @throws \RuntimeException if event call recursion reaches the max depth limit */ public function call() : void{ - $this->callDepth(function() { + $this->callDepth(function(){ $handlers = HandlerListManager::global()->getHandlersFor(static::class); foreach($handlers as $registration){ diff --git a/src/event/RegisteredAsyncListener.php b/src/event/RegisteredAsyncListener.php index e5bbde84b21..a9fd77f80e3 100644 --- a/src/event/RegisteredAsyncListener.php +++ b/src/event/RegisteredAsyncListener.php @@ -38,7 +38,7 @@ public function __construct( private bool $noConcurrentCall, TimingsHandler $timings ){ - $handler = function(AsyncEvent&Event $event) use($handler) : void { + $handler = function(AsyncEvent&Event $event) use($handler) : void{ $this->returnPromise = $handler($event); if(!$this->returnPromise instanceof Promise){ throw new \TypeError("Async event handler must return a Promise"); diff --git a/src/plugin/PluginManager.php b/src/plugin/PluginManager.php index f918d916a68..9d0d428271e 100644 --- a/src/plugin/PluginManager.php +++ b/src/plugin/PluginManager.php @@ -692,7 +692,7 @@ public function registerEvent(string $event, \Closure $handler, int $priority, P * @phpstan-template TEvent of Event * @phpstan-param (\Closure(TEvent) : void)|(\Closure(AsyncEvent&TEvent) : Promise) $handler */ - private function canHandleAsyncEvent(\Closure $handler) : bool { + private function canHandleAsyncEvent(\Closure $handler) : bool{ $reflection = new \ReflectionFunction($handler); $return = $reflection->getReturnType(); From c250bb0da7848fb99f465f0d8fd8d85b7fe8612b Mon Sep 17 00:00:00 2001 From: ShockedPlot7560 Date: Sun, 22 Oct 2023 15:08:50 +0200 Subject: [PATCH 06/47] undo Promise covariant + improve array types --- src/promise/Promise.php | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/promise/Promise.php b/src/promise/Promise.php index aae08159f19..2601a3cbf2b 100644 --- a/src/promise/Promise.php +++ b/src/promise/Promise.php @@ -27,7 +27,7 @@ use function spl_object_id; /** - * @phpstan-template-covariant TValue + * @phpstan-template TValue */ final class Promise{ /** @@ -61,20 +61,19 @@ public function isResolved() : bool{ * `$promises` indexed by the respective Promises' array keys. * * @template TPromiseValue - * @phpstan-param Promise[] $promises + * @template TKey of array-key + * @phpstan-param array> $promises * - * @phpstan-return Promise> + * @phpstan-return Promise> */ public static function all(array $promises) : Promise { - /** @phpstan-var PromiseResolver> $resolver */ + /** @phpstan-var PromiseResolver> $resolver */ $resolver = new PromiseResolver(); $values = []; $toResolve = count($promises); $continue = true; foreach($promises as $key => $promise){ - $values[$key] = null; - $promise->onCompletion( function(mixed $value) use ($resolver, $key, &$toResolve, &$continue, &$values) : void{ $values[$key] = $value; From 58155a77fb5dff99d2e018c106b70855b689c4b1 Mon Sep 17 00:00:00 2001 From: ShockedPlot7560 Date: Sun, 22 Oct 2023 15:18:39 +0200 Subject: [PATCH 07/47] fix PHPstan --- src/event/AsyncEventDelegate.php | 2 +- src/event/Event.php | 5 +++++ src/event/RegisteredAsyncListener.php | 7 +++++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/event/AsyncEventDelegate.php b/src/event/AsyncEventDelegate.php index 661ae8f3ac3..e598c2d54b3 100644 --- a/src/event/AsyncEventDelegate.php +++ b/src/event/AsyncEventDelegate.php @@ -120,7 +120,7 @@ private function callPriority(int $priority) : Promise{ } /** - * @phpstan-return Promise + * @phpstan-return Promise> */ private function waitForPromises() : Promise{ $array = $this->promises->toArray(); diff --git a/src/event/Event.php b/src/event/Event.php index 660290a6087..77746c8e81d 100644 --- a/src/event/Event.php +++ b/src/event/Event.php @@ -56,6 +56,11 @@ public function call() : void{ }); } + /** + * @template T + * @phpstan-param \Closure() : T $closure + * @phpstan-return T + */ final protected function callDepth(\Closure $closure) : mixed{ if(self::$eventCallDepth >= self::MAX_EVENT_CALL_DEPTH){ //this exception will be caught by the parent event call if all else fails diff --git a/src/event/RegisteredAsyncListener.php b/src/event/RegisteredAsyncListener.php index a9fd77f80e3..3f492add377 100644 --- a/src/event/RegisteredAsyncListener.php +++ b/src/event/RegisteredAsyncListener.php @@ -28,8 +28,12 @@ use pocketmine\timings\TimingsHandler; class RegisteredAsyncListener extends RegisteredListener{ + /** @phpstan-var Promise $returnPromise */ private Promise $returnPromise; + /** + * @phpstan-param \Closure(AsyncEvent&Event) : Promise $handler + */ public function __construct( \Closure $handler, int $priority, @@ -51,6 +55,9 @@ public function canBeCallConcurrently() : bool{ return !$this->noConcurrentCall; } + /** + * @phpstan-return Promise + */ public function callAsync(AsyncEvent&Event $event) : Promise{ $this->callEvent($event); return $this->returnPromise; From 2b2fa9ddf18bac2e5f7b12fe2a2ee43413d1ea0e Mon Sep 17 00:00:00 2001 From: ShockedPlot7560 Date: Sun, 22 Oct 2023 15:45:05 +0200 Subject: [PATCH 08/47] phpstan: populate baseline --- tests/phpstan/configs/actual-problems.neon | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/phpstan/configs/actual-problems.neon b/tests/phpstan/configs/actual-problems.neon index 4278cc3a81d..a2ff4519b68 100644 --- a/tests/phpstan/configs/actual-problems.neon +++ b/tests/phpstan/configs/actual-problems.neon @@ -1210,3 +1210,14 @@ parameters: count: 1 path: ../../../src/promise/Promise.php + - + message: "#^Parameter \\#1 \\$value of method pocketmine\\\\promise\\\\PromiseResolver\\\\:\\:resolve\\(\\) expects null, string given\\.$#" + count: 2 + path: ../../../src/event/AsyncEventDelegate.php + + - + message: "#^Parameter \\#1 \\$handler of class pocketmine\\\\event\\\\RegisteredAsyncListener constructor expects Closure\\(pocketmine\\\\event\\\\AsyncEvent&pocketmine\\\\event\\\\Event\\)\\: pocketmine\\\\promise\\\\Promise\\, \\(Closure\\(TEvent\\)\\: void\\)\\|\\(Closure\\(pocketmine\\\\event\\\\AsyncEvent&TEvent\\)\\: pocketmine\\\\promise\\\\Promise\\\\) given\\.$#" + count: 1 + path: ../../../src/plugin/PluginManager.php + + From 1176b7090cffab9652678fcbe6ec506afc8eeae0 Mon Sep 17 00:00:00 2001 From: Dylan T Date: Mon, 23 Oct 2023 17:31:53 +0100 Subject: [PATCH 09/47] Update src/player/Player.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Javier León <58715544+JavierLeon9966@users.noreply.github.com> --- src/player/Player.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/player/Player.php b/src/player/Player.php index c9af9bb94b6..0afa0bffe36 100644 --- a/src/player/Player.php +++ b/src/player/Player.php @@ -1517,7 +1517,6 @@ public function chat(string $message) : bool{ if(!$ev->isCancelled()){ $this->server->broadcastMessage($ev->getFormatter()->format($ev->getPlayer()->getDisplayName(), $ev->getMessage()), $ev->getRecipients()); } - } } } From ed739cff4ff47349b40b536a68fe52cb3a526f64 Mon Sep 17 00:00:00 2001 From: ShockedPlot7560 Date: Fri, 27 Oct 2023 21:37:56 +0200 Subject: [PATCH 10/47] cannot call async event in sync context + remove Event dependency for AsyncEventDelegate --- src/event/AsyncEventDelegate.php | 28 +++++++++++++++++++++++++--- src/event/AsyncEventTrait.php | 5 +---- src/event/Event.php | 24 ++++++++---------------- src/timings/Timings.php | 12 ++++++++++++ 4 files changed, 46 insertions(+), 23 deletions(-) diff --git a/src/event/AsyncEventDelegate.php b/src/event/AsyncEventDelegate.php index e598c2d54b3..c12ed8fc293 100644 --- a/src/event/AsyncEventDelegate.php +++ b/src/event/AsyncEventDelegate.php @@ -25,13 +25,17 @@ use pocketmine\promise\Promise; use pocketmine\promise\PromiseResolver; +use pocketmine\timings\Timings; use pocketmine\utils\ObjectSet; use function array_shift; use function count; -final class AsyncEventDelegate extends Event{ +final class AsyncEventDelegate{ /** @phpstan-var ObjectSet> $promises */ private ObjectSet $promises; + /** @var array, int> $delegatesCall */ + private static array $delegatesCallDepth = []; + private const MAX_EVENT_CALL_DEPTH = 50; public function __construct( private AsyncEvent&Event $event @@ -42,9 +46,27 @@ public function __construct( /** * @phpstan-return Promise */ - public function callAsync() : Promise{ + public function call() : Promise{ $this->promises->clear(); - return $this->callDepth($this->callAsyncDepth(...)); + if(!isset(self::$delegatesCallDepth[$class = $this->event::class])){ + self::$delegatesCallDepth[$class] = 0; + } + + if(self::$delegatesCallDepth[$class] >= self::MAX_EVENT_CALL_DEPTH){ + //this exception will be caught by the parent event call if all else fails + throw new \RuntimeException("Recursive event call detected (reached max depth of " . self::MAX_EVENT_CALL_DEPTH . " calls)"); + } + + $timings = Timings::getAsyncEventTimings($this->event); + $timings->startTiming(); + + ++self::$delegatesCallDepth[$class]; + try{ + return $this->callAsyncDepth(); + }finally{ + --self::$delegatesCallDepth[$class]; + $timings->stopTiming(); + } } /** diff --git a/src/event/AsyncEventTrait.php b/src/event/AsyncEventTrait.php index 5fd12aca028..c5da9bac4b9 100644 --- a/src/event/AsyncEventTrait.php +++ b/src/event/AsyncEventTrait.php @@ -32,9 +32,6 @@ trait AsyncEventTrait { * @phpstan-return Promise */ final public function callAsync() : Promise{ - if(!isset($this->delegate)){ - $this->delegate = new AsyncEventDelegate($this); - } - return $this->delegate->callAsync(); + return (new AsyncEventDelegate($this))->call(); } } diff --git a/src/event/Event.php b/src/event/Event.php index 77746c8e81d..7d1d19d706f 100644 --- a/src/event/Event.php +++ b/src/event/Event.php @@ -47,32 +47,24 @@ final public function getEventName() : string{ * @throws \RuntimeException if event call recursion reaches the max depth limit */ public function call() : void{ - $this->callDepth(function(){ - $handlers = HandlerListManager::global()->getHandlersFor(static::class); - - foreach($handlers as $registration){ - $registration->callEvent($this); - } - }); - } - - /** - * @template T - * @phpstan-param \Closure() : T $closure - * @phpstan-return T - */ - final protected function callDepth(\Closure $closure) : mixed{ if(self::$eventCallDepth >= self::MAX_EVENT_CALL_DEPTH){ //this exception will be caught by the parent event call if all else fails throw new \RuntimeException("Recursive event call detected (reached max depth of " . self::MAX_EVENT_CALL_DEPTH . " calls)"); } + if($this instanceof AsyncEvent){ + throw new \InvalidArgumentException("Cannot call async event synchronously"); + } $timings = Timings::getEventTimings($this); $timings->startTiming(); + $handlers = HandlerListManager::global()->getHandlersFor(static::class); + ++self::$eventCallDepth; try{ - return $closure(); + foreach($handlers as $registration){ + $registration->callEvent($this); + } }finally{ --self::$eventCallDepth; $timings->stopTiming(); diff --git a/src/timings/Timings.php b/src/timings/Timings.php index 0d8a256dea5..e8d37c8983a 100644 --- a/src/timings/Timings.php +++ b/src/timings/Timings.php @@ -25,6 +25,7 @@ use pocketmine\block\tile\Tile; use pocketmine\entity\Entity; +use pocketmine\event\AsyncEvent; use pocketmine\event\Event; use pocketmine\network\mcpe\protocol\ClientboundPacket; use pocketmine\network\mcpe\protocol\ServerboundPacket; @@ -113,6 +114,8 @@ abstract class Timings{ /** @var TimingsHandler[] */ private static array $events = []; + /** @var TimingsHandler[] */ + private static array $asyncEvents = []; /** @var TimingsHandler[][] */ private static array $eventHandlers = []; @@ -304,6 +307,15 @@ public static function getEventTimings(Event $event) : TimingsHandler{ return self::$events[$eventClass]; } + public static function getAsyncEventTimings(AsyncEvent&Event $event) : TimingsHandler{ + $eventClass = get_class($event); + if(!isset(self::$asyncEvents[$eventClass])){ + self::$asyncEvents[$eventClass] = new TimingsHandler(self::shortenCoreClassName($eventClass, "pocketmine\\event\\"), group: "Events"); + } + + return self::$asyncEvents[$eventClass]; + } + /** * @phpstan-template TEvent of Event * @phpstan-param class-string $event From 7e87fbbb7a0381f45b8920a413bf1b635fc57aab Mon Sep 17 00:00:00 2001 From: ShockedPlot7560 Date: Fri, 27 Oct 2023 21:39:18 +0200 Subject: [PATCH 11/47] clarifying the exception message --- src/event/Event.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/event/Event.php b/src/event/Event.php index 7d1d19d706f..0dd16aa64d3 100644 --- a/src/event/Event.php +++ b/src/event/Event.php @@ -52,7 +52,7 @@ public function call() : void{ throw new \RuntimeException("Recursive event call detected (reached max depth of " . self::MAX_EVENT_CALL_DEPTH . " calls)"); } if($this instanceof AsyncEvent){ - throw new \InvalidArgumentException("Cannot call async event synchronously"); + throw new \InvalidArgumentException("Cannot call async event synchronously, use callAsync() instead"); } $timings = Timings::getEventTimings($this); From 5beaa3ce4e9a1e807cfc8e9aa0fbb21569a116eb Mon Sep 17 00:00:00 2001 From: ShockedPlot7560 Date: Fri, 27 Oct 2023 21:45:22 +0200 Subject: [PATCH 12/47] correction of various problems --- src/event/AsyncEventDelegate.php | 2 +- src/event/AsyncEventTrait.php | 2 -- src/event/HandlerList.php | 7 ++----- src/event/RegisteredAsyncListener.php | 2 +- 4 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/event/AsyncEventDelegate.php b/src/event/AsyncEventDelegate.php index c12ed8fc293..6c20ad0b92b 100644 --- a/src/event/AsyncEventDelegate.php +++ b/src/event/AsyncEventDelegate.php @@ -106,7 +106,7 @@ private function callPriority(int $priority) : Promise{ $nonConcurrentHandlers = []; foreach($handlers as $registration){ if($registration instanceof RegisteredAsyncListener){ - if($registration->canBeCallConcurrently()){ + if($registration->canBeCalledConcurrently()){ $this->promises->add($registration->callAsync($this->event)); }else{ $nonConcurrentHandlers[] = $registration; diff --git a/src/event/AsyncEventTrait.php b/src/event/AsyncEventTrait.php index c5da9bac4b9..9cea1d36c94 100644 --- a/src/event/AsyncEventTrait.php +++ b/src/event/AsyncEventTrait.php @@ -26,8 +26,6 @@ use pocketmine\promise\Promise; trait AsyncEventTrait { - private AsyncEventDelegate $delegate; - /** * @phpstan-return Promise */ diff --git a/src/event/HandlerList.php b/src/event/HandlerList.php index b99a128542d..faf48f43dde 100644 --- a/src/event/HandlerList.php +++ b/src/event/HandlerList.php @@ -142,11 +142,8 @@ public function getListenerList() : array{ } foreach($asyncListenersByPriority as $priority => $asyncListeners){ usort($asyncListeners, static function(RegisteredAsyncListener $a, RegisteredAsyncListener $b) : int{ - if($a->canBeCallConcurrently()){ - return $b->canBeCallConcurrently() ? 0 : -1; - }else{ - return $b->canBeCallConcurrently() ? -1 : 0; - } + // concurrent listeners are sorted to the end of the list + return $b->canBeCalledConcurrently() <=> $a->canBeCalledConcurrently(); }); $listenersByPriority[$priority] = array_merge($listenersByPriority[$priority] ?? [], $asyncListeners); } diff --git a/src/event/RegisteredAsyncListener.php b/src/event/RegisteredAsyncListener.php index 3f492add377..ce235cbe2e7 100644 --- a/src/event/RegisteredAsyncListener.php +++ b/src/event/RegisteredAsyncListener.php @@ -51,7 +51,7 @@ public function __construct( parent::__construct($handler, $priority, $plugin, $handleCancelled, $timings); } - public function canBeCallConcurrently() : bool{ + public function canBeCalledConcurrently() : bool{ return !$this->noConcurrentCall; } From cc6e8ef2327da14ab26d427bbf4e2576071065b3 Mon Sep 17 00:00:00 2001 From: ShockedPlot7560 Date: Fri, 27 Oct 2023 21:58:43 +0200 Subject: [PATCH 13/47] move the asynchronous registration of handlers to a dedicated PluginManager function --- src/event/ListenerMethodTags.php | 2 +- src/plugin/PluginManager.php | 63 ++++++++++++++++++++++++-------- 2 files changed, 49 insertions(+), 16 deletions(-) diff --git a/src/event/ListenerMethodTags.php b/src/event/ListenerMethodTags.php index 0941fb94a98..e65f25f80ba 100644 --- a/src/event/ListenerMethodTags.php +++ b/src/event/ListenerMethodTags.php @@ -31,5 +31,5 @@ final class ListenerMethodTags{ public const HANDLE_CANCELLED = "handleCancelled"; public const NOT_HANDLER = "notHandler"; public const PRIORITY = "priority"; - public const NO_CONCURRENT_CALL = "noConcurrentCall"; + public const EXCLUSIVE_CALL = "exclusiveCall"; } diff --git a/src/plugin/PluginManager.php b/src/plugin/PluginManager.php index 9d0d428271e..b1050a97118 100644 --- a/src/plugin/PluginManager.php +++ b/src/plugin/PluginManager.php @@ -629,29 +629,33 @@ public function registerEvents(Listener $listener, Plugin $plugin) : void{ throw new PluginException("Event handler " . Utils::getNiceClosureName($handlerClosure) . "() declares invalid @" . ListenerMethodTags::HANDLE_CANCELLED . " value \"" . $tags[ListenerMethodTags::HANDLE_CANCELLED] . "\""); } } - $noConcurrentCall = false; - if(isset($tags[ListenerMethodTags::NO_CONCURRENT_CALL])){ + $exclusiveCall = false; + if(isset($tags[ListenerMethodTags::EXCLUSIVE_CALL])){ if(!is_a($eventClass, AsyncEvent::class, true)){ throw new PluginException(sprintf( "Event handler %s() declares @%s for non-async event of type %s", Utils::getNiceClosureName($handlerClosure), - ListenerMethodTags::NO_CONCURRENT_CALL, + ListenerMethodTags::EXCLUSIVE_CALL, $eventClass )); } - switch(strtolower($tags[ListenerMethodTags::NO_CONCURRENT_CALL])){ + switch(strtolower($tags[ListenerMethodTags::EXCLUSIVE_CALL])){ case "true": case "": - $noConcurrentCall = true; + $exclusiveCall = true; break; case "false": break; default: - throw new PluginException("Event handler " . Utils::getNiceClosureName($handlerClosure) . "() declares invalid @" . ListenerMethodTags::NO_CONCURRENT_CALL . " value \"" . $tags[ListenerMethodTags::NO_CONCURRENT_CALL] . "\""); + throw new PluginException("Event handler " . Utils::getNiceClosureName($handlerClosure) . "() declares invalid @" . ListenerMethodTags::EXCLUSIVE_CALL . " value \"" . $tags[ListenerMethodTags::EXCLUSIVE_CALL] . "\""); } } - $this->registerEvent($eventClass, $handlerClosure, $priority, $plugin, $handleCancelled, $noConcurrentCall); + if(is_subclass_of($eventClass, AsyncEvent::class) && $this->canHandleAsyncEvent($handlerClosure)){ + $this->registerAsyncEvent($eventClass, $handlerClosure, $priority, $plugin, $handleCancelled, $exclusiveCall); + }else{ + $this->registerEvent($eventClass, $handlerClosure, $priority, $plugin, $handleCancelled); + } } } @@ -660,11 +664,11 @@ public function registerEvents(Listener $listener, Plugin $plugin) : void{ * * @phpstan-template TEvent of Event * @phpstan-param class-string $event - * @phpstan-param (\Closure(TEvent) : void)|(\Closure(AsyncEvent&TEvent) : Promise) $handler + * @phpstan-param \Closure(TEvent) : void $handler * * @throws \ReflectionException */ - public function registerEvent(string $event, \Closure $handler, int $priority, Plugin $plugin, bool $handleCancelled = false, bool $noConcurrentCall = false) : RegisteredListener{ + public function registerEvent(string $event, \Closure $handler, int $priority, Plugin $plugin, bool $handleCancelled = false) : RegisteredListener{ if(!is_subclass_of($event, Event::class)){ throw new PluginException($event . " is not an Event"); } @@ -677,11 +681,38 @@ public function registerEvent(string $event, \Closure $handler, int $priority, P $timings = Timings::getEventHandlerTimings($event, $handlerName, $plugin->getDescription()->getFullName()); - if(is_subclass_of($event, AsyncEvent::class) && $this->canHandleAsyncEvent($handler)){ - $registeredListener = new RegisteredAsyncListener($handler, $priority, $plugin, $handleCancelled, $noConcurrentCall, $timings); - }else{ - $registeredListener = new RegisteredListener($handler, $priority, $plugin, $handleCancelled, $timings); + $registeredListener = new RegisteredListener($handler, $priority, $plugin, $handleCancelled, $timings); + HandlerListManager::global()->getListFor($event)->register($registeredListener); + return $registeredListener; + } + + /** + * @param string $event Class name that extends Event and AsyncEvent + * + * @phpstan-template TEvent of Event&AsyncEvent + * @phpstan-param class-string $event + * @phpstan-param \Closure(TEvent) : Promise $handler + * + * @throws \ReflectionException + */ + public function registerAsyncEvent(string $event, \Closure $handler, int $priority, Plugin $plugin, bool $handleCancelled = false, bool $exclusiveCall = false) : RegisteredAsyncListener{ + if(!is_subclass_of($event, Event::class)){ + throw new PluginException($event . " is not an Event"); } + + if(!is_subclass_of($event, AsyncEvent::class)){ + throw new PluginException($event . " is not an AsyncEvent"); + } + + $handlerName = Utils::getNiceClosureName($handler); + + if(!$plugin->isEnabled()){ + throw new PluginException("Plugin attempted to register event handler " . $handlerName . "() to event " . $event . " while not enabled"); + } + + $timings = Timings::getEventHandlerTimings($event, $handlerName, $plugin->getDescription()->getFullName()); + + $registeredListener = new RegisteredAsyncListener($handler, $priority, $plugin, $handleCancelled, $exclusiveCall, $timings); HandlerListManager::global()->getListFor($event)->register($registeredListener); return $registeredListener; } @@ -689,8 +720,10 @@ public function registerEvent(string $event, \Closure $handler, int $priority, P /** * Check if the given handler return type is async-compatible (equal to Promise) * - * @phpstan-template TEvent of Event - * @phpstan-param (\Closure(TEvent) : void)|(\Closure(AsyncEvent&TEvent) : Promise) $handler + * @phpstan-template TEvent of Event&AsyncEvent + * @phpstan-param \Closure(TEvent) : Promise $handler + * + * @throws \ReflectionException */ private function canHandleAsyncEvent(\Closure $handler) : bool{ $reflection = new \ReflectionFunction($handler); From ca95b2f28d58cbc4b1e2a9d65ea647b61117d3fb Mon Sep 17 00:00:00 2001 From: ShockedPlot7560 Date: Fri, 27 Oct 2023 22:08:49 +0200 Subject: [PATCH 14/47] fix PHPStan --- src/event/AsyncEventDelegate.php | 2 +- tests/phpstan/configs/actual-problems.neon | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/event/AsyncEventDelegate.php b/src/event/AsyncEventDelegate.php index 6c20ad0b92b..7cccb3660ff 100644 --- a/src/event/AsyncEventDelegate.php +++ b/src/event/AsyncEventDelegate.php @@ -33,7 +33,7 @@ final class AsyncEventDelegate{ /** @phpstan-var ObjectSet> $promises */ private ObjectSet $promises; - /** @var array, int> $delegatesCall */ + /** @var array, int> $delegatesCallDepth */ private static array $delegatesCallDepth = []; private const MAX_EVENT_CALL_DEPTH = 50; diff --git a/tests/phpstan/configs/actual-problems.neon b/tests/phpstan/configs/actual-problems.neon index 47b2615b91a..f48f611ee0a 100644 --- a/tests/phpstan/configs/actual-problems.neon +++ b/tests/phpstan/configs/actual-problems.neon @@ -1205,4 +1205,8 @@ parameters: count: 1 path: ../../../src/plugin/PluginManager.php + - + message: "#^Parameter \\#1 \\$handler of class pocketmine\\\\event\\\\RegisteredAsyncListener constructor expects Closure\\(pocketmine\\\\event\\\\AsyncEvent&pocketmine\\\\event\\\\Event\\)\\: pocketmine\\\\promise\\\\Promise\\, Closure\\(TEvent\\)\\: pocketmine\\\\promise\\\\Promise\\ given\\.$#" + count: 1 + path: ../../../src/plugin/PluginManager.php From 823d4ead6a55833977067229d5625aeb14ffa0d7 Mon Sep 17 00:00:00 2001 From: ShockedPlot7560 Date: Fri, 27 Oct 2023 22:12:03 +0200 Subject: [PATCH 15/47] inconsistency correction --- src/event/RegisteredAsyncListener.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/event/RegisteredAsyncListener.php b/src/event/RegisteredAsyncListener.php index ce235cbe2e7..821b88154e3 100644 --- a/src/event/RegisteredAsyncListener.php +++ b/src/event/RegisteredAsyncListener.php @@ -39,7 +39,7 @@ public function __construct( int $priority, Plugin $plugin, bool $handleCancelled, - private bool $noConcurrentCall, + private bool $exclusiveCall, TimingsHandler $timings ){ $handler = function(AsyncEvent&Event $event) use($handler) : void{ @@ -52,7 +52,7 @@ public function __construct( } public function canBeCalledConcurrently() : bool{ - return !$this->noConcurrentCall; + return !$this->exclusiveCall; } /** From 243a3035ba5f0559111c6d03631e4af6fb37cf1d Mon Sep 17 00:00:00 2001 From: ShockedPlot7560 Date: Fri, 27 Oct 2023 22:14:21 +0200 Subject: [PATCH 16/47] follow up of #6110 --- src/event/AsyncEventDelegate.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/event/AsyncEventDelegate.php b/src/event/AsyncEventDelegate.php index 7cccb3660ff..c3054ebe57c 100644 --- a/src/event/AsyncEventDelegate.php +++ b/src/event/AsyncEventDelegate.php @@ -79,7 +79,7 @@ private function callAsyncDepth() : Promise{ $priorities = EventPriority::ALL; $testResolve = function () use (&$testResolve, &$priorities, $globalResolver){ if(count($priorities) === 0){ - $globalResolver->resolve(""); // TODO: see #6110 + $globalResolver->resolve(null); }else{ $this->callPriority(array_shift($priorities))->onCompletion(function() use ($testResolve) : void{ $testResolve(); @@ -119,7 +119,7 @@ private function callPriority(int $priority) : Promise{ $testResolve = function() use (&$nonConcurrentHandlers, &$testResolve, $resolver){ if(count($nonConcurrentHandlers) === 0){ $this->waitForPromises()->onCompletion(function() use ($resolver){ - $resolver->resolve(""); // TODO: see #6110 + $resolver->resolve(null); }, function() use ($resolver){ $resolver->reject(); }); From aaa37baf2e4e6c7ae663838ac6e50a6b1c08656b Mon Sep 17 00:00:00 2001 From: ShockedPlot7560 Date: Fri, 27 Oct 2023 22:28:04 +0200 Subject: [PATCH 17/47] handlerListe: reduce code complexity --- src/event/HandlerList.php | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/event/HandlerList.php b/src/event/HandlerList.php index faf48f43dde..9b905fbf91a 100644 --- a/src/event/HandlerList.php +++ b/src/event/HandlerList.php @@ -24,11 +24,10 @@ namespace pocketmine\event; use pocketmine\plugin\Plugin; -use function array_filter; use function array_merge; +use function array_merge_recursive; use function krsort; use function spl_object_id; -use function usort; use const SORT_NUMERIC; class HandlerList{ @@ -129,24 +128,23 @@ public function getListenerList() : array{ $handlerLists[] = $currentList; } - $listenersByPriority = []; - $asyncListenersByPriority = []; + $listeners = []; + $asyncListeners = []; + $exclusiveAsyncListeners = []; foreach($handlerLists as $currentList){ - foreach($currentList->handlerSlots as $priority => $listeners){ - $syncListeners = array_filter($listeners, static function(RegisteredListener $listener) : bool{ return !($listener instanceof RegisteredAsyncListener); }); - $listenersByPriority[$priority] = array_merge($listenersByPriority[$priority] ?? [], $syncListeners); - - $asyncListeners = array_filter($listeners, static function(RegisteredListener $listener) : bool{ return $listener instanceof RegisteredAsyncListener; }); - $asyncListenersByPriority[$priority] = array_merge($asyncListenersByPriority[$priority] ?? [], $asyncListeners); + 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($asyncListenersByPriority as $priority => $asyncListeners){ - usort($asyncListeners, static function(RegisteredAsyncListener $a, RegisteredAsyncListener $b) : int{ - // concurrent listeners are sorted to the end of the list - return $b->canBeCalledConcurrently() <=> $a->canBeCalledConcurrently(); - }); - $listenersByPriority[$priority] = array_merge($listenersByPriority[$priority] ?? [], $asyncListeners); - } + $listenersByPriority = array_merge_recursive($listeners, $asyncListeners, $exclusiveAsyncListeners); //TODO: why on earth do the priorities have higher values for lower priority? krsort($listenersByPriority, SORT_NUMERIC); From f82c422f64cdd99043ee128d0a81b7f47a1b6359 Mon Sep 17 00:00:00 2001 From: ShockedPlot7560 Date: Sun, 21 Jan 2024 11:25:34 +0100 Subject: [PATCH 18/47] remove using of Event API --- src/event/AsyncEvent.php | 127 +++++++++++++++++- src/event/AsyncEventDelegate.php | 153 ---------------------- src/event/AsyncEventTrait.php | 35 ----- src/event/HandlerListManager.php | 2 +- src/event/RegisteredAsyncListener.php | 34 ++--- src/event/player/PlayerChatAsyncEvent.php | 92 +++++++++++++ src/player/Player.php | 15 +++ src/plugin/PluginManager.php | 6 +- src/timings/Timings.php | 4 +- 9 files changed, 249 insertions(+), 219 deletions(-) delete mode 100644 src/event/AsyncEventDelegate.php delete mode 100644 src/event/AsyncEventTrait.php create mode 100644 src/event/player/PlayerChatAsyncEvent.php diff --git a/src/event/AsyncEvent.php b/src/event/AsyncEvent.php index 9f1784b9029..3757744851e 100644 --- a/src/event/AsyncEvent.php +++ b/src/event/AsyncEvent.php @@ -24,20 +24,133 @@ namespace pocketmine\event; use pocketmine\promise\Promise; +use pocketmine\promise\PromiseResolver; +use pocketmine\timings\Timings; +use pocketmine\utils\ObjectSet; +use function array_shift; +use function assert; +use function count; /** - * This interface is implemented by an Event subclass if and only if it can be called asynchronously. + * This class is used to permit asynchronous event handling. * - * Used with {@see AsyncEventTrait} to provide a way to call an event asynchronously. * When an event is called asynchronously, the event handlers are called by priority level. * When all the promises of a priority level have been resolved, the next priority level is called. */ -interface AsyncEvent{ +abstract class AsyncEvent{ + /** @phpstan-var ObjectSet> $promises */ + private ObjectSet $promises; + /** @var array, int> $delegatesCallDepth */ + private static array $delegatesCallDepth = []; + private const MAX_EVENT_CALL_DEPTH = 50; + + /** + * @phpstan-return Promise + */ + final public function call() : Promise{ + $this->promises = new ObjectSet(); + if(!isset(self::$delegatesCallDepth[$class = static::class])){ + self::$delegatesCallDepth[$class] = 0; + } + + if(self::$delegatesCallDepth[$class] >= self::MAX_EVENT_CALL_DEPTH){ + //this exception will be caught by the parent event call if all else fails + throw new \RuntimeException("Recursive event call detected (reached max depth of " . self::MAX_EVENT_CALL_DEPTH . " calls)"); + } + + $timings = Timings::getAsyncEventTimings($this); + $timings->startTiming(); + + ++self::$delegatesCallDepth[$class]; + try{ + return $this->callAsyncDepth(); + }finally{ + --self::$delegatesCallDepth[$class]; + $timings->stopTiming(); + } + } + + /** + * @phpstan-return Promise + */ + private function callAsyncDepth() : Promise{ + /** @phpstan-var PromiseResolver $globalResolver */ + $globalResolver = new PromiseResolver(); + + $priorities = EventPriority::ALL; + $testResolve = function () use (&$testResolve, &$priorities, $globalResolver){ + if(count($priorities) === 0){ + $globalResolver->resolve(null); + }else{ + $this->callPriority(array_shift($priorities))->onCompletion(function() use ($testResolve) : void{ + $testResolve(); + }, function () use ($globalResolver) { + $globalResolver->reject(); + }); + } + }; + + $testResolve(); + + return $globalResolver->getPromise(); + } + /** - * Be prudent, calling an event asynchronously can produce unexpected results. - * During the execution of the event, the server, the player and the event context may have changed state. - * * @phpstan-return Promise */ - public function callAsync() : Promise; + private function callPriority(int $priority) : Promise{ + $handlers = HandlerListManager::global()->getListFor(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) { + $this->promises->add($result); + } + }else{ + $nonConcurrentHandlers[] = $registration; + } + } + + $testResolve = function() use (&$nonConcurrentHandlers, &$testResolve, $resolver){ + if(count($nonConcurrentHandlers) === 0){ + $this->waitForPromises()->onCompletion(function() use ($resolver){ + $resolver->resolve(null); + }, function() use ($resolver){ + $resolver->reject(); + }); + }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); + } + $testResolve(); + }, function() use ($resolver) { + $resolver->reject(); + }); + } + }; + + $testResolve(); + + return $resolver->getPromise(); + } + + /** + * @phpstan-return Promise> + */ + private function waitForPromises() : Promise{ + $array = $this->promises->toArray(); + $this->promises->clear(); + + return Promise::all($array); + } } diff --git a/src/event/AsyncEventDelegate.php b/src/event/AsyncEventDelegate.php deleted file mode 100644 index c3054ebe57c..00000000000 --- a/src/event/AsyncEventDelegate.php +++ /dev/null @@ -1,153 +0,0 @@ -> $promises */ - private ObjectSet $promises; - /** @var array, int> $delegatesCallDepth */ - private static array $delegatesCallDepth = []; - private const MAX_EVENT_CALL_DEPTH = 50; - - public function __construct( - private AsyncEvent&Event $event - ){ - $this->promises = new ObjectSet(); - } - - /** - * @phpstan-return Promise - */ - public function call() : Promise{ - $this->promises->clear(); - if(!isset(self::$delegatesCallDepth[$class = $this->event::class])){ - self::$delegatesCallDepth[$class] = 0; - } - - if(self::$delegatesCallDepth[$class] >= self::MAX_EVENT_CALL_DEPTH){ - //this exception will be caught by the parent event call if all else fails - throw new \RuntimeException("Recursive event call detected (reached max depth of " . self::MAX_EVENT_CALL_DEPTH . " calls)"); - } - - $timings = Timings::getAsyncEventTimings($this->event); - $timings->startTiming(); - - ++self::$delegatesCallDepth[$class]; - try{ - return $this->callAsyncDepth(); - }finally{ - --self::$delegatesCallDepth[$class]; - $timings->stopTiming(); - } - } - - /** - * @phpstan-return Promise - */ - private function callAsyncDepth() : Promise{ - /** @phpstan-var PromiseResolver $globalResolver */ - $globalResolver = new PromiseResolver(); - - $priorities = EventPriority::ALL; - $testResolve = function () use (&$testResolve, &$priorities, $globalResolver){ - if(count($priorities) === 0){ - $globalResolver->resolve(null); - }else{ - $this->callPriority(array_shift($priorities))->onCompletion(function() use ($testResolve) : void{ - $testResolve(); - }, function () use ($globalResolver) { - $globalResolver->reject(); - }); - } - }; - - $testResolve(); - - return $globalResolver->getPromise(); - } - - /** - * @phpstan-return Promise - */ - private function callPriority(int $priority) : Promise{ - $handlers = HandlerListManager::global()->getListFor($this->event::class)->getListenersByPriority($priority); - - /** @phpstan-var PromiseResolver $resolver */ - $resolver = new PromiseResolver(); - - $nonConcurrentHandlers = []; - foreach($handlers as $registration){ - if($registration instanceof RegisteredAsyncListener){ - if($registration->canBeCalledConcurrently()){ - $this->promises->add($registration->callAsync($this->event)); - }else{ - $nonConcurrentHandlers[] = $registration; - } - }else{ - $registration->callEvent($this->event); - } - } - - $testResolve = function() use (&$nonConcurrentHandlers, &$testResolve, $resolver){ - if(count($nonConcurrentHandlers) === 0){ - $this->waitForPromises()->onCompletion(function() use ($resolver){ - $resolver->resolve(null); - }, function() use ($resolver){ - $resolver->reject(); - }); - }else{ - $this->waitForPromises()->onCompletion(function() use (&$nonConcurrentHandlers, $testResolve){ - $handler = array_shift($nonConcurrentHandlers); - if($handler instanceof RegisteredAsyncListener){ - $this->promises->add($handler->callAsync($this->event)); - } - $testResolve(); - }, function() use ($resolver) { - $resolver->reject(); - }); - } - }; - - $testResolve(); - - return $resolver->getPromise(); - } - - /** - * @phpstan-return Promise> - */ - private function waitForPromises() : Promise{ - $array = $this->promises->toArray(); - $this->promises->clear(); - - return Promise::all($array); - } -} diff --git a/src/event/AsyncEventTrait.php b/src/event/AsyncEventTrait.php deleted file mode 100644 index 9cea1d36c94..00000000000 --- a/src/event/AsyncEventTrait.php +++ /dev/null @@ -1,35 +0,0 @@ - - */ - final public function callAsync() : Promise{ - return (new AsyncEventDelegate($this))->call(); - } -} diff --git a/src/event/HandlerListManager.php b/src/event/HandlerListManager.php index 047632f5483..92fd3f14170 100644 --- a/src/event/HandlerListManager.php +++ b/src/event/HandlerListManager.php @@ -86,7 +86,7 @@ private static function resolveNearestHandleableParent(\ReflectionClass $class) * * Calling this method also lazily initializes the $classMap inheritance tree of handler lists. * - * @phpstan-template TEvent of Event + * @phpstan-template TEvent of Event|AsyncEvent * @phpstan-param class-string $event * * @throws \ReflectionException diff --git a/src/event/RegisteredAsyncListener.php b/src/event/RegisteredAsyncListener.php index 821b88154e3..6a0413d9930 100644 --- a/src/event/RegisteredAsyncListener.php +++ b/src/event/RegisteredAsyncListener.php @@ -28,26 +28,17 @@ use pocketmine\timings\TimingsHandler; class RegisteredAsyncListener extends RegisteredListener{ - /** @phpstan-var Promise $returnPromise */ - private Promise $returnPromise; - /** - * @phpstan-param \Closure(AsyncEvent&Event) : Promise $handler + * @phpstan-param \Closure(AsyncEvent) : Promise $handler */ public function __construct( - \Closure $handler, + protected \Closure $handler, int $priority, Plugin $plugin, bool $handleCancelled, private bool $exclusiveCall, - TimingsHandler $timings + protected TimingsHandler $timings ){ - $handler = function(AsyncEvent&Event $event) use($handler) : void{ - $this->returnPromise = $handler($event); - if(!$this->returnPromise instanceof Promise){ - throw new \TypeError("Async event handler must return a Promise"); - } - }; parent::__construct($handler, $priority, $plugin, $handleCancelled, $timings); } @@ -55,11 +46,22 @@ public function canBeCalledConcurrently() : bool{ return !$this->exclusiveCall; } + public function callEvent(Event $event) : void{ + throw new \BadMethodCallException("Cannot call async event synchronously, use callAsync() instead"); + } + /** - * @phpstan-return Promise + * @phpstan-return Promise|null */ - public function callAsync(AsyncEvent&Event $event) : Promise{ - $this->callEvent($event); - return $this->returnPromise; + public function callAsync(AsyncEvent $event) : ?Promise{ + if($event instanceof Cancellable && $event->isCancelled() && !$this->isHandlingCancelled()){ + return null; + } + $this->timings->startTiming(); + try{ + return ($this->handler)($event); + }finally{ + $this->timings->stopTiming(); + } } } diff --git a/src/event/player/PlayerChatAsyncEvent.php b/src/event/player/PlayerChatAsyncEvent.php new file mode 100644 index 00000000000..c520aa5a117 --- /dev/null +++ b/src/event/player/PlayerChatAsyncEvent.php @@ -0,0 +1,92 @@ +message; + } + + public function setMessage(string $message) : void{ + $this->message = $message; + } + + /** + * Changes the player that is sending the message + */ + public function setPlayer(Player $player) : void{ + $this->player = $player; + } + + public function getPlayer() : Player{ + return $this->player; + } + + public function getFormatter() : ChatFormatter{ + return $this->formatter; + } + + public function setFormatter(ChatFormatter $formatter) : void{ + $this->formatter = $formatter; + } + + /** + * @return CommandSender[] + */ + public function getRecipients() : array{ + return $this->recipients; + } + + /** + * @param CommandSender[] $recipients + */ + public function setRecipients(array $recipients) : void{ + Utils::validateArrayValueType($recipients, function(CommandSender $_) : void{}); + $this->recipients = $recipients; + } +} diff --git a/src/player/Player.php b/src/player/Player.php index 0ee9d7dc2a2..4deab6aa6d1 100644 --- a/src/player/Player.php +++ b/src/player/Player.php @@ -51,6 +51,7 @@ use pocketmine\event\player\PlayerBedLeaveEvent; use pocketmine\event\player\PlayerBlockPickEvent; use pocketmine\event\player\PlayerChangeSkinEvent; +use pocketmine\event\player\PlayerChatAsyncEvent; use pocketmine\event\player\PlayerChatEvent; use pocketmine\event\player\PlayerDeathEvent; use pocketmine\event\player\PlayerDisplayNameChangeEvent; @@ -158,6 +159,7 @@ use function strtolower; use function substr; use function trim; +use function var_dump; use const M_PI; use const M_SQRT3; use const PHP_INT_MAX; @@ -1517,6 +1519,19 @@ public function chat(string $message) : bool{ if(!$ev->isCancelled()){ $this->server->broadcastMessage($ev->getFormatter()->format($ev->getPlayer()->getDisplayName(), $ev->getMessage()), $ev->getRecipients()); } + + $ev = new PlayerChatAsyncEvent( + $this, $messagePart, + $this->server->getBroadcastChannelSubscribers(Server::BROADCAST_CHANNEL_USERS), + new StandardChatFormatter() + ); + $ev->call()->onCompletion(function () use ($ev) { + if(!$ev->isCancelled()){ + $this->server->broadcastMessage($ev->getFormatter()->format($ev->getPlayer()->getDisplayName(), $ev->getMessage()), $ev->getRecipients()); + } + }, function () { + var_dump("Failed to send chat message"); + }); } } } diff --git a/src/plugin/PluginManager.php b/src/plugin/PluginManager.php index b1050a97118..520acae6492 100644 --- a/src/plugin/PluginManager.php +++ b/src/plugin/PluginManager.php @@ -689,17 +689,13 @@ public function registerEvent(string $event, \Closure $handler, int $priority, P /** * @param string $event Class name that extends Event and AsyncEvent * - * @phpstan-template TEvent of Event&AsyncEvent + * @phpstan-template TEvent of AsyncEvent * @phpstan-param class-string $event * @phpstan-param \Closure(TEvent) : Promise $handler * * @throws \ReflectionException */ public function registerAsyncEvent(string $event, \Closure $handler, int $priority, Plugin $plugin, bool $handleCancelled = false, bool $exclusiveCall = false) : RegisteredAsyncListener{ - if(!is_subclass_of($event, Event::class)){ - throw new PluginException($event . " is not an Event"); - } - if(!is_subclass_of($event, AsyncEvent::class)){ throw new PluginException($event . " is not an AsyncEvent"); } diff --git a/src/timings/Timings.php b/src/timings/Timings.php index e8d37c8983a..053f7970c62 100644 --- a/src/timings/Timings.php +++ b/src/timings/Timings.php @@ -307,7 +307,7 @@ public static function getEventTimings(Event $event) : TimingsHandler{ return self::$events[$eventClass]; } - public static function getAsyncEventTimings(AsyncEvent&Event $event) : TimingsHandler{ + public static function getAsyncEventTimings(AsyncEvent $event) : TimingsHandler{ $eventClass = get_class($event); if(!isset(self::$asyncEvents[$eventClass])){ self::$asyncEvents[$eventClass] = new TimingsHandler(self::shortenCoreClassName($eventClass, "pocketmine\\event\\"), group: "Events"); @@ -317,7 +317,7 @@ public static function getAsyncEventTimings(AsyncEvent&Event $event) : TimingsHa } /** - * @phpstan-template TEvent of Event + * @phpstan-template TEvent of Event|AsyncEvent * @phpstan-param class-string $event */ public static function getEventHandlerTimings(string $event, string $handlerName, string $group) : TimingsHandler{ From eb9814197b631b3aa69bce7e5004540ead2e3e16 Mon Sep 17 00:00:00 2001 From: ShockedPlot7560 Date: Sun, 21 Jan 2024 11:50:31 +0100 Subject: [PATCH 19/47] resolve AsyncEvent with self instance --- src/event/AsyncEvent.php | 8 ++++---- src/player/Player.php | 15 --------------- src/plugin/PluginManager.php | 2 +- src/promise/Promise.php | 6 ++++-- 4 files changed, 9 insertions(+), 22 deletions(-) diff --git a/src/event/AsyncEvent.php b/src/event/AsyncEvent.php index 3757744851e..12337602473 100644 --- a/src/event/AsyncEvent.php +++ b/src/event/AsyncEvent.php @@ -45,7 +45,7 @@ abstract class AsyncEvent{ private const MAX_EVENT_CALL_DEPTH = 50; /** - * @phpstan-return Promise + * @phpstan-return Promise */ final public function call() : Promise{ $this->promises = new ObjectSet(); @@ -71,16 +71,16 @@ final public function call() : Promise{ } /** - * @phpstan-return Promise + * @phpstan-return Promise */ private function callAsyncDepth() : Promise{ - /** @phpstan-var PromiseResolver $globalResolver */ + /** @phpstan-var PromiseResolver $globalResolver */ $globalResolver = new PromiseResolver(); $priorities = EventPriority::ALL; $testResolve = function () use (&$testResolve, &$priorities, $globalResolver){ if(count($priorities) === 0){ - $globalResolver->resolve(null); + $globalResolver->resolve($this); }else{ $this->callPriority(array_shift($priorities))->onCompletion(function() use ($testResolve) : void{ $testResolve(); diff --git a/src/player/Player.php b/src/player/Player.php index e4c055c56d7..92e15ddd380 100644 --- a/src/player/Player.php +++ b/src/player/Player.php @@ -51,7 +51,6 @@ use pocketmine\event\player\PlayerBedLeaveEvent; use pocketmine\event\player\PlayerBlockPickEvent; use pocketmine\event\player\PlayerChangeSkinEvent; -use pocketmine\event\player\PlayerChatAsyncEvent; use pocketmine\event\player\PlayerChatEvent; use pocketmine\event\player\PlayerDeathEvent; use pocketmine\event\player\PlayerDisplayNameChangeEvent; @@ -159,7 +158,6 @@ use function strtolower; use function substr; use function trim; -use function var_dump; use const M_PI; use const M_SQRT3; use const PHP_INT_MAX; @@ -1519,19 +1517,6 @@ public function chat(string $message) : bool{ if(!$ev->isCancelled()){ $this->server->broadcastMessage($ev->getFormatter()->format($ev->getPlayer()->getDisplayName(), $ev->getMessage()), $ev->getRecipients()); } - - $ev = new PlayerChatAsyncEvent( - $this, $messagePart, - $this->server->getBroadcastChannelSubscribers(Server::BROADCAST_CHANNEL_USERS), - new StandardChatFormatter() - ); - $ev->call()->onCompletion(function () use ($ev) { - if(!$ev->isCancelled()){ - $this->server->broadcastMessage($ev->getFormatter()->format($ev->getPlayer()->getDisplayName(), $ev->getMessage()), $ev->getRecipients()); - } - }, function () { - var_dump("Failed to send chat message"); - }); } } } diff --git a/src/plugin/PluginManager.php b/src/plugin/PluginManager.php index 997a5f451c0..9f3b678de29 100644 --- a/src/plugin/PluginManager.php +++ b/src/plugin/PluginManager.php @@ -578,7 +578,7 @@ private function getEventsHandledBy(\ReflectionMethod $method) : ?string{ /** @phpstan-var class-string $paramClass */ $paramClass = $paramType->getName(); $eventClass = new \ReflectionClass($paramClass); - if(!$eventClass->isSubclassOf(Event::class)){ + if(!$eventClass->isSubclassOf(Event::class) && !$eventClass->isSubclassOf(AsyncEvent::class)){ return null; } diff --git a/src/promise/Promise.php b/src/promise/Promise.php index ec028a2d338..8ee270749e9 100644 --- a/src/promise/Promise.php +++ b/src/promise/Promise.php @@ -83,8 +83,10 @@ public static function all(array $promises) : Promise { function(mixed $value) use ($resolver, $key, &$toResolve, &$continue, &$values) : void{ $values[$key] = $value; - if(--$toResolve === 0 && $continue){ + $toResolve--; + if($toResolve === 0 && $continue){ $resolver->resolve($values); + $continue = false; } }, function() use ($resolver, &$continue) : void{ @@ -100,7 +102,7 @@ function() use ($resolver, &$continue) : void{ } } - if($toResolve === 0){ + if($toResolve === 0 && $continue){ $continue = false; $resolver->resolve($values); } From c1e39039347126e6638a206338378e2e8f94a89d Mon Sep 17 00:00:00 2001 From: ShockedPlot7560 Date: Sun, 21 Jan 2024 12:04:23 +0100 Subject: [PATCH 20/47] fix PHPstan --- src/event/Event.php | 3 --- src/event/HandlerList.php | 2 +- src/event/HandlerListManager.php | 10 +++++----- src/plugin/PluginManager.php | 8 +++----- 4 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/event/Event.php b/src/event/Event.php index 0dd16aa64d3..21b8ae36a5d 100644 --- a/src/event/Event.php +++ b/src/event/Event.php @@ -51,9 +51,6 @@ public function call() : void{ //this exception will be caught by the parent event call if all else fails throw new \RuntimeException("Recursive event call detected (reached max depth of " . self::MAX_EVENT_CALL_DEPTH . " calls)"); } - if($this instanceof AsyncEvent){ - throw new \InvalidArgumentException("Cannot call async event synchronously, use callAsync() instead"); - } $timings = Timings::getEventTimings($this); $timings->startTiming(); diff --git a/src/event/HandlerList.php b/src/event/HandlerList.php index 4b6541ada15..89629e140e3 100644 --- a/src/event/HandlerList.php +++ b/src/event/HandlerList.php @@ -38,7 +38,7 @@ class HandlerList{ private array $affectedHandlerCaches = []; /** - * @phpstan-param class-string $class + * @phpstan-param class-string $class */ public function __construct( private string $class, diff --git a/src/event/HandlerListManager.php b/src/event/HandlerListManager.php index 15199f65285..4fc9ac3052d 100644 --- a/src/event/HandlerListManager.php +++ b/src/event/HandlerListManager.php @@ -38,7 +38,7 @@ public static function global() : self{ private array $allLists = []; /** * @var RegisteredListenerCache[] event class name => cache - * @phpstan-var array, RegisteredListenerCache> + * @phpstan-var array, RegisteredListenerCache> */ private array $handlerCaches = []; @@ -59,7 +59,7 @@ public function unregisterAll(RegisteredListener|Plugin|Listener|null $object = } /** - * @phpstan-param \ReflectionClass $class + * @phpstan-param \ReflectionClass $class */ private static function isValidClass(\ReflectionClass $class) : bool{ $tags = Utils::parseDocComment((string) $class->getDocComment()); @@ -67,9 +67,9 @@ private static function isValidClass(\ReflectionClass $class) : bool{ } /** - * @phpstan-param \ReflectionClass $class + * @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()){ @@ -113,7 +113,7 @@ public function getListFor(string $event) : HandlerList{ } /** - * @phpstan-param class-string $event + * @phpstan-param class-string $event * * @return RegisteredListener[] */ diff --git a/src/plugin/PluginManager.php b/src/plugin/PluginManager.php index 9f3b678de29..c850596d0c9 100644 --- a/src/plugin/PluginManager.php +++ b/src/plugin/PluginManager.php @@ -692,9 +692,8 @@ public function registerEvent(string $event, \Closure $handler, int $priority, P /** * @param string $event Class name that extends Event and AsyncEvent * - * @phpstan-template TEvent of AsyncEvent - * @phpstan-param class-string $event - * @phpstan-param \Closure(TEvent) : Promise $handler + * @phpstan-param class-string $event + * @phpstan-param \Closure(AsyncEvent) : Promise $handler * * @throws \ReflectionException */ @@ -719,8 +718,7 @@ public function registerAsyncEvent(string $event, \Closure $handler, int $priori /** * Check if the given handler return type is async-compatible (equal to Promise) * - * @phpstan-template TEvent of Event&AsyncEvent - * @phpstan-param \Closure(TEvent) : Promise $handler + * @phpstan-param \Closure(AsyncEvent) : Promise $handler * * @throws \ReflectionException */ From 48d243006416bfcf08f96165f989543a046b6890 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 13 Nov 2024 14:58:09 +0000 Subject: [PATCH 21/47] Update PHPStan baseline --- tests/phpstan/configs/actual-problems.neon | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/phpstan/configs/actual-problems.neon b/tests/phpstan/configs/actual-problems.neon index f3300817b94..e778cf0047f 100644 --- a/tests/phpstan/configs/actual-problems.neon +++ b/tests/phpstan/configs/actual-problems.neon @@ -545,11 +545,6 @@ parameters: count: 1 path: ../../../src/entity/projectile/Projectile.php - - - message: "#^Parameter \\#1 \\$promises of static method pocketmine\\\\promise\\\\Promise\\\\:\\:all\\(\\) expects non\\-empty\\-array\\\\>, array\\\\> given\\.$#" - count: 1 - path: ../../../src/event/AsyncEvent.php - - message: "#^Parameter \\#2 \\$recipe of class pocketmine\\\\event\\\\inventory\\\\CraftItemEvent constructor expects pocketmine\\\\crafting\\\\CraftingRecipe, pocketmine\\\\crafting\\\\CraftingRecipe\\|null given\\.$#" count: 1 From 8f48fe48561f1902f25c532f64ee9c3c594698fb Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 13 Nov 2024 15:35:41 +0000 Subject: [PATCH 22/47] 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 12337602473..24c97224cd3 100644 --- a/src/event/AsyncEvent.php +++ b/src/event/AsyncEvent.php @@ -28,7 +28,6 @@ use pocketmine\timings\Timings; use pocketmine\utils\ObjectSet; use function array_shift; -use function assert; use function count; /** @@ -99,14 +98,13 @@ private function callAsyncDepth() : Promise{ * @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 @@ private function callPriority(int $priority) : Promise{ }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 00000000000..a2e238afb2b --- /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 6a0413d9930..1d0f65b7159 100644 --- a/src/event/RegisteredAsyncListener.php +++ b/src/event/AsyncRegisteredListener.php @@ -26,28 +26,35 @@ 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 getPlugin() : Plugin{ + return $this->plugin; } - public function callEvent(Event $event) : void{ - throw new \BadMethodCallException("Cannot call async event synchronously, use callAsync() instead"); + public function getPriority() : int{ + return $this->priority; } /** @@ -64,4 +71,11 @@ public function callAsync(AsyncEvent $event) : ?Promise{ $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 89629e140e3..2072cd5226f 100644 --- a/src/event/HandlerList.php +++ b/src/event/HandlerList.php @@ -25,7 +25,6 @@ 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 @@ public function getListenerList() : array{ $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 4fc9ac3052d..aedf065a3d4 100644 --- a/src/event/HandlerListManager.php +++ b/src/event/HandlerListManager.php @@ -35,24 +35,37 @@ public static function global() : self{ } /** @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 @@ private static function isValidClass(\ReflectionClass $class) : bool{ } /** - * @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 @@ private static function resolveNearestHandleableParent(\ReflectionClass $class) * * 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 @@ public function getListFor(string $event) : HandlerList{ } /** - * @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 @@ public function getHandlersFor(string $event) : array{ * @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 ca126ab4cfd..8c99e9f400b 100644 --- a/src/plugin/PluginManager.php +++ b/src/plugin/PluginManager.php @@ -32,7 +32,7 @@ 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 @@ public function registerEvent(string $event, \Closure $handler, int $priority, P * * @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 @@ public function registerAsyncEvent(string $event, \Closure $handler, int $priori $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; } From 17ae932d31ed414bd8a1779517733fa8003a3b01 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 13 Nov 2024 15:36:51 +0000 Subject: [PATCH 23/47] HandlerListManager: added getter --- src/event/HandlerListManager.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/event/HandlerListManager.php b/src/event/HandlerListManager.php index aedf065a3d4..725b3b8e045 100644 --- a/src/event/HandlerListManager.php +++ b/src/event/HandlerListManager.php @@ -173,4 +173,11 @@ public function getHandlersFor(string $event) : array{ public function getAll() : array{ return $this->allSyncLists; } + + /** + * @return AsyncHandlerList[] + */ + public function getAllAsync() : array{ + return $this->allAsyncLists; + } } From c426677841761c36a1dc5b94ad6b8a1639a7b46a Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 13 Nov 2024 16:03:10 +0000 Subject: [PATCH 24/47] optimization --- src/event/AsyncEvent.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/event/AsyncEvent.php b/src/event/AsyncEvent.php index 24c97224cd3..905da5fb002 100644 --- a/src/event/AsyncEvent.php +++ b/src/event/AsyncEvent.php @@ -76,12 +76,13 @@ private function callAsyncDepth() : Promise{ /** @phpstan-var PromiseResolver $globalResolver */ $globalResolver = new PromiseResolver(); + $handlerList = HandlerListManager::global()->getAsyncListFor(static::class); $priorities = EventPriority::ALL; - $testResolve = function () use (&$testResolve, &$priorities, $globalResolver){ + $testResolve = function () use ($handlerList, &$testResolve, &$priorities, $globalResolver){ if(count($priorities) === 0){ $globalResolver->resolve($this); }else{ - $this->callPriority(array_shift($priorities))->onCompletion(function() use ($testResolve) : void{ + $this->callPriority($handlerList, array_shift($priorities))->onCompletion(function() use ($testResolve) : void{ $testResolve(); }, function () use ($globalResolver) { $globalResolver->reject(); @@ -97,8 +98,8 @@ private function callAsyncDepth() : Promise{ /** * @phpstan-return Promise */ - private function callPriority(int $priority) : Promise{ - $handlers = HandlerListManager::global()->getAsyncListFor(static::class)->getListenersByPriority($priority); + private function callPriority(AsyncHandlerList $handlerList, int $priority) : Promise{ + $handlers = $handlerList->getListenersByPriority($priority); /** @phpstan-var PromiseResolver $resolver */ $resolver = new PromiseResolver(); From db88e543fa9b168d1ddf1e570a0e8c8ebb2c14b7 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 13 Nov 2024 16:04:27 +0000 Subject: [PATCH 25/47] Fix PHPStan error --- src/event/AsyncEvent.php | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/event/AsyncEvent.php b/src/event/AsyncEvent.php index 905da5fb002..14fc208b2ea 100644 --- a/src/event/AsyncEvent.php +++ b/src/event/AsyncEvent.php @@ -117,24 +117,20 @@ private function callPriority(AsyncHandlerList $handlerList, int $priority) : Pr } $testResolve = function() use (&$nonConcurrentHandlers, &$testResolve, $resolver){ - if(count($nonConcurrentHandlers) === 0){ - $this->waitForPromises()->onCompletion(function() use ($resolver){ - $resolver->resolve(null); - }, function() use ($resolver){ - $resolver->reject(); - }); - }else{ - $this->waitForPromises()->onCompletion(function() use (&$nonConcurrentHandlers, $testResolve){ - $handler = array_shift($nonConcurrentHandlers); + $this->waitForPromises()->onCompletion(function() use (&$nonConcurrentHandlers, $testResolve, $resolver){ + $handler = array_shift($nonConcurrentHandlers); + if($handler !== null){ $result = $handler->callAsync($this); if($result !== null) { $this->promises->add($result); } $testResolve(); - }, function() use ($resolver) { - $resolver->reject(); - }); - } + }else{ + $resolver->resolve(null); + } + }, function() use ($resolver) { + $resolver->reject(); + }); }; $testResolve(); From a14afb4bb520473371ec8e7b91e10685b3eb5642 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 13 Nov 2024 18:39:14 +0000 Subject: [PATCH 26/47] Add integration tests most of these are failing - needs to be investigated --- .../src/AsyncEventConcurrencyTest.php | 151 ++++++++++++++++++ .../src/AsyncEventInheritanceTest.php | 83 ++++++++++ .../src/AsyncEventPriorityTest.php | 96 +++++++++++ tests/plugins/TesterPlugin/src/Main.php | 3 + .../src/event/ChildAsyncEvent.php | 28 ++++ .../src/event/GrandchildAsyncEvent.php | 28 ++++ .../src/event/ParentAsyncEvent.php | 28 ++++ 7 files changed, 417 insertions(+) create mode 100644 tests/plugins/TesterPlugin/src/AsyncEventConcurrencyTest.php create mode 100644 tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php create mode 100644 tests/plugins/TesterPlugin/src/AsyncEventPriorityTest.php create mode 100644 tests/plugins/TesterPlugin/src/event/ChildAsyncEvent.php create mode 100644 tests/plugins/TesterPlugin/src/event/GrandchildAsyncEvent.php create mode 100644 tests/plugins/TesterPlugin/src/event/ParentAsyncEvent.php diff --git a/tests/plugins/TesterPlugin/src/AsyncEventConcurrencyTest.php b/tests/plugins/TesterPlugin/src/AsyncEventConcurrencyTest.php new file mode 100644 index 00000000000..f372e8015b0 --- /dev/null +++ b/tests/plugins/TesterPlugin/src/AsyncEventConcurrencyTest.php @@ -0,0 +1,151 @@ +> + */ + private array $resolvers = []; + + private bool $activeExclusiveHandler = false; + private bool $activeConcurrentHandler = false; + + private int $done = 0; + + public function getName() : string{ + return "Async Event Concurrency Lock"; + } + + public function getDescription() : string{ + return "Test that exclusive lock on async event handlers works correctly"; + } + + public function run() : void{ + HandlerListManager::global()->unregisterAll(); + + $main = $this->getPlugin(); + $pluginManager = $main->getServer()->getPluginManager(); + + $pluginManager->registerAsyncEvent( + GrandchildAsyncEvent::class, + function(GrandchildAsyncEvent $event) use ($main) : ?Promise{ + if($this->activeExclusiveHandler){ + $main->getLogger()->error("Concurrent handler can't run while exclusive handlers are waiting to complete"); + $this->setResult(Test::RESULT_FAILED); + return null; + } + $this->activeConcurrentHandler = true; + $resolver = new PromiseResolver(); + $this->resolvers[] = $resolver; + $resolver->getPromise()->onCompletion( + fn() => $this->complete($this->activeConcurrentHandler, "concurrent"), + fn() => $main->getLogger()->error("Not expecting this to be rejected") + ); + return $resolver->getPromise(); + }, + EventPriority::NORMAL, + $main, + //non-exclusive - this must be completed before any exclusive handlers are run (or run after them) + ); + $pluginManager->registerAsyncEvent( + GrandchildAsyncEvent::class, + function(GrandchildAsyncEvent $event) use ($main) : ?Promise{ + $main->getLogger()->info("Entering exclusive handler 1"); + if($this->activeExclusiveHandler || $this->activeConcurrentHandler){ + $main->getLogger()->error("Can't run multiple exclusive handlers at once"); + $this->setResult(Test::RESULT_FAILED); + return null; + } + $this->activeExclusiveHandler = true; + $resolver = new PromiseResolver(); + $this->resolvers[] = $resolver; + $resolver->getPromise()->onCompletion( + fn() => $this->complete($this->activeExclusiveHandler, "exclusive 1"), + fn() => $main->getLogger()->error("Not expecting this to be rejected") + ); + return null; + }, + EventPriority::NORMAL, + $main, + exclusiveCall: true + ); + + $pluginManager->registerAsyncEvent( + GrandchildAsyncEvent::class, + function(GrandchildAsyncEvent $event) use ($main) : ?Promise{ + $this->getPlugin()->getLogger()->info("Entering exclusive handler 2"); + if($this->activeExclusiveHandler || $this->activeConcurrentHandler){ + $main->getLogger()->error("Exclusive lock handlers must not run at the same time as any other handlers"); + $this->setResult(Test::RESULT_FAILED); + return null; + } + $this->activeExclusiveHandler = true; + /** @phpstan-var PromiseResolver $resolver */ + $resolver = new PromiseResolver(); + $this->resolvers[] = $resolver; + $resolver->getPromise()->onCompletion( + function() use ($main) : void{ + $main->getLogger()->info("Exiting exclusive handler asynchronously"); + $this->complete($this->activeExclusiveHandler, "exclusive 2"); + }, + function() use ($main) : void{ + $main->getLogger()->error("Not expecting this promise to be rejected"); + $this->setResult(Test::RESULT_ERROR); + } + ); + return $resolver->getPromise(); + }, + EventPriority::NORMAL, + $main, + exclusiveCall: true + ); + + (new GrandchildAsyncEvent())->call(); + } + + private function complete(bool &$flag, string $what) : void{ + $this->getPlugin()->getLogger()->info("Completing $what"); + $flag = false; + if(++$this->done === 3){ + $this->setResult(Test::RESULT_OK); + } + } + + public function tick() : void{ + foreach($this->resolvers as $k => $resolver){ + $resolver->resolve(null); + //don't clear the array here - resolving this will trigger adding the next resolver + unset($this->resolvers[$k]); + } + } +} diff --git a/tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php b/tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php new file mode 100644 index 00000000000..5fc889e706a --- /dev/null +++ b/tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php @@ -0,0 +1,83 @@ +unregisterAll(); + + $plugin = $this->getPlugin(); + $classes = self::EXPECTED_ORDER; + shuffle($classes); + foreach($classes as $event){ + $plugin->getServer()->getPluginManager()->registerAsyncEvent( + $event, + function(AsyncEvent $event) : ?Promise{ + $this->callOrder[] = $event::class; + return null; + }, + EventPriority::NORMAL, + $plugin + ); + } + + $event = new GrandchildAsyncEvent(); + $promise = $event->call(); + $promise->onCompletion(onSuccess: $this->collectResults(...), onFailure: $this->collectResults(...)); + } + + private function collectResults() : void{ + if($this->callOrder === self::EXPECTED_ORDER){ + $this->setResult(Test::RESULT_OK); + }else{ + $this->getPlugin()->getLogger()->error("Expected order: " . implode(", ", self::EXPECTED_ORDER) . ", got: " . implode(", ", $this->callOrder)); + $this->setResult(Test::RESULT_FAILED); + } + } +} diff --git a/tests/plugins/TesterPlugin/src/AsyncEventPriorityTest.php b/tests/plugins/TesterPlugin/src/AsyncEventPriorityTest.php new file mode 100644 index 00000000000..ac8b56407c4 --- /dev/null +++ b/tests/plugins/TesterPlugin/src/AsyncEventPriorityTest.php @@ -0,0 +1,96 @@ +> + */ + private array $resolvers = []; + + private bool $firstHandlerCompleted = false; + + public function getName() : string{ + return "Async Event Handler Priority Lock"; + } + + public function getDescription() : string{ + return "Tests that async events do not call handlers from the next priority until all promises from the current priority are resolved"; + } + + public function run() : void{ + HandlerListManager::global()->unregisterAll(); + + $main = $this->getPlugin(); + $pluginManager = $main->getServer()->getPluginManager(); + $pluginManager->registerAsyncEvent( + GrandchildAsyncEvent::class, + function(GrandchildAsyncEvent $event) use ($main) : Promise{ + $resolver = new PromiseResolver(); + $this->resolvers[] = $resolver; + + $resolver->getPromise()->onCompletion(function() : void{ + $this->firstHandlerCompleted = true; + }, function() use ($main) : void{ + $main->getLogger()->error("Not expecting this to be rejected"); + $this->setResult(Test::RESULT_ERROR); + }); + + return $resolver->getPromise(); + }, + EventPriority::LOW, //anything below NORMAL is fine + $main + ); + $pluginManager->registerAsyncEvent( + GrandchildAsyncEvent::class, + function(GrandchildAsyncEvent $event) use ($main) : ?Promise{ + if(!$this->firstHandlerCompleted){ + $main->getLogger()->error("This shouldn't run until the previous priority is done"); + $this->setResult(Test::RESULT_FAILED); + }else{ + $this->setResult(Test::RESULT_OK); + } + return null; + }, + EventPriority::NORMAL, + $main + ); + + (new GrandchildAsyncEvent())->call(); + } + + public function tick() : void{ + foreach($this->resolvers as $k => $resolver){ + $resolver->resolve(null); + unset($this->resolvers[$k]); + } + } +} diff --git a/tests/plugins/TesterPlugin/src/Main.php b/tests/plugins/TesterPlugin/src/Main.php index 26d3441f4a6..f330af6d9da 100644 --- a/tests/plugins/TesterPlugin/src/Main.php +++ b/tests/plugins/TesterPlugin/src/Main.php @@ -58,6 +58,9 @@ public function onEnable() : void{ $this->waitingTests = [ new EventHandlerInheritanceTest($this), + new AsyncEventInheritanceTest($this), + new AsyncEventConcurrencyTest($this), + new AsyncEventPriorityTest($this) ]; } diff --git a/tests/plugins/TesterPlugin/src/event/ChildAsyncEvent.php b/tests/plugins/TesterPlugin/src/event/ChildAsyncEvent.php new file mode 100644 index 00000000000..c54a2503c8c --- /dev/null +++ b/tests/plugins/TesterPlugin/src/event/ChildAsyncEvent.php @@ -0,0 +1,28 @@ + Date: Wed, 13 Nov 2024 18:47:46 +0000 Subject: [PATCH 27/47] Fixed bug in concurrency integration test --- tests/plugins/TesterPlugin/src/AsyncEventConcurrencyTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/plugins/TesterPlugin/src/AsyncEventConcurrencyTest.php b/tests/plugins/TesterPlugin/src/AsyncEventConcurrencyTest.php index f372e8015b0..865eae5f413 100644 --- a/tests/plugins/TesterPlugin/src/AsyncEventConcurrencyTest.php +++ b/tests/plugins/TesterPlugin/src/AsyncEventConcurrencyTest.php @@ -93,7 +93,7 @@ function(GrandchildAsyncEvent $event) use ($main) : ?Promise{ fn() => $this->complete($this->activeExclusiveHandler, "exclusive 1"), fn() => $main->getLogger()->error("Not expecting this to be rejected") ); - return null; + return $resolver->getPromise(); }, EventPriority::NORMAL, $main, From 409066c8f51b142aace1c8797d1534a6ad2190e1 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 13 Nov 2024 18:49:15 +0000 Subject: [PATCH 28/47] AsyncEvent: make the code easier to make sense of --- src/event/AsyncEvent.php | 104 ++++++++++++++++++++------------------- 1 file changed, 54 insertions(+), 50 deletions(-) diff --git a/src/event/AsyncEvent.php b/src/event/AsyncEvent.php index 14fc208b2ea..28b9a6d568a 100644 --- a/src/event/AsyncEvent.php +++ b/src/event/AsyncEvent.php @@ -26,7 +26,6 @@ use pocketmine\promise\Promise; use pocketmine\promise\PromiseResolver; use pocketmine\timings\Timings; -use pocketmine\utils\ObjectSet; use function array_shift; use function count; @@ -37,17 +36,14 @@ * When all the promises of a priority level have been resolved, the next priority level is called. */ abstract class AsyncEvent{ - /** @phpstan-var ObjectSet> $promises */ - private ObjectSet $promises; /** @var array, int> $delegatesCallDepth */ private static array $delegatesCallDepth = []; private const MAX_EVENT_CALL_DEPTH = 50; /** - * @phpstan-return Promise + * @phpstan-return Promise */ final public function call() : Promise{ - $this->promises = new ObjectSet(); if(!isset(self::$delegatesCallDepth[$class = static::class])){ self::$delegatesCallDepth[$class] = 0; } @@ -62,7 +58,12 @@ final public function call() : Promise{ ++self::$delegatesCallDepth[$class]; try{ - return $this->callAsyncDepth(); + /** @phpstan-var PromiseResolver $globalResolver */ + $globalResolver = new PromiseResolver(); + + $this->asyncEachPriority(HandlerListManager::global()->getAsyncListFor(static::class), EventPriority::ALL, $globalResolver); + + return $globalResolver->getPromise(); }finally{ --self::$delegatesCallDepth[$class]; $timings->stopTiming(); @@ -70,81 +71,84 @@ final public function call() : Promise{ } /** - * @phpstan-return Promise + * TODO: this should use EventPriority constants for the list type but it's inconvenient with the current design + * @phpstan-param list $remaining + * @phpstan-param PromiseResolver $globalResolver */ - private function callAsyncDepth() : Promise{ - /** @phpstan-var PromiseResolver $globalResolver */ - $globalResolver = new PromiseResolver(); - - $handlerList = HandlerListManager::global()->getAsyncListFor(static::class); - $priorities = EventPriority::ALL; - $testResolve = function () use ($handlerList, &$testResolve, &$priorities, $globalResolver){ - if(count($priorities) === 0){ + private function asyncEachPriority(AsyncHandlerList $handlerList, array $remaining, PromiseResolver $globalResolver) : void{ + while(true){ + $nextPriority = array_shift($remaining); + if($nextPriority === null){ $globalResolver->resolve($this); - }else{ - $this->callPriority($handlerList, array_shift($priorities))->onCompletion(function() use ($testResolve) : void{ - $testResolve(); - }, function () use ($globalResolver) { - $globalResolver->reject(); - }); + break; } - }; - $testResolve(); - - return $globalResolver->getPromise(); + $promise = $this->callPriority($handlerList, $nextPriority); + if($promise !== null){ + $promise->onCompletion( + onSuccess: fn() => $this->asyncEachPriority($handlerList, $remaining, $globalResolver), + onFailure: $globalResolver->reject(...) + ); + break; + } + } } /** * @phpstan-return Promise */ - private function callPriority(AsyncHandlerList $handlerList, int $priority) : Promise{ + private function callPriority(AsyncHandlerList $handlerList, int $priority) : ?Promise{ $handlers = $handlerList->getListenersByPriority($priority); + if(count($handlers) === 0){ + return null; + } /** @phpstan-var PromiseResolver $resolver */ $resolver = new PromiseResolver(); + $concurrentPromises = []; $nonConcurrentHandlers = []; foreach($handlers as $registration){ if($registration->canBeCalledConcurrently()){ $result = $registration->callAsync($this); if($result !== null) { - $this->promises->add($result); + $concurrentPromises[] = $result; } }else{ $nonConcurrentHandlers[] = $registration; } } - $testResolve = function() use (&$nonConcurrentHandlers, &$testResolve, $resolver){ - $this->waitForPromises()->onCompletion(function() use (&$nonConcurrentHandlers, $testResolve, $resolver){ - $handler = array_shift($nonConcurrentHandlers); - if($handler !== null){ - $result = $handler->callAsync($this); - if($result !== null) { - $this->promises->add($result); - } - $testResolve(); - }else{ - $resolver->resolve(null); - } - }, function() use ($resolver) { - $resolver->reject(); - }); - }; - - $testResolve(); + Promise::all($concurrentPromises)->onCompletion( + onSuccess: fn() => $this->processExclusiveHandlers($nonConcurrentHandlers, $resolver), + onFailure: $resolver->reject(...) + ); return $resolver->getPromise(); } /** - * @phpstan-return Promise> + * @param AsyncRegisteredListener[] $handlers + * @phpstan-param PromiseResolver $resolver */ - private function waitForPromises() : Promise{ - $array = $this->promises->toArray(); - $this->promises->clear(); + private function processExclusiveHandlers(array $handlers, PromiseResolver $resolver) : void{ + while(true){ + $handler = array_shift($handlers); + if($handler === null){ + $resolver->resolve(null); + break; + } + $result = $handler->callAsync($this); + if($result instanceof Promise){ + //wait for this promise to resolve before calling the next handler + $result->onCompletion( + onSuccess: fn() => $this->processExclusiveHandlers($handlers, $resolver), + onFailure: $resolver->reject(...) + ); + break; + } - return Promise::all($array); + //this handler didn't return a promise - continue directly to the next one + } } } From a6a44bde904781151b8568964f90e0581d954edd Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 13 Nov 2024 18:49:32 +0000 Subject: [PATCH 29/47] Fix doc comments --- src/event/AsyncRegisteredListener.php | 2 +- src/plugin/PluginManager.php | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/event/AsyncRegisteredListener.php b/src/event/AsyncRegisteredListener.php index 1d0f65b7159..81be52f731a 100644 --- a/src/event/AsyncRegisteredListener.php +++ b/src/event/AsyncRegisteredListener.php @@ -30,7 +30,7 @@ class AsyncRegisteredListener{ /** - * @phpstan-param \Closure(AsyncEvent) : Promise $handler + * @phpstan-param \Closure(AsyncEvent) : ?Promise $handler */ public function __construct( private \Closure $handler, diff --git a/src/plugin/PluginManager.php b/src/plugin/PluginManager.php index 8c99e9f400b..85167447bc0 100644 --- a/src/plugin/PluginManager.php +++ b/src/plugin/PluginManager.php @@ -697,8 +697,9 @@ public function registerEvent(string $event, \Closure $handler, int $priority, P /** * @param string $event Class name that extends Event and AsyncEvent * - * @phpstan-param class-string $event - * @phpstan-param \Closure(AsyncEvent) : Promise $handler + * @phpstan-template TEvent of AsyncEvent + * @phpstan-param class-string $event + * @phpstan-param \Closure(TEvent) : ?Promise $handler * * @throws \ReflectionException */ From 6f40c6fc1d2252cb8d725626b7ceb82bfcc9d686 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 13 Nov 2024 18:49:40 +0000 Subject: [PATCH 30/47] CS --- src/plugin/PluginManager.php | 2 +- tests/plugins/TesterPlugin/src/AsyncEventConcurrencyTest.php | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/plugin/PluginManager.php b/src/plugin/PluginManager.php index 85167447bc0..afbdf64fdc0 100644 --- a/src/plugin/PluginManager.php +++ b/src/plugin/PluginManager.php @@ -24,6 +24,7 @@ namespace pocketmine\plugin; use pocketmine\event\AsyncEvent; +use pocketmine\event\AsyncRegisteredListener; use pocketmine\event\Cancellable; use pocketmine\event\Event; use pocketmine\event\EventPriority; @@ -32,7 +33,6 @@ use pocketmine\event\ListenerMethodTags; use pocketmine\event\plugin\PluginDisableEvent; use pocketmine\event\plugin\PluginEnableEvent; -use pocketmine\event\AsyncRegisteredListener; use pocketmine\event\RegisteredListener; use pocketmine\lang\KnownTranslationFactory; use pocketmine\permission\DefaultPermissions; diff --git a/tests/plugins/TesterPlugin/src/AsyncEventConcurrencyTest.php b/tests/plugins/TesterPlugin/src/AsyncEventConcurrencyTest.php index 865eae5f413..2eed286ae4c 100644 --- a/tests/plugins/TesterPlugin/src/AsyncEventConcurrencyTest.php +++ b/tests/plugins/TesterPlugin/src/AsyncEventConcurrencyTest.php @@ -28,7 +28,6 @@ use pocketmine\event\HandlerListManager; use pocketmine\promise\Promise; use pocketmine\promise\PromiseResolver; -use function var_dump; final class AsyncEventConcurrencyTest extends Test{ /** From 32b1d6c0c2d4d6ff6b3b281156261df4703df815 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 13 Nov 2024 18:52:56 +0000 Subject: [PATCH 31/47] Fixed test code the test still doesn't pass, but at least it's actually testing the problem now... --- tests/plugins/TesterPlugin/src/event/ChildAsyncEvent.php | 2 +- tests/plugins/TesterPlugin/src/event/GrandchildAsyncEvent.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/plugins/TesterPlugin/src/event/ChildAsyncEvent.php b/tests/plugins/TesterPlugin/src/event/ChildAsyncEvent.php index c54a2503c8c..79d88b5d3a8 100644 --- a/tests/plugins/TesterPlugin/src/event/ChildAsyncEvent.php +++ b/tests/plugins/TesterPlugin/src/event/ChildAsyncEvent.php @@ -23,6 +23,6 @@ namespace pmmp\TesterPlugin\event; -final class ChildAsyncEvent extends ParentAsyncEvent{ +class ChildAsyncEvent extends ParentAsyncEvent{ } diff --git a/tests/plugins/TesterPlugin/src/event/GrandchildAsyncEvent.php b/tests/plugins/TesterPlugin/src/event/GrandchildAsyncEvent.php index f29bf487b8b..3325268e005 100644 --- a/tests/plugins/TesterPlugin/src/event/GrandchildAsyncEvent.php +++ b/tests/plugins/TesterPlugin/src/event/GrandchildAsyncEvent.php @@ -23,6 +23,6 @@ namespace pmmp\TesterPlugin\event; -class GrandchildAsyncEvent extends \pocketmine\event\AsyncEvent{ +class GrandchildAsyncEvent extends ChildAsyncEvent{ } From fa796535ff8f2a95def0d452364271b68defec35 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 13 Nov 2024 19:09:52 +0000 Subject: [PATCH 32/47] ah hello my old friend, impossible-generics.neon propagating generics all the way through the likes of HandlerList etc is more trouble than it's worth. --- src/event/AsyncRegisteredListener.php | 3 --- tests/phpstan/configs/impossible-generics.neon | 10 ++++++++++ .../TesterPlugin/src/AsyncEventInheritanceTest.php | 3 +++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/event/AsyncRegisteredListener.php b/src/event/AsyncRegisteredListener.php index 81be52f731a..4e6158e9efe 100644 --- a/src/event/AsyncRegisteredListener.php +++ b/src/event/AsyncRegisteredListener.php @@ -29,9 +29,6 @@ use function in_array; class AsyncRegisteredListener{ - /** - * @phpstan-param \Closure(AsyncEvent) : ?Promise $handler - */ public function __construct( private \Closure $handler, private int $priority, diff --git a/tests/phpstan/configs/impossible-generics.neon b/tests/phpstan/configs/impossible-generics.neon index b0e67d294bf..a2d10becfe6 100644 --- a/tests/phpstan/configs/impossible-generics.neon +++ b/tests/phpstan/configs/impossible-generics.neon @@ -1,5 +1,15 @@ parameters: ignoreErrors: + - + message: "#^Method pocketmine\\\\event\\\\AsyncRegisteredListener\\:\\:__construct\\(\\) has parameter \\$handler with no signature specified for Closure\\.$#" + count: 1 + path: ../../../src/event/AsyncRegisteredListener.php + + - + message: "#^Method pocketmine\\\\event\\\\AsyncRegisteredListener\\:\\:getHandler\\(\\) return type has no signature specified for Closure\\.$#" + count: 1 + path: ../../../src/event/AsyncRegisteredListener.php + - message: "#^Method pocketmine\\\\event\\\\RegisteredListener\\:\\:__construct\\(\\) has parameter \\$handler with no signature specified for Closure\\.$#" count: 1 diff --git a/tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php b/tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php index 5fc889e706a..091555fed3e 100644 --- a/tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php +++ b/tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php @@ -39,6 +39,9 @@ final class AsyncEventInheritanceTest extends Test{ ChildAsyncEvent::class, ParentAsyncEvent::class ]; + /** + * @var string[] + */ private array $callOrder = []; public function getName() : string{ From 8aed5d6b2742b6a11df80437cbc8d4af5278ed84 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 13 Nov 2024 20:35:14 +0000 Subject: [PATCH 33/47] Handler inheritance is now working this code should also perform somewhat better --- src/event/AsyncEvent.php | 106 ++++++------------ src/event/AsyncHandlerList.php | 74 +++++++++++- src/event/HandlerList.php | 6 +- src/event/HandlerListManager.php | 23 +++- src/event/RegisteredListenerCache.php | 4 +- .../src/AsyncEventInheritanceTest.php | 9 +- 6 files changed, 141 insertions(+), 81 deletions(-) diff --git a/src/event/AsyncEvent.php b/src/event/AsyncEvent.php index 28b9a6d568a..2db63a0df5d 100644 --- a/src/event/AsyncEvent.php +++ b/src/event/AsyncEvent.php @@ -26,7 +26,6 @@ use pocketmine\promise\Promise; use pocketmine\promise\PromiseResolver; use pocketmine\timings\Timings; -use function array_shift; use function count; /** @@ -61,7 +60,7 @@ final public function call() : Promise{ /** @phpstan-var PromiseResolver $globalResolver */ $globalResolver = new PromiseResolver(); - $this->asyncEachPriority(HandlerListManager::global()->getAsyncListFor(static::class), EventPriority::ALL, $globalResolver); + $this->processRemainingHandlers(HandlerListManager::global()->getAsyncHandlersFor(static::class), $globalResolver); return $globalResolver->getPromise(); }finally{ @@ -71,84 +70,51 @@ final public function call() : Promise{ } /** - * TODO: this should use EventPriority constants for the list type but it's inconvenient with the current design - * @phpstan-param list $remaining + * @param AsyncRegisteredListener[] $handlers * @phpstan-param PromiseResolver $globalResolver */ - private function asyncEachPriority(AsyncHandlerList $handlerList, array $remaining, PromiseResolver $globalResolver) : void{ - while(true){ - $nextPriority = array_shift($remaining); - if($nextPriority === null){ - $globalResolver->resolve($this); + private function processRemainingHandlers(array $handlers, PromiseResolver $globalResolver) : void{ + $currentPriority = null; + $awaitPromises = []; + foreach($handlers as $k => $handler){ + $priority = $handler->getPriority(); + if(count($awaitPromises) > 0 && $currentPriority !== null && $currentPriority !== $priority){ + //wait for concurrent promises from previous priority to complete break; } - $promise = $this->callPriority($handlerList, $nextPriority); - if($promise !== null){ - $promise->onCompletion( - onSuccess: fn() => $this->asyncEachPriority($handlerList, $remaining, $globalResolver), - onFailure: $globalResolver->reject(...) - ); - break; - } - } - } - - /** - * @phpstan-return Promise - */ - private function callPriority(AsyncHandlerList $handlerList, int $priority) : ?Promise{ - $handlers = $handlerList->getListenersByPriority($priority); - if(count($handlers) === 0){ - return null; - } - - /** @phpstan-var PromiseResolver $resolver */ - $resolver = new PromiseResolver(); - - $concurrentPromises = []; - $nonConcurrentHandlers = []; - foreach($handlers as $registration){ - if($registration->canBeCalledConcurrently()){ - $result = $registration->callAsync($this); - if($result !== null) { - $concurrentPromises[] = $result; + $currentPriority = $priority; + if($handler->canBeCalledConcurrently()){ + unset($handlers[$k]); + $promise = $handler->callAsync($this); + if($promise !== null){ + $awaitPromises[] = $promise; } }else{ - $nonConcurrentHandlers[] = $registration; - } - } - - Promise::all($concurrentPromises)->onCompletion( - onSuccess: fn() => $this->processExclusiveHandlers($nonConcurrentHandlers, $resolver), - onFailure: $resolver->reject(...) - ); - - return $resolver->getPromise(); - } + if(count($awaitPromises) > 0){ + //wait for concurrent promises to complete + break; + } - /** - * @param AsyncRegisteredListener[] $handlers - * @phpstan-param PromiseResolver $resolver - */ - private function processExclusiveHandlers(array $handlers, PromiseResolver $resolver) : void{ - while(true){ - $handler = array_shift($handlers); - if($handler === null){ - $resolver->resolve(null); - break; - } - $result = $handler->callAsync($this); - if($result instanceof Promise){ - //wait for this promise to resolve before calling the next handler - $result->onCompletion( - onSuccess: fn() => $this->processExclusiveHandlers($handlers, $resolver), - onFailure: $resolver->reject(...) - ); - break; + unset($handlers[$k]); + $promise = $handler->callAsync($this); + if($promise !== null){ + $promise->onCompletion( + onSuccess: fn() => $this->processRemainingHandlers($handlers, $globalResolver), + onFailure: $globalResolver->reject(...) + ); + return; + } } + } - //this handler didn't return a promise - continue directly to the next one + if(count($awaitPromises) > 0){ + Promise::all($awaitPromises)->onCompletion( + onSuccess: fn() => $this->processRemainingHandlers($handlers, $globalResolver), + onFailure: $globalResolver->reject(...) + ); + }else{ + $globalResolver->resolve($this); } } } diff --git a/src/event/AsyncHandlerList.php b/src/event/AsyncHandlerList.php index a2e238afb2b..6a78f0f6500 100644 --- a/src/event/AsyncHandlerList.php +++ b/src/event/AsyncHandlerList.php @@ -24,25 +24,47 @@ namespace pocketmine\event; use pocketmine\plugin\Plugin; +use function array_merge; +use function krsort; use function spl_object_id; +use function uasort; +use const SORT_NUMERIC; class AsyncHandlerList{ + //TODO: we can probably deduplicate most of this code with the sync side if we throw in some generics + /** @var AsyncRegisteredListener[][] */ private array $handlerSlots = []; /** - * @phpstan-param class-string $class + * @var RegisteredListenerCache[] + * @phpstan-var array> + */ + private array $affectedHandlerCaches = []; + + /** + * @phpstan-param class-string $class + * @phpstan-param RegisteredListenerCache $handlerCache */ public function __construct( private string $class, private ?AsyncHandlerList $parentList, - ){} + private RegisteredListenerCache $handlerCache = new RegisteredListenerCache() + ){ + for($list = $this; $list !== null; $list = $list->parentList){ + $list->affectedHandlerCaches[spl_object_id($this->handlerCache)] = $this->handlerCache; + } + } + /** + * @throws \Exception + */ 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; + $this->invalidateAffectedCaches(); } /** @@ -52,10 +74,10 @@ public function registerAll(array $listeners) : void{ foreach($listeners as $listener){ $this->register($listener); } + $this->invalidateAffectedCaches(); } 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){ @@ -69,10 +91,12 @@ public function unregister(AsyncRegisteredListener|Plugin|Listener $object) : vo }else{ unset($this->handlerSlots[$object->getPriority()][spl_object_id($object)]); } + $this->invalidateAffectedCaches(); } public function clear() : void{ $this->handlerSlots = []; + $this->invalidateAffectedCaches(); } /** @@ -85,4 +109,48 @@ public function getListenersByPriority(int $priority) : array{ public function getParent() : ?AsyncHandlerList{ return $this->parentList; } + + /** + * Invalidates all known caches which might be affected by this list's contents. + */ + private function invalidateAffectedCaches() : void{ + foreach($this->affectedHandlerCaches as $cache){ + $cache->list = null; + } + } + + /** + * @return AsyncRegisteredListener[] + * @phpstan-return list + */ + public function getListenerList() : array{ + if($this->handlerCache->list !== null){ + return $this->handlerCache->list; + } + + $handlerLists = []; + for($currentList = $this; $currentList !== null; $currentList = $currentList->parentList){ + $handlerLists[] = $currentList; + } + + $listenersByPriority = []; + foreach($handlerLists as $currentList){ + foreach($currentList->handlerSlots as $priority => $listeners){ + uasort($listeners, function(AsyncRegisteredListener $left, AsyncRegisteredListener $right) : int{ + //While the system can handle these in any order, it's better for latency if concurrent handlers + //are processed together. It doesn't matter whether they are processed before or after exclusive handlers. + if($right->canBeCalledConcurrently()){ + return $left->canBeCalledConcurrently() ? 0 : 1; + } + return -1; + }); + $listenersByPriority[$priority] = array_merge($listenersByPriority[$priority] ?? [], $listeners); + } + } + + //TODO: why on earth do the priorities have higher values for lower priority? + krsort($listenersByPriority, SORT_NUMERIC); + + return $this->handlerCache->list = array_merge(...$listenersByPriority); + } } diff --git a/src/event/HandlerList.php b/src/event/HandlerList.php index 2072cd5226f..0c6b0f66ecf 100644 --- a/src/event/HandlerList.php +++ b/src/event/HandlerList.php @@ -33,11 +33,15 @@ class HandlerList{ /** @var RegisteredListener[][] */ private array $handlerSlots = []; - /** @var RegisteredListenerCache[] */ + /** + * @var RegisteredListenerCache[] + * @phpstan-var array> + */ private array $affectedHandlerCaches = []; /** * @phpstan-param class-string $class + * @phpstan-param RegisteredListenerCache $handlerCache */ public function __construct( private string $class, diff --git a/src/event/HandlerListManager.php b/src/event/HandlerListManager.php index 725b3b8e045..ecfe073f6a3 100644 --- a/src/event/HandlerListManager.php +++ b/src/event/HandlerListManager.php @@ -38,12 +38,17 @@ public static function global() : self{ private array $allSyncLists = []; /** * @var RegisteredListenerCache[] event class name => cache - * @phpstan-var array, RegisteredListenerCache> + * @phpstan-var array, RegisteredListenerCache> */ private array $syncHandlerCaches = []; /** @var AsyncHandlerList[] classname => AsyncHandlerList */ private array $allAsyncLists = []; + /** + * @var RegisteredListenerCache[] event class name => cache + * @phpstan-var array, RegisteredListenerCache> + */ + private array $asyncHandlerCaches = []; /** * Unregisters all the listeners @@ -119,6 +124,7 @@ public function getListFor(string $event) : HandlerList{ } $parent = self::resolveNearestHandleableParent($class); + /** @phpstan-var RegisteredListenerCache $cache */ $cache = new RegisteredListenerCache(); $this->syncHandlerCaches[$event] = $cache; return $this->allSyncLists[$event] = new HandlerList( @@ -150,9 +156,13 @@ public function getAsyncListFor(string $event) : AsyncHandlerList{ } $parent = self::resolveNearestHandleableParent($class); + /** @phpstan-var RegisteredListenerCache $cache */ + $cache = new RegisteredListenerCache(); + $this->asyncHandlerCaches[$event] = $cache; return $this->allAsyncLists[$event] = new AsyncHandlerList( $event, parentList: $parent !== null ? $this->getAsyncListFor($parent->getName()) : null, + handlerCache: $cache ); } @@ -167,6 +177,17 @@ public function getHandlersFor(string $event) : array{ return $cache?->list ?? $this->getListFor($event)->getListenerList(); } + /** + * @phpstan-param class-string $event + * + * @return AsyncRegisteredListener[] + */ + public function getAsyncHandlersFor(string $event) : array{ + $cache = $this->asyncHandlerCaches[$event] ?? null; + //getListFor() will populate the cache for the next call + return $cache?->list ?? $this->getAsyncListFor($event)->getListenerList(); + } + /** * @return HandlerList[] */ diff --git a/src/event/RegisteredListenerCache.php b/src/event/RegisteredListenerCache.php index e5e3155465b..ce7bfcb2c48 100644 --- a/src/event/RegisteredListenerCache.php +++ b/src/event/RegisteredListenerCache.php @@ -25,14 +25,14 @@ /** * @internal + * @phpstan-template TListener */ final class RegisteredListenerCache{ /** * List of all handlers that will be called for a particular event, ordered by execution order. * - * @var RegisteredListener[] - * @phpstan-var list + * @phpstan-var list */ public ?array $list = null; } diff --git a/tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php b/tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php index 091555fed3e..d63ba05cd7f 100644 --- a/tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php +++ b/tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php @@ -58,11 +58,12 @@ public function run() : void{ $plugin = $this->getPlugin(); $classes = self::EXPECTED_ORDER; shuffle($classes); - foreach($classes as $event){ + foreach($classes as $class){ $plugin->getServer()->getPluginManager()->registerAsyncEvent( - $event, - function(AsyncEvent $event) : ?Promise{ - $this->callOrder[] = $event::class; + $class, + function(AsyncEvent $event) use ($class) : ?Promise{ + var_dump($class); + $this->callOrder[] = $class; return null; }, EventPriority::NORMAL, From 96989d1dc4e7ba11cd60c70be69b03d07848b20e Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 13 Nov 2024 20:44:35 +0000 Subject: [PATCH 34/47] cleanup --- tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php b/tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php index d63ba05cd7f..b55bd081283 100644 --- a/tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php +++ b/tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php @@ -39,9 +39,7 @@ final class AsyncEventInheritanceTest extends Test{ ChildAsyncEvent::class, ParentAsyncEvent::class ]; - /** - * @var string[] - */ + /** @var string[] */ private array $callOrder = []; public function getName() : string{ @@ -62,7 +60,6 @@ public function run() : void{ $plugin->getServer()->getPluginManager()->registerAsyncEvent( $class, function(AsyncEvent $event) use ($class) : ?Promise{ - var_dump($class); $this->callOrder[] = $class; return null; }, From ac1cf73f8eacbd61058f36d93c2cfe1f96a97626 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 13 Nov 2024 21:09:52 +0000 Subject: [PATCH 35/47] Reduce code duplication --- src/event/AsyncHandlerList.php | 139 ++------------ src/event/AsyncRegisteredListener.php | 32 +--- src/event/BaseHandlerList.php | 170 ++++++++++++++++++ src/event/BaseRegisteredListener.php | 58 ++++++ src/event/HandlerList.php | 124 +------------ src/event/RegisteredListener.php | 33 +--- .../phpstan/configs/impossible-generics.neon | 13 +- 7 files changed, 259 insertions(+), 310 deletions(-) create mode 100644 src/event/BaseHandlerList.php create mode 100644 src/event/BaseRegisteredListener.php diff --git a/src/event/AsyncHandlerList.php b/src/event/AsyncHandlerList.php index 6a78f0f6500..db5c612ff66 100644 --- a/src/event/AsyncHandlerList.php +++ b/src/event/AsyncHandlerList.php @@ -23,134 +23,21 @@ namespace pocketmine\event; -use pocketmine\plugin\Plugin; -use function array_merge; -use function krsort; -use function spl_object_id; use function uasort; -use const SORT_NUMERIC; -class AsyncHandlerList{ - //TODO: we can probably deduplicate most of this code with the sync side if we throw in some generics - - /** @var AsyncRegisteredListener[][] */ - private array $handlerSlots = []; - - /** - * @var RegisteredListenerCache[] - * @phpstan-var array> - */ - private array $affectedHandlerCaches = []; - - /** - * @phpstan-param class-string $class - * @phpstan-param RegisteredListenerCache $handlerCache - */ - public function __construct( - private string $class, - private ?AsyncHandlerList $parentList, - private RegisteredListenerCache $handlerCache = new RegisteredListenerCache() - ){ - for($list = $this; $list !== null; $list = $list->parentList){ - $list->affectedHandlerCaches[spl_object_id($this->handlerCache)] = $this->handlerCache; - } - } - - /** - * @throws \Exception - */ - 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; - $this->invalidateAffectedCaches(); - } - - /** - * @param AsyncRegisteredListener[] $listeners - */ - public function registerAll(array $listeners) : void{ - foreach($listeners as $listener){ - $this->register($listener); - } - $this->invalidateAffectedCaches(); - } - - public function unregister(AsyncRegisteredListener|Plugin|Listener $object) : void{ - 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)]); - } - $this->invalidateAffectedCaches(); - } - - public function clear() : void{ - $this->handlerSlots = []; - $this->invalidateAffectedCaches(); - } - - /** - * @return AsyncRegisteredListener[] - */ - public function getListenersByPriority(int $priority) : array{ - return $this->handlerSlots[$priority] ?? []; - } - - public function getParent() : ?AsyncHandlerList{ - return $this->parentList; - } - - /** - * Invalidates all known caches which might be affected by this list's contents. - */ - private function invalidateAffectedCaches() : void{ - foreach($this->affectedHandlerCaches as $cache){ - $cache->list = null; - } - } - - /** - * @return AsyncRegisteredListener[] - * @phpstan-return list - */ - public function getListenerList() : array{ - if($this->handlerCache->list !== null){ - return $this->handlerCache->list; - } - - $handlerLists = []; - for($currentList = $this; $currentList !== null; $currentList = $currentList->parentList){ - $handlerLists[] = $currentList; - } - - $listenersByPriority = []; - foreach($handlerLists as $currentList){ - foreach($currentList->handlerSlots as $priority => $listeners){ - uasort($listeners, function(AsyncRegisteredListener $left, AsyncRegisteredListener $right) : int{ - //While the system can handle these in any order, it's better for latency if concurrent handlers - //are processed together. It doesn't matter whether they are processed before or after exclusive handlers. - if($right->canBeCalledConcurrently()){ - return $left->canBeCalledConcurrently() ? 0 : 1; - } - return -1; - }); - $listenersByPriority[$priority] = array_merge($listenersByPriority[$priority] ?? [], $listeners); +/** + * @phpstan-extends BaseHandlerList + */ +class AsyncHandlerList extends BaseHandlerList{ + protected function sortSamePriorityListeners(array $listeners) : array{ + uasort($listeners, function(AsyncRegisteredListener $left, AsyncRegisteredListener $right) : int{ + //While the system can handle these in any order, it's better for latency if concurrent handlers + //are processed together. It doesn't matter whether they are processed before or after exclusive handlers. + if($right->canBeCalledConcurrently()){ + return $left->canBeCalledConcurrently() ? 0 : 1; } - } - - //TODO: why on earth do the priorities have higher values for lower priority? - krsort($listenersByPriority, SORT_NUMERIC); - - return $this->handlerCache->list = array_merge(...$listenersByPriority); + return -1; + }); + return $listeners; } } diff --git a/src/event/AsyncRegisteredListener.php b/src/event/AsyncRegisteredListener.php index 4e6158e9efe..4c5c0b957dc 100644 --- a/src/event/AsyncRegisteredListener.php +++ b/src/event/AsyncRegisteredListener.php @@ -26,32 +26,17 @@ use pocketmine\plugin\Plugin; use pocketmine\promise\Promise; use pocketmine\timings\TimingsHandler; -use function in_array; -class AsyncRegisteredListener{ +class AsyncRegisteredListener extends BaseRegisteredListener{ public function __construct( - private \Closure $handler, - private int $priority, - private Plugin $plugin, - private bool $handleCancelled, + \Closure $handler, + int $priority, + Plugin $plugin, + bool $handleCancelled, private bool $exclusiveCall, - private TimingsHandler $timings + TimingsHandler $timings ){ - if(!in_array($priority, EventPriority::ALL, true)){ - throw new \InvalidArgumentException("Invalid priority number $priority"); - } - } - - public function getHandler() : \Closure{ - return $this->handler; - } - - public function getPlugin() : Plugin{ - return $this->plugin; - } - - public function getPriority() : int{ - return $this->priority; + parent::__construct($handler, $priority, $plugin, $handleCancelled, $timings); } /** @@ -69,9 +54,6 @@ public function callAsync(AsyncEvent $event) : ?Promise{ } } - public function isHandlingCancelled() : bool{ - return $this->handleCancelled; - } public function canBeCalledConcurrently() : bool{ return !$this->exclusiveCall; } diff --git a/src/event/BaseHandlerList.php b/src/event/BaseHandlerList.php new file mode 100644 index 00000000000..ebdd7c89082 --- /dev/null +++ b/src/event/BaseHandlerList.php @@ -0,0 +1,170 @@ +> + */ + private array $handlerSlots = []; + + /** + * @var RegisteredListenerCache[] + * @phpstan-var array> + */ + private array $affectedHandlerCaches = []; + + /** + * @phpstan-param class-string $class + * @phpstan-param ?static $parentList + * @phpstan-param RegisteredListenerCache $handlerCache + */ + public function __construct( + private string $class, + private ?BaseHandlerList $parentList, + private RegisteredListenerCache $handlerCache = new RegisteredListenerCache() + ){ + for($list = $this; $list !== null; $list = $list->parentList){ + $list->affectedHandlerCaches[spl_object_id($this->handlerCache)] = $this->handlerCache; + } + } + + /** + * @phpstan-param TListener $listener + */ + public function register(BaseRegisteredListener $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; + $this->invalidateAffectedCaches(); + } + + /** + * @param BaseRegisteredListener[] $listeners + * @phpstan-param array $listeners + */ + public function registerAll(array $listeners) : void{ + foreach($listeners as $listener){ + $this->register($listener); + } + $this->invalidateAffectedCaches(); + } + + /** + * @phpstan-param TListener|Plugin|Listener $object + */ + public function unregister(BaseRegisteredListener|Plugin|Listener $object) : void{ + 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)]); + } + $this->invalidateAffectedCaches(); + } + + public function clear() : void{ + $this->handlerSlots = []; + $this->invalidateAffectedCaches(); + } + + /** + * @return BaseRegisteredListener[] + * @phpstan-return array + */ + public function getListenersByPriority(int $priority) : array{ + return $this->handlerSlots[$priority] ?? []; + } + + /** + * @phpstan-return static + */ + public function getParent() : ?BaseHandlerList{ + return $this->parentList; + } + + /** + * Invalidates all known caches which might be affected by this list's contents. + */ + private function invalidateAffectedCaches() : void{ + foreach($this->affectedHandlerCaches as $cache){ + $cache->list = null; + } + } + + /** + * @param BaseRegisteredListener[] $listeners + * @phpstan-param array $listeners + * + * @return BaseRegisteredListener[] + * @phpstan-return array + */ + abstract protected function sortSamePriorityListeners(array $listeners) : array; + + /** + * @return BaseRegisteredListener[] + * @phpstan-return list + */ + public function getListenerList() : array{ + if($this->handlerCache->list !== null){ + return $this->handlerCache->list; + } + + $handlerLists = []; + for($currentList = $this; $currentList !== null; $currentList = $currentList->parentList){ + $handlerLists[] = $currentList; + } + + $listenersByPriority = []; + foreach($handlerLists as $currentList){ + foreach($currentList->handlerSlots as $priority => $listeners){ + $listenersByPriority[$priority] = array_merge($listenersByPriority[$priority] ?? [], $this->sortSamePriorityListeners($listeners)); + } + } + + //TODO: why on earth do the priorities have higher values for lower priority? + krsort($listenersByPriority, SORT_NUMERIC); + + return $this->handlerCache->list = array_merge(...$listenersByPriority); + } +} diff --git a/src/event/BaseRegisteredListener.php b/src/event/BaseRegisteredListener.php new file mode 100644 index 00000000000..0c685f09b10 --- /dev/null +++ b/src/event/BaseRegisteredListener.php @@ -0,0 +1,58 @@ +handler; + } + + public function getPlugin() : Plugin{ + return $this->plugin; + } + + public function getPriority() : int{ + return $this->priority; + } + + public function isHandlingCancelled() : bool{ + return $this->handleCancelled; + } +} diff --git a/src/event/HandlerList.php b/src/event/HandlerList.php index 0c6b0f66ecf..53c5148faf7 100644 --- a/src/event/HandlerList.php +++ b/src/event/HandlerList.php @@ -23,123 +23,11 @@ namespace pocketmine\event; -use pocketmine\plugin\Plugin; -use function array_merge; -use function krsort; -use function spl_object_id; -use const SORT_NUMERIC; - -class HandlerList{ - /** @var RegisteredListener[][] */ - private array $handlerSlots = []; - - /** - * @var RegisteredListenerCache[] - * @phpstan-var array> - */ - private array $affectedHandlerCaches = []; - - /** - * @phpstan-param class-string $class - * @phpstan-param RegisteredListenerCache $handlerCache - */ - public function __construct( - private string $class, - private ?HandlerList $parentList, - private RegisteredListenerCache $handlerCache = new RegisteredListenerCache() - ){ - for($list = $this; $list !== null; $list = $list->parentList){ - $list->affectedHandlerCaches[spl_object_id($this->handlerCache)] = $this->handlerCache; - } - } - - /** - * @throws \Exception - */ - public function register(RegisteredListener $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; - $this->invalidateAffectedCaches(); - } - - /** - * @param RegisteredListener[] $listeners - */ - public function registerAll(array $listeners) : void{ - foreach($listeners as $listener){ - $this->register($listener); - } - $this->invalidateAffectedCaches(); - } - - public function unregister(RegisteredListener|Plugin|Listener $object) : void{ - 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)]); - } - $this->invalidateAffectedCaches(); - } - - public function clear() : void{ - $this->handlerSlots = []; - $this->invalidateAffectedCaches(); - } - - /** - * @return RegisteredListener[] - */ - public function getListenersByPriority(int $priority) : array{ - return $this->handlerSlots[$priority] ?? []; - } - - public function getParent() : ?HandlerList{ - return $this->parentList; - } - - /** - * Invalidates all known caches which might be affected by this list's contents. - */ - private function invalidateAffectedCaches() : void{ - foreach($this->affectedHandlerCaches as $cache){ - $cache->list = null; - } - } - - /** - * @return RegisteredListener[] - * @phpstan-return list - */ - public function getListenerList() : array{ - if($this->handlerCache->list !== null){ - return $this->handlerCache->list; - } - - $handlerLists = []; - for($currentList = $this; $currentList !== null; $currentList = $currentList->parentList){ - $handlerLists[] = $currentList; - } - - $listenersByPriority = []; - foreach($handlerLists as $currentList){ - foreach($currentList->handlerSlots as $priority => $listeners){ - $listenersByPriority[$priority] = array_merge($listenersByPriority[$priority] ?? [], $listeners); - } - } - - //TODO: why on earth do the priorities have higher values for lower priority? - krsort($listenersByPriority, SORT_NUMERIC); - - return $this->handlerCache->list = array_merge(...$listenersByPriority); +/** + * @phpstan-extends BaseHandlerList + */ +class HandlerList extends BaseHandlerList{ + protected function sortSamePriorityListeners(array $listeners) : array{ + return $listeners; } } diff --git a/src/event/RegisteredListener.php b/src/event/RegisteredListener.php index 6b29dfec30f..a2217c2996c 100644 --- a/src/event/RegisteredListener.php +++ b/src/event/RegisteredListener.php @@ -23,34 +23,7 @@ namespace pocketmine\event; -use pocketmine\plugin\Plugin; -use pocketmine\timings\TimingsHandler; -use function in_array; - -class RegisteredListener{ - public function __construct( - private \Closure $handler, - private int $priority, - private Plugin $plugin, - private bool $handleCancelled, - private TimingsHandler $timings - ){ - if(!in_array($priority, EventPriority::ALL, true)){ - throw new \InvalidArgumentException("Invalid priority number $priority"); - } - } - - public function getHandler() : \Closure{ - return $this->handler; - } - - public function getPlugin() : Plugin{ - return $this->plugin; - } - - public function getPriority() : int{ - return $this->priority; - } +class RegisteredListener extends BaseRegisteredListener{ public function callEvent(Event $event) : void{ if($event instanceof Cancellable && $event->isCancelled() && !$this->isHandlingCancelled()){ @@ -63,8 +36,4 @@ public function callEvent(Event $event) : void{ $this->timings->stopTiming(); } } - - public function isHandlingCancelled() : bool{ - return $this->handleCancelled; - } } diff --git a/tests/phpstan/configs/impossible-generics.neon b/tests/phpstan/configs/impossible-generics.neon index a2d10becfe6..9b57d6604c0 100644 --- a/tests/phpstan/configs/impossible-generics.neon +++ b/tests/phpstan/configs/impossible-generics.neon @@ -6,17 +6,12 @@ parameters: path: ../../../src/event/AsyncRegisteredListener.php - - message: "#^Method pocketmine\\\\event\\\\AsyncRegisteredListener\\:\\:getHandler\\(\\) return type has no signature specified for Closure\\.$#" + message: "#^Method pocketmine\\\\event\\\\BaseRegisteredListener\\:\\:__construct\\(\\) has parameter \\$handler with no signature specified for Closure\\.$#" count: 1 - path: ../../../src/event/AsyncRegisteredListener.php - - - - message: "#^Method pocketmine\\\\event\\\\RegisteredListener\\:\\:__construct\\(\\) has parameter \\$handler with no signature specified for Closure\\.$#" - count: 1 - path: ../../../src/event/RegisteredListener.php + path: ../../../src/event/BaseRegisteredListener.php - - message: "#^Method pocketmine\\\\event\\\\RegisteredListener\\:\\:getHandler\\(\\) return type has no signature specified for Closure\\.$#" + message: "#^Method pocketmine\\\\event\\\\BaseRegisteredListener\\:\\:getHandler\\(\\) return type has no signature specified for Closure\\.$#" count: 1 - path: ../../../src/event/RegisteredListener.php + path: ../../../src/event/BaseRegisteredListener.php From 972a9fb201cd144d58dee87e9369f1bf6c8c377e Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 13 Nov 2024 21:30:24 +0000 Subject: [PATCH 36/47] PluginManager: ensure that handler candidates of async events with wrong return types don't attempt to register as sync events this will cause other, more confusing errors to be thrown. to be honest, I'm not sure if enforcing the return type here is even necessary (or desirable). --- src/plugin/PluginManager.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/plugin/PluginManager.php b/src/plugin/PluginManager.php index afbdf64fdc0..355d1eeaeb3 100644 --- a/src/plugin/PluginManager.php +++ b/src/plugin/PluginManager.php @@ -654,7 +654,10 @@ public function registerEvents(Listener $listener, Plugin $plugin) : void{ } } - if(is_subclass_of($eventClass, AsyncEvent::class) && $this->canHandleAsyncEvent($handlerClosure)){ + if(is_subclass_of($eventClass, AsyncEvent::class)){ + if(!$this->canHandleAsyncEvent($handlerClosure)){ + throw new PluginException("Event handler " . Utils::getNiceClosureName($handlerClosure) . " must return null|Promise to be able to handle async events"); + } $this->registerAsyncEvent($eventClass, $handlerClosure, $priority, $plugin, $handleCancelled, $exclusiveCall); }else{ $this->registerEvent($eventClass, $handlerClosure, $priority, $plugin, $handleCancelled); From 667656b1c61cf57394b45c6125ea0e000bf60baf Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 13 Nov 2024 22:08:28 +0000 Subject: [PATCH 37/47] Split AsyncHandlerListManager this allows further code deduplication at the expense of needing 2 calls to unregister all handlers --- src/Server.php | 2 + src/event/AsyncEvent.php | 2 +- src/event/AsyncHandlerListManager.php | 43 +++++ src/event/BaseHandlerListManager.php | 156 +++++++++++++++ src/event/HandlerListManager.php | 177 +----------------- src/plugin/PluginManager.php | 4 +- .../src/AsyncEventConcurrencyTest.php | 4 +- .../src/AsyncEventInheritanceTest.php | 4 +- .../src/AsyncEventPriorityTest.php | 4 +- 9 files changed, 219 insertions(+), 177 deletions(-) create mode 100644 src/event/AsyncHandlerListManager.php create mode 100644 src/event/BaseHandlerListManager.php diff --git a/src/Server.php b/src/Server.php index a34349bb5cd..046e4a4819d 100644 --- a/src/Server.php +++ b/src/Server.php @@ -38,6 +38,7 @@ use pocketmine\crash\CrashDumpRenderer; use pocketmine\entity\EntityDataHelper; use pocketmine\entity\Location; +use pocketmine\event\AsyncHandlerListManager; use pocketmine\event\HandlerListManager; use pocketmine\event\player\PlayerCreationEvent; use pocketmine\event\player\PlayerDataSaveEvent; @@ -1485,6 +1486,7 @@ public function forceShutdown() : void{ $this->logger->debug("Removing event handlers"); HandlerListManager::global()->unregisterAll(); + AsyncHandlerListManager::global()->unregisterAll(); if(isset($this->asyncPool)){ $this->logger->debug("Shutting down async task worker pool"); diff --git a/src/event/AsyncEvent.php b/src/event/AsyncEvent.php index 2db63a0df5d..6f5c6fc1437 100644 --- a/src/event/AsyncEvent.php +++ b/src/event/AsyncEvent.php @@ -60,7 +60,7 @@ final public function call() : Promise{ /** @phpstan-var PromiseResolver $globalResolver */ $globalResolver = new PromiseResolver(); - $this->processRemainingHandlers(HandlerListManager::global()->getAsyncHandlersFor(static::class), $globalResolver); + $this->processRemainingHandlers(AsyncHandlerListManager::global()->getHandlersFor(static::class), $globalResolver); return $globalResolver->getPromise(); }finally{ diff --git a/src/event/AsyncHandlerListManager.php b/src/event/AsyncHandlerListManager.php new file mode 100644 index 00000000000..8dd9461c955 --- /dev/null +++ b/src/event/AsyncHandlerListManager.php @@ -0,0 +1,43 @@ + + */ +final class AsyncHandlerListManager extends BaseHandlerListManager{ + private static ?self $globalInstance = null; + + public static function global() : self{ + return self::$globalInstance ?? (self::$globalInstance = new self()); + } + + protected function getBaseEventClass() : string{ + return AsyncEvent::class; + } + + protected function createHandlerList(string $event, ?BaseHandlerList $parentList, RegisteredListenerCache $handlerCache) : BaseHandlerList{ + return new AsyncHandlerList($event, $parentList, $handlerCache); + } +} diff --git a/src/event/BaseHandlerListManager.php b/src/event/BaseHandlerListManager.php new file mode 100644 index 00000000000..c8f0a59e7e6 --- /dev/null +++ b/src/event/BaseHandlerListManager.php @@ -0,0 +1,156 @@ + + */ +abstract class BaseHandlerListManager{ + /** + * @var BaseHandlerList[] classname => BaseHandlerList + * @phpstan-var array, THandlerList> + */ + private array $allLists = []; + /** + * @var RegisteredListenerCache[] event class name => cache + * @phpstan-var array, RegisteredListenerCache> + */ + private array $handlerCaches = []; + + /** + * Unregisters all the listeners + * If a Plugin or Listener is passed, all the listeners with that object will be removed + * + * @phpstan-param TRegisteredListener|Plugin|Listener|null $object + */ + public function unregisterAll(BaseRegisteredListener|Plugin|Listener|null $object = null) : void{ + if($object !== null){ + foreach($this->allLists as $h){ + $h->unregister($object); + } + }else{ + foreach($this->allLists as $h){ + $h->clear(); + } + } + } + + /** + * @phpstan-param \ReflectionClass $class + */ + private static function isValidClass(\ReflectionClass $class) : bool{ + $tags = Utils::parseDocComment((string) $class->getDocComment()); + return !$class->isAbstract() || isset($tags["allowHandle"]); + } + + /** + * @phpstan-param \ReflectionClass $class + * + * @phpstan-return \ReflectionClass|null + */ + private static function resolveNearestHandleableParent(\ReflectionClass $class) : ?\ReflectionClass{ + for($parent = $class->getParentClass(); $parent !== false; $parent = $parent->getParentClass()){ + if(self::isValidClass($parent)){ + return $parent; + } + //NOOP + } + return null; + } + + /** + * @phpstan-return class-string + */ + abstract protected function getBaseEventClass() : string; + + /** + * @phpstan-param class-string $event + * @phpstan-param THandlerList|null $parentList + * @phpstan-param RegisteredListenerCache $handlerCache + * + * @phpstan-return THandlerList + */ + abstract protected function createHandlerList(string $event, ?BaseHandlerList $parentList, RegisteredListenerCache $handlerCache) : BaseHandlerList; + + /** + * Returns the HandlerList for listeners that explicitly handle this event. + * + * Calling this method also lazily initializes the $classMap inheritance tree of handler lists. + * + * @phpstan-param class-string $event + * @phpstan-return THandlerList + * + * @throws \ReflectionException + * @throws \InvalidArgumentException + */ + public function getListFor(string $event) : BaseHandlerList{ + if(isset($this->allLists[$event])){ + return $this->allLists[$event]; + } + + $class = new \ReflectionClass($event); + if(!$class->isSubclassOf($this->getBaseEventClass())){ + 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); + /** @phpstan-var RegisteredListenerCache $cache */ + $cache = new RegisteredListenerCache(); + $this->handlerCaches[$event] = $cache; + return $this->allLists[$event] = $this->createHandlerList( + $event, + parentList: $parent !== null ? $this->getListFor($parent->getName()) : null, + handlerCache: $cache + ); + } + + /** + * @phpstan-param class-string $event + * + * @return RegisteredListener[] + * @phpstan-return list + */ + public function getHandlersFor(string $event) : array{ + $cache = $this->handlerCaches[$event] ?? null; + //getListFor() will populate the cache for the next call + return $cache?->list ?? $this->getListFor($event)->getListenerList(); + } + + /** + * @return HandlerList[] + * @phpstan-return array, THandlerList> + */ + public function getAll() : array{ + return $this->allLists; + } +} diff --git a/src/event/HandlerListManager.php b/src/event/HandlerListManager.php index ecfe073f6a3..85be5238703 100644 --- a/src/event/HandlerListManager.php +++ b/src/event/HandlerListManager.php @@ -23,182 +23,21 @@ namespace pocketmine\event; -use pocketmine\plugin\Plugin; -use pocketmine\utils\Utils; - -class HandlerListManager{ - +/** + * @phpstan-extends BaseHandlerListManager + */ +class HandlerListManager extends BaseHandlerListManager{ private static ?self $globalInstance = null; public static function global() : self{ return self::$globalInstance ?? (self::$globalInstance = new self()); } - /** @var HandlerList[] classname => HandlerList */ - private array $allSyncLists = []; - /** - * @var RegisteredListenerCache[] event class name => cache - * @phpstan-var array, RegisteredListenerCache> - */ - private array $syncHandlerCaches = []; - - /** @var AsyncHandlerList[] classname => AsyncHandlerList */ - private array $allAsyncLists = []; - /** - * @var RegisteredListenerCache[] event class name => cache - * @phpstan-var array, RegisteredListenerCache> - */ - private array $asyncHandlerCaches = []; - - /** - * Unregisters all the listeners - * If a Plugin or Listener is passed, all the listeners with that object will be removed - */ - 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->allSyncLists as $h){ - $h->clear(); - } - foreach($this->allAsyncLists as $h){ - $h->clear(); - } - } - } - - /** - * @phpstan-param \ReflectionClass $class - */ - private static function isValidClass(\ReflectionClass $class) : bool{ - $tags = Utils::parseDocComment((string) $class->getDocComment()); - return !$class->isAbstract() || isset($tags["allowHandle"]); - } - - /** - * @phpstan-template TEvent of Event|AsyncEvent - * @phpstan-param \ReflectionClass $class - * - * @phpstan-return \ReflectionClass|null - */ - private static function resolveNearestHandleableParent(\ReflectionClass $class) : ?\ReflectionClass{ - for($parent = $class->getParentClass(); $parent !== false; $parent = $parent->getParentClass()){ - if(self::isValidClass($parent)){ - return $parent; - } - //NOOP - } - return null; - } - - /** - * Returns the HandlerList for listeners that explicitly handle this event. - * - * Calling this method also lazily initializes the $classMap inheritance tree of handler lists. - * - * @phpstan-param class-string $event - * - * @throws \ReflectionException - * @throws \InvalidArgumentException - */ - public function getListFor(string $event) : HandlerList{ - 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); - /** @phpstan-var RegisteredListenerCache $cache */ - $cache = new RegisteredListenerCache(); - $this->syncHandlerCaches[$event] = $cache; - return $this->allSyncLists[$event] = new HandlerList( - $event, - parentList: $parent !== null ? $this->getListFor($parent->getName()) : null, - handlerCache: $cache - ); - } - - /** - * 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); - /** @phpstan-var RegisteredListenerCache $cache */ - $cache = new RegisteredListenerCache(); - $this->asyncHandlerCaches[$event] = $cache; - return $this->allAsyncLists[$event] = new AsyncHandlerList( - $event, - parentList: $parent !== null ? $this->getAsyncListFor($parent->getName()) : null, - handlerCache: $cache - ); - } - - /** - * @phpstan-param class-string $event - * - * @return RegisteredListener[] - */ - public function getHandlersFor(string $event) : array{ - $cache = $this->syncHandlerCaches[$event] ?? null; - //getListFor() will populate the cache for the next call - return $cache?->list ?? $this->getListFor($event)->getListenerList(); - } - - /** - * @phpstan-param class-string $event - * - * @return AsyncRegisteredListener[] - */ - public function getAsyncHandlersFor(string $event) : array{ - $cache = $this->asyncHandlerCaches[$event] ?? null; - //getListFor() will populate the cache for the next call - return $cache?->list ?? $this->getAsyncListFor($event)->getListenerList(); - } - - /** - * @return HandlerList[] - */ - public function getAll() : array{ - return $this->allSyncLists; + protected function getBaseEventClass() : string{ + return Event::class; } - /** - * @return AsyncHandlerList[] - */ - public function getAllAsync() : array{ - return $this->allAsyncLists; + protected function createHandlerList(string $event, ?BaseHandlerList $parentList, RegisteredListenerCache $handlerCache) : BaseHandlerList{ + return new HandlerList($event, $parentList, $handlerCache); } } diff --git a/src/plugin/PluginManager.php b/src/plugin/PluginManager.php index 355d1eeaeb3..6e793e04f2c 100644 --- a/src/plugin/PluginManager.php +++ b/src/plugin/PluginManager.php @@ -24,6 +24,7 @@ namespace pocketmine\plugin; use pocketmine\event\AsyncEvent; +use pocketmine\event\AsyncHandlerListManager; use pocketmine\event\AsyncRegisteredListener; use pocketmine\event\Cancellable; use pocketmine\event\Event; @@ -525,6 +526,7 @@ public function disablePlugin(Plugin $plugin) : void{ $plugin->onEnableStateChange(false); $plugin->getScheduler()->shutdown(); HandlerListManager::global()->unregisterAll($plugin); + AsyncHandlerListManager::global()->unregisterAll($plugin); } } @@ -721,7 +723,7 @@ public function registerAsyncEvent(string $event, \Closure $handler, int $priori $timings = Timings::getEventHandlerTimings($event, $handlerName, $plugin->getDescription()->getFullName()); $registeredListener = new AsyncRegisteredListener($handler, $priority, $plugin, $handleCancelled, $exclusiveCall, $timings); - HandlerListManager::global()->getAsyncListFor($event)->register($registeredListener); + AsyncHandlerListManager::global()->getListFor($event)->register($registeredListener); return $registeredListener; } diff --git a/tests/plugins/TesterPlugin/src/AsyncEventConcurrencyTest.php b/tests/plugins/TesterPlugin/src/AsyncEventConcurrencyTest.php index 2eed286ae4c..856ee45adcd 100644 --- a/tests/plugins/TesterPlugin/src/AsyncEventConcurrencyTest.php +++ b/tests/plugins/TesterPlugin/src/AsyncEventConcurrencyTest.php @@ -24,8 +24,8 @@ namespace pmmp\TesterPlugin; use pmmp\TesterPlugin\event\GrandchildAsyncEvent; +use pocketmine\event\AsyncHandlerListManager; use pocketmine\event\EventPriority; -use pocketmine\event\HandlerListManager; use pocketmine\promise\Promise; use pocketmine\promise\PromiseResolver; @@ -50,7 +50,7 @@ public function getDescription() : string{ } public function run() : void{ - HandlerListManager::global()->unregisterAll(); + AsyncHandlerListManager::global()->unregisterAll(); $main = $this->getPlugin(); $pluginManager = $main->getServer()->getPluginManager(); diff --git a/tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php b/tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php index b55bd081283..d952e36b10e 100644 --- a/tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php +++ b/tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php @@ -27,8 +27,8 @@ use pmmp\TesterPlugin\event\GrandchildAsyncEvent; use pmmp\TesterPlugin\event\ParentAsyncEvent; use pocketmine\event\AsyncEvent; +use pocketmine\event\AsyncHandlerListManager; use pocketmine\event\EventPriority; -use pocketmine\event\HandlerListManager; use pocketmine\promise\Promise; use function implode; use function shuffle; @@ -51,7 +51,7 @@ public function getDescription() : string{ } public function run() : void{ - HandlerListManager::global()->unregisterAll(); + AsyncHandlerListManager::global()->unregisterAll(); $plugin = $this->getPlugin(); $classes = self::EXPECTED_ORDER; diff --git a/tests/plugins/TesterPlugin/src/AsyncEventPriorityTest.php b/tests/plugins/TesterPlugin/src/AsyncEventPriorityTest.php index ac8b56407c4..4ac5766d1e5 100644 --- a/tests/plugins/TesterPlugin/src/AsyncEventPriorityTest.php +++ b/tests/plugins/TesterPlugin/src/AsyncEventPriorityTest.php @@ -24,8 +24,8 @@ namespace pmmp\TesterPlugin; use pmmp\TesterPlugin\event\GrandchildAsyncEvent; +use pocketmine\event\AsyncHandlerListManager; use pocketmine\event\EventPriority; -use pocketmine\event\HandlerListManager; use pocketmine\promise\Promise; use pocketmine\promise\PromiseResolver; @@ -47,7 +47,7 @@ public function getDescription() : string{ } public function run() : void{ - HandlerListManager::global()->unregisterAll(); + AsyncHandlerListManager::global()->unregisterAll(); $main = $this->getPlugin(); $pluginManager = $main->getServer()->getPluginManager(); From edae9f26e4f25a6a49b3bc6fb955f3bb105804c8 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 13 Nov 2024 22:23:43 +0000 Subject: [PATCH 38/47] Reduce number of classes --- src/event/AsyncHandlerList.php | 43 ------- src/event/AsyncHandlerListManager.php | 22 +++- src/event/BaseHandlerList.php | 170 -------------------------- src/event/BaseHandlerListManager.php | 10 +- src/event/HandlerList.php | 142 ++++++++++++++++++++- src/event/HandlerListManager.php | 2 +- 6 files changed, 164 insertions(+), 225 deletions(-) delete mode 100644 src/event/AsyncHandlerList.php delete mode 100644 src/event/BaseHandlerList.php diff --git a/src/event/AsyncHandlerList.php b/src/event/AsyncHandlerList.php deleted file mode 100644 index db5c612ff66..00000000000 --- a/src/event/AsyncHandlerList.php +++ /dev/null @@ -1,43 +0,0 @@ - - */ -class AsyncHandlerList extends BaseHandlerList{ - protected function sortSamePriorityListeners(array $listeners) : array{ - uasort($listeners, function(AsyncRegisteredListener $left, AsyncRegisteredListener $right) : int{ - //While the system can handle these in any order, it's better for latency if concurrent handlers - //are processed together. It doesn't matter whether they are processed before or after exclusive handlers. - if($right->canBeCalledConcurrently()){ - return $left->canBeCalledConcurrently() ? 0 : 1; - } - return -1; - }); - return $listeners; - } -} diff --git a/src/event/AsyncHandlerListManager.php b/src/event/AsyncHandlerListManager.php index 8dd9461c955..b6d24991906 100644 --- a/src/event/AsyncHandlerListManager.php +++ b/src/event/AsyncHandlerListManager.php @@ -23,6 +23,8 @@ namespace pocketmine\event; +use function uasort; + /** * @phpstan-extends BaseHandlerListManager */ @@ -37,7 +39,23 @@ protected function getBaseEventClass() : string{ return AsyncEvent::class; } - protected function createHandlerList(string $event, ?BaseHandlerList $parentList, RegisteredListenerCache $handlerCache) : BaseHandlerList{ - return new AsyncHandlerList($event, $parentList, $handlerCache); + /** + * @phpstan-param array $listeners + * @phpstan-return array + */ + private static function sortSamePriorityHandlers(array $listeners) : array{ + uasort($listeners, function(AsyncRegisteredListener $left, AsyncRegisteredListener $right) : int{ + //While the system can handle these in any order, it's better for latency if concurrent handlers + //are processed together. It doesn't matter whether they are processed before or after exclusive handlers. + if($right->canBeCalledConcurrently()){ + return $left->canBeCalledConcurrently() ? 0 : 1; + } + return -1; + }); + return $listeners; + } + + protected function createHandlerList(string $event, ?HandlerList $parentList, RegisteredListenerCache $handlerCache) : HandlerList{ + return new HandlerList($event, $parentList, $handlerCache, self::sortSamePriorityHandlers(...)); } } diff --git a/src/event/BaseHandlerList.php b/src/event/BaseHandlerList.php deleted file mode 100644 index ebdd7c89082..00000000000 --- a/src/event/BaseHandlerList.php +++ /dev/null @@ -1,170 +0,0 @@ -> - */ - private array $handlerSlots = []; - - /** - * @var RegisteredListenerCache[] - * @phpstan-var array> - */ - private array $affectedHandlerCaches = []; - - /** - * @phpstan-param class-string $class - * @phpstan-param ?static $parentList - * @phpstan-param RegisteredListenerCache $handlerCache - */ - public function __construct( - private string $class, - private ?BaseHandlerList $parentList, - private RegisteredListenerCache $handlerCache = new RegisteredListenerCache() - ){ - for($list = $this; $list !== null; $list = $list->parentList){ - $list->affectedHandlerCaches[spl_object_id($this->handlerCache)] = $this->handlerCache; - } - } - - /** - * @phpstan-param TListener $listener - */ - public function register(BaseRegisteredListener $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; - $this->invalidateAffectedCaches(); - } - - /** - * @param BaseRegisteredListener[] $listeners - * @phpstan-param array $listeners - */ - public function registerAll(array $listeners) : void{ - foreach($listeners as $listener){ - $this->register($listener); - } - $this->invalidateAffectedCaches(); - } - - /** - * @phpstan-param TListener|Plugin|Listener $object - */ - public function unregister(BaseRegisteredListener|Plugin|Listener $object) : void{ - 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)]); - } - $this->invalidateAffectedCaches(); - } - - public function clear() : void{ - $this->handlerSlots = []; - $this->invalidateAffectedCaches(); - } - - /** - * @return BaseRegisteredListener[] - * @phpstan-return array - */ - public function getListenersByPriority(int $priority) : array{ - return $this->handlerSlots[$priority] ?? []; - } - - /** - * @phpstan-return static - */ - public function getParent() : ?BaseHandlerList{ - return $this->parentList; - } - - /** - * Invalidates all known caches which might be affected by this list's contents. - */ - private function invalidateAffectedCaches() : void{ - foreach($this->affectedHandlerCaches as $cache){ - $cache->list = null; - } - } - - /** - * @param BaseRegisteredListener[] $listeners - * @phpstan-param array $listeners - * - * @return BaseRegisteredListener[] - * @phpstan-return array - */ - abstract protected function sortSamePriorityListeners(array $listeners) : array; - - /** - * @return BaseRegisteredListener[] - * @phpstan-return list - */ - public function getListenerList() : array{ - if($this->handlerCache->list !== null){ - return $this->handlerCache->list; - } - - $handlerLists = []; - for($currentList = $this; $currentList !== null; $currentList = $currentList->parentList){ - $handlerLists[] = $currentList; - } - - $listenersByPriority = []; - foreach($handlerLists as $currentList){ - foreach($currentList->handlerSlots as $priority => $listeners){ - $listenersByPriority[$priority] = array_merge($listenersByPriority[$priority] ?? [], $this->sortSamePriorityListeners($listeners)); - } - } - - //TODO: why on earth do the priorities have higher values for lower priority? - krsort($listenersByPriority, SORT_NUMERIC); - - return $this->handlerCache->list = array_merge(...$listenersByPriority); - } -} diff --git a/src/event/BaseHandlerListManager.php b/src/event/BaseHandlerListManager.php index c8f0a59e7e6..5183fed9b36 100644 --- a/src/event/BaseHandlerListManager.php +++ b/src/event/BaseHandlerListManager.php @@ -30,11 +30,11 @@ * @phpstan-template TEvent of Event|AsyncEvent * @phpstan-template TRegisteredListener of BaseRegisteredListener * - * @phpstan-type THandlerList BaseHandlerList + * @phpstan-type THandlerList HandlerList */ abstract class BaseHandlerListManager{ /** - * @var BaseHandlerList[] classname => BaseHandlerList + * @var HandlerList[] classname => HandlerList * @phpstan-var array, THandlerList> */ private array $allLists = []; @@ -92,12 +92,12 @@ abstract protected function getBaseEventClass() : string; /** * @phpstan-param class-string $event - * @phpstan-param THandlerList|null $parentList + * @phpstan-param HandlerList|null $parentList * @phpstan-param RegisteredListenerCache $handlerCache * * @phpstan-return THandlerList */ - abstract protected function createHandlerList(string $event, ?BaseHandlerList $parentList, RegisteredListenerCache $handlerCache) : BaseHandlerList; + abstract protected function createHandlerList(string $event, ?HandlerList $parentList, RegisteredListenerCache $handlerCache) : HandlerList; /** * Returns the HandlerList for listeners that explicitly handle this event. @@ -110,7 +110,7 @@ abstract protected function createHandlerList(string $event, ?BaseHandlerList $p * @throws \ReflectionException * @throws \InvalidArgumentException */ - public function getListFor(string $event) : BaseHandlerList{ + public function getListFor(string $event) : HandlerList{ if(isset($this->allLists[$event])){ return $this->allLists[$event]; } diff --git a/src/event/HandlerList.php b/src/event/HandlerList.php index 53c5148faf7..b1d53ab8638 100644 --- a/src/event/HandlerList.php +++ b/src/event/HandlerList.php @@ -23,11 +23,145 @@ namespace pocketmine\event; +use pocketmine\plugin\Plugin; +use function array_merge; +use function krsort; +use function spl_object_id; +use const SORT_NUMERIC; + /** - * @phpstan-extends BaseHandlerList + * @phpstan-template TListener of BaseRegisteredListener */ -class HandlerList extends BaseHandlerList{ - protected function sortSamePriorityListeners(array $listeners) : array{ - return $listeners; +class HandlerList{ + /** + * @var BaseRegisteredListener[][] + * @phpstan-var array> + */ + private array $handlerSlots = []; + + /** + * @var RegisteredListenerCache[] + * @phpstan-var array> + */ + private array $affectedHandlerCaches = []; + + /** + * @phpstan-param class-string $class + * @phpstan-param ?static $parentList + * @phpstan-param RegisteredListenerCache $handlerCache + * @phpstan-param ?\Closure(array) : array $sortSamePriorityHandlers + */ + public function __construct( + private string $class, + private ?HandlerList $parentList, + private RegisteredListenerCache $handlerCache = new RegisteredListenerCache(), + private ?\Closure $sortSamePriorityHandlers = null + ){ + for($list = $this; $list !== null; $list = $list->parentList){ + $list->affectedHandlerCaches[spl_object_id($this->handlerCache)] = $this->handlerCache; + } + } + + /** + * @phpstan-param TListener $listener + */ + public function register(BaseRegisteredListener $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; + $this->invalidateAffectedCaches(); + } + + /** + * @param BaseRegisteredListener[] $listeners + * @phpstan-param array $listeners + */ + public function registerAll(array $listeners) : void{ + foreach($listeners as $listener){ + $this->register($listener); + } + $this->invalidateAffectedCaches(); + } + + /** + * @phpstan-param TListener|Plugin|Listener $object + */ + public function unregister(BaseRegisteredListener|Plugin|Listener $object) : void{ + 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)]); + } + $this->invalidateAffectedCaches(); + } + + public function clear() : void{ + $this->handlerSlots = []; + $this->invalidateAffectedCaches(); + } + + /** + * @return BaseRegisteredListener[] + * @phpstan-return array + */ + public function getListenersByPriority(int $priority) : array{ + return $this->handlerSlots[$priority] ?? []; + } + + /** + * @phpstan-return static + */ + public function getParent() : ?HandlerList{ + return $this->parentList; + } + + /** + * Invalidates all known caches which might be affected by this list's contents. + */ + private function invalidateAffectedCaches() : void{ + foreach($this->affectedHandlerCaches as $cache){ + $cache->list = null; + } + } + + /** + * @return BaseRegisteredListener[] + * @phpstan-return list + */ + public function getListenerList() : array{ + if($this->handlerCache->list !== null){ + return $this->handlerCache->list; + } + + $handlerLists = []; + for($currentList = $this; $currentList !== null; $currentList = $currentList->parentList){ + $handlerLists[] = $currentList; + } + + $listenersByPriority = []; + foreach($handlerLists as $currentList){ + foreach($currentList->handlerSlots as $priority => $listeners){ + $listenersByPriority[$priority] = array_merge( + $listenersByPriority[$priority] ?? [], + $this->sortSamePriorityHandlers !== null ? + ($this->sortSamePriorityHandlers)($listeners) : + $listeners + ); + } + } + + //TODO: why on earth do the priorities have higher values for lower priority? + krsort($listenersByPriority, SORT_NUMERIC); + + return $this->handlerCache->list = array_merge(...$listenersByPriority); } } diff --git a/src/event/HandlerListManager.php b/src/event/HandlerListManager.php index 85be5238703..489a3af90a2 100644 --- a/src/event/HandlerListManager.php +++ b/src/event/HandlerListManager.php @@ -37,7 +37,7 @@ protected function getBaseEventClass() : string{ return Event::class; } - protected function createHandlerList(string $event, ?BaseHandlerList $parentList, RegisteredListenerCache $handlerCache) : BaseHandlerList{ + protected function createHandlerList(string $event, ?HandlerList $parentList, RegisteredListenerCache $handlerCache) : HandlerList{ return new HandlerList($event, $parentList, $handlerCache); } } From 11fdf79a7ca0910f0f33c10df9958fe5c29807b3 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 13 Nov 2024 22:55:08 +0000 Subject: [PATCH 39/47] ... --- src/event/AsyncEvent.php | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/event/AsyncEvent.php b/src/event/AsyncEvent.php index 6f5c6fc1437..0d55f63bb71 100644 --- a/src/event/AsyncEvent.php +++ b/src/event/AsyncEvent.php @@ -60,7 +60,12 @@ final public function call() : Promise{ /** @phpstan-var PromiseResolver $globalResolver */ $globalResolver = new PromiseResolver(); - $this->processRemainingHandlers(AsyncHandlerListManager::global()->getHandlersFor(static::class), $globalResolver); + $handlers = AsyncHandlerListManager::global()->getHandlersFor(static::class); + if(count($handlers) > 0){ + $this->processRemainingHandlers($handlers, fn() => $globalResolver->resolve($this), $globalResolver->reject(...)); + }else{ + $globalResolver->resolve($this); + } return $globalResolver->getPromise(); }finally{ @@ -71,9 +76,10 @@ final public function call() : Promise{ /** * @param AsyncRegisteredListener[] $handlers - * @phpstan-param PromiseResolver $globalResolver + * @phpstan-param \Closure() : void $resolve + * @phpstan-param \Closure() : void $reject */ - private function processRemainingHandlers(array $handlers, PromiseResolver $globalResolver) : void{ + private function processRemainingHandlers(array $handlers, \Closure $resolve, \Closure $reject) : void{ $currentPriority = null; $awaitPromises = []; foreach($handlers as $k => $handler){ @@ -100,8 +106,8 @@ private function processRemainingHandlers(array $handlers, PromiseResolver $glob $promise = $handler->callAsync($this); if($promise !== null){ $promise->onCompletion( - onSuccess: fn() => $this->processRemainingHandlers($handlers, $globalResolver), - onFailure: $globalResolver->reject(...) + onSuccess: fn() => $this->processRemainingHandlers($handlers, $resolve, $reject), + onFailure: $reject ); return; } @@ -110,11 +116,11 @@ private function processRemainingHandlers(array $handlers, PromiseResolver $glob if(count($awaitPromises) > 0){ Promise::all($awaitPromises)->onCompletion( - onSuccess: fn() => $this->processRemainingHandlers($handlers, $globalResolver), - onFailure: $globalResolver->reject(...) + onSuccess: fn() => $this->processRemainingHandlers($handlers, $resolve, $reject), + onFailure: $reject ); }else{ - $globalResolver->resolve($this); + $resolve(); } } } From 0a56cf877bd47fdd73eae9b026c9719d7b909477 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 13 Nov 2024 22:58:29 +0000 Subject: [PATCH 40/47] Remove unused class --- src/event/player/PlayerChatAsyncEvent.php | 92 ----------------------- 1 file changed, 92 deletions(-) delete mode 100644 src/event/player/PlayerChatAsyncEvent.php diff --git a/src/event/player/PlayerChatAsyncEvent.php b/src/event/player/PlayerChatAsyncEvent.php deleted file mode 100644 index c520aa5a117..00000000000 --- a/src/event/player/PlayerChatAsyncEvent.php +++ /dev/null @@ -1,92 +0,0 @@ -message; - } - - public function setMessage(string $message) : void{ - $this->message = $message; - } - - /** - * Changes the player that is sending the message - */ - public function setPlayer(Player $player) : void{ - $this->player = $player; - } - - public function getPlayer() : Player{ - return $this->player; - } - - public function getFormatter() : ChatFormatter{ - return $this->formatter; - } - - public function setFormatter(ChatFormatter $formatter) : void{ - $this->formatter = $formatter; - } - - /** - * @return CommandSender[] - */ - public function getRecipients() : array{ - return $this->recipients; - } - - /** - * @param CommandSender[] $recipients - */ - public function setRecipients(array $recipients) : void{ - Utils::validateArrayValueType($recipients, function(CommandSender $_) : void{}); - $this->recipients = $recipients; - } -} From a7a1077676502a0d6814c064bd95befb6da59f5c Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 13 Nov 2024 23:00:49 +0000 Subject: [PATCH 41/47] CONTRIBUTING: changing an event from sync to async or vice versa is a BC break --- CONTRIBUTING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3490542c5b2..eb475e76c8a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -56,6 +56,7 @@ PocketMine-MP has three primary branches of development. | Deprecating API classes, methods or constants | ❌ | ✔️ | ✔️ | | Adding optional parameters to an API method | ❌ | ✔️ | ✔️ | | Changing API behaviour | ❌ | 🟡 Only if backwards-compatible | ✔️ | +| Changing an event from sync to async or vice versa | ❌ | ❌ | ✔️ | | Removal of API | ❌ | ❌ | ✔️ | | Backwards-incompatible API change (e.g. renaming a method) | ❌ | ❌ | ✔️ | | Backwards-incompatible internals change (e.g. changing things in `pocketmine\network\mcpe`) | ❌ | ✔️ | ✔️ | From d2d663b1c970bb7eccaa2c41df65c1ec6e2fb2d0 Mon Sep 17 00:00:00 2001 From: "Dylan T." Date: Thu, 14 Nov 2024 13:11:38 +0000 Subject: [PATCH 42/47] Simplify handler sorting --- src/event/AsyncHandlerListManager.php | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/event/AsyncHandlerListManager.php b/src/event/AsyncHandlerListManager.php index b6d24991906..1f7be2e5c5c 100644 --- a/src/event/AsyncHandlerListManager.php +++ b/src/event/AsyncHandlerListManager.php @@ -45,12 +45,9 @@ protected function getBaseEventClass() : string{ */ private static function sortSamePriorityHandlers(array $listeners) : array{ uasort($listeners, function(AsyncRegisteredListener $left, AsyncRegisteredListener $right) : int{ - //While the system can handle these in any order, it's better for latency if concurrent handlers - //are processed together. It doesn't matter whether they are processed before or after exclusive handlers. - if($right->canBeCalledConcurrently()){ - return $left->canBeCalledConcurrently() ? 0 : 1; - } - return -1; + //Promise::all() can be used more efficiently if concurrent handlers are grouped together. + //It's not important whether they are grouped before or after exclusive handlers. + return $left->canBeCalledConcurrently() <=> $right->canBeCalledConcurrently(); }); return $listeners; } From 406e2c6c576fa8ebd3f446d03abcc66e295a1b33 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 20 Nov 2024 16:35:20 +0000 Subject: [PATCH 43/47] Convert integration tests to unit tests this required mocking to get around #6524. Longer term we should make improvements to avoid the need for mocking here. --- .../event/AsyncEventConcurrencyTest.php | 125 +++++++++++++++ tests/phpunit/event/AsyncEventTest.php | 134 ++++++++++++++++ .../event/fixtures/TestChildAsyncEvent.php} | 4 +- .../fixtures/TestGrandchildAsyncEvent.php} | 4 +- .../event/fixtures/TestParentAsyncEvent.php} | 6 +- .../src/AsyncEventConcurrencyTest.php | 150 ------------------ .../src/AsyncEventInheritanceTest.php | 84 ---------- .../src/AsyncEventPriorityTest.php | 96 ----------- tests/plugins/TesterPlugin/src/Main.php | 4 +- 9 files changed, 268 insertions(+), 339 deletions(-) create mode 100644 tests/phpunit/event/AsyncEventConcurrencyTest.php create mode 100644 tests/phpunit/event/AsyncEventTest.php rename tests/{plugins/TesterPlugin/src/event/GrandchildAsyncEvent.php => phpunit/event/fixtures/TestChildAsyncEvent.php} (88%) rename tests/{plugins/TesterPlugin/src/event/ParentAsyncEvent.php => phpunit/event/fixtures/TestGrandchildAsyncEvent.php} (88%) rename tests/{plugins/TesterPlugin/src/event/ChildAsyncEvent.php => phpunit/event/fixtures/TestParentAsyncEvent.php} (86%) delete mode 100644 tests/plugins/TesterPlugin/src/AsyncEventConcurrencyTest.php delete mode 100644 tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php delete mode 100644 tests/plugins/TesterPlugin/src/AsyncEventPriorityTest.php diff --git a/tests/phpunit/event/AsyncEventConcurrencyTest.php b/tests/phpunit/event/AsyncEventConcurrencyTest.php new file mode 100644 index 00000000000..bfbe7438463 --- /dev/null +++ b/tests/phpunit/event/AsyncEventConcurrencyTest.php @@ -0,0 +1,125 @@ +> + */ + private array $resolvers = []; + + private bool $activeExclusiveHandler = false; + private bool $activeConcurrentHandler = false; + + private int $done = 0; + + protected function setUp() : void{ + AsyncHandlerListManager::global()->unregisterAll(); + + //TODO: this is a really bad hack and could break any time if PluginManager decides to access its Server field + //we really need to make it possible to register events without a Plugin or Server context + $this->mockServer = $this->createMock(Server::class); + $this->mockPlugin = self::createStub(Plugin::class); + $this->mockPlugin->method('isEnabled')->willReturn(true); + $this->pluginManager = new PluginManager($this->mockServer, null); + } + + public static function tearDownAfterClass() : void{ + AsyncHandlerListManager::global()->unregisterAll(); + } + + /** + * @phpstan-return Promise + */ + private function handler(bool &$flag, string $label) : Promise{ + $flag = true; + $resolver = new PromiseResolver(); + $this->resolvers[] = $resolver; + $resolver->getPromise()->onCompletion( + function() use (&$flag) : void{ + $flag = false; + $this->done++; + }, + fn() => self::fail("Not expecting this to be rejected for $label") + ); + return $resolver->getPromise(); + } + + public function testConcurrency() : void{ + $this->pluginManager->registerAsyncEvent( + TestGrandchildAsyncEvent::class, + function(TestGrandchildAsyncEvent $event) : Promise{ + self::assertFalse($this->activeExclusiveHandler, "Concurrent handler can't run while exclusive handlers are waiting to complete"); + + return $this->handler($this->activeConcurrentHandler, "concurrent"); + }, + EventPriority::NORMAL, + $this->mockPlugin, + //non-exclusive - this must be completed before any exclusive handlers are run (or run after them) + ); + for($i = 0; $i < 2; $i++){ + $this->pluginManager->registerAsyncEvent( + TestGrandchildAsyncEvent::class, + function(TestGrandchildAsyncEvent $event) use ($i) : Promise{ + self::assertFalse($this->activeExclusiveHandler, "Exclusive handler $i can't run alongside other exclusive handlers"); + self::assertFalse($this->activeConcurrentHandler, "Exclusive handler $i can't run alongside concurrent handler"); + + return $this->handler($this->activeExclusiveHandler, "exclusive $i"); + }, + EventPriority::NORMAL, + $this->mockPlugin, + exclusiveCall: true + ); + } + + (new TestGrandchildAsyncEvent())->call(); + + while(count($this->resolvers) > 0 && $this->done < 3){ + foreach($this->resolvers as $k => $resolver){ + unset($this->resolvers[$k]); + //don't clear the array here - resolving this will trigger adding the next resolver + $resolver->resolve(null); + } + } + + self::assertSame(3, $this->done, "Expected feedback from exactly 3 handlers"); + } +} diff --git a/tests/phpunit/event/AsyncEventTest.php b/tests/phpunit/event/AsyncEventTest.php new file mode 100644 index 00000000000..f9c20b41233 --- /dev/null +++ b/tests/phpunit/event/AsyncEventTest.php @@ -0,0 +1,134 @@ +unregisterAll(); + + //TODO: this is a really bad hack and could break any time if PluginManager decides to access its Server field + //we really need to make it possible to register events without a Plugin or Server context + $this->mockServer = $this->createMock(Server::class); + $this->mockPlugin = self::createStub(Plugin::class); + $this->mockPlugin->method('isEnabled')->willReturn(true); + $this->pluginManager = new PluginManager($this->mockServer, null); + } + + public static function tearDownAfterClass() : void{ + AsyncHandlerListManager::global()->unregisterAll(); + } + + public function testHandlerInheritance() : void{ + $expectedOrder = [ + TestGrandchildAsyncEvent::class, + TestChildAsyncEvent::class, + TestParentAsyncEvent::class + ]; + $classes = $expectedOrder; + $actualOrder = []; + shuffle($classes); + foreach($classes as $class){ + $this->pluginManager->registerAsyncEvent( + $class, + function(AsyncEvent $event) use (&$actualOrder, $class) : ?Promise{ + $actualOrder[] = $class; + return null; + }, + EventPriority::NORMAL, + $this->mockPlugin + ); + } + + $event = new TestGrandchildAsyncEvent(); + $promise = $event->call(); + + $resolved = false; + $promise->onCompletion( + function() use ($expectedOrder, $actualOrder, &$resolved){ + self::assertSame($expectedOrder, $actualOrder, "Expected event handlers to be called from most specific to least specific"); + $resolved = true; + }, + fn() => self::fail("Not expecting this to be rejected") + ); + + self::assertTrue($resolved, "No promises were used, expected this promise to resolve immediately"); + } + + public function testPriorityLock() : void{ + $resolver = null; + $firstCompleted = false; + $run = 0; + + $this->pluginManager->registerAsyncEvent( + TestGrandchildAsyncEvent::class, + function(TestGrandchildAsyncEvent $event) use (&$resolver, &$firstCompleted, &$run) : Promise{ + $run++; + $resolver = new PromiseResolver(); + + $resolver->getPromise()->onCompletion( + function() use (&$firstCompleted) : void{ $firstCompleted = true; }, + fn() => self::fail("Not expecting this to be rejected") + ); + + return $resolver->getPromise(); + }, + EventPriority::LOW, //anything below NORMAL is fine + $this->mockPlugin + ); + $this->pluginManager->registerAsyncEvent( + TestGrandchildAsyncEvent::class, + function(TestGrandchildAsyncEvent $event) use (&$firstCompleted, &$run) : ?Promise{ + $run++; + self::assertTrue($firstCompleted, "This shouldn't run until the previous priority is done"); + return null; + }, + EventPriority::NORMAL, + $this->mockPlugin + ); + + (new TestGrandchildAsyncEvent())->call(); + self::assertNotNull($resolver, "First handler didn't provide a resolver"); + $resolver->resolve(null); + self::assertSame(2, $run, "Expected feedback from 2 handlers"); + } +} diff --git a/tests/plugins/TesterPlugin/src/event/GrandchildAsyncEvent.php b/tests/phpunit/event/fixtures/TestChildAsyncEvent.php similarity index 88% rename from tests/plugins/TesterPlugin/src/event/GrandchildAsyncEvent.php rename to tests/phpunit/event/fixtures/TestChildAsyncEvent.php index 3325268e005..4b19d31d180 100644 --- a/tests/plugins/TesterPlugin/src/event/GrandchildAsyncEvent.php +++ b/tests/phpunit/event/fixtures/TestChildAsyncEvent.php @@ -21,8 +21,8 @@ declare(strict_types=1); -namespace pmmp\TesterPlugin\event; +namespace pocketmine\event\fixtures; -class GrandchildAsyncEvent extends ChildAsyncEvent{ +class TestChildAsyncEvent extends TestParentAsyncEvent{ } diff --git a/tests/plugins/TesterPlugin/src/event/ParentAsyncEvent.php b/tests/phpunit/event/fixtures/TestGrandchildAsyncEvent.php similarity index 88% rename from tests/plugins/TesterPlugin/src/event/ParentAsyncEvent.php rename to tests/phpunit/event/fixtures/TestGrandchildAsyncEvent.php index 5c77966974f..7be2373c6a1 100644 --- a/tests/plugins/TesterPlugin/src/event/ParentAsyncEvent.php +++ b/tests/phpunit/event/fixtures/TestGrandchildAsyncEvent.php @@ -21,8 +21,8 @@ declare(strict_types=1); -namespace pmmp\TesterPlugin\event; +namespace pocketmine\event\fixtures; -class ParentAsyncEvent extends \pocketmine\event\AsyncEvent{ +class TestGrandchildAsyncEvent extends TestChildAsyncEvent{ } diff --git a/tests/plugins/TesterPlugin/src/event/ChildAsyncEvent.php b/tests/phpunit/event/fixtures/TestParentAsyncEvent.php similarity index 86% rename from tests/plugins/TesterPlugin/src/event/ChildAsyncEvent.php rename to tests/phpunit/event/fixtures/TestParentAsyncEvent.php index 79d88b5d3a8..17b3d1189bf 100644 --- a/tests/plugins/TesterPlugin/src/event/ChildAsyncEvent.php +++ b/tests/phpunit/event/fixtures/TestParentAsyncEvent.php @@ -21,8 +21,10 @@ declare(strict_types=1); -namespace pmmp\TesterPlugin\event; +namespace pocketmine\event\fixtures; -class ChildAsyncEvent extends ParentAsyncEvent{ +use pocketmine\event\AsyncEvent; + +class TestParentAsyncEvent extends AsyncEvent{ } diff --git a/tests/plugins/TesterPlugin/src/AsyncEventConcurrencyTest.php b/tests/plugins/TesterPlugin/src/AsyncEventConcurrencyTest.php deleted file mode 100644 index 856ee45adcd..00000000000 --- a/tests/plugins/TesterPlugin/src/AsyncEventConcurrencyTest.php +++ /dev/null @@ -1,150 +0,0 @@ -> - */ - private array $resolvers = []; - - private bool $activeExclusiveHandler = false; - private bool $activeConcurrentHandler = false; - - private int $done = 0; - - public function getName() : string{ - return "Async Event Concurrency Lock"; - } - - public function getDescription() : string{ - return "Test that exclusive lock on async event handlers works correctly"; - } - - public function run() : void{ - AsyncHandlerListManager::global()->unregisterAll(); - - $main = $this->getPlugin(); - $pluginManager = $main->getServer()->getPluginManager(); - - $pluginManager->registerAsyncEvent( - GrandchildAsyncEvent::class, - function(GrandchildAsyncEvent $event) use ($main) : ?Promise{ - if($this->activeExclusiveHandler){ - $main->getLogger()->error("Concurrent handler can't run while exclusive handlers are waiting to complete"); - $this->setResult(Test::RESULT_FAILED); - return null; - } - $this->activeConcurrentHandler = true; - $resolver = new PromiseResolver(); - $this->resolvers[] = $resolver; - $resolver->getPromise()->onCompletion( - fn() => $this->complete($this->activeConcurrentHandler, "concurrent"), - fn() => $main->getLogger()->error("Not expecting this to be rejected") - ); - return $resolver->getPromise(); - }, - EventPriority::NORMAL, - $main, - //non-exclusive - this must be completed before any exclusive handlers are run (or run after them) - ); - $pluginManager->registerAsyncEvent( - GrandchildAsyncEvent::class, - function(GrandchildAsyncEvent $event) use ($main) : ?Promise{ - $main->getLogger()->info("Entering exclusive handler 1"); - if($this->activeExclusiveHandler || $this->activeConcurrentHandler){ - $main->getLogger()->error("Can't run multiple exclusive handlers at once"); - $this->setResult(Test::RESULT_FAILED); - return null; - } - $this->activeExclusiveHandler = true; - $resolver = new PromiseResolver(); - $this->resolvers[] = $resolver; - $resolver->getPromise()->onCompletion( - fn() => $this->complete($this->activeExclusiveHandler, "exclusive 1"), - fn() => $main->getLogger()->error("Not expecting this to be rejected") - ); - return $resolver->getPromise(); - }, - EventPriority::NORMAL, - $main, - exclusiveCall: true - ); - - $pluginManager->registerAsyncEvent( - GrandchildAsyncEvent::class, - function(GrandchildAsyncEvent $event) use ($main) : ?Promise{ - $this->getPlugin()->getLogger()->info("Entering exclusive handler 2"); - if($this->activeExclusiveHandler || $this->activeConcurrentHandler){ - $main->getLogger()->error("Exclusive lock handlers must not run at the same time as any other handlers"); - $this->setResult(Test::RESULT_FAILED); - return null; - } - $this->activeExclusiveHandler = true; - /** @phpstan-var PromiseResolver $resolver */ - $resolver = new PromiseResolver(); - $this->resolvers[] = $resolver; - $resolver->getPromise()->onCompletion( - function() use ($main) : void{ - $main->getLogger()->info("Exiting exclusive handler asynchronously"); - $this->complete($this->activeExclusiveHandler, "exclusive 2"); - }, - function() use ($main) : void{ - $main->getLogger()->error("Not expecting this promise to be rejected"); - $this->setResult(Test::RESULT_ERROR); - } - ); - return $resolver->getPromise(); - }, - EventPriority::NORMAL, - $main, - exclusiveCall: true - ); - - (new GrandchildAsyncEvent())->call(); - } - - private function complete(bool &$flag, string $what) : void{ - $this->getPlugin()->getLogger()->info("Completing $what"); - $flag = false; - if(++$this->done === 3){ - $this->setResult(Test::RESULT_OK); - } - } - - public function tick() : void{ - foreach($this->resolvers as $k => $resolver){ - $resolver->resolve(null); - //don't clear the array here - resolving this will trigger adding the next resolver - unset($this->resolvers[$k]); - } - } -} diff --git a/tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php b/tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php deleted file mode 100644 index d952e36b10e..00000000000 --- a/tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php +++ /dev/null @@ -1,84 +0,0 @@ -unregisterAll(); - - $plugin = $this->getPlugin(); - $classes = self::EXPECTED_ORDER; - shuffle($classes); - foreach($classes as $class){ - $plugin->getServer()->getPluginManager()->registerAsyncEvent( - $class, - function(AsyncEvent $event) use ($class) : ?Promise{ - $this->callOrder[] = $class; - return null; - }, - EventPriority::NORMAL, - $plugin - ); - } - - $event = new GrandchildAsyncEvent(); - $promise = $event->call(); - $promise->onCompletion(onSuccess: $this->collectResults(...), onFailure: $this->collectResults(...)); - } - - private function collectResults() : void{ - if($this->callOrder === self::EXPECTED_ORDER){ - $this->setResult(Test::RESULT_OK); - }else{ - $this->getPlugin()->getLogger()->error("Expected order: " . implode(", ", self::EXPECTED_ORDER) . ", got: " . implode(", ", $this->callOrder)); - $this->setResult(Test::RESULT_FAILED); - } - } -} diff --git a/tests/plugins/TesterPlugin/src/AsyncEventPriorityTest.php b/tests/plugins/TesterPlugin/src/AsyncEventPriorityTest.php deleted file mode 100644 index 4ac5766d1e5..00000000000 --- a/tests/plugins/TesterPlugin/src/AsyncEventPriorityTest.php +++ /dev/null @@ -1,96 +0,0 @@ -> - */ - private array $resolvers = []; - - private bool $firstHandlerCompleted = false; - - public function getName() : string{ - return "Async Event Handler Priority Lock"; - } - - public function getDescription() : string{ - return "Tests that async events do not call handlers from the next priority until all promises from the current priority are resolved"; - } - - public function run() : void{ - AsyncHandlerListManager::global()->unregisterAll(); - - $main = $this->getPlugin(); - $pluginManager = $main->getServer()->getPluginManager(); - $pluginManager->registerAsyncEvent( - GrandchildAsyncEvent::class, - function(GrandchildAsyncEvent $event) use ($main) : Promise{ - $resolver = new PromiseResolver(); - $this->resolvers[] = $resolver; - - $resolver->getPromise()->onCompletion(function() : void{ - $this->firstHandlerCompleted = true; - }, function() use ($main) : void{ - $main->getLogger()->error("Not expecting this to be rejected"); - $this->setResult(Test::RESULT_ERROR); - }); - - return $resolver->getPromise(); - }, - EventPriority::LOW, //anything below NORMAL is fine - $main - ); - $pluginManager->registerAsyncEvent( - GrandchildAsyncEvent::class, - function(GrandchildAsyncEvent $event) use ($main) : ?Promise{ - if(!$this->firstHandlerCompleted){ - $main->getLogger()->error("This shouldn't run until the previous priority is done"); - $this->setResult(Test::RESULT_FAILED); - }else{ - $this->setResult(Test::RESULT_OK); - } - return null; - }, - EventPriority::NORMAL, - $main - ); - - (new GrandchildAsyncEvent())->call(); - } - - public function tick() : void{ - foreach($this->resolvers as $k => $resolver){ - $resolver->resolve(null); - unset($this->resolvers[$k]); - } - } -} diff --git a/tests/plugins/TesterPlugin/src/Main.php b/tests/plugins/TesterPlugin/src/Main.php index 0a7c524795f..08b59dbacb6 100644 --- a/tests/plugins/TesterPlugin/src/Main.php +++ b/tests/plugins/TesterPlugin/src/Main.php @@ -57,9 +57,7 @@ public function onEnable() : void{ }), 10); $this->waitingTests = [ - new AsyncEventInheritanceTest($this), - new AsyncEventConcurrencyTest($this), - new AsyncEventPriorityTest($this) + //Add test objects here ]; } From d9f56342620dfe2dad7dc68e7bc925846be97686 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 20 Nov 2024 16:37:49 +0000 Subject: [PATCH 44/47] CS --- tests/phpunit/event/AsyncEventConcurrencyTest.php | 1 + tests/phpunit/event/AsyncEventTest.php | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/phpunit/event/AsyncEventConcurrencyTest.php b/tests/phpunit/event/AsyncEventConcurrencyTest.php index bfbe7438463..fed1edf5e78 100644 --- a/tests/phpunit/event/AsyncEventConcurrencyTest.php +++ b/tests/phpunit/event/AsyncEventConcurrencyTest.php @@ -30,6 +30,7 @@ use pocketmine\promise\Promise; use pocketmine\promise\PromiseResolver; use pocketmine\Server; +use function count; final class AsyncEventConcurrencyTest extends TestCase{ diff --git a/tests/phpunit/event/AsyncEventTest.php b/tests/phpunit/event/AsyncEventTest.php index f9c20b41233..f8d49e855a3 100644 --- a/tests/phpunit/event/AsyncEventTest.php +++ b/tests/phpunit/event/AsyncEventTest.php @@ -25,11 +25,8 @@ use PHPUnit\Framework\TestCase; use pocketmine\event\fixtures\TestChildAsyncEvent; -use pocketmine\event\fixtures\TestChildEvent; use pocketmine\event\fixtures\TestGrandchildAsyncEvent; -use pocketmine\event\fixtures\TestGrandchildEvent; use pocketmine\event\fixtures\TestParentAsyncEvent; -use pocketmine\event\fixtures\TestParentEvent; use pocketmine\plugin\Plugin; use pocketmine\plugin\PluginManager; use pocketmine\promise\Promise; From d9080f182cb02a5b058ffcb7ff6e5e21f1273403 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 20 Nov 2024 16:44:00 +0000 Subject: [PATCH 45/47] we don't need a fake server instance outside of setUp() in these tests --- tests/phpunit/event/AsyncEventConcurrencyTest.php | 6 ++---- tests/phpunit/event/AsyncEventTest.php | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/phpunit/event/AsyncEventConcurrencyTest.php b/tests/phpunit/event/AsyncEventConcurrencyTest.php index fed1edf5e78..7533ddeb755 100644 --- a/tests/phpunit/event/AsyncEventConcurrencyTest.php +++ b/tests/phpunit/event/AsyncEventConcurrencyTest.php @@ -35,8 +35,6 @@ final class AsyncEventConcurrencyTest extends TestCase{ private Plugin $mockPlugin; - private Server $mockServer; - private PluginManager $pluginManager; //this one gets its own class because it requires a bunch of context variables @@ -57,10 +55,10 @@ protected function setUp() : void{ //TODO: this is a really bad hack and could break any time if PluginManager decides to access its Server field //we really need to make it possible to register events without a Plugin or Server context - $this->mockServer = $this->createMock(Server::class); + $mockServer = $this->createMock(Server::class); $this->mockPlugin = self::createStub(Plugin::class); $this->mockPlugin->method('isEnabled')->willReturn(true); - $this->pluginManager = new PluginManager($this->mockServer, null); + $this->pluginManager = new PluginManager($mockServer, null); } public static function tearDownAfterClass() : void{ diff --git a/tests/phpunit/event/AsyncEventTest.php b/tests/phpunit/event/AsyncEventTest.php index f8d49e855a3..ecc03c19307 100644 --- a/tests/phpunit/event/AsyncEventTest.php +++ b/tests/phpunit/event/AsyncEventTest.php @@ -35,8 +35,6 @@ use function shuffle; final class AsyncEventTest extends TestCase{ - - private Server $mockServer; private Plugin $mockPlugin; private PluginManager $pluginManager; @@ -45,10 +43,10 @@ protected function setUp() : void{ //TODO: this is a really bad hack and could break any time if PluginManager decides to access its Server field //we really need to make it possible to register events without a Plugin or Server context - $this->mockServer = $this->createMock(Server::class); + $mockServer = $this->createMock(Server::class); $this->mockPlugin = self::createStub(Plugin::class); $this->mockPlugin->method('isEnabled')->willReturn(true); - $this->pluginManager = new PluginManager($this->mockServer, null); + $this->pluginManager = new PluginManager($mockServer, null); } public static function tearDownAfterClass() : void{ From e8ec81d123af08232609562a9319559349f3bc98 Mon Sep 17 00:00:00 2001 From: "Dylan T." Date: Fri, 29 Nov 2024 14:08:06 +0000 Subject: [PATCH 46/47] fix PHPStan error --- src/event/AsyncEvent.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/event/AsyncEvent.php b/src/event/AsyncEvent.php index 0d55f63bb71..137042e0572 100644 --- a/src/event/AsyncEvent.php +++ b/src/event/AsyncEvent.php @@ -76,6 +76,7 @@ final public function call() : Promise{ /** * @param AsyncRegisteredListener[] $handlers + * @phpstan-param list $handlers * @phpstan-param \Closure() : void $resolve * @phpstan-param \Closure() : void $reject */ From a0d69a9fb88e5dab65820a45a5a388903714d134 Mon Sep 17 00:00:00 2001 From: "Dylan T." Date: Fri, 29 Nov 2024 14:13:25 +0000 Subject: [PATCH 47/47] github web editor don't fuck up indentation, challenge impossible --- src/event/AsyncEvent.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/event/AsyncEvent.php b/src/event/AsyncEvent.php index 137042e0572..f5ed00dd8be 100644 --- a/src/event/AsyncEvent.php +++ b/src/event/AsyncEvent.php @@ -76,7 +76,7 @@ final public function call() : Promise{ /** * @param AsyncRegisteredListener[] $handlers - * @phpstan-param list $handlers + * @phpstan-param list $handlers * @phpstan-param \Closure() : void $resolve * @phpstan-param \Closure() : void $reject */