minetest_game icon indicating copy to clipboard operation
minetest_game copied to clipboard

Weather mod needs API to disable it

Open Zughy opened this issue 3 years ago • 10 comments

Quoting from https://github.com/minetest/minetest/issues/11915, talking about weather mod:

I had no idea such mod existed. I looked at the code and it's just all about cycling clouds, so "weather" is a bit of a stretch

Since the introduction of set_sky() etc. functions, having a mod that only cycles clouds feels useless in my opinion. There are no atmospheric agents implemented and I doubt any mod depends from something like this. Right now is only something that messes with the set_clouds() function, and it's not like it'll be expanded anyway (as MTG is in mainteinance mode only)

Zughy avatar Jan 01 '22 13:01 Zughy

:-1: on removing it, but it should be ensured that an API is provided for other mods to inhibit this.

sfan5 avatar Jan 01 '22 14:01 sfan5

I disagree, as it'd mean that every mod should take in consideration MTG (also, more work to do here). I would have agreed if MTG hadn't been the default game experience, but that's not the case. What I personally see is an unfinished mod that went deprecated by the sky PR, causing more troubles than anything.

Disclaimer: it's not a big problem for me, as I only need the sky change for my server, where there are just a couple mods from MTG (meaning that it's bothering me only when testing locally), but this can turn into a problem for people creating singleplayer mods

Zughy avatar Jan 01 '22 15:01 Zughy

Uh yes? Mods are largely game specific so they obviously have to take the defaults and APIs provided by the games it intends to support into account. In the same fashion you could complain that player_api claims model and animation management for its own or that map claims the minimap modes and so on.

I would have agreed if MTG hadn't been the default game experience, but that's not the case.

I don't see how that's relevant here or why it changes anything.

What I personally see is an unfinished mod that went deprecated by the sky PR

What makes you think the mod is deprecated? It changes the cloud characteristics depending on some parameters including the biome a player is in and it does that well.

sfan5 avatar Jan 01 '22 17:01 sfan5

True, thanks for your time sfan5. Closing

Zughy avatar Jan 01 '22 18:01 Zughy

Your need to disable this is valid though so I'll keep this issue open.

sfan5 avatar Jan 03 '22 11:01 sfan5

@paramat

Wuzzy2 avatar Feb 03 '22 05:02 Wuzzy2

@Zughy: Would you prefer disabling the weather mod entirely (globally, for all players) or rather a more fine-grained per-player control?

I suggest the following APIs:

  • weather.enabled: boolean indicating whether the weather mod is enabled
  • weather.get_enabled(): alternative way to obtain weather.enabled, see below
  • weather.set_enabled(enabled): sets the boolean, clears everything set by the weather mod

Using metatables the setter could even be removed and replaced with just variable access. Alternatively, if the variable is considered not clean (as it may be changed without notifying the weather mod, bypassing set_enabled), a getter may be used instead. To summarize, the options here are:

  • Full func style: Getter + setter (the way it's currently done by most engine APIs)
  • A little sugar: Variable + setter (somewhat inconsistent?)
  • All the sugar: Just variable (requires metatable)

For players, this would just be parameterized with another player param, possibly exposing a table of "enabled" bools.

appgurueu avatar Apr 04 '22 18:04 appgurueu

It needs to be per-player.

sfan5 avatar Apr 04 '22 21:04 sfan5

It needs to be per-player.

Then I suggest weather.get_enabled(player) (or weather.is_enabled(player)) as well as weather.set_enabled(player, enabled)

appgurueu avatar Apr 05 '22 14:04 appgurueu

I would like two throw in my two cents here as well. The bundled "weather" mod is pretty much the main reason why I stopped active development on my Regional Weather mod.

Obviously weather mods have a strong need to tie cloud density, etc. to their own active weather effects. Rain would look pretty stupid otherwise. The inclusion of the "weather" mod breaks compatibility with no easy way to resolve it. I keep telling people to deactivate the mod in the settings but they're very well hidden and many people just download stuff in-game without looking at a readme. I've seen others try to disable the setting programmatically but this depends on load order with no way to ensure that the own mod loads before the bundled "weather" mod.

Having some kind of API to disable it per-player would be nice to have but I very much prefer a global setting (or both). Chances are, if you need to disable this mod then you already have made some kind of replacement and will no longer need it at all. Having to iterate through the players joining just adds an unnecessary work overhead.

t-affeldt avatar Mar 10 '23 20:03 t-affeldt