Skip to content
This repository has been archived by the owner on Nov 29, 2022. It is now read-only.

Event CollisionEvent::Stopped is not emitted in some cases #223

Open
Shatur opened this issue Mar 9, 2022 · 6 comments
Open

Event CollisionEvent::Stopped is not emitted in some cases #223

Shatur opened this issue Mar 9, 2022 · 6 comments
Labels
bug Something isn't working

Comments

@Shatur
Copy link
Contributor

Shatur commented Mar 9, 2022

Event CollisionEvent::Stopped is not emitted when:

  • You are deleting the colliding entity.
  • You are changing/adding CollisionLayers which should disable collision between colliding objects.
@jcornaz jcornaz added the bug Something isn't working label Mar 9, 2022
@jcornaz
Copy link
Owner

jcornaz commented Mar 9, 2022

I also opened an issue in rapier: dimforge/rapier#299

I mark this issue as "upstream" for now until we know if rapier agrees to address it. If rapier rejects the proposal, we'll try to figure out a way to solve it in heron.

@jcornaz jcornaz added the upstream Wait for changes in a dependency of heron (like Bevy or Rapier) label Mar 9, 2022
@Shatur
Copy link
Contributor Author

Shatur commented Mar 9, 2022

You are changing/adding CollisionLayers which should disable collision between colliding objects.

I think this one is also could be resolved in Rapier. Is it worth mentioning this issue as well?

@jcornaz
Copy link
Owner

jcornaz commented Mar 9, 2022

I think this one is also could be resolved in Rapier. Is it worth mentioning this issue as well?

that is kind of a different issue to me. But if the problem is indeed in rapier then we could open another issue there.

@Shatur
Copy link
Contributor Author

Shatur commented Mar 9, 2022

that is kind of a different issue to me.

Sure, the cause is different. I just assumed that grouping this two issues together may result in a better designed solution for events.

@jcornaz
Copy link
Owner

jcornaz commented Mar 9, 2022

Sure, the cause is different. I just assumed that grouping this two issues together may result in a better designed solution for events.

I understand, and surely it is important to consider both problems to help design a good solution.

But that may still be two different issues. Eventually mentioning the other issue in each of them can be enough to make sure the two problems are considered when designing solutions. If there are enough related problems, the maintainer may group them via an epic/umbrella issue, a milestone or a dedicated tag.

Having multiple different problems (even related) in the same issue on the other hand may lead to an awkward situation when we have a fix ready for only part of the issue, but cannot close it. Or worse we close it by mistake and forget there is another part still waiting for a fix. It may also make prioritization more difficult because it's either all or nothing. One of the problems may be more problematic/urgent than the another. We may even decide to fix one problem but not the other. Finally, it also increase the perceived cost of solving the issue, and reduce the likelihood that someone start working on it.

I usually find that splitting too much in different issues is less problematic than aggregating too much into a single issue (the same applies to PRs by the way)

@Shatur
Copy link
Contributor Author

Shatur commented Mar 9, 2022

Agree, opened a separate issue in Rapier: dimforge/rapier#300
I will split this issue later based on upstream response.

@jcornaz jcornaz removed the upstream Wait for changes in a dependency of heron (like Bevy or Rapier) label May 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants