Server enforcement for fog_distance (#13448) to block cheating

[lhofhansl]

Followup from #13448

• Goal of the PR
  Enforce fog_distance at the server to block cheating clients.
  Only allow reasonable values to set for fog_distance, i.e. the same limits we have now: 20 to 4000

To do

This PR is Ready for Review.

How to test

Set fog_distance to 0. Ensure it uses 20.
Set fog_distance to 1000000, make sure it all works.
Can't really test a cheating client without changing the code. If you want to hard code wanted_range in client and see how the server still won't send more data. At least make sure the server never sends less that what is expected.

[grorp]

I think the minimum is too high. A fog distance of 10 allows for really nice effects, but there could also be use cases for something like 2. The only value I really see no use for is 0.

I don't think we should define an artificial minimum here. Instead, we could just change this

Any value >= 0 sets the desired upper bound for the client's viewing_range and disables range_all.

into this

Any value > 0 sets the desired upper bound for the client's viewing_range and disables range_all.

and call it a day. (Not only in the docs, but also in the implementation, of course.)

[lhofhansl]

The client won't allow less than 20.

minetest/src/client/game.cpp

Line 2513 in 25ef8f3

if (range_new < 20) {

[grorp]

The client won't allow the user to set a value below 20, but via the current set_sky API, you can set values below 20 and they will be applied. E.g. this works:

minetest.register_on_joinplayer(function(player)
    player:set_sky({ fog = { fog_distance = 2 }})
end)

And I think there's no good reason to disallow it.

[lhofhansl]

I do not feel strongly. The main part was to avoid was allowing setting this to 0 - which would not render anything.

[lhofhansl]

I'd like to get some consensus. I can change this to only disallow 0 as valid value.

[Desour]

I don't see an issue with setting fog_distance even to 0. Why do we need a limit?

[lhofhansl]

At 0 you see nothing. Even the wield item is not shaded correctly.
It's not just black (or fog color) it looks like a bug.

Actually: Can you try and see what you think (without this PR)?

[grorp]

At 0 you see nothing. Even the wield item is not shaded correctly.
It's not just black (or fog color) it looks like a bug.

Some more testing reveals that for me, any fog_distance value below 1 results in only the fog color being visible, both with shaders and with the fixed pipeline. The value of fog_start doesn't seem to make any difference.

-- for Minetest game
minetest.register_on_joinplayer(function(player)
    minetest.after(0, function()
        player:set_sky({ fog = { fog_distance = <insert value here>, fog_start = 0.5 }, type = "plain", base_color = "#ff0000", clouds = false })
    end)
end)

All screenshots were taken from the same point of view. The first screenshot is always with shaders (enable_shaders = true) and the second with the fixed pipeline (enable_shaders = false). The differences are probably (partially) due to the fact that tone mapping is enabled (tone_mapping = true).

fog_distance = 10

fog_distance = 1

fog_distance = 0.999999

fog_distance = 0.1

fog_distance = 0

So, something seems to break with values below 1.

[Desour]

fog_distance is an s16, that's why it's rounded.

Not seeing anything bug fog is the intended behaviour for fog_distance=0, I'd say.
And the wieldhand issues are not that severe, imo.

[lhofhansl]

Ok... Make this simpler :)
I'll post an update soon.