Document & extend testing for rotation conventions (#16200)

* Document Luanti rotation conventions
* Add test for setPitchYawRollRad (entity) rotation conventions
* Test and document that `vector.rotate` uses (extrinsic) Z-X-Y rotation order

builtin/common/tests/vector_spec.lua

@@ -432,7 +432,32 @@ describe("vector", function()
 			assert.True(almost_equal({x = 1, y = 0, z = 0},
 				vector.rotate({x = 1, y = 0, z = 0}, {x = math.pi / 123, y = 0, z = 0})))
 		end)
-		it("is counterclockwise", function()
+		it("rotation order is Z-X-Y", function()
+			local r = vector.new(1, 2, 3)
+			for _, v in ipairs({
+				vector.new(1, 0, 0),
+				vector.new(0, 1, 0),
+				vector.new(0, 0, 1),
+			}) do
+				local expected = v:rotate(r)
+				local function try(order)
+					local rotated = v
+					for axis in order:gmatch(".") do
+						local r_axis = vector.zero()
+						r_axis[axis] = r[axis]
+						rotated = vector.rotate(rotated, r_axis)
+					end
+					return almost_equal(rotated, expected)
+				end
+				assert.False(try("xyz"))
+				assert.False(try("xzy"))
+				assert.False(try("yxz"))
+				assert.False(try("yzx"))
+				assert.True(try("zxy"))
+				assert.False(try("zyx"))
+			end
+		end)
+		it("is right handed", function()
 			local v_before1 = {x = 0, y = 1, z = -1}
 			local v_after1 = vector.rotate(v_before1, {x = math.pi / 4, y = 0, z = 0})
 			assert.True(almost_equal(vector.normalize(vector.cross(v_after1, v_before1)), {x = 1, y = 0, z = 0}))

doc/lua_api.md

@@ -3935,6 +3935,32 @@ The following functions provide escape sequences:
     * Removes all color escape sequences.
 
 
+Coordinate System
+=================
+
+Luanti uses a **left-handed** coordinate system: Y is "up", X is "right", Z is "forward".
+This is the convention used by Unity, DirectX and Irrlicht.
+It means that when you're pointing in +Z direction in-game ("forward"), +X is to your right; +Y is up.
+
+Consistently, rotation is [**left-handed**](https://en.wikipedia.org/w/index.php?title=Right-hand_rule) as well.
+Luanti uses [Tait-Bryan angles](https://en.wikipedia.org/wiki/Euler_angles#Tait%E2%80%93Bryan_angles) for rotations,
+often referred to simply as "euler angles" (even though they are not "proper" euler angles).
+The rotation order is extrinsic X-Y-Z:
+First rotation around the (unrotated) X-axis is applied,
+then rotation around the (unrotated) Y-axis follows,
+and finally rotation around the (unrotated) Z-axis is applied.
+(Note: As a product of rotation matrices, this will be written in reverse, so `Z*Y*X`.)
+
+Attachment and bone override rotations both use these conventions.
+
+There is an exception, however: Object rotation (`ObjectRef:set_rotation`, `ObjectRef:get_rotation`, `automatic_rotate`)
+**does not** use left-handed (extrinsic) X-Y-Z rotations.
+Instead, it uses **right-handed (extrinsic) Z-X-Y** rotations:
+First roll (Z) is applied, then pitch (X); yaw (Y) is applied last.
+
+See [Scratchapixel](https://www.scratchapixel.com/lessons/mathematics-physics-for-computer-graphics/geometry/coordinate-systems.html)
+or [Wikipedia](https://en.wikipedia.org/wiki/Cartesian_coordinate_system#Orientation_and_handedness)
+for a more detailed and pictorial explanation of these terms.
 
 
 Spatial Vectors
@@ -4134,6 +4160,7 @@ angles in radians.
 
 * `vector.rotate(v, r)`:
     * Applies the rotation `r` to `v` and returns the result.
+    * Uses (extrinsic) Z-X-Y rotation order and is right-handed, consistent with `ObjectRef:set_rotation`.
     * `vector.rotate(vector.new(0, 0, 1), r)` and
       `vector.rotate(vector.new(0, 1, 0), r)` return vectors pointing
       forward and up relative to an entity's rotation `r`.
@@ -8506,9 +8533,9 @@ child will follow movement and rotation of that bone.
         * `interpolation`: The old and new overrides are interpolated over this timeframe (in seconds).
         * `absolute`: If set to `false` (which is the default),
           the override will be relative to the animated property:
-            * Translation in the case of `position`;
-            * Composition in the case of `rotation`;
-            * Per-axis multiplication in the case of `scale`
+          * Translation in the case of `position`;
+          * Composition in the case of `rotation`;
+          * Per-axis multiplication in the case of `scale`
     * `property = nil` is equivalent to no override on that property
     * **Note:** Unlike `set_bone_position`, the rotation is in radians, not degrees.
     * Compatibility note: Clients prior to 5.9.0 only support absolute position and rotation.
@@ -8589,9 +8616,10 @@ child will follow movement and rotation of that bone.
     * `acc` is a vector
 * `get_acceleration()`: returns the acceleration, a vector
 * `set_rotation(rot)`
-    * Sets the rotation
     * `rot` is a vector (radians). X is pitch (elevation), Y is yaw (heading)
       and Z is roll (bank).
+    * Sets the **right-handed Z-X-Y** rotation:
+      First roll (Z) is applied, then pitch (X); yaw (Y) is applied last.
     * Does not reset rotation incurred through `automatic_rotate`.
       Remove & re-add your objects to force a certain rotation.
 * `get_rotation()`: returns the rotation, a vector (radians)
@@ -9506,7 +9534,7 @@ Player properties need to be saved manually.
     -- (see node sound definition for details).
 
     automatic_rotate = 0,
-    -- Set constant rotation in radians per second, positive or negative.
+    -- Set constant right-handed rotation in radians per second, positive or negative.
     -- Object rotates along the local Y-axis, and works with set_rotation.
     -- Set to 0 to disable constant rotation.
 

irr/include/matrix4.h

@@ -167,8 +167,9 @@ public:
 	vector3d<T> getTranslation() const;
 
 	//! Make a rotation matrix from Euler angles. The 4th row and column are unmodified.
-	//! NOTE: Rotation order is ZYX. This means that vectors are
-	//! first rotated around the X, then the Y, and finally the Z axis.
+	//! NOTE: Rotation order is (extrinsic) X-Y-Z.
+	//! This means that vectors are first rotated around the X,
+	//! then the (unrotated) Y, and finally the (unrotated) Z axis.
 	//! NOTE: The rotation is done as per the right-hand rule.
 	//! See test_irr_matrix4.cpp if you're still unsure about the conventions used here.
 	inline CMatrix4<T> &setRotationRadians(const vector3d<T> &rotation);

src/script/lua_api/l_object.cpp

@@ -1158,6 +1158,8 @@ int ObjectRef::l_set_rotation(lua_State *L)
 
 	v3f rotation = check_v3f(L, 2) * core::RADTODEG;
 
+	// Note: These angles are inverted before being applied using setPitchYawRoll,
+	// hence we end up with a right-handed rotation
 	entitysao->setRotation(rotation);
 	return 0;
 }

src/unittest/test_irr_matrix4.cpp

@@ -6,6 +6,8 @@
 #include "irrMath.h"
 #include "matrix4.h"
 #include "irr_v3d.h"
+#include "util/numeric.h"
+#include <functional>
 
 using matrix4 = core::matrix4;
 
@@ -17,10 +19,60 @@ constexpr v3f x{1, 0, 0};
 constexpr v3f y{0, 1, 0};
 constexpr v3f z{0, 0, 1};
 
+constexpr f32 QUARTER_TURN = core::PI / 2;
+
+static void LEFT_HANDED(const std::function<void(core::matrix4 &m, const v3f &rot_rad)> &f) {
+	SECTION("rotation is left-handed") {
+		SECTION("around the X-axis") {
+			matrix4 X;
+			f(X, {QUARTER_TURN, 0 , 0});
+			CHECK(X.transformVect(x).equals(x));
+			CHECK(X.transformVect(y).equals(z));
+			CHECK(X.transformVect(z).equals(-y));
+		}
+
+		SECTION("around the Y-axis") {
+			matrix4 Y;
+			f(Y, {0, QUARTER_TURN, 0});
+			CHECK(Y.transformVect(y).equals(y));
+			CHECK(Y.transformVect(x).equals(-z));
+			CHECK(Y.transformVect(z).equals(x));
+		}
+
+		SECTION("around the Z-axis") {
+			matrix4 Z;
+			f(Z, {0, 0, QUARTER_TURN});
+			CHECK(Z.transformVect(z).equals(z));
+			CHECK(Z.transformVect(x).equals(y));
+			CHECK(Z.transformVect(y).equals(-x));
+		}
+	}
+}
+
 TEST_CASE("matrix4") {
 
+// This is in numeric.h rather than matrix4.h, but is conceptually a matrix4 method as well
+SECTION("setPitchYawRollRad") {
+	SECTION("rotation order is Y*X*Z (matrix notation)") {
+		v3f rot{1, 2, 3};
+		matrix4 X, Y, Z, YXZ;
+		setPitchYawRollRad(X, {rot.X, 0, 0});
+		setPitchYawRollRad(Y, {0, rot.Y, 0});
+		setPitchYawRollRad(Z, {0, 0, rot.Z});
+		setPitchYawRollRad(YXZ, rot);
+		CHECK(!matrix_equals(X * Y * Z, YXZ));
+		CHECK(!matrix_equals(X * Z * Y, YXZ));
+		CHECK(matrix_equals(Y * X * Z, YXZ));
+		CHECK(!matrix_equals(Y * Z * X, YXZ));
+		CHECK(!matrix_equals(Z * X * Y, YXZ));
+		CHECK(!matrix_equals(Z * Y * X, YXZ));
+	}
+
+	LEFT_HANDED(setPitchYawRollRad);
+}
+
 SECTION("setRotationRadians") {
-	SECTION("rotation order is ZYX (matrix notation)") {
+	SECTION("rotation order is Z*Y*X (matrix notation)") {
 		v3f rot{1, 2, 3};
 		matrix4 X, Y, Z, ZYX;
 		X.setRotationRadians({rot.X, 0, 0});
@@ -35,36 +87,12 @@ SECTION("setRotationRadians") {
 		CHECK(matrix_equals(Z * Y * X, ZYX));
 	}
 
-	const f32 quarter_turn = core::PI / 2;
-
 	// See https://en.wikipedia.org/wiki/Right-hand_rule#/media/File:Cartesian_coordinate_system_handedness.svg
 	// for a visualization of what handedness means for rotations
 
-	SECTION("rotation is right-handed") {
-		SECTION("rotation around the X-axis is Z-up, counter-clockwise") {
-			matrix4 X;
-			X.setRotationRadians({quarter_turn, 0, 0});
-			CHECK(X.transformVect(x).equals(x));
-			CHECK(X.transformVect(y).equals(z));
-			CHECK(X.transformVect(z).equals(-y));
-		}
-
-		SECTION("rotation around the Y-axis is Z-up, clockwise") {
-			matrix4 Y;
-			Y.setRotationRadians({0, quarter_turn, 0});
-			CHECK(Y.transformVect(y).equals(y));
-			CHECK(Y.transformVect(x).equals(-z));
-			CHECK(Y.transformVect(z).equals(x));
-		}
-
-		SECTION("rotation around the Z-axis is Y-up, counter-clockwise") {
-			matrix4 Z;
-			Z.setRotationRadians({0, 0, quarter_turn});
-			CHECK(Z.transformVect(z).equals(z));
-			CHECK(Z.transformVect(x).equals(y));
-			CHECK(Z.transformVect(y).equals(-x));
-		}
-	}
+	LEFT_HANDED([](core::matrix4 &m, const v3f &rot_rad) {
+		m.setRotationRadians(rot_rad);
+	});
 }
 
 SECTION("getScale") {

src/unittest/test_irr_rotation.cpp

@@ -5,8 +5,6 @@
 #include "catch_amalgamated.hpp"
 #include "irrMath.h"
 #include "matrix4.h"
-#include "irrMath.h"
-#include "matrix4.h"
 #include "irr_v3d.h"
 #include "quaternion.h"
 #include <functional>

src/util/numeric.h

@@ -478,6 +478,8 @@ inline void wrappedApproachShortest(T &current, const T target, const T stepsize
 	}
 }
 
+/// @note Uses (extrinsic) Z-X-Y rotation order, left-handed rotation
+/// @note This is not consistent with matrix4::setRotationRadians
 void setPitchYawRollRad(core::matrix4 &m, v3f rot);
 
 inline void setPitchYawRoll(core::matrix4 &m, v3f rot)
@@ -485,6 +487,7 @@ inline void setPitchYawRoll(core::matrix4 &m, v3f rot)
 	setPitchYawRollRad(m, rot * core::DEGTORAD);
 }
 
+/// @see setPitchYawRollRad
 v3f getPitchYawRollRad(const core::matrix4 &m);
 
 inline v3f getPitchYawRoll(const core::matrix4 &m)