Add Lua methods 'set_rotation()' and 'get_rotation()'

[jmdevy]

This PR adds Lua methods called 'set_rotation(rot)' where rot is a vector {x=0, y=0, z=0} and ''get_rotation()' returns a vector. As far as the network code goes; instead of just a float being sent over the network now a v3f is sent for XYZ axes. I preserved setYaw/set_yaw so that old mods still work. This also changes some of the C++ PlayerSAO names to be more clear.

One issue that needs to be dealt with: should the new Lua methods take radians or degrees? Right now it takes degrees (which I think is more intuitive). setYaw already uses radians though but once it has the angle in radians it uses core::RADTODEG right away. (EDIT as of July 11, 2018: decided on using degrees. EDIT2: it's back to radians)

For testing, I wrote a mod that has various commands for making sure this all works: prytest. Starting a world with the mod enabled will display the commands in chat.

If you're curious to see the Lua code that a mod could use: Link

For some good discussion on entity rotation: #7331

Here's what I know works so far:

• Set to various rotations and back to zero: GIF
• Spin on each axis: GIF
• Set to a rotation with each axis edited (not zero), quit Minetest/world, come back and entity still at same rotation: GIF
• The player is still rotating correctly: GIF
• If an entity is added to another entity, a call to set_rotation() on the child entity will not override the attach rotation. If the parent entity is spinning then any child entity will behave correctly (does not spin on its own axis, spins like a dot on the surface of a rotating sphere would, for example, like you and me on Earth): GIF
• Negative rotations: comment
• Mods that use setYaw() still work like they should when using this PR.

[stujones11]

One issue that needs to be dealt with: should the new Lua methods take radians or degrees?

Using degrees would be consistent with attachment rotations and is what Irrlicht uses. It may also help to avoid network conversion errors with small floating-point values.

I still think a rotate or maybe rotate_local method would be very useful for 'flight' mods in particular as it would allow for local axis rotation without having to overcome problems like 'gimbal-lock' in Lua.

Of course that should be a separate PR I could even make myself if need be :)

[jmdevy]

@stujones11 I plan on adding rotate after this PR is merged. For a rotate method I will have to add an extra bool to the network code but first I want this to be merged so that a v3f for representing rotation is already there.

The reason I have done all this work is that I wanted to make a mod that had flight mechanics but realized that Minetest did not have many resources for that. After this PR is merged then I will work on and push the code for rotate.

There really is no reason for you do it, yet, I know this part Minetest well now so it should not take me long to do it, but in about a week and a half I start a summer semester and will be swamped with school. Once I am back in school I will not be able to start a whole new PR I don't think.

[stujones11]

@CoderForTheBetter There is really no hurry, your school work should definitely come first. It would be nice to see these features included in 0.5.0 but I think that might still be a little way off. The code itself is very simple, it's all the other boiler-plate and documentation that takes time.

I have also always wanted to make a semi-realistic flying mod and have had several attempts at trying to use dummy attachments, animation key-frames and all sorts of other hackery to mimic pitch and roll, none of which have been very successful =)

[jmdevy]

I know you guys are busy so I made a GIF for each thing I tested and know works (can't think of anything else to test) and I summarized questions that need answers:

Here's what I know works so far:

• Set to various rotations and back to zero: GIF
• Spin on each axis: GIF
• Set to a rotation with each axis edited (not zero), quit Minetest, come back and entity still at same rotation: GIF
• The player is still rotating correctly: GIF
• If an entity is added to another entity, a call to set_rotation() on the child entity will not override the attach rotation. If the parent entity is spinning then any child entity will behave correctly (does not spin on its own axis, spins like a dot on the surface of a rotating sphere would, for example, like you and me on Earth): GIF

Questions:

1. Should set_rotation() take degrees or radians? Stu pointed out set_attach() takes degrees.
2. Should I change back to using getYaw(), instead of getRotation().Y, in areas like here?

Hopefully, @SmallJoker and/or @nerzhul can answer my above questions.

[stujones11]

Should I change back to using getYaw()

FWIW, I think we should move away from the aircraft terminology (yaw, pitch & roll), this is already done in the api by deprecating get_look_yaw to get_look_horizontal etc.

[SmallJoker]

@CoderForTheBetter set_rotation should take, like the other rotation functions, radians. The attachment functions are inconsistent IMO, but changing them is not possible due to compatibility.
Most of the time, only yaw is used to rotate objects (horizontal maps), so it does not matter which functions are used as long it's clear for the context. For your highlighted line I'd personally keep getYaw() (horizontal look angle) because the camera cannot be rotated in a 3rd axis (yet).

[stujones11]

The attachment functions are inconsistent IMO

Irrlicht uses degrees for rotation, using radians (IMO) is not only less intuitive for most people, it would require unnecessary conversions which may result in loss of precision, especially with sending complex floats over the network.

[jmdevy]

@stujones11 Didn't see your comment. I can change back my commit if needed, someone needs to decide on degrees or radians.

EDIT: @nerzhul, what do you think? Degrees or radians? I like stu's points:

• Irrlicht uses degrees already.
• Degrees are more intuitive than radians.
• More math for the engine to do when using radians (unnecessary conversions it seems).
• Using radians can result in a loss of precision when sending complex floats over the network.

It is ultimately up to you, @rubenwardy, and @SmallJoker since you guys can approve PRs.

[jmdevy]

Tested everything using radians, seems to be working. I don't don't mind using radians, I just want this approved before 0.5.0 so that the network code is in place. Degrees or radians can be decided on later, the network can't.

Also, it has been three years that people have wanted this basic feature: #2251 This is the only chance for the network code to be put into place since 0.5.0 breaks stuff anyways.

[ClobberXD]

Rotation doesn't seem to be consistent:

The entity is jerking back and forth half the time. Is the issue on my side?

[jmdevy]

@ClobberXD weird. Are you using the spin command from my mod? It spins it fast now that it is using radians. Try lowering the increment amount to something smaller. Seems to be on your side.

Here's what I have with the default increment amount from my mod (a little jerky but that's just Minetest when moving things fast):

Could be your minetest.conf settings. The important one for this is the server step; here's mine: dedicated_server_step = 0.03 (default is 0.09: value)

EDIT: Just tried with 0.09 server step and got the same problem as you. The entity is just spinning too fast for the server step. Realistically, no mod will want to spin an object that fast anyway, which in this case is 1 radian (57 degrees) every server step (very fast).

[ClobberXD]

Oh right, dedicated_server_step was 0.1 for me. It rotates much more smoothly at 0.03, thanks :)

[nerzhul]

the old 0.1 was before MT was optimized from 0.4.15 to 0.4.17. Now you should use 0.05 by default it's a common accepted choice

[ClobberXD]

Right, so is there any specific issue preventing this PR from being merged?

[paramat]

Rubenwardy, we don't have to change everything to one or the other, at first i considered that too but later realised there's actually no reason to enforce a hard rule for everything, and very good reason to be flexible and practical and use one or the other unit according to what makes sense for that particular usage.

For example decoration rotation being 0 90 180 270 should obviously be in degrees and these values are never used in Lua maths code.
Likewise for treedef, place_schematic, attachments and bones, degrees is obviously the best choice as these are only used as intuitive values set by a modder.
Whereas values heavily used in Lua maths code, as in rotations, are best in radians as the Lua maths functions require radians. Otherwise large amounts of conversion code is required in already complex code.

Once you remove the obvious degrees items from your list you end up with Lua maths and values heavily used in Lua maths (object rotations), all are radians for good reason.

Existing mods using yaw will have code written for radians, so it's easier to update these mods to 3D rotations and easier for modders who are already thinking in radians.
I recently coded my 'driftcar' mod which uses physics modelling of forces and has a lot of angle calculations. It was a huge help that everything was in radians as used in Lua maths code.

This would make more sense anyway, given that it's usual to have angles in degrees but in radians for calculations

Nope, it doesn't make sense to have angles in degrees when those angles are being heavily used in calculations, as with rotations.

[jmdevy]

Latest commit doesn't change anything major. It is just an improvement to the quality of the code.

[paramat]

My 👍 still stands.

[paramat]

Thanks for reviewing! =D

[jmdevy]

Nice! Thank you all for your time reviewing and testing! In about two weeks I will start on trying to make a PR that will make rotations more intuitive when it comes to rolling objects.

[GreenXenith]

I am ecstatic that this has been merged 🎉
However, I am wondering why this was merged without proper roll.
When using the code @paramat posted for the boat example, I got this:
Rotating around Y axis: Works as expected
Pitching at any yaw: Works as expected
Rolling at any yaw: Rolls when aligned on z, pitches when aligned on x.
This sounds broken to me. Maybe I missed something?

[jmdevy]

We discussed this a long time ago (in my first issue on the subject). This is first needed to be able to get the network protocol where it needs to be. Also, if a function such as 'minetest.rotate_on_axis' existed then the rotation of the object can easily be set back to zero (or any other rotation) using the methods this PR provides.

Remember, this PR does not provide a roll on-axis method, that is not its purpose, this just behaves like when attaching an entity to an entity and then setting its rotation. Also, you could test and see that this same problem is in the mentioned entity rotation method.

Once I have the time, I will work on another PR that will allow more intuitive rotations. Please, look back on the discussions we have had on this matter in this PR and in #7331.

However, if you do have any ideas for something that could work better (although I think this fulfills the purpose it was made for) then please share those ideas.

EDIT: Here's a good comment in my last issue that is what decided this PR: link

[GreenXenith]

Ah, thank you for the explanation :)

[Desour]

Is the rotation actually saved in the world? doc/world_format.txt only mentions yaw:

minetest/doc/world_format.txt

Line 523 in 1d3f5af

s32 yaw * 1000

[SmallJoker]

@DS-Minetest

minetest/src/content_sao.cpp

Lines 614 to 620 in 1d3f5af

	writeF1000(os, m_rotation.Y);
	// version2. Increase this variable for new values
	writeU8(os, 1); // PROTOCOL_VERSION >= 37
	writeF1000(os, m_rotation.X);
	writeF1000(os, m_rotation.Z);

F1000 calls are basically just floats converted to integer, multiplied by 1000. Hence precision errors in some places and weird errors due floats being too large.

minetest/src/util/serialize.h

Lines 305 to 310 in 1d3f5af

	inline void writeF1000(u8 *data, f32 i)
	{
	assert(i >= F1000_MIN && i <= F1000_MAX);
	writeS32(data, i * FIXEDPOINT_FACTOR);
	}

EDIT: FYI: If you see writeF32 or -alike calls, then it's either temporary data and/or networking stuff.

[Desour]

Ah, thanks,
I'll make a PR to document this. Edit: #8864