This cleans up some cargo-cult code poorly copied from Bukkit, which has negative performance effects and also makes internal event handling more complex than necessary.
## API changes
- Removed `EventExecutor` and `MethodEventExecutor`.
- A listener is no longer required for an event handler to be registered. Closure objects can now be used directly provided that they meet the conditions for registration.
- `PluginManager->registerEvent()` signature has changed: the `Listener` and `EventExecutor` parameters have been removed and a `\Closure $handler` has been added in its place.
- `RegisteredListener` now requires a `Closure` parameter instead of `Listener, EventExecutor`.
## Behavioural changes
These changes reduce the execution complexity involved with calling an event handler. Since event calls can happen in hot paths, this may have visible positive effects on performance.
Initial testing reveals a performance improvement of ~15% per event handler call compared to the old method.
This was observed in a recent crashdump where a plugin triggered a recursion error, but the stack trace did not contain any sign of a recursive event call. I conclude that this must have been caused by previous event handlers triggering errors 50 times in order to make the recursion detection break, because the recursion detection did not decrement the counter in cases where an exception was thrown.
These methods:
a) add concrete dependencies
b) are pointless (event->getEntity() instanceof Creature, anyone? an IDE can better understand this as well...)
c) encourage bad code (they don't enforce type contracts the same way an instanceof check does - oh, and why not let's add an is*() for every new mob that gets added ever?
This check is completely unnecessary since handlers get unregistered when a plugin is disabled. Additionally, this is an extremely hot path and this change produces a modest 5% performance improvement to event calls.
This is dependent on the changes made in b1e0f82cbf. This now makes it possible to call events without fetching a Server reference, allowing to eliminate a vast array of Server dependencies.
This solves the age-old issue of PlayerInteractEvent firing twice when a player clicks a block with an item in the hand.
API changes:
- Removed PlayerInteractEvent constants LEFT_CLICK_AIR, RIGHT_CLICK_AIR and PHYSICAL.
- PlayerInteractEvent no longer fires when the player right-clicks the air.
- Added new event PlayerItemUseEvent.
the same behaviour can be achieved by cancelling the event and setting the held item manually. Allowing this to be changed increases the complexity of dealing with survival block picking.
* Added a new PermissionManager, remove ridiculous cyclic dependencies of Permissions on Server
Aside from all the other ridiculous design problems with the permission system, the biggest problems are its API. This is, once again, a result of poor API design copied from Bukkit.
This pull request removes all permission-related functionality from `PluginManager` and moves it to the `pocketmine\permission\PermissionManager` class.
As can be observed from the removed code in the diff, the permissions system was previously entirely dependent on the Server, because it needed to get the PluginManager for registering permissions. This is utterly ridiculous. This refactor isolates _most_ permission-related functionality within the `permission` namespace.
As mentioned above, this stupid API is a direct result of copying from Bukkit. If you look at the API documentation for Bukkit for `PluginManager` you will see that the methods I'm deprecating here are also in there.
## Changes
- Added a new `PermissionManager` class. This can be accessed via its singleton `getInstance()` static method.
- Deprecated the following `PluginManager` methods - these will be removed no later than 4.0.0:
- `getPermission()`
- `addPermission()`
- `removePermission()`
- `getDefaultPermissions()`
- `recalculatePermissionDefaults()`
- `subscribeToPermission()`
- `unsubscribeFromPermission()`
- `getPermissionSubscriptions()`
- `subscribeToDefaultPerms()`
- `unsubscribeFromDefaultPerms()`
- `getDefaultPermSubscriptions()`
- `getPermissions()`
- Separated player handling and creation from network interfaces
- Rewire disconnects to make them not be recursive
- Batching now uses sessions instead of players
- Fixed DisconnectPacket getting sent to players who disconnect of their own accord
PhpStorm can't see these or understand how they are being called, which is very annoying for bug hunting. Additionally, we already have the CodeOptimizer for this.
Totem usage can be detected using the MODIFIER_TOTEM constant of EntityDamageEvent.
This does not currently support using the totem in the offhand because offhand is not implemented yet.
The general purpose of this is to split up base damage from modifiers.
- Added methods getBaseDamage(), setBaseDamage(), getOriginalBaseDamage(), getModifiers(), getOriginalModifiers()
- setDamage() renamed to setModifier() and type is now mandatory
- getDamage() renamed to getModifier() and type is now mandatory
- getOriginalDamage() renamed to getOriginalModifier() and type is now mandatory
- Removed MODIFIER_BASE constant
- Constructors now accept: float baseDamage, float[] modifiers instead of just float[] modifiers
