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

Clarify behavior of RayCast when get_collision_point() is used inside a collision shape #84085

Merged
merged 1 commit into from
Jan 22, 2024

Conversation

CardboardCarl
Copy link
Contributor

@CardboardCarl CardboardCarl commented Oct 27, 2023

Clarified documentation on a behavior of get_collision_point() in both RayCast2D and RayCast3D objects when it is used while the ray is inside a collision shape

@CardboardCarl CardboardCarl requested a review from a team as a code owner October 27, 2023 23:15
@MewPurPur
Copy link
Contributor

MewPurPur commented Oct 28, 2023

Here it should maybe just say "See also [member hit_from_inside]" as it explains the same thing verbatim, and also it explains the limitation with concave shapes.

@Calinou Calinou added enhancement documentation cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release labels Oct 28, 2023
@Calinou Calinou added this to the 4.2 milestone Oct 28, 2023
doc/classes/RayCast2D.xml Outdated Show resolved Hide resolved
doc/classes/RayCast3D.xml Outdated Show resolved Hide resolved
@CardboardCarl
Copy link
Contributor Author

Here it should maybe just say "See also [member hit_from_inside]" as it explains the same thing verbatim, and also it explains the limitation with concave shapes.

The issue with this is that the returned value is not Vector2(0, 0), but instead it's the global position of the ray, so the behavior is slightly different.

@CardboardCarl
Copy link
Contributor Author

Oops! Going to rebase this in a little bit.

@CardboardCarl
Copy link
Contributor Author

CardboardCarl commented Oct 29, 2023

Changes have been rebased into one commit. 👍

@AThousandShips

@AThousandShips
Copy link
Member

Someone knowledgeable on the area will come and review this, was just fixing errors in the docs that caused issues

@AThousandShips AThousandShips requested a review from a team October 30, 2023 16:01
@CardboardCarl
Copy link
Contributor Author

Someone knowledgeable on the area will come and review this, was just fixing errors in the docs that caused issues

No worries, thanks for checking in!

@CardboardCarl CardboardCarl requested a review from Calinou November 3, 2023 01:41
@akien-mga akien-mga changed the title Clarified behavior of RayCast objects when get_collision_point() is used inside a collision shape Clarified behavior of RayCast objects when get_collision_point() is used inside a collision shape Nov 12, 2023
@akien-mga akien-mga modified the milestones: 4.2, 4.x Nov 12, 2023
Copy link
Contributor

@Mickeon Mickeon left a comment

Choose a reason for hiding this comment

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

Very simple but nice addition. Should probably be cherrypicked for 4.2, too.

@akien-mga akien-mga added the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Jan 16, 2024
@akien-mga akien-mga modified the milestones: 4.x, 4.3 Jan 16, 2024
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Language looks good. I'm assuming the doc change is actually correct, didn't double check :)

@YuriSizov
Copy link
Contributor

Could you please squash your commits into one? Make sure that the final commit has a short but descriptive message (the title of this PR is a good option). See this documentation, if you need help with squashing.

@CardboardCarl
Copy link
Contributor Author

@YuriSizov very sorry about that, I could have sworn I did earlier! Working on it.

@CardboardCarl
Copy link
Contributor Author

Had to reorganize a little bit. @YuriSizov commits have been squashed!

@CardboardCarl CardboardCarl reopened this Jan 21, 2024
@YuriSizov YuriSizov merged commit 52bac73 into godotengine:master Jan 22, 2024
16 checks passed
@YuriSizov
Copy link
Contributor

Thanks! And congrats on your first merged Godot contribution!

@YuriSizov YuriSizov changed the title Clarified behavior of RayCast objects when get_collision_point() is used inside a collision shape Clarify behavior of RayCast when get_collision_point() is used inside a collision shape Jan 24, 2024
@YuriSizov YuriSizov removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Jan 24, 2024
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.2.2.

@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Jan 24, 2024
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.4.

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.

7 participants