[RSDK-10933] use PlanningAlgorithm enum in favor of function pointer #5067
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
viambot
added
the
safe to test
raybjork
reviewed
Jun 18, 2025
motionplan/planning_algorithms.go
Outdated
| } | ||
| } | ||
|
|
||
| func getPlanner( |
There was a problem hiding this comment.
We've been using the idiom of newX when writing functions that are constructors. This one definitely feels like a constructor rather than a getter. newPlanner is already taken so not sure what we should call this though
raybjork
approved these changes
Jun 18, 2025
biotinker
reviewed
Jun 18, 2025
motionplan/planning_algorithms.go
Outdated
| } | ||
| } | ||
|
|
||
| func getPlanner( |
There was a problem hiding this comment.
I usually think of "get" as returning an existing thing.
We're actually calling the constructor here and generating a new planner. Maybe we name this "constructPlanner"?
biotinker
approved these changes
Jun 18, 2025
raybjork
reviewed
Jun 18, 2025
motionplan/plannerOptions.go
Outdated
Comment on lines
110
to
112
| // Note the direct reference to a default here. | ||
| // This is due to a Go compiler issue where it will incorrectly refuse to compile with a circular reference error if this | ||
| // is placed in a global default var. |
There was a problem hiding this comment.
also not sure if this is relevant anymore
viambot
added
safe to test
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
First of many PRs to clean up
PlanRequestandplannerOptions.This particular PR is the start of trying to remove function pointers from these structs wherever possible (due to them not being serializable or explicit)