Align onXYZ method signatures to match event payloads

[mattjennings]

Context

Actors (and maybe other classes) expose methods such as onPreUpdate or onCollisionStart as convenient handlers for events to avoid manually wiring up to events.

However, the method signature is different than what you would get if you did manually wire up to the event.

// method on Actor
class Player extends ex.Actor {
   onPreUpdate(engine: Engine, delta: number) {}
}

// wiring up to event manually e.g. when extending Entity
class Player extends ex.Entity {
   constructor() {
     super()

     this.on('preupdate', this.onPreUpdate)
   }

   onPreUpdate({ engine, delta }: ex.PreUpdateEvent) {}
}

This makes it inconvenient if I want to move code around in such a way that i'm going to/from an event handler/class method.

Proposal

This is a severely breaking change

Align all onXYZ convenience methods to use event as the parameter rather than multiple parameters from the event.

class Player extends ex.Actor {
   onPreUpdate({ engine, delta }: ex.PreUpdateEvent) {}
}

[mattjennings]

we could consider writing a codemod to help with the breaking change

[github-actions]

This issue hasn't had any recent activity lately and is being marked as stale automatically.

[Autsider666]

Playing devil's advocate: What about deprecating and later removing the hardcoded onXYZ functions altogether? The added value of these, for as far as I could see separately implemented most of the time, onXYZ functions seems a bit low with on already existing. A dev could extend a class to create the same onXYZ functions with it if they prefer using them.

[mattjennings]

They should definitely stay. There's a very high chance your extended actor will need to use one of those events, so it removes boilerplate by having these. It's also a familiar concept for devs coming from Unity etc.

[Autsider666]

Ah, good to know. I'm not familiar with Unity at all, so that explains their usage.
Does this (or some other industry standard) include the custom code used for some of the existing onXYZ functions (example) instead of using on in the background?

[mattjennings]

I could be wrong but I'm guessing it's done that way to save setting up an event listener unnecessarily in the case that the extended actor doesn't need it.