-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Weather Tweaks - new light drizzle category and fix wind #35759
Conversation
src/weather_gen.cpp
Outdated
@@ -225,7 +227,7 @@ weather_type weather_generator::get_weather_conditions( const w_point &w ) const | |||
int weather_generator::get_wind_direction( const season_type season, unsigned seed ) const | |||
{ | |||
unsigned dirseed = seed; | |||
cata_default_random_engine wind_dir_gen( dirseed ); | |||
static cata_default_random_engine wind_dir_gen( dirseed ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means the seed is essentially on any subsequent call. But this may get called with a new seed when a new game is started or loaded.
static cata_default_random_engine wind_dir_gen( dirseed ); | |
cata_default_random_engine wind_dir_gen( dirseed ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without that static, then it gets passed the same seed, and therefore wind never changes direction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you mean: each invocation (while playing the same game) uses the same seed, so the random engine is in the same state each time.
But my argument still stands: loading a second save will carry over the random engine created from within the first game and will thereby ignore the seed of the second game.
Btw. what's the point of copying the function parameter seed
into a local variable and using that one? Why not use the function parameter directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seeing as the initial wind PR that added that code, over a year ago, was one of my first PRs, and I was even more inexperienced than I am now, I can only guess as to the reasons behind not using the function parameter directly, and I think the answer is " I have no idea what I was doing" , so I will change that. I will also think of a solution that solves the random engine seed problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it suitable to use the time_since_epoch based seed for the random engine? this was originally the case when I first added wind direction, then I passed the game seed down to it in a subsequent PR, for reasons I cannot remember.
If you want trace rainfall, why not change Cataclysm to represent rainfall continuously instead of with discrete categories? |
That'd be something to aim for in the future sure, this is using the system we have, for the time being. |
reduce drench wetness remove snow forming form light drizzle fallthrough fix fallthrough fix use time since epoch for rng seed
Summary
SUMMARY: Bugfixes "Weather Tweaks - new light drizzle category and fix wind"
Purpose of change
Fixes #35535
Describe the solution
After #34515 Occurences of rain and drizzle were significantly reduced, correctly.
This however opened up the space for very very light precipitation, which was the type of precipitation that occurs most irl in that geographic region, but there was no applicable category for it, as rain and drizzle had in-game effects that would've been too much for that light precipitation.
Also, due to that PRs fix to air pressure values generation, the windspeed calculations which were previously tailored to fit the previous pressure calculation, then became unvariable and stagnant.
So what ive done is added a new weather type - LIGHT_DRIZZLE, this has barely any wet effects or rain accumulation ( for funnel purposes ) etc, but is more common than drizzle or rain, this closely matches expectations of weather , based on real-life, without skewing their actual in-game effects.
Also I discovered that at some point the rng for wind direction changing was changed to a non-static rng engine, so wind direction was not changing at all, ive fixed that.
Also I've boosted the variability of wind back up again, this is a brute force approach of just amplifying it a bit, I spent a long time tailoring the wind algorithm to the previous pressure calculation, now that pressure is correct and realistic, eventually the wind algorithm will need a similar amount of time given to it , to correlate it to real-world numbers, but in the meantime, this at least fixes it remaining at 5mph the entire year.
Describe alternatives you've considered
N/A
Testing
Used the debug menu to output 2 years of weather data, and pored over the data.
debug changed weather to light drizzle an dobsserved its in-game effects( morale, funnels ) etc, and considered it a mostly flavour change with minimal effects, that at least gives the perception of changeable weather and light rain.
Additional context
Screenshot of data summary over 2 years, before this change.
You can see how, rain+drizzle is very rare compared to the total non-precip hours, you can see theres hardly any wind variation.
After this change -
Rain and drizzle are mostly unchanged, but total hours of precipitation is increased, most of which comes from the new light drizzle category
and Wind is much more variable, whilst still retaining the average.