Skip to content
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

[CR]Regional weather settings #17500

Merged
merged 4 commits into from Sep 14, 2016

Conversation

Projects
None yet
5 participants
@Coolthulhu
Copy link
Contributor

commented Jul 3, 2016

Per-regional-setting temperature, humidity, pressure and acidity.
As an example I changed the desert region settings to be very dry and slightly warmer than default ones.

The acid setting isn't used anywhere, but someone might want to turn it on and it was trivial to add.

[CR] because I moved randomness seed from weather generator to game and weather generator out of game and into regional settings (which in turn are an overmap field). And because some of the code looks rather unelegant because of that.

@Coolthulhu Coolthulhu force-pushed the cataclysmbnteam:weather-gen branch Jul 25, 2016

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Jul 25, 2016

Rebased

@Coolthulhu Coolthulhu force-pushed the cataclysmbnteam:weather-gen branch Aug 9, 2016

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Aug 9, 2016

Rebased

@mugling

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2016

Sorry I haven't given this much prior attention. I'm not completely certain what it does. Could you explain a little and I'll take a look?

@pisskop

This comment has been minimized.

Copy link
Contributor

commented Aug 17, 2016

. <--

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Aug 18, 2016

Could you explain a little and I'll take a look?

Moves the responsibility to provide weather data from game to overmap. This allows regional settings to change the weather settings, thus allowing (some parts of) weather to by set by jsons. Not much - just amplitudes - but it's still a necessary step towards json weather.

@mugling mugling self-assigned this Sep 11, 2016

@mugling

This comment has been minimized.

Copy link
Contributor

commented Sep 11, 2016

I'm struggling to rebase, in particular init.cpp:212

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2016

Rebased on my side just fine. Could be different mergetools or git settings. Will push if it compiles and works.

@Coolthulhu Coolthulhu force-pushed the cataclysmbnteam:weather-gen branch to 928dc84 Sep 11, 2016

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2016

OK, I think I got it working.
Desert worlds have rare rains, while default ones have common rains (as in master).

@mugling

This comment has been minimized.

Copy link
Contributor

commented Sep 11, 2016

How do I generate deserts?

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2016

When generating a world, there is a terrain type option defaulting to "default". It should allow switching it to desert_test (or test_desert).
Sometimes it does not allow selecting due to a bug. In this case a game must be loaded, then exited to menu (without closing the game), then a new world must be created.

@mugling

This comment has been minimized.

Copy link
Contributor

commented Sep 11, 2016

Still fails to compile at init.cpp:212

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2016

What error?
Jenkins doesn't have problems with it, so it's probably Clang being stricter than GCC.

constexpr double base_h = 66.0; // Average humidity
constexpr double base_p = 1015.0; // Average atmospheric pressure
// GCC doesn't like M_PI here for some reason
constexpr double PI = 3.141592653589793238463;

This comment has been minimized.

Copy link
@codemime

codemime Sep 11, 2016

Member

It's safely defined in game_constants.h.

double base_temperature = 6.5; // Average temperature of New England
double base_humidity = 66.0; // Average humidity
double base_pressure = 1015.0; // Average atmospheric pressure
double base_acid = 0.0;

This comment has been minimized.

Copy link
@codemime

codemime Sep 11, 2016

Member

Do these variables have to be public?

@mugling

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2016

src/init.cpp:212:5: error: no matching member function for call to 'add'
    add( "weather_settings", &weather_generator::load );
    ^~~
src/init.cpp:96:25: note: candidate function not viable: no known conversion from 'weather_generator (*)(JsonObject &)' to 'std::function<void (JsonObject &, const std::string &)>' for 2nd argument
void DynamicDataLoader::add( const std::string &type, std::function<void(JsonObject &, const std::string &)> f )
                        ^
src/init.cpp:104:25: note: candidate function not viable: no known conversion from 'weather_generator (*)(JsonObject &)' to 'std::function<void (JsonObject &)>' for 2nd argument
void DynamicDataLoader::add( const std::string &type, std::function<void(JsonObject&)> f )
@@ -208,6 +209,7 @@ void DynamicDataLoader::initialize()
add( "gate", &gates::load_gates );
add( "overlay_order", &load_overlay_ordering );
add( "mission_definition", []( JsonObject &jo ) { mission_type::load_mission_type( jo ); } );
add( "weather_settings", &weather_generator::load );

This comment has been minimized.

Copy link
@BevapDin

BevapDin Sep 13, 2016

Contributor

The static load function loads and returns the object, but it's not stored anywhere. And there is no object with "type": "weather_settings" in any JSON file, so this will (currently) not be triggered anyway. The code for the regional settings loads the generator explicitly, without using the DynamicDataLoader.

@mugling

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2016

So we can just drop the offending line entirely?

@mugling

This comment has been minimized.

Copy link
Contributor

commented Sep 14, 2016

Now compiles ok

@mugling mugling merged commit 86f3574 into CleverRaven:master Sep 14, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.