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

correction #13031

Merged
merged 3 commits into from
Sep 28, 2022
Merged

correction #13031

merged 3 commits into from
Sep 28, 2022

Conversation

BabylonJSGuide
Copy link
Contributor

I did not check https://playground.babylonjs.com/?snapshot=refs/pull/13012/merge#S7HDGQ#5 before merge, the green axis should be tangential to the line and it was not. Should be OK after this PR

Reason for correction

PitchYawRollToMoveBetweenPointsToRef(start: Vector3, target: Vector3, ref: Vector3)

Cannot guarentee ref with be a zero vector and so need to make ref.z explicitly 0

@azure-pipelines
Copy link

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@james-pre
Copy link
Contributor

Thanks for spotting this!

@azure-pipelines
Copy link

@BabylonJSGuide
Copy link
Contributor Author

BabylonJSGuide commented Sep 27, 2022

https://playground.babylonjs.com/?snapshot=refs/pull/13031/merge#S7HDGQ#5

Still not correct, should be. Will need to double check before approval.

This is what I based the change on and it works https://playground.babylonjs.com/#QYA3JC#32

RaananW
RaananW previously approved these changes Sep 27, 2022
@BabylonJSGuide
Copy link
Contributor Author

BabylonJSGuide commented Sep 28, 2022

I was wrong using TmpVectors.Vector3[0] instead of Vector3.Zero() is not always the right thing to do!

Happy this works now https://playground.babylonjs.com/?snapshot=refs/pull/13031/merge#S7HDGQ#5

@RaananW can now merge after approving review.

@RaananW RaananW merged commit 417f4b0 into BabylonJS:master Sep 28, 2022
RaananW pushed a commit that referenced this pull request Dec 9, 2022
* correction

* make ref 0 vector

Former-commit-id: 3a67b52ab5098c05cd7569d10065886d3da75923
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants