Reimplement slerp on Vector3

[T4rmyn]

Reimplemented the slerp method on Vector3 with the addition of documentation and several tests on it. Addresses part of #310 to (possibly?) complete Vector3 methods (leaving the lacking documentations aside).

A notable change from the original Godot calculations is just directly using .normalized() on axis for unit_axis than manually doing it by axis / axis_length_sq.sqrt() on line 314.

[GodotRust]

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-316

[Bromeon]

Thanks a lot! Just minor comments.

Here's the link to your method documentation, by the way 🙂

[Bromeon]

The "directly calls" is an implementation detail (and makes the sentence more complicated to read), no need to mention that.

Instead, it would be nice if you added such comments in the code itself, e.g. in each if branch, to explain why this is equivalent to lerp.

[T4rmyn]

This update was an attempt at explaining the if statements (and the other fixes) and I hope it was sufficient.

I wonder though, could the line 315 if check be moved one statement before to instead check directly for a zero Vector3 since only that can produce a length of zero (right?), saving the length_squared() operation for after the check. Also that would make axis_length_sq obsolete I think?

[Bromeon]

That's correct, you could do

if axis == Vector3::ZERO {

Thanks for the comments!

[Bromeon]

Regarding the test cases, are they from Godot?

Otherwise, one thing we could do is see if Vector3::slerp and InnerVector3::slerp yield approximately the same results. This would need #[itest] though.

[T4rmyn]

No, it's not. I will replace it with the Godot ones then. It's also getting really late so expect it tomorrow.

[Bromeon]

No rush here! 😊

I also think the tests are fine as-is. We might automate some integration-testing against Godot in the future, so it can still be good to have some unit tests. Those also make ensure that the engine is not required to run.

[T4rmyn]

Uh oh, I think I found a difference in the checks by the repo's clippy and the local ./check.sh clippy. When I ran ./check.sh clippy it showed the previous force push as without flaws, but the repo one found the float has excessive precision.

When I ran the repo clippy command locally however, it also found the excessive precision issue.

For reference, the repo clippy command is:
cargo clippy --all-targets $GDEXT_FEATURES -- \ -D clippy::suspicious -D clippy::style -D clippy::complexity -D clippy::perf \ -D clippy::dbg_macro -D clippy::todo -D clippy::unimplemented -D warnings.

While the ./check.sh one is:
cargo clippy --no-default-features -- -D clippy::suspicious -D clippy::style -D clippy::complexity -D clippy::perf -D clippy::dbg_macro -D clippy::todo -D clippy::unimplemented -D warnings.

If this is a real issue I could open an issue about it (pun not intended).

[T4rmyn]

On another note, I ported the Godot tests :D.

[Bromeon]

Thanks for the note about clippy, I can adjust the local check.sh to match CI. In case of doubt, CI is always the source of truth.

bors r+

[bors]

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

• full-ci