feat: colliding entities component

[Shatur]

Closes #207.

I added a component that automatically filled with colliding object entities.

Things to discuss:

• I used CollidingEntities because the suggested Collision could be confusing (and already used in Bevy, but not in prelude). Is this okay name?
• Should we provide Deref for CollidingEntities?

[Shatur]

Should I provide an example for this feature? Or simple enough?

[jcornaz]

• I used CollidingEntities because the suggested Collision could be confusing (and already used in Bevy, but not in prelude). Is this okay name?

Yeah Collision wouldn't be good. Collisions is already a little bit better. I kind of like CollidingEntities though we could go for that.

We may also consider: CollisionSet or CollidedEntities, stuff like that.

No need to bikeshed too much though, if we find a better name later we can change it without making a breaking change (by making the old name an alias)

• Should we provide Deref for CollidingEntities?

No. That would expose internal details that we couldn't change without breaking the API.

[jcornaz]

Should I provide an example for this feature? Or simple enough?

I would like to make this new way for observing collisions the "main" way. As I think it is simpler and more ergonomic in most cases.

Therefore I would suggest updating the existing event example to use this pattern. And then leave the way of observing collision via EventReader<CollisionEvent> without example (that one being actually more intuitive anyway)

But if you don't feel like doing it in this PR, it's fine. We can do it later.

[Shatur]

But if you don't feel like doing it in this PR, it's fine. We can do it later.

I will do it in this PR.

[Shatur]

Yeah Collision wouldn't be good. Collisions is already a little bit better. I kind of like CollidingEntities though we could go for that.

I somehow misread your first message in the issue and thought that you proposed Collision. I renamed into Collisions, I really like how this name fits.

[Shatur]

I applied your suggestions, but discovered the issue with the event example. When I remove an entity, it doesn't emit CollisionEvent::Stopped event. So If user deletes entity that collides it stays forever in Collisions which is unexpected. How we could solve this on Heron side:

1. Emit CollisionEvent::Stopped on entity deletion.
2. Remove entity from Collisions on deletion.
   Which solution you would prefer?

[jcornaz]

I applied your suggestions, but discovered the issue with the event example. When I remove an entity, it doesn't emit CollisionEvent::Stopped event. So If user deletes entity that collides it stays forever in Collisions which is unexpected. How we could solve this on Heron side:

1. Emit CollisionEvent::Stopped on entity deletion.
2. Remove entity from Collisions on deletion.
   Which solution you would prefer?

I think I would lean toward option 1. What do you think?

[Shatur]

I think I would lean toward option 1. What do you think?

Yes, I would expect CollisionEvent::Stopped to be called on entity deletion.
I will send a separate PR for this first and go back to this PR.

[Shatur]

On second thought I should have a list of colliding entities to emit this CollisionEvent::Stopped. Should I go with option 2 then? Or do you have any idea of how can I implement 1?

[jcornaz]

Or do you have any idea of how can I implement 1?

I have to look into it. Somehow, I am surprised, that it isn't already the case, that kind of sounds like a bug to me. I'll investigate that, and come back to you.

[Shatur]

I discovered another case when CollisionEvent::Stopped is not emitted. Opened a general issue: #223

[Shatur]

Until the mentioned issue fixed upstream, I think I can start working on option 2:

Remove entity from Collisions on deletion.

When the fix arrives, we can just delete the removal detection.

[Shatur]

Implemented. I also rebased to the latest master and improved tests description.

[jcornaz]

Thanks! I'll try to look at it this week.

[jcornaz]

Looks great thanks!

[jcornaz]

Just change iter to:

pub fn iter(&self) -> impl Iterator<Item = Entity> {
  self.0.iter().copied()
}

[Shatur]

Just change iter to

Done! I also had to add anonymous lifetime.