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

Feature: Snow line height can be autodetermined upon world generation #8404

Closed

Conversation

@SamuXarick
Copy link
Contributor

@SamuXarick SamuXarick commented Dec 20, 2020

Adds an autofill snow line height option for World Generation GUI

First, it counts the land mass in number of tiles, excluding those that are at sea level.
Then, starting with a snow line height of 1, it runs multiple passes on the map and counts the number of tiles that are of that height.
While the number of tiles counted don't reach 50% of the land mass, it keeps adding +1 to the snow line height each pass.
Once the number of tiles surpass 50%, the snow line height is whichever value it stopped at.
The resulting value is checked against some industry restraints, like farms, which can't be placed on snow, and forests which can only be placed on snow. Should the result need adjusting, it is clamped accordingly.

Due to these industry constraints, the lowest value for the maximum height for generating Sub-Arctic terrain with TerraGenesis has been increased to 6, which affect the 'Very Flat' and, to a minor extent, the 'Flat' presets.

@SamuXarick SamuXarick force-pushed the auto-determine-snow-line-height branch from 4ae867b to 0b60e76 Dec 20, 2020
@glx22
Copy link
Contributor

@glx22 glx22 commented Dec 21, 2020

I like the idea, but the implementation looks like a ugly hack, and I fear it will be very slow on big maps.

@SamuXarick SamuXarick marked this pull request as draft Dec 21, 2020
@SamuXarick
Copy link
Contributor Author

@SamuXarick SamuXarick commented Dec 21, 2020

I like the idea, but the implementation looks like a ugly hack, and I fear it will be very slow on big maps.

I changed it so it doesn't run multiple loops on the map. It only needs to loop once now.

As for the ugly hack, I think you're referring to snow_line_height variable having a -1 / + 1 to their min/max and the various code trying to keep it working the way it's supposed. I thought about reverting all the changes related to snow_line_height, and instead, create a new setting, a gui setting that isn't stored in savegames, that enables or disables whether to determine the snow line height upon world generation.

I'm unsure what to do about the 'Snow line height' part in the new world gen window, though. Any ideas?

@SamuXarick SamuXarick force-pushed the auto-determine-snow-line-height branch from 16240fe to 66cecf3 Dec 21, 2020
@SamuXarick
Copy link
Contributor Author

@SamuXarick SamuXarick commented Dec 22, 2020

An update, these last commits, made it so that when changing the snow line height in the World Generation window, it disables the 'autofill' option, and uses the value entered or set via up/down arrows. To select 'autofill', edit the value using the Query window and click 'Default'.

It's probably not the way this should be handled, but at least, it's possible to chose it via World Gen window.

@SamuXarick SamuXarick force-pushed the auto-determine-snow-line-height branch from 66cecf3 to 4734d16 Dec 23, 2020
@SamuXarick
Copy link
Contributor Author

@SamuXarick SamuXarick commented Dec 23, 2020

Update on this last force push: reduced the number of commits to just 3. Some commits were adding things only to be reverted in another, which would create confusion for reviewers. Now it's saner for reviewing.

@SamuXarick SamuXarick force-pushed the auto-determine-snow-line-height branch from 4734d16 to bdb5953 Jan 8, 2021
@SamuXarick
Copy link
Contributor Author

@SamuXarick SamuXarick commented Jan 8, 2021

With 7463c46 in master, the purpose of determining the snow line height is rather... pointless now.

Example:
1 - That commit reads a snow_line_height value of 10, and generates hills above it so that they're covered in snow. Once that is done...
2 - ... that value of 10 is 'discarded' and a new value is computed by this PR. It comes up with 4.

The end result is a world that was created with one value in mind, and then becomes another. In this example, with more snow. Seems counterproductive. I guess this PR serves little purpose now.

SamuXarick and others added 3 commits Jan 8, 2021
First, it counts the land mass in number of tiles, excluding those that are at sea level.
Then, starting with a snow line height of 1, it runs multiple passes on the map and counts the number of tiles that are of that height.
While the number of tiles counted don't reach 50% of the land mass, it keeps adding +1 to the snow line height each pass.
Once the number of tiles surpass 50%, the snow line height is whichever value it stopped at.
The resulting value is checked against some industry restraints, like farms, which can't be placed on snow, and forests which can only be placed on snow. Should the result need adjusting, it is clamped accordingly.

Due to these industry constraints, the lowest value for the maximum height for generating Sub-Arctic terrain with TerraGenesis has been increased to 6, which affect the 'Very Flat' and, to a minor extent, the 'Flat' presets.
@SamuXarick SamuXarick force-pushed the auto-determine-snow-line-height branch from bdb5953 to 742fe30 Jan 8, 2021
@SamuXarick SamuXarick closed this Jan 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants