-
-
Notifications
You must be signed in to change notification settings - Fork 60
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
Add pathfinder #648
Add pathfinder #648
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.
Apologies for the slow review. This looks great! 🚀
I've left a few minor comments for your consideration - mostly just for style / consistency. I think the next things are:
-
I'd like to see it working / have an example to turn to for reference. Usually at this point I'd set up a scene in the sandbox, maybe just a simple grid and have it solve and show coloured boxes for the path or something. I can do this if you'd rather not.
-
Decide whether or not to just remove the old search grid. I think we could. Indigo isn't in a place where people are moaning about breaking API changes.
We could just raise issues for those two points, and I can make a demo and then consider removing SearchGrid as steps outside this PR.
indigo/indigo-extras/src/main/scala/indigoextras/pathfinding/DefaultPathBuilders.scala
Outdated
Show resolved
Hide resolved
indigo/indigo-extras/src/main/scala/indigoextras/pathfinding/DefaultPathBuilders.scala
Outdated
Show resolved
Hide resolved
indigo/indigo-extras/src/main/scala/indigoextras/pathfinding/DefaultPathBuilders.scala
Outdated
Show resolved
Hide resolved
indigo/indigo-extras/src/main/scala/indigoextras/pathfinding/DefaultPathBuilders.scala
Outdated
Show resolved
Hide resolved
import scala.annotation.tailrec | ||
|
||
@deprecated("Use the new indigoextras.pathfinding.PathFinder", "0.15.3") | ||
@nowarn("cat=deprecation") | ||
final case class SearchGrid( |
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.
It's tempting to just remove this stuff. I doubt many people are using it. 🤔
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.
I did not remove it.
indigo/indigo-extras/src/main/scala/indigoextras/pathfinding/PathFinder.scala
Outdated
Show resolved
Hide resolved
indigo/indigo-extras/src/main/scala/indigoextras/pathfinding/PathFinder.scala
Outdated
Show resolved
Hide resolved
The PR is updated according to the comments but the removal of the old pathfinder. |
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.
Fantastic work @mprevel, and the demo is very cool. 😎
Hi,
This is a pathfinding algorithm as discussed in #646
It took me a little bit more time than what I was expecting.
Please let me know if you have questions, comments or want some changes about it.
Regards,