Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: onComponentTypeCheck support for ShapeHitbox #1981

Merged

Conversation

ASGAlex
Copy link
Contributor

@ASGAlex ASGAlex commented Oct 1, 2022

Description

onComponentTypeCheck was only supported for components but not for hitboxes, because ShapeHitbox implements GenericCollisionCallbacks instead CollisionCallbacks.

Specifying onComponentTypeCheck for individual hitboxes might be useful in cases where Component have hitboxes for collisions and also some utility hitboxes (for example, to check available directions for pathfinding). Colliding between this two types of hitboxes should not lead to movement restrictions and so on. But without onComponentTypeCheck in ShapeHitbox we can not to perform such check.

Checklist

  • The title of my PR starts with a [Conventional Commit] prefix (fix:, feat:, docs: etc).
  • I have followed the [Contributor Guide] when preparing my PR.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • I have updated/added relevant examples in examples or docs.

Breaking Change?

  • Yes, this PR is a breaking change.
  • No, this PR is not a breaking change.

@spydon spydon merged commit f840210 into flame-engine:main Oct 1, 2022
@ASGAlex
Copy link
Contributor Author

ASGAlex commented Oct 2, 2022

@spydon I also implemented onComponentTypeCheck for Sweep algorithm. But after performing a benchmark I decided that it absolutely not worth in performance, it only makes code more complicated. Maybe we should to document this somewhere? To avoid new "issues" after years, when everithing will be forgotten 😉

@spydon
Copy link
Member

spydon commented Oct 2, 2022

@spydon I also implemented onComponentTypeCheck for Sweep algorithm. But after performing a benchmark I decided that it absolutely not worth in performance, it only makes code more complicated. Maybe we should to document this somewhere? To avoid new "issues" after years, when everithing will be forgotten wink

It is probably worth having it just for consistency, because it will also avoid callbacks being called for collisions that the user doesn't care about, even though the performance gain isn't that high?

@ASGAlex
Copy link
Contributor Author

ASGAlex commented Oct 2, 2022

Ok, I pushed what I have for now: #1996
Let's to discuss realization there. This code does not looks good for me but fixing it in right way will lead to breaking changes, I think

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants