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

@@ -28,7 +28,6 @@ use pocketmine\promise\PromiseResolver;
 use pocketmine\timings\Timings;
 use pocketmine\utils\ObjectSet;
 use function array_shift;
-use function assert;
 use function count;
 
 /**
@@ -99,14 +98,13 @@ abstract class AsyncEvent{
 	 * @phpstan-return Promise<null>
 	 */
 	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<null> $resolver */
 		$resolver = new PromiseResolver();
 
 		$nonConcurrentHandlers = [];
 		foreach($handlers as $registration){
-			assert($registration instanceof RegisteredAsyncListener);
 			if($registration->canBeCalledConcurrently()){
 				$result = $registration->callAsync($this);
 				if($result !== null) {
@@ -127,7 +125,6 @@ abstract class AsyncEvent{
 			}else{
 				$this->waitForPromises()->onCompletion(function() use (&$nonConcurrentHandlers, $testResolve){
 					$handler = array_shift($nonConcurrentHandlers);
-					assert($handler instanceof RegisteredAsyncListener);
 					$result = $handler->callAsync($this);
 					if($result !== null) {
 						$this->promises->add($result);

src/event/AsyncHandlerList.php

@@ -0,0 +1,88 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+ */
+
+declare(strict_types=1);
+
+namespace pocketmine\event;
+
+use pocketmine\plugin\Plugin;
+use function spl_object_id;
+
+class AsyncHandlerList{
+	/** @var AsyncRegisteredListener[][] */
+	private array $handlerSlots = [];
+
+	/**
+	 * @phpstan-param class-string<AsyncEvent> $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;
+	}
+}

src/event/AsyncRegisteredListener.php

@@ -26,28 +26,35 @@ namespace pocketmine\event;
 use pocketmine\plugin\Plugin;
 use pocketmine\promise\Promise;
 use pocketmine\timings\TimingsHandler;
+use function in_array;
 
-class RegisteredAsyncListener extends RegisteredListener{
+class AsyncRegisteredListener{
 	/**
 	 * @phpstan-param \Closure(AsyncEvent) : Promise<null> $handler
 	 */
 	public function __construct(
-		protected \Closure $handler,
-		int $priority,
-		Plugin $plugin,
-		bool $handleCancelled,
+		private \Closure $handler,
+		private int $priority,
+		private Plugin $plugin,
+		private bool $handleCancelled,
 		private bool $exclusiveCall,
-		protected TimingsHandler $timings
+		private TimingsHandler $timings
 	){
-		parent::__construct($handler, $priority, $plugin, $handleCancelled, $timings);
+		if(!in_array($priority, EventPriority::ALL, true)){
+			throw new \InvalidArgumentException("Invalid priority number $priority");
+		}
 	}
 
-	public function canBeCalledConcurrently() : bool{
-		return !$this->exclusiveCall;
+	public function getHandler() : \Closure{
+		return $this->handler;
 	}
 
-	public function callEvent(Event $event) : void{
-		throw new \BadMethodCallException("Cannot call async event synchronously, use callAsync() instead");
+	public function getPlugin() : Plugin{
+		return $this->plugin;
+	}
+
+	public function getPriority() : int{
+		return $this->priority;
 	}
 
 	/**
@@ -64,4 +71,11 @@ class RegisteredAsyncListener extends RegisteredListener{
 			$this->timings->stopTiming();
 		}
 	}
+
+	public function isHandlingCancelled() : bool{
+		return $this->handleCancelled;
+	}
+	public function canBeCalledConcurrently() : bool{
+		return !$this->exclusiveCall;
+	}
 }

src/event/HandlerList.php

@@ -25,7 +25,6 @@ namespace pocketmine\event;
 
 use pocketmine\plugin\Plugin;
 use function array_merge;
-use function array_merge_recursive;
 use function krsort;
 use function spl_object_id;
 use const SORT_NUMERIC;
@@ -38,7 +37,7 @@ class HandlerList{
 	private array $affectedHandlerCaches = [];
 
 	/**
-	 * @phpstan-param class-string<Event|AsyncEvent> $class
+	 * @phpstan-param class-string<covariant Event> $class
 	 */
 	public function __construct(
 		private string $class,
@@ -127,23 +126,12 @@ class HandlerList{
 			$handlerLists[] = $currentList;
 		}
 
-		$listeners = [];
-		$asyncListeners = [];
-		$exclusiveAsyncListeners = [];
+		$listenersByPriority = [];
 		foreach($handlerLists as $currentList){
-			foreach($currentList->handlerSlots as $priority => $listenersToSort){
-				foreach($listenersToSort as $listener){
-					if(!$listener instanceof RegisteredAsyncListener){
-						$listeners[$priority][] = $listener;
-					}elseif(!$listener->canBeCalledConcurrently()){
-						$asyncListeners[$priority][] = $listener;
-					}else{
-						$exclusiveAsyncListeners[$priority][] = $listener;
-					}
-				}
+			foreach($currentList->handlerSlots as $priority => $listeners){
+				$listenersByPriority[$priority] = array_merge($listenersByPriority[$priority] ?? [], $listeners);
 			}
 		}
-		$listenersByPriority = array_merge_recursive($listeners, $asyncListeners, $exclusiveAsyncListeners);
 
 		//TODO: why on earth do the priorities have higher values for lower priority?
 		krsort($listenersByPriority, SORT_NUMERIC);

src/event/HandlerListManager.php

@@ -35,24 +35,37 @@ class HandlerListManager{
 	}
 
 	/** @var HandlerList[] classname => HandlerList */
-	private array $allLists = [];
+	private array $allSyncLists = [];
 	/**
 	 * @var RegisteredListenerCache[] event class name => cache
 	 * @phpstan-var array<class-string<Event|AsyncEvent>, RegisteredListenerCache>
 	 */
-	private array $handlerCaches = [];
+	private array $syncHandlerCaches = [];
+
+	/** @var AsyncHandlerList[] classname => AsyncHandlerList */
+	private array $allAsyncLists = [];
 
 	/**
 	 * Unregisters all the listeners
 	 * If a Plugin or Listener is passed, all the listeners with that object will be removed
 	 */
-	public function unregisterAll(RegisteredListener|Plugin|Listener|null $object = null) : void{
-		if($object instanceof Listener || $object instanceof Plugin || $object instanceof RegisteredListener){
-			foreach($this->allLists as $h){
-				$h->unregister($object);
+	public function unregisterAll(RegisteredListener|AsyncRegisteredListener|Plugin|Listener|null $object = null) : void{
+		if($object !== null){
+			if(!$object instanceof AsyncRegisteredListener){
+				foreach($this->allSyncLists as $h){
+					$h->unregister($object);
+				}
+			}
+			if(!$object instanceof RegisteredListener){
+				foreach($this->allAsyncLists as $h){
+					$h->unregister($object);
+				}
 			}
 		}else{
-			foreach($this->allLists as $h){
+			foreach($this->allSyncLists as $h){
+				$h->clear();
+			}
+			foreach($this->allAsyncLists as $h){
 				$h->clear();
 			}
 		}
@@ -67,9 +80,10 @@ class HandlerListManager{
 	}
 
 	/**
-	 * @phpstan-param \ReflectionClass<Event|AsyncEvent> $class
+	 * @phpstan-template TEvent of Event|AsyncEvent
+	 * @phpstan-param \ReflectionClass<TEvent> $class
 	 *
-	 * @phpstan-return \ReflectionClass<Event|AsyncEvent>|null
+	 * @phpstan-return \ReflectionClass<TEvent>|null
 	 */
 	private static function resolveNearestHandleableParent(\ReflectionClass $class) : ?\ReflectionClass{
 		for($parent = $class->getParentClass(); $parent !== false; $parent = $parent->getParentClass()){
@@ -86,26 +100,28 @@ class HandlerListManager{
 	 *
 	 * Calling this method also lazily initializes the $classMap inheritance tree of handler lists.
 	 *
-	 * @phpstan-template TEvent of Event|AsyncEvent
-	 * @phpstan-param class-string<TEvent> $event
+	 * @phpstan-param class-string<covariant Event> $event
 	 *
 	 * @throws \ReflectionException
 	 * @throws \InvalidArgumentException
 	 */
 	public function getListFor(string $event) : HandlerList{
-		if(isset($this->allLists[$event])){
-			return $this->allLists[$event];
+		if(isset($this->allSyncLists[$event])){
+			return $this->allSyncLists[$event];
 		}
 
 		$class = new \ReflectionClass($event);
+		if(!$class->isSubclassOf(Event::class)){
+			throw new \InvalidArgumentException("Cannot get sync handler list for async event");
+		}
 		if(!self::isValidClass($class)){
 			throw new \InvalidArgumentException("Event must be non-abstract or have the @allowHandle annotation");
 		}
 
 		$parent = self::resolveNearestHandleableParent($class);
 		$cache = new RegisteredListenerCache();
-		$this->handlerCaches[$event] = $cache;
-		return $this->allLists[$event] = new HandlerList(
+		$this->syncHandlerCaches[$event] = $cache;
+		return $this->allSyncLists[$event] = new HandlerList(
 			$event,
 			parentList: $parent !== null ? $this->getListFor($parent->getName()) : null,
 			handlerCache: $cache
@@ -113,12 +129,40 @@ class HandlerListManager{
 	}
 
 	/**
-	 * @phpstan-param class-string<Event|AsyncEvent> $event
+	 * Returns the HandlerList for async listeners that explicitly handle this event.
+	 *
+	 * @phpstan-param class-string<covariant AsyncEvent> $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<covariant Event> $event
 	 *
 	 * @return RegisteredListener[]
 	 */
 	public function getHandlersFor(string $event) : array{
-		$cache = $this->handlerCaches[$event] ?? null;
+		$cache = $this->syncHandlerCaches[$event] ?? null;
 		//getListFor() will populate the cache for the next call
 		return $cache?->list ?? $this->getListFor($event)->getListenerList();
 	}
@@ -127,6 +171,6 @@ class HandlerListManager{
 	 * @return HandlerList[]
 	 */
 	public function getAll() : array{
-		return $this->allLists;
+		return $this->allSyncLists;
 	}
 }

src/plugin/PluginManager.php

@@ -32,7 +32,7 @@ use pocketmine\event\Listener;
 use pocketmine\event\ListenerMethodTags;
 use pocketmine\event\plugin\PluginDisableEvent;
 use pocketmine\event\plugin\PluginEnableEvent;
-use pocketmine\event\RegisteredAsyncListener;
+use pocketmine\event\AsyncRegisteredListener;
 use pocketmine\event\RegisteredListener;
 use pocketmine\lang\KnownTranslationFactory;
 use pocketmine\permission\DefaultPermissions;
@@ -702,7 +702,8 @@ class PluginManager{
 	 *
 	 * @throws \ReflectionException
 	 */
-	public function registerAsyncEvent(string $event, \Closure $handler, int $priority, Plugin $plugin, bool $handleCancelled = false, bool $exclusiveCall = false) : RegisteredAsyncListener{
+	public function registerAsyncEvent(string $event, \Closure $handler, int $priority, Plugin $plugin, bool $handleCancelled = false, bool $exclusiveCall = false) : AsyncRegisteredListener{
+		//TODO: Not loving the code duplication here
 		if(!is_subclass_of($event, AsyncEvent::class)){
 			throw new PluginException($event . " is not an AsyncEvent");
 		}
@@ -715,8 +716,8 @@ class PluginManager{
 
 		$timings = Timings::getEventHandlerTimings($event, $handlerName, $plugin->getDescription()->getFullName());
 
-		$registeredListener = new RegisteredAsyncListener($handler, $priority, $plugin, $handleCancelled, $exclusiveCall, $timings);
-		HandlerListManager::global()->getListFor($event)->register($registeredListener);
+		$registeredListener = new AsyncRegisteredListener($handler, $priority, $plugin, $handleCancelled, $exclusiveCall, $timings);
+		HandlerListManager::global()->getAsyncListFor($event)->register($registeredListener);
 		return $registeredListener;
 	}