-
-
Notifications
You must be signed in to change notification settings - Fork 21.3k
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
C#: Add IntersectsRay to Aabb
#97695
base: master
Are you sure you want to change the base?
Conversation
Aabb
with CoreAabb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for contributing to the .NET module! Looks like the code matches the Core implementation, and is consistent with the other Intersects*
methods in Aabb
. I'm making a few comments, mostly about code-style.
Also, since you are a new contributor, make sure to read CONTRIBUTING.md and the contributing documentation if you haven't already.
You'll need to squash the commits before this PR can be merged. The contributing documentation contains information about squashing in case you need it.
Feel free to reach out in the development chat if you need help.
/// </returns> | ||
public readonly bool IntersectsRay(Vector3 from, Vector3 dir) | ||
{ | ||
if (HasPoint(from)) return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see this in the Core implementation, and seems a bit redundant.
if (HasPoint(from)) return true; | |
if (HasPoint(from)) | |
{ | |
return true; | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're saying remove it? Seems like a potential optimisation to me but I may be wrong
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'd have to benchmark it to see if makes a difference.
Co-authored-by: Raul Santos <[email protected]>
Co-authored-by: Raul Santos <[email protected]>
…odot into dotnet/sync-aabb
IntersectsRay
intersects_ray
implementation without inside/normal/intersection point calculation