excaliburjs / Excalibur

🎮 Your friendly TypeScript 2D game engine for the web 🗡️
https://excaliburjs.com
BSD 2-Clause "Simplified" License
1.75k stars 188 forks source link

precollision event inconsistencies between Actor hook and ColliderComponent event #3098

Open mattjennings opened 2 months ago

mattjennings commented 2 months ago

The value of other in the precollision event is either a collider or the actor, depending on where you're listening to the event from.

Actor's onPreCollisionResolve is a collider

ColliderComponent event is the entity, which is wiring from what the System emits

I would suggest keeping them always as colliders.


Tangentially (this could be its own issue but I figured i'd throw it in here), I wonder if lifecycle methods like onXYZ() on actors should always match the event payload. In other words 1 param as the event. Sometimes it's a pain when you want to move an event handler as a .on() handler to/from an Actor lifecycle method because one is multiple params whereas the other is an event object.

eonarheim commented 2 months ago

I agree, I think keeping them as the Collider would be less confusing

Tangentially (this could be its own issue but I figured i'd throw it in here), I wonder if lifecycle methods like onXYZ() on actors should always match the event payload. In other words 1 param as the event. Sometimes it's a pain when you want to move an event handler as a .on() handler to/from an Actor lifecycle method because one is multiple params whereas the other is an event object.

It definitely feel like it should match... I'd support changes to make this so

mattjennings commented 2 months ago

cool, I've split that out into a separate issue then! https://github.com/excaliburjs/Excalibur/issues/3106

github-actions[bot] commented 2 weeks ago

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