Remove PlayerCreationEvent

[SOF3]

Applications of PlayerCreationEvent are basically as unreliable as using reflections. PocketMine is currently not semver-compliant because Player internal semantics are not polymorphism-friendly enough; most changes on Player methods do not consider BC with plugins using PlayerCreationEvent. Hence, supporting PlayerCreeationEvent is a maintainability burden and should be removed from the API.

Plugins currently using PlayerCreationEvent should seek alternative methods. In particular, for public plugins, it is always preferrable to send pull requests to extend PocketMine's API, while private developers unwilling to contribute can still patch the source directly (this is unreliable, but PlayerCreationEvent is unreliable as well).

[dktapps]

what "alternative methods"? the whole reason I haven't put forth this proposal yet myself is because there isn't any suitable alternative methods for some scenarios.

most changes on Player methods do not consider BC with plugins using PlayerCreationEvent

citation needed; this is actually the exact reason why #3167 has not yet been merged.

Sending PRs is fool's gold, since usually the changes needed are still specialized to one plugin or another anyway (e.g. economy plugins adding getBalance() or similar), and also because of the duration of time it takes for a PR to get from initial creation to release. That said, any application which doesn't require directly modifying Player functionality doesn't belong in a custom Player class anyway.

[dktapps]

Some additional things to consider, which aren't mentioned in the main issue:

• Users of PlayerCreationEvent are non-cooperative. Only one plugin using custom player classes can operate at a time. This goes against PM's entire plugin architecture, which is designed from ground-up to be cooperative.
  • This means that custom player classes cannot be used in public plugins, since public plugins can't know whether they'll be used in isolation or alongside other plugins.
  • Following from the above point, it's clear that only private core plugins can benefit from this feature. If the required use case is so specialized that it can't be filled by a general-purpose PM API addition, private servers can fork PM and modify it to their desires anyway.
• The intended, original use case for custom player classes was to allow overriding handleDataPacket() in the old days when packet handling was embedded into Player. This is no longer the case since many years.
• The only remaining legitimate use cases in PM4 for custom Player classes are to override protected or public methods of Player to alter behaviour that isn't directly exposed, or to intercept stuff that don't currently have events associated with them.
  • In both of these cases, it makes more sense to add events and new APIs to allow altering behaviour where appropriate, such as #4937 and #4563
  • Modifying internals behaviour is not a supported use case anyway and might break at any time.
• The vast majority of real-world (ab)uses of custom player classes can be filled by using session classes.
• Usage of custom players requires boilerplate code everywhere (such as in event handlers), e.g. /** @var CustomPlayerClass */ or instanceof CustomPlayerClass if the code is to be statically analysable. This ties into the above point: using session classes actually produces cleaner code.
• There is a legitimate case to be made for storing objects in the Player for future retrieval, so that objects such as sessions are automatically GC'd when the player disconnects without having to handle PlayerQuitEvent. This ties back into the earlier point about adding new APIs explicitly designed for this kind of use case.

The bottom line is that PlayerCreationEvent is one gigantic hack that's not fit for any purpose other than enabling plugins to do hacks. These hacks are typically only necessary because of:

• Poor internals design (e.g. packet handlers being baked into the Player class) - these should be rectified by internals improvements
• API deficiencies - these should be addressed with RFCs and API improvements and additions
• Bugs - these should be fixed in PM itself, not worked around by plugin devs
• Plugin developers refusing to see that there's more than one (and often better) ways of doing things than the ones they are fixated on - nothing we can really do except rip the rug from under them.

Example code: sessions vs custom Player classes

$this->getSession($player)->doSomething();

vs

if($player instanceof CustomPlayerClass){
    $player->doSomething();
}

[SOF3]

Relevant: the problem described in the OP is commonly referred to as the Fragile base class problem.

The fact that PlayerCreationEvent is non-cooperative is not exactly critical in the context of private plugins, but the fragility introduced on the Player class is causing much more potential compatibility and maintainability problems.

[dktapps]

This is a problem that extends beyond Player classes, which is the reason why I make liberal use of final and private in new code