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

Add closest_points_between_segments() basis path tests for Geometry2D #48652

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

alexbilledeaux
Copy link
Contributor

New tests for the closest_points_between_segments method in Geometry2D, based on basis path testing.

@alexbilledeaux alexbilledeaux requested a review from a team as a code owner May 11, 2021 18:20
@Calinou Calinou added this to the 4.0 milestone May 11, 2021
@akien-mga
Copy link
Member

There are some style issues, see the diff from CI: https://github.com/godotengine/godot/pull/48652/checks?check_run_id=2559024734

See Code style guidelines for details, there are instructions for running clang-format locally to validate your changes.

tests/test_geometry_2d.h Outdated Show resolved Hide resolved
tests/test_geometry_2d.h Outdated Show resolved Hide resolved
@akien-mga akien-mga modified the milestones: 4.0, 4.x Jan 19, 2023
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally (rebased on top of master 83b916b), it works as expected. Code looks good to me.

I force-pushed a rebased version which also removed a few existing tests, as I think they are made redundant by this PR (please double-check this).

@Calinou Calinou changed the title Add basis path testing for get_closest_points_between_segments 2D Add closest_points_between_segments() basis path tests for Geometry2D Apr 10, 2024
@akien-mga akien-mga merged commit 76c4ed9 into godotengine:master Apr 11, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

And sorry for the delay getting this reviewed and merged.

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.

4 participants