Add physics overrides for walk speed and Fast Mode

[Wuzzy2]

Follow-up PR for #11465 based on a suggestion by @grorp.

This PR adds the following new fields to set_physics_override:

• speed_walk: Normal walking speed
• speed_fast: Speed in Fast Mode
• acceleration_fast: Acceleration in Fast Mode

speed_fast and acceleration_fast modify the movement_speed_fast and movement_acceleration_fast settings by simple multiplication. Like most physics modifiers, they default to 1.

speed_walk will multiply movement_speed_walk and only affects the walking speed, i.e. not crouching, climbing or Fast Mode.

The player physics are not changed, this PR merely exposes some player physics values to be modifiable by Lua.

Additionally, the Client mod documentation is updated for the new physics overrides (including those added by #11465).

Also, I fixed a small bug in the anticheat because it underestimated the possible max. speed.

With this PR, every player physics value (movement_ settings) will be overridable by Lua. The walk speed and Fast Mode settings are the missing ones.

How to test

Please test this extensively before merging!

I recommend you to use luacmd and orienteering mods. Get yourself a speedometer with /giveme orienteering:speedometer and put it in the hotbar. This displays your speed at the top which is very useful for testing.

Now, in DevTest, call me:set_physics_override(table_of_physics_you_want_to_test). Test the normal walk speed and Fast Mode. Also test sneaking and climbing if you want to, just to make sure those still work as expected.
Then check if it does what you expect to / what the documentation promises.

Note that the player physics must be completely unchanged if you have not called this function so far. It is also very important this PR is 100% backwards-compatible.

I made some basic tests playing around with various physics overrides. But I did not test networking.

Notes about speed

Note that speed and the other speed_* settings are multiplied with each other. speed is supposed to affect all player speed, no matter the movement type. This is required to keep backwards-compability. All the other speed_* values like speed_fast will only affect the corresponding movement type.

Example 1: speed=2, speed_fast=6 must result in 6× speed in fast mode but only 2× speed for all other movement types.

Example 2: speed=0 must stop all player movement.

Example 3: speed_walk=0 must only stop walking speed but Fast Mode, sneaking and climbing must be unaffected.

[grorp]

movement_speed_fast * physics_override.speed_fast exists way too many times. Perhaps you could make it a local variable in LocalPlayer::applyControl?

EDIT: That's not related to this PR, but I wonder if movement_speed_walk should get its own speed_walk override. Modifying movement_speed_walk via the speed override has the disadvantage that it also affects all other speed and acceleration overrides.

With this PR, every player physics value (movement_ settings) will be overridable by Lua. The Fast Mode settings are the two missing ones.

So this isn't exactly true, movement_speed_walk is missing too.

[Wuzzy2]

Thanks for pointing out the mistakes, seems like I overlooked some things. This should be fixed now.

About speed_walk: I just totally forgot this one exists, too. I'm not sure how to proceed without breaking compability. Yeah, I generally agree that it would be best to expose this setting, too, so we actually have full Lua exposure. But the question is how to make it interact with the rest without breaking things.
Currently, speed affects normal speed as well as sneak and fast speed. This makes things tricky. I need to think.

The most important thing to me that any change must be 100% backwards-compatible with the previous stable version. So for now I would rather err on the side of caution but maybe a nice solution can be found. In any case, this discussion should be resolved BEFORE the next release.

[grorp]

About speed_walk: I just totally forgot this one exists, too. I'm not sure how to proceed without breaking compability. Yeah, I generally agree that it would be best to expose this setting, too, so we actually have full Lua exposure. But the question is how to make it interact with the rest without breaking things. Currently, speed affects normal speed as well as sneak and fast speed. This makes things tricky. I need to think.

Adding a speed_walk override without breaking backwards compatibility should be quite easy. One can simply do it the same way you did with speed_fast: Still multiply movement_speed_walk by speed, but additionally multiply it by speed_walk as well.

Something like this should work:

diff --git a/src/client/localplayer.cpp b/src/client/localplayer.cpp
index 0dd70a884..658d8d2ae 100644
--- a/src/client/localplayer.cpp
+++ b/src/client/localplayer.cpp
@@ -536,6 +536,7 @@ void LocalPlayer::applyControl(float dtime, Environment *env)
 	// Whether superspeed mode is used or not
 	bool superspeed = false;
 
+	f32 speed_walk = movement_speed_walk * physics_override.speed_walk;
 	f32 speed_fast = movement_speed_fast * physics_override.speed_fast;
 
 	if (always_fly_fast && free_move && fast_move)
@@ -554,9 +555,9 @@ void LocalPlayer::applyControl(float dtime, Environment *env)
 				if (fast_move)
 					speedV.Y = -speed_fast;
 				else
-					speedV.Y = -movement_speed_walk;
+					speedV.Y = -speed_walk;
 			} else if ((in_liquid || in_liquid_stable) && !m_disable_descend) {
-				speedV.Y = -movement_speed_walk;
+				speedV.Y = -speed_walk;
 				swimming_vertical = true;
 			} else if (is_climbing && !m_disable_descend) {
 				speedV.Y = -movement_speed_climb * physics_override.speed_climb;
@@ -586,12 +587,12 @@ void LocalPlayer::applyControl(float dtime, Environment *env)
 				if (fast_move && (control.aux1 || always_fly_fast))
 					speedV.Y = -speed_fast;
 				else
-					speedV.Y = -movement_speed_walk;
+					speedV.Y = -speed_walk;
 			} else if ((in_liquid || in_liquid_stable) && !m_disable_descend) {
 				if (fast_climb)
 					speedV.Y = -speed_fast;
 				else
-					speedV.Y = -movement_speed_walk;
+					speedV.Y = -speed_walk;
 				swimming_vertical = true;
 			} else if (is_climbing && !m_disable_descend) {
 				if (fast_climb)
@@ -619,12 +620,12 @@ void LocalPlayer::applyControl(float dtime, Environment *env)
 					if (fast_move)
 						speedV.Y = speed_fast;
 					else
-						speedV.Y = movement_speed_walk;
+						speedV.Y = speed_walk;
 				} else {
 					if (fast_move && control.aux1)
 						speedV.Y = speed_fast;
 					else
-						speedV.Y = movement_speed_walk;
+						speedV.Y = speed_walk;
 				}
 			}
 		} else if (m_can_jump) {
@@ -643,7 +644,7 @@ void LocalPlayer::applyControl(float dtime, Environment *env)
 			if (fast_climb)
 				speedV.Y = speed_fast;
 			else
-				speedV.Y = movement_speed_walk;
+				speedV.Y = speed_walk;
 			swimming_vertical = true;
 		} else if (is_climbing && !m_disable_jump && !control.sneak) {
 			if (fast_climb)
@@ -660,7 +661,7 @@ void LocalPlayer::applyControl(float dtime, Environment *env)
 	else if (control.sneak && !free_move && !in_liquid && !in_liquid_stable)
 		speedH = speedH.normalize() * movement_speed_crouch * physics_override.speed_crouch;
 	else
-		speedH = speedH.normalize() * movement_speed_walk;
+		speedH = speedH.normalize() * speed_walk;
 
 	speedH *= control.movement_speed; /* Apply analog input */
 

(I wanted to put this into a <details><pre>, but that seems to mess up code formatting.)

The most important thing to me that any change must be 100% backwards-compatible with the previous stable version. So for now I would rather err on the side of caution but maybe a nice solution can be found. In any case, this discussion should be resolved BEFORE the next release.

I don't see any backwards compatibility problems here.

[grorp]

Looks good to me now, but I suggest also adding speed_walk in this PR. (This is not a requirement.)

[Wuzzy2]

Okay, you convinced me. I am now willing to add support for speed_walk later, but I also want to test this and properly think this trough to make sure nothing weird happens.

[Wuzzy2]

Walking speed added. I also added some remarks to the documentation and updated the 1st post.
I also spotted and fixed a minor bug in the anticheat that underestimated the max. legal speed.

[grorp]

Walking speed added. I also added some remarks to the documentation and updated the 1st post.
I also spotted and fixed a minor bug in the anticheat that underestimated the max. legal speed.

Thank you.

[Wuzzy2]

Oh no, it looks like we just entered feature freeze so this change is apparently not going to make it into 5.8. :-(

[Bituvo]

I think that renaming the fields like speed_fast -> fast_speed would make them more readable.

[grorp]

Hey @Wuzzy2, how's it going with this PR?

[Wuzzy2]

I didn't forget about this, but there was this game jam. XD I think I may pick this up again later.

[Wuzzy2]

I've resolved the latest @grorp comments and also did a rebase since the code has diverged again.

Since this PR won't make it into 5.8.0, does this need changes?

Not having fallbacks is asking for trouble.

[grorp]

Not having fallbacks is asking for trouble.

That's not what I meant.

https://github.com/minetest/minetest/blob/eb57ae6cabe0abce6f986b734b291da09a03762b/src/client/content_cao.cpp#L1797-L1821

What I meant is that you're resetting too many fields here in some cases. If you connect a 5.9.0-dev client (with this PR) to a 5.8.0 server that sends a speed_climb override, the is.eof() condition triggers and the speed_climb override is ignored.

[Wuzzy2]

Sorry. The reason why I stopped updating this is because I didn't know what I was supposed to do. *shrug*

[grorp]

Alright, just fixed it myself. *shrug*

What I meant is that you're resetting too many fields here in some cases. If you connect a 5.9.0-dev client (with this PR) to a 5.8.0 server that sends a speed_climb override, the is.eof() condition triggers and the speed_climb override is ignored.
#13815 (comment)

Fixed by grorp@661e3dc

Adopted in #14475.