Implement API to disable weather

[t-affeldt]

I implemented an API for the weather mod. This resolves #2913 .

This is not a new feature per se, it is designed to enable compatibility between MTG and weather mods that add other effects to the sky or lighting. See the respective issue for the discussion.

This also reduces the amount of flashing due to clouds updating. When a new player joins, the sky will no longer be immediately recalculated for everyone but only for the new player.

The new API methods are exposed through a global weather object and are as follows:

-- returns bool for whether weather is active
weather.is_enabled()

-- enables / disables weather globally.
-- Has no effect if enable_weather == false in minetest.conf
-- If disabled, completely stops update cycle and reset every player to default values
weather.set_enabled(enable)

-- Most important part of this PR
-- Allows for overriding this function
-- Passes player object and tables that are about to be updated
-- Return values determine actual update to player object
-- Returning nil values for a table prevents it from getting updated
weather.on_update = function(player, { clouds, lighting })

Edit: Updated this post in line with redesign. Previous iteration suggested a set_enable function per-player instead of the on_update.

Tested on Minetest 5.6.1 and 5.7.0 RC1

[appgurueu]

Should work in its current state (only concern is that if weather for a player is enabled later on, the default shadow intensity may not be set for mgv6 and singlenode). However, I have an overarching reliability concern: You consistently use player refs as indices. This works fine in practice since player refs currently don't change during the session; in theory there is no such API guarantee though, which is why I like to err on the side of caution, using player names as keys instead.

[t-affeldt]

I have now updated the code to use a proper set and to store players by name. Shadows should now get correctly updated in v6 and singlenode.

[appgurueu]

Seems fine to me. Luacheck will probably complain because you're shadowing playerset though (even though IMO this shadowing is valid)...

[t-affeldt]

Seems fine to me. Luacheck will probably complain because you're shadowing playerset though (even though IMO this shadowing is valid)...

Yes, I just got the email about the failed check. I thought shadowing is valid here because it's semantically the same. Just renamed it.

[sfan5]

The need to have weather.enable_on_join in addition to the API functions is pretty inconvenient.
What about defining a public weather.update = function(player) that mods can override?
This gives:

• clean way for mods to disable weather, no need for synchronization on join
• multiple mods can use this mechanism without interference (mostly)

It does not provide ability to "reset" the weather changes but that's a lost cause anyway since values set by mods cannot stack.

[appgurueu]

I forgot to raise that you should document this in game_api.txt as sfan5 has pointed out on IRC.

[t-affeldt]

What about defining a public weather.update = function(player) that mods can override? This gives:

This would require a complete rewrite, so we should probably have this discussion before I do that.

The current implementation offers, in my opinion, the easiest way to toggle weather effects per player. The enable_on_join is sadly necessary to prevent new players from ever getting any effects in the first place.

In an ideal scenario, mod authors would not only be able to toggle the weather but also adjust it slightly without a rewrite.
In order to do so, they would need to be handed the cloud and lighting tables before they get written to the player object. However, if disabling the mod is the only purpose then generating those tables would be unnecessary overhead.

So, an alternative suggestion would be the following:

weather.enabled = true
weather.on_update = function(player, { clouds, lighting })

The weather.enabled would simply toggle the weather globally.

The weather.on_update would be called after the tables have been generated and they would be passed as an argument. You could then override individual values and return the modified tables before they get written to the player object. Returning nil would instead prevent a table from getting written at all. If weather.enabled == false then this function would never be called.

Edit: It is possible that one mod wants to disable it completely and another one only temporarily (and then re-enable it). For those cases, I would note in the API doc to override the on_update method for temporary effects rather than the enabled. Alternatively, the on_update could be split into a before_update (to determine whether or not to generate the effects) and an after_update. I feel like a simple boolean is easier to understand, though.

[t-affeldt]

I have redesigned the API again. This iteration should be as flexible as possible.
Overriding the on_update method can be used to disable functionality, filter by players, add additional effects, apply modifiers, etc. with not much of a hassle.

The set_enabled function can be used to completely disable the update cycle, improving performance when weather is unused.

See updated original post for details. I have also added the respective explanations to the game_api.txt

[appgurueu]

Should be fine code-wise now. Note that

local a, b = 42, 101
local t = { a, b }

is not equivalent to

let a = 42, b = 101;
let t = { a, b };

in JS this will create an object {a: 42, b: 101}, whereas in Lua it will simply create a "list" {[1] = 42, [2] = 101}. I've fixed this.

[sfan5]

I can't think of a way how more than one mod could correctly use set_enabled without stepping on the toes of the other mod.

An alternative would be to have just is_enabled and tell modders to override it.

[sfan5]

this will constantly reset the lighting parameters for everyone.
I suppose it's not bad because that's what this mod does anyway (on supported mapgens), but is this intended?
Asking because it didn't do so before.