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.

Sign up for GitHub

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

Jump to bottom

Refactor: navigation_obstacle_3d #70239

Closed
wants to merge 1 commit into from
Closed

Refactor: navigation_obstacle_3d #70239

wants to merge 1 commit into from

Conversation

quentinguidee
Copy link
Contributor

@quentinguidee quentinguidee commented Dec 18, 2022 •
edited
Loading

Clean NavigationObstacle3D

  • Add _ before private methods
  • Fix code duplication in NOTIFICATION_PAUSED and NOTIFICATION_UNPAUSED
  • Add some const keywords
  • Remove redundant parenthesis
  • Simplify if-if
  • Rename one-letter variables

@quentinguidee
Refactor: navigation_obstacle_3d
eb2529f 
* Add `_` before private methods
* Fix code duplication in NOTIFICATION_PAUSED and NOTIFICATION_UNPAUSED
* Add some const keywords
* Remove redundant parenthesis
* Simplify if-if
* Rename one-letter variables

Signed-off-by: Quentin Guidée <[email protected]>
@quentinguidee quentinguidee requested a review from a team as a code owner December 18, 2022 03:16
@Chaosus Chaosus added this to the 4.0 milestone Dec 18, 2022
@smix8
Copy link
Contributor

smix8 commented Dec 18, 2022 •
edited
Loading

See #69988.

Could prepare a patch for those refactor changes that still apply but parent node and estimate radius are removed.

Obstacle3D and Obstacle2D share mostly the same code so should be always refactored together. Some is also shared with the NavigationAgent2D and NavigationAgent3D that still use a parent node so might be worth to take a look and apply there.

@quentinguidee quentinguidee marked this pull request as draft December 18, 2022 14:22
@akien-mga akien-mga modified the milestones: 4.0, 4.x Feb 9, 2023
@smix8
Copy link
Contributor

smix8 commented Aug 16, 2024

Closing as the obstacle has evolved so much since that everything in this draft is obsolete by now.

Thanks for the PR contribution nonetheless!

@smix8 smix8 closed this Aug 16, 2024
@smix8 smix8 removed this from the 4.x milestone Aug 16, 2024
@smix8 smix8 added the archived label Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
archived enhancement topic:navigation topic:3d
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@quentinguidee @smix8 @Chaosus @akien-mga