[3.x] Allow constructing Quat from two Vector3s

[timothyqiu]

Closes https://github.com/godotengine/godot-proposals/issues/424

This quaternion constructor is exposed in 4.x.

~~But GDScript in 3.x doesn't allow having multiple constructors with the same argument count but different types. Nor are static methods available. Therefore, I added the quat_to() method to Vector3, similar to angle_to().~~

Adds set_rotation(from, to) to Quat.

[lawnjelly]

Could this be a method of Quat instead of Vector3?

It seems slightly counter-intuitive (and difficult for discoverability) to try and put this as a method of Vector3. :thinking:

@AThousandShips : Note this is for 3.x, as 4.x has it exposed already it sounds like, but 3.x can not use the same method.

[AThousandShips]

Yep, realized after, you caught it before I got to it

[timothyqiu]

Could this be a method of Quat instead of Vector3?

Could be. The downside is that you have to create an empty Quat first, as there is no static method. Do you think it's acceptable?

var q1 := v1.quat_to(v2)

var q2 := Quat()
q2.make_rotation(v1, v2)

[lawnjelly]

Could be. The downside is that you have to create an empty Quat first, as there is no static method. Do you think it's acceptable?

Hmm, certainly worth getting a few more opinions on before merging (I'm open to either, but would be nice to be sure before committing to an API). In fact I'd prefer to leave this for @akien-mga to look at before we commit to it.

I'll admit the former looks more efficient (in e.g. bound languages with no optimization), but on the other hand it seems to make more logical sense to keep Quat methods with Quat rather than moving to Vector3.

[akien-mga]

I think moving it to Quat makes sense, even though the lack of static method is a bit awkward.

The way the method is now defined in the middle of variant_call.cpp instead of relying on VCALL_ macros to use the member function seems to also advocate for a less hacky approach.

[timothyqiu]

Changed to Quat.set_rotation(from, to).

Just noticed that Quat already has similar methods set_euler(euler) and set_axis_angle(axis, angle). So I adapted the naming.

[lawnjelly]

The name set_rotation() seems a little of a cop out, given that quaternions are usually rotations, and there are multiple ways of setting them (e.g. axis angle etc).

Would set_shortest_arc() or something like that be any better? :thinking:

[lawnjelly]

I think shortest arc is probably the best term. There is also "great circle arc" and "minor arc" but shortest arc seems to convey what we are after.

Double checking with any mathematicians, @kleonc does this naming sound okay?

[kleonc]

Double checking with any mathematicians, @kleonc does this naming sound okay?

Sounds fine to me, but got to clarify it n-th time: I'm not a mathematician. :smile: I do get why I could be considered a math-guy though, and I'm absolutely fine with it! Still had to clarify. :slightly_smiling_face:

Besides the naming I want you to be aware of #80249, see my comment: https://github.com/godotengine/godot/issues/80249#issuecomment-1666581678 (seems still relevant).

To be sure I'll link this / ask for some potential feedback on Godot Discord's math channel (there are some more-mathy-than-me folks in there).

[lawnjelly]

Besides the naming I want you to be aware of https://github.com/godotengine/godot/issues/80249, see my comment: https://github.com/godotengine/godot/issues/80249#issuecomment-1666581678 (seems still relevant).

Yes the input could be verified (at the least in DEV_ENABLED). And the output is undefined in the case of 180 degrees. We've had issues on this in the past if I remember right: https://github.com/godotengine/godot/issues/53928

I wonder whether we should return a bool for success or failure to cover this, or an error code. There is an opportunity to make a more sensible function than is present in 4.x.

e.g.

OK
ERR_PARAMETER_RANGE_ERROR
FAILED

What do you guys think?