Add snow option to Change Weather module

[johnb432]

When merged this pull request will:

• Addresses #681
• Adds a simple "Rain/Snow" option to the weather module.
• QGVAR(applyWeather) event is kept backwards compatible.

Notes:

• These changes might require some edits to the rain strength slider text, in order to be more clear: At the moment, there is "Rain" to set the rain strength and "Precipitation" to set the type of precipitation, which makes me believe it could lead to some confusion. Suggestions:

• Previously: "Rain" -> Suggested: "Precipitation"

• Previously: "Precipitation" -> Suggested: "Precipitation Type"

• The rainParticles can be changed (color, drop speed, etc., see setRain, but I'm unsure what options should be added to the weather module. I didn't add any, as I felt the module might become too cluttered and maybe even complicated for some to use. If it is wanted, so that curators can change snow particle properties, making an entirely new module just for that might be necessary.

• I haven't done much in terms of optimisation (e.g. not comparing the current precipitation state with the new one), but I'm not sure it's worth it.

• I have tested it in SP. As it's relatively simple, I doubt there should be any issues in MP, but I haven't tested it, so I can't say for certain if it's MP ready.

[Kexanone]

Works fine in MP so far.

[mharis001]

I have not looked into it yet but would using BIS_fnc_setRain be better here? The wiki implies that it helps with rainParams synchronization in MP. I agree with changing the wording to "precipitation".

I am unsure if there is much value in making all rainParams options available via a module. Maybe just providing a small set of curated "precipitation types" (with values that work well together) would be more useful. Does not need to be a part of this PR of course and can be done later.

[Kexanone]

We gonna need to have a look on what BIS_fnc_setRain actually does. I tested changes with a second client and also with JIP and both worked.

[johnb432]

We gonna need to have a look on what BIS_fnc_setRain actually does. I tested changes with a second client and also with JIP and both worked.

Looking at the code of the function, it should be MP and JIP compatible. I have only tested it the latest commit (c4f7531) in SP though.

[Kexanone]

Maybe my previous comment was confusing. What I meant is that I tested your initial PR and it worked fine on dedicated with a second client and JIP as well.

On a different note, we first need to know what BIS_fnc_setRain actually does before using it. Biki says you should execute it on the server, but at the moment you are executing it everywhere and on JIPs as well.

[Kexanone]

Just checked out BIS_fnc_setRain. The function is meant to be executed on the server only and basically just remote executes setRain with JIP. Hence, I suggest we revert to the original approach with directly using setRain.

[johnb432]

Maybe my previous comment was confusing. What I meant is that I tested your initial PR and it worked fine on dedicated with a second client and JIP as well.

No, no, I understood what you meant the first time. My communication abilities are poor, so I assume I didn't express myself well enough: I have tested both options in SP only (by options I mean either CBA Events or BIS_fnc_setRain).

On a different note, we first need to know what BIS_fnc_setRain actually does before using it. Biki says you should execute it on the server, but at the moment you are executing it everywhere and on JIPs as well.

I had looked into BIS_fnc_setRain (as in looked its code) before I used it and I know you need to execute on the server to get MP and JIP compatibility. At the moment, BIS_fnc_setRain is only executed on the server as can be seen here.

Just checked out BIS_fnc_setRain. The function is meant to be executed on the server only and basically just remote executes setRain with JIP. Hence, I suggest we revert to the original approach with directly using setRain.

Ultimately, I don't care, as both options work (well, in theory, I still haven't tested the option with BIS_fnc_setRain). Maybe the option using BIS_fnc_setRain is a bit more elegant, as requires less code written in ZEN, but on the other hand, there will be something with the JIP ID "BIS_fnc_setRain" in the JIP queue, as it's never cleared, only overwritten (at least as far as I could tell). I guess it's down to personal taste.

[Kexanone]

I had looked into BIS_fnc_setRain (as in looked its code) before I used it and I know you need to execute on the server to get MP and JIP compatibility. At the moment, BIS_fnc_setRain is only executed on the server as can be seen here.

Nvm, I somehow missed that you put it in the isServer part. We can leave it as it is then.

[johnb432]

I have tested it with 2 clients, one acting as the server, and by the looks of it it seems to work (MP and JIP compatible).

On a different note: The particle arguments require some tweaking. For example: Wind doesn't seem to affect snow much, even if you set the windCoeff argument of the particles higher. I'm not sure what parameters would be best, so I appreciate any feedback/ideas.