Allow the server to control fog_distance and fog_start via the sky-api

[lhofhansl]

See #13421 ...

Edit: Actually this does not, yet, achieve was is asked in #13421 . For that we need to override the fog color, and probably also disable MT's scanning of surrounding brightness to see if something is black (resulting in black fog)
Edit II: Color is actually not that easy to do. Has to work with and without shaders, etc, I'd prefer that as a separate PR.

There is no separate API to set the fog color. That is already possible via the sky_color.
Disabling fog (F3 by default) is still possible, the range is still enforced, so I wasn't sure about this. Easy to add if we wanted that.

To do

This PR is Ready for Review.

How to test

Test with shaders and without.
Set the fog distance/start via the API:

core.register_on_joinplayer(function(player)
      player:set_sky({fog = {fog_distance = 400, fog_start = 0.9}})
end)

Try "unsetting" via a negative value. Messages on the client should make sense.
Set only one of the two, and make sure everything works as expected. (large fog_start with unchanged fog_distance enhances sunny days for example)

[lhofhansl]

BTW. I like the model that the server can force a value, and if the server game/mod does not care then that client gets to control it. Orbital tilt was the first to do that.

[Zughy]

Disabling fog (F3 by default) is still possible, the range is still enforced, so I wasn't sure about this. Easy to add if we wanted that

Yes, please. Right now people can cheat by disabling fog, so modders are forced to lower clouds and put players inside them to emulate the effect

[lhofhansl]

@Zughy Just be clear, viewing range is still limited when fog is disabled... (Unlike viewing range all). So you cannot cheat into seeing more when the server limits the fog distance (which is effectively the client's viewing_range)

[lhofhansl]

Question: In order to do what we want in #13421 we would have to override MT's logic to determine the sky color, specifically the logic that determines the average lighting around the player and choose a black background if there is no (or little) light.

Do we want that? It's not really about setting the color (you can already set the sky/fog color) but about optionally disabling the "darkness-check".

[Iniquitatis]

It's not really about setting the color (you can already set the sky/fog color) but about optionally disabling the "darkness-check".

Given that MT poses itself as an engine, giving as much as possible control of these not-so-technical things is a natural thing to do. Not to mention it's a little bit buggy right now and can trigger when player's avatar just stands under an overhang, and not every game needs caves and all that stuff.

Funnily enough, I thought about this thing just yesterday. And yes, calling minetest.settings:set to modify fog distance isn't cool.

[lhofhansl]

OK. Then there is the question about the API. Could one of the following:

• a forced fog color
• a bit to disable MT's darkness detection. As part of the sky setting(?)

[Desour]

I haven't looked up how the darkness check thing works, or how useful it is, but it should definitely be possible to turn it off, for dehardcoding reasons.

It kinda sounds like fog color is a somewhat complex topic. A separate PR may be better.

[lhofhansl]

This is ready...

[mecha-moonboy]

Nice! 🥇

[Desour]

Tried:
//lua local p=core.get_player_by_name("singleplayer"); p:set_sky({fog={fog_distance=10}})
and then:
//lua local p=core.get_player_by_name("singleplayer"); p:set_sky({fog={fog_distance=-1}})

But setting to -1 didn't work.

[Desour]

Why 0.99? Is 0.999 not ok? A limit to percent feels weird.

[lhofhansl]

That's what the client setting has (see settingtypes). It's can be 100 and more than 99 does not make sense.
See float fogShadingParameter = 1.0 / (1.0 - fogStart);

[Desour]

The 0.99 is only to prevent a division by 0, right? But 0.99 is pretty arbitrary, it could also be 0.999, for example. My point is that it's kinda ugly to expose this to the API. Can't we just take [0.0, 1.0] here, and hide the clamp as implementation detail in the client?

[lhofhansl]

Setting to -1 should work now. Did not realize that the client might keep its setting persistent. Should all work now.

[Desour]

Issue I found while testing: The view distance setting shouldn't be limited by the fog distance. It should rather be independent instead.
Example of current situation:

• View distance is set to 100.
• Server sets a lower fog distance (e.g. 19).
• User presses button to increase view distance, view distance setting gets limited to 19.
• Server sets a higher fog distance again (e.g. -1).
• View distance stays at 19.
  This is undesirable for two reasons:
  • (19 would under normal circumstances not be settable with the buttons. It feels buggy.)
  • The user should get instant feedback when the fog vanishes. They should not have to increase their view distance again.

[lhofhansl]

This is undesirable for two reasons:

(19 would under normal circumstances not be settable with the buttons. It feels buggy.)
The user should get instant feedback when the fog vanishes. They should not have to increase their view distance again.

Hmm... On the other hand setting a viewing_range on the client is also a performance setting. If the server sets it to 1000 that does not mean the client should immediately set it to 1000 if it was smaller before.

We would have to remember the last manual client setting...? Even then, what if the viewing_range was 200, then server sets it 100, then the user reduces that 50. Now the server unsets it, what should it revert to?

We can round to multiples of 10 of course.

[Desour]

I'd say just let the user change their view distance setting freely, and only clamp the used value. So the actual view distance is always the minimum of the setting and the server-set value.
(The user could still be informed about the limit, i.e. by a message like this: "View distance increased to 100 (but limited by server to 20).")

[lhofhansl]

@Desour That makes a lot of sense, I'll do that.

[lhofhansl]

@Desour How about this?

[Desour]

@Desour How about this?

Works.
Will rereview the other parts of the PR at a later point.

[lhofhansl]

Anything else needed here?

[Desour]

Anything else needed here?

Sorry, I forgot about this PR.

---

Still unresolved: #13448 (comment) and #13448 (comment)

[lhofhansl]

Review comments addressed, except for the two that I kept unresolved.
Update: All comments addressed except request for adding a NaN note, which would set a bad precedent I fear/

[Desour]

Fine otherwise.

[Desour]

Just noticed a thing: It would be good if the server also used the fog distance as a limit to the block and object send distance, to avoid cheating with modified or old clients. Can be done in another PR though.

[lhofhansl]

Just noticed a thing: It would be good if the server also used the fog distance as a limit to the block and object send distance, to avoid cheating with modified or old clients. Can be done in another PR though.

The server relies on what client requests and its own max setting max_block_send_distance.
That would need to some discussion.