-
-
Notifications
You must be signed in to change notification settings - Fork 905
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: quad tree broadphase support #1894
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good job!
Tests are missing, but I guess that might have been intentional so that you could show the API first.
packages/flame/lib/src/collisions/quadtree/quadtree_collision_detection.dart
Outdated
Show resolved
Hide resolved
Co-authored-by: Lukas Klingsbo <lukas.klingsbo@gmail.com>
Co-authored-by: Lukas Klingsbo <lukas.klingsbo@gmail.com>
Co-authored-by: Lukas Klingsbo <lukas.klingsbo@gmail.com>
Co-authored-by: Lukas Klingsbo <lukas.klingsbo@gmail.com>
I have done with tests - looks like old tests are reused correctly 🎉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some very minor last comments.
You've done an amazing job! 🎉
packages/flame/lib/src/collisions/quadtree/has_quadtree_collision_detection.dart
Outdated
Show resolved
Hide resolved
Co-authored-by: Lukas Klingsbo <lukas.klingsbo@gmail.com>
# Conflicts: # packages/flame/test/collisions/collision_callback_test.dart
@ASGAlex if you can just fix the merge conflict this PR is ready to be merged! 😍 |
15 minutes 👌 |
# Conflicts: # packages/flame/test/collisions/collision_callback_test.dart
Head branch was pushed to by a user without write access
@spydon looks like done =^_^= |
Great job! I think it ended up quite nice :) |
@ASGAlex congrats! You did great job! :) |
…1943) A variant of refactoring mentioned here #1894 (comment) Sweep broadphase have no any additional specific classes, it's based only on core classes. So it is discussion question, do we need to keep int in separate folder (to avoid mixing with core broadphase.dart file) or just leave it in the root folder 'broadphase'.
Description
Quad tree broadphase support.
Checklist
fix:
,feat:
,docs:
etc).docs
and added dartdoc comments with///
.examples
.Breaking Change