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 static methods to create RayQueryParameters #61918

Merged
merged 1 commit into from
Aug 4, 2022

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Jun 10, 2022

The intersect_ray() method was changed to take single PhysicsRayQueryParameters object instead of several arguments. This is great for flexibility, but not for convenience, as a one-line is turned into a 4-liner (in the minimal case). So I added some static methods with most common parameters to quickly create query objects.

Originally discussed here:
https://chat.godotengine.org/channel/physics?msg=i4n2dWWm3XbYAGHHt

Copy link
Member

@rburing rburing left a comment

Choose a reason for hiding this comment

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

Convenient, and useful for --convert-3to4 project conversion. I think implementing the _init constructor would also be a reasonable alternative, but I don't have strong feelings about it.

@akien-mga akien-mga merged commit 036258b into godotengine:master Aug 4, 2022
@akien-mga
Copy link
Member

Thanks!

@KoBeWi KoBeWi deleted the static_body branch August 4, 2022 09:01
@and3rson
Copy link
Contributor

and3rson commented Aug 6, 2022

Is there any reason for swapped collision_mask & excludes arguments? It used to be from/to/excludes/mask

@KoBeWi
Copy link
Member Author

KoBeWi commented Aug 6, 2022

I don't know about excludes, but given what masks are usually used for, the default value makes little sense and you always want it customized. In my project, out of 23 uses of intersect_ray(), all of them change the mask, but only 2 use excludes. So I just moved the more commonly used argument earlier in the list.

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