More height levels (in trunk since r27010)

[ChillCore]

In tgp, I only adjusted scaling so that no too high maps can be generated. The drawback of this is that the generated maps for some of the settings simply don´t have the expected characteristics. Example for what I mean here:

Generate a 1024x1024 map, landscape type Alpinist, max. allowed heightlevel 30. You can with no doubt generate quite mountainious maps using 30 heightlevels. However, in this case, the algorithm will generate something that can at most be called hilly - simply because all mountains are scaled by some factor compared to the originally intended height of an Alpinist map on a 1024x1024 map.

So, I think the question is how could we change this, and how important is changing this? On the one hand, by setting the max. allowed heightlevel high enough, you can get senseful results for any combination of map size and landscape type, on the other hand by setting it too low you can get unexpected results.

I did not yet have time to test your new code, sorry about that.
So from memory ...


- I think the easiest way is to adjust "static const int _smoothing_parameters[7][3]" to take into account the limited height. However as this is already a nested array this might not be too easy ... you are better than me with programming so you be the judge on feasability.


- You could also try to revert this change I have done a long time ago (4 times)

Code: Select all

-			max_x = max((smallest_size * smallest_size / 16) + max_x, (smallest_size * smallest_size / 16) + margin - max_x);
+			max_x = max((smallest_size * smallest_size / 64) + max_x, (smallest_size * smallest_size / 64) + margin - max_x);

I did this because the resulting maps looked nicer but otherwise there was no reason for this change and the code will not break if you do revert this.


- Else there might not be no other option than to adjust ... "static const amplitude_t _amplitudes_by_smoothness_and_frequency[7][TGP_FREQUENCY_MAX + 1] = {"
Note that it took me a LOT of trial and error to get the terrain too look as it does/did ... Also if you change this then you may end up with a similar problem that you are having now. (looking good on low maps but way too steep terrain on high maps)



PS:
I understand that you want the patch in trunk ASAP but please do not be discouraged by Devs (additional) requests.
You are very close to your goal (else they would not take the time to review and comment on the patch) and it would really be a shame if you gave up now.
You have to understand that once the code gets in trunk it is Dev's task to maintain the code.

[ic111]

Note that it took me a LOT of trial and error to get the terrain too look as it does/did ... Also if you change this then you may end up with a similar problem that you are having now. (looking good on low maps but way too steep terrain on high maps)

I know this. And this is the reason for me not really wanting to touch this again.

Thinking about it again, I think the following two solutions are feasible:

(a) leave it as is - people will learn how to generate senseful maps
(b) Remove the max. height selection from the map generation dialog, and let the map generator choose the max. height based on the max. generated height.

I prefer (b).

@devs: Is (b) ok for you?

I personally will probably not have the time for such a project, but I think that the tgp map generator has its limits regarding creation of realistic landscape anyway. For example, in reality rivers usually flow in valleys, but... tgp doesn´t actually generate valleys. It also has no possibility for generating considerably different landscape kinds at different parts of the map - e.g. flat land in Northern Germany, mountains in the Alps as an example in reality. And such limits are intrinsic to the way it works - so maybe we just need someone implementing a new map generator... A map generator that simulates processes like rivers forming a valley by erosion, to give just one example.

PS:
I understand that you want the patch in trunk ASAP

No, mere time for me is not that relevant (at least as long as devs don´t commit too much things interfering with the patch).

but please do not be discouraged by Devs (additional) requests.

For me this is mainly a question of communication.

[frosch]

Since the topic was only about snow till now...
How does the queue handle the "rainforest" zone in tropic? In trunk rainforrest appears above heightlevel 4.

On my list, Z scaling is needed for:

• Provide NewGRFs with an idea what is "high" and "low". For snow, and other climatic stuff. E.g. "iron ore mines" only in "high places". "Daylight mining" only in "low" areas....
• Define Rainforest.
• Keeping behaviour when loading old savegames (e.g. snow and NewGRFs)
• To keep stuff consistent, the maximum Z level needs to be a hard limit, which also limits terraforming from companies.

Esp. the "savegame compatibility" issue requires that the max-Z-level is stored in the savegame. For new maps it could be decided depending on the map size (16 height levels on 64x64, 256 on 2kx2k; or similar). But actually I would leave this decision to the player. E.g. the mapgen window could allow selecting the maximum height level, while defaulting to "automatic depending on mapsize" or whatever.

[Eddi]

imho (a) is the only future proof way.

[ic111]

Since the topic was only about snow till now...
How does the queue handle the "rainforest" zone in tropic? In trunk rainforrest appears above heightlevel 4.

Ah, thanks for pointing me on that. I wasn´t aware of that as I never play with that climate zone... I just generated a test map and it looks like the behaviour didn´t change at all.

Do you know, where this behaviour is controlled?

[*] Provide NewGRFs with an idea what is "high" and "low". For snow, and other climatic stuff. E.g. "iron ore mines" only in "high places". "Daylight mining" only in "low" areas....

Hm, but we now have the max. heightlevel setting. One can simply return that max. heightlevel setting to the Newgrf. And the Newgrf can do its decisions based on that setting.

Or do I miss something?

[*] Define Rainforest.
[*] Keeping behaviour when loading old savegames (e.g. snow and NewGRFs)

Rainforest: What about keeping the "rainforest on heightlevel >= 4" rule unchanged in game, and letting NewGrfs adjust this based on a max. heightlevel setting returned as described above?
Snow:As the NewGrf code wasn´t adjusted by me, I can´t say much about how to achive that currently.

What do you mean with "e.g. NewGRFs"?

[*] To keep stuff consistent, the maximum Z level needs to be a hard limit, which also limits terraforming from companies.

I didn´t test it, but I hope that the companies use the same code for executing terraforming as the human players, so my hope would be that that already works. Can someone tell us more about this?

Esp. the "savegame compatibility" issue requires that the max-Z-level is stored in the savegame.

I implemented it as an advanced setting. I.e. it should already be stored in the savegame, shouldn´t it?

For new maps it could be decided depending on the map size (16 height levels on 64x64, 256 on 2kx2k; or similar). But actually I would leave this decision to the player. E.g. the mapgen window could allow selecting the maximum height level, while defaulting to "automatic depending on mapsize" or whatever.

Does this mean you vote for (a) in the above post from me? Setting the maximum height level from the map generation window IMHO implys that (b) is not an option.
(if you know a feasible way to deal with this in terms of tgp, feel free to give me a hint for an option (c)).

[ChillCore]

Do you know, where this behaviour is controlled?

landscape.cpp
+- line 890

if (t != INVALID_TILE && (TileHeight(t) >= 4 || IsTileType(t, MP_WATER))) break;


Imho with increased heights the amount of desert needs to be increased too. Else it will look silly.

Please find attached a desert amount patch written for plain trunk. (that is also included in you_know_where but there it is adjusted to mhl)
Not that you have to use it but you may get some inspiration.



Also I stick by my point that in regards of tgp:

- I think the easiest way is to adjust "static const int _smoothing_parameters[7][3]" to take into account the limited height.

Code: Select all

+/**
+ * Array holding some extra smoothing parameters that are applied after _amplitudes_and_frequency.
+ * They represent in order of appearance :
+ * { smoothing radius, power of smoothing (1= smoothest, 10 = almost unsmoothed), min. smoothing step size }
+ *
+ * @see Smooth()
+ */
+static const int _smoothing_parameters[7][3] = {
+	{3, 3, 8}, ///< Very  smooth
+	{4, 3, 8}, ///< Smooth
+	{2, 7, 8}, ///< Rough
+	{2, 4, 8}, ///< Very rough
+	{2, 5, 8}, ///< Cereally rough - bonus -
+	{2, 6, 8}, ///< CommutorZ - bonus -
+	{2, 5, 4}  ///< Go with the flow - bonus -
+};

Current parameters apply a lot of smoothing and can be made a lot rougher for lower maps without touching the noise parameters.

If only I knew how to correctly write the array in case of:
static const int _smoothing_parameters[7][3][max_height]
Where max height can be either 16, 32, 64 etc.
or more flexible from level "0 to 16", "17 to 32", " 33 to 48", etc. if you do not want to be pinned on fixed values.



I still need to read the rest of the recent posts in order to reply ...

Edit: forgot to attach patch.

[Honza_]

Seriously guys, what is this topic about?

[ChillCore]

Duh ... what does the topic title say.


Changing it has a lot of implications that need to be solved too.
Unlike some other patches that are written ... this one aims for trunk.


Sorry for being rude but please add constructive comments or stay out.

[ic111]

Imho with increased heights the amount of desert needs to be increased too. Else it will look silly.

Please find attached a desert amount patch written for plain trunk. (that is also included in you_know_where but there it is adjusted to mhl)
Not that you have to use it but you may get some inspiration.

If one decides for controlling this from the new game dialog, I think it´s the way to solve it. Thanks for the patch!

But one thing is clear: If desert yes/no purely depends on the heightlevel, on mountainious maps one will get rings of desert around the mountains.

Opinions? If there are no objections I will probably make it a mhl patch today in the evening.

Current parameters apply a lot of smoothing and can be made a lot rougher for lower maps without touching the noise parameters.

Hm, I so far thougth that smoothing works on the small scale, i.e. determines wether you have many small hills and basins in the scale of 1 or 2 heightlevels. But I will play a bit with the settings. If I succeed it´s good, if not I think I will stick with solution (a) (leave it as is) in the sense of the post yesterday.

[ChillCore]

Imho with increased heights the amount of desert needs to be increased too. Else it will look silly.

Please find attached a desert amount patch written for plain trunk. (that is also included in you_know_where but there it is adjusted to mhl)
Not that you have to use it but you may get some inspiration.

If one decides for controlling this from the new game dialog, I think it´s the way to solve it. Thanks for the patch!

My pleasure, as you can see from the revision I have it already for quite some time.
I forgot about it untill frosh mentioned deserts.

As the patch is, it is configurable for the player with almost no limits, one can have deserts all the way up. I am not sure if that is a good idea for trunk.
It could also be rewritten as a oneliner without the gui stuff.
Read: Set the upper "desertline" automatically based on max height of the map if that is prefered. ¿max_height / 4?

Also it does not change the lower desertline, that still starts ¿two or four? tiles from the shore.
I do not think this needs to be changed

But one thing is clear: If desert yes/no purely depends on the heightlevel, on mountainious maps one will get rings of desert around the mountains.

When you set it close to the tops then indeed you will get that effect.

Current parameters apply a lot of smoothing and can be made a lot rougher for lower maps without touching the noise parameters.

Hm, I so far thougth that smoothing works on the small scale, i.e. determines wether you have many small hills and basins in the scale of 1 or 2 heightlevels. But I will play a bit with the settings.

It works on a smaller scale yes.

The noise parameter set the rough shape of the map and it was hard to get something playable for all mapsizes / 8.
So CommanderZ gave me the extra smoothing code to play with and that helped a lot.
Also note that most of tgp code was written by him and I toyed with the numbers if you recall.
I documented some of the code but I do not fully understand how Smoothing() works.

If you set the radius smaller, the power of smoothing to almost unsmoothed and the step in tiles larger ...
The overal look of the map will still look the same only rougher which is what we want for smaller maps.

While playing with the numbers ... I suggest disabling generating a new seed between new games or test in the scenario editor, where the seed does not change all the time, to more easily see what effect changing these numbers has.

In genworld_gui.cpp:

Code: Select all

	/* Always give a new seed if not editor */
//	if (_game_mode != GM_EDITOR) _settings_newgame.game_creation.generation_seed = InteractiveRandom();

This will allow you to keep the same seed between "normal" game sessions.
if you want a new one just press the random button or Ctrl+Newgame from the main menu

If I succeed it´s good, if not I think I will stick with solution (a) (leave it as is) in the sense of the post yesterday.

Fair enough IMHO.

[ic111]

Version 20 of the patch. The only change: A new setting for the maximal desert height.

Taking Froschs list as a checklist, what issues are open?

Provide NewGRFs with an idea what is "high" and "low". For snow, and other climatic stuff. E.g. "iron ore mines" only in "high places". "Daylight mining" only in "low" areas....

I´m unsure what this means for me.

Define Rainforest.

Done.

Keeping behaviour when loading old savegames (e.g. snow and NewGRFs)

The two new variables, desert_height and max_heightlevel: Is this the purpose of the def = ... setting in gameopt_setting.ini / setting.ini? I.e. can I set that value to desert_height = 4 and max_heightlevel = 16 for older savegames, without affecting future games? Or does this setting (also) have another purpose?

NewGRFs: I have to admit that I am not the expert for them. Is there anything left to be done regarding this? If yes, does someone familiar feel responsible for it, or tells me in more detail what issues are left here?

To keep stuff consistent, the maximum Z level needs to be a hard limit, which also limits terraforming from companies.

Done for human players. Can anyone tell me by codereview etc. wether this is also done for companies?

Esp. the "savegame compatibility" issue requires that the max-Z-level is stored in the savegame.

It is a setting in the construction advanced settings, thus I think this is done.

If in this list is missing anything, feel free to add it.

[planetmaker]

Just a partial reply before zzzZZZzzz time:

Keeping behaviour when loading old savegames (e.g. snow and NewGRFs)

The two new variables, desert_height and max_heightlevel: Is this the purpose of the def = ... setting in gameopt_setting.ini / setting.ini? I.e. can I set that value to desert_height = 4 and max_heightlevel = 16 for older savegames, without affecting future games? Or does this setting (also) have another purpose?

That defines the range of allowed values for the newly introduced variables and has no influence on how old savegames are converted. Old savegames need conversion in src/saveload/afterload.cpp and friends, i.e. the old default values have to be assigned to the new variables there explicitly. And yes, the variables are saved in the savegame when they're added to settings.ini (like you said you did, I didn't yet read the patch(es))

[ChillCore]

Hello ic111,
I had a look at patch 155 and 240.

Here are some of my thoughts,
Disclaimer:
- The following are my personal opinions and devs might disagree (I have not asked).
- ¿I am not sure and would like some more comment from someone else?
- I do not differenciate between scenarios and normal games here.



Both 155 and 240 are stored in the savegame but not in the config file.
-> should be stored in both places (like snowline).

static const uint MIN_DESERT_HEIGHT = 1; ///< Minimum allowed desert height.
-> Should be 4.

static const uint MAX_DESERT_HEIGHT = 255; ///< Maximum allowed desert height.
-> Too much: ¿192? should be more then plenty we still want rainforest or the game becomes unplayable (missing industries).

Gui clamping of selecting desertline.
->The actual clamping of the desert line height in the gui, when creating a new game, could be based on the selected max height?
¿ "upper clamp = _settings_game.construction.max_heightlevel / 4 * 3" ? instead of MAX_DESERT_HEIGHT. see previous comment.

Is this the purpose of the def = ... setting in gameopt_setting.ini / setting.ini?

The purpose is to set reasonable values for when the game is started for the first time after the new setting was introduced.
You can easily see most of the default values when you look at the config file. -> After exiting the game for the first time when you started with a clean one and nothing was changed ingame. (IIRC some values are not stored there.)

I.e. can I set that value to desert_height = 4 and max_heightlevel = 16 for older savegames, without affecting future games?

yes those defaults are fine, but the latter should be 15 imho -> mhl off, no will not have an impact on future games.

As planetmaker said, handling compatibility with older savegames is handled in afterload cpp and should still be taking care of still. (unless you did that somewhere in one of the other patches.)
The old games do not have the new settings in them (unlike heightlevels) so you need to give them the correct values to avoid problems,

Code: Select all

	if (IsSavegameVersionBefore(MORE_HEIGHTLEVEL_SAVEGAME_VERSION)) {
		_settings_newgame.game_creation.desert_height = 4;
	}

, somewhere near the end of the file.
¿For the max_heightlevel you give them 15.?

[ChillCore]

ic111,
As I mentioned in my previous post I did not ask Devs opinion on my suggestions above.
So I did that after posting.

12:40:54 <frosch123> i would leave everything to a single map-gen setting "max height"

In regards of desert line:

12:46:38 <frosch123> just fix it at max-height / 4
12:46:42 <frosch123> everything else is a different patch
12:47:12 <frosch123> and imo, a desert height makes no sense. it should be "few" or "many" etc, and not only related to height

In regards of max height:

12:41:24 <frosch123> also, i think the max-height must be fixed at map generation, any may not be altered in game

[Eddi]

12:47:12 <frosch123> and imo, a desert height makes no sense. it should be "few" or "many" etc, and not only related to height

that could be done similar to determining the water level, i.e. by using the histogram of the heightlevels, to choose a level with the closest percentage. but really, that's an entirely different patch.

[ic111]

Reply for the previous posts:

ChillCore: As legal values for settings, I generally used the range that makes technically sense and that one might want to use. Specifically, if there would be a desert height, anything from <all rainforest> to <all desert> would be senseful values IMHO. Why not play a map with all rainforest? (and missing industries are IMHO no argument, as industries can be altered by NewGRFs). Thus, I used range 1..255.

ChillCore: You are right, default max. heightlevel should be 15, my mistake.

Regarding desert height, I am open for any solution that doesn´t make things significantly more complicate than the 240 patch. As I have written previuosly, in fact a much more sophisticated landscape generator would be desirable, but is clearly out of scope for this patch. So, it´s up to you (devs) wether you use the 240 patch, or do simply nothing about desert height, leaving the hardcoded value of 4.

Both 155 and 240 are stored in the savegame but not in the config file.
-> should be stored in both places (like snowline)

So what did I miss technically?

Gui clamping of selecting desertline.
->The actual clamping of the desert line height in the gui, when creating a new game, could be based on the selected max height?
¿ "upper clamp = _settings_game.construction.max_heightlevel / 4 * 3" ? instead of MAX_DESERT_HEIGHT. see previous comment.

You mean, forbidding a desert height higher than max. heightlevel? Well, one might implement this, makes code a bit more complicate as then for any change of these two values, the respective other value has to be checked. On the other hand, both are maximal values, if you choose max. desert height 100 on a max. heightlevel 50 map, you simply won´t get any rainforest. So, it´s a defined result, and thus I decided on not implementing additional checks for this. But this can be changed if we want it to be changed.

12:41:24 <frosch123> also, i think the max-height must be fixed at map generation, any may not be altered in game

Can we agree on making this an option in the cheat window?

I.e. on the same level as changes to money, time, climate, etc.

I don´t care about where and how, but I think there should really be a way to change this. I, and I think many other players too, play this game in single player mode, without even AI or network players. Thus, I want to be able to form the game (of course within the technically possible limits) the way I like it. And this involves corrections of this kind if after starting a game, I realize that some aspect of the game is not yet in the state I want it.

[ChillCore]

Disclaimer:
*most* of this is still my personal opinion:
When I mention Devs ... I take into account the opinion of some of them but these are not their words, except for the quotes I made previously which was the view shared by more than 1 of them.

ChillCore: As legal values for settings, I generally used the range that makes technically sense and that one might want to use. Specifically, if there would be a desert height, anything from <all rainforest> to <all desert> would be senseful values IMHO. Why not play a map with all rainforest? (and missing industries are IMHO no argument, as industries can be altered by NewGRFs). Thus, I used range 1..255.

I understand your reasoning, but to avoid OpenTTD possibly behaving badly you should not:
- Go higher than MAX_TILE_HEIGHT - 2. (snowline in trunk does not go all the way down to zero neither.)
- MIN_DESERT_HEIGHT = 1 is a bad idea too IMHO, the lower line of the desert (as in trunk) does not start at a fixed height but at a fixed number of tiles from the shore ... this could very well be higher than 1 and then you will be defining the upper desert line below the lower line, which might cause problems. I have never experimented with this so I might be wrong. I'd stick with 4 as minimum (the value current trunk has -> proven safe value).

ChillCore: You are right, default max. heightlevel should be 15, my mistake.

My pleasure.

Regarding desert height, I am open for any solution that doesn´t make things significantly more complicate than the 240 patch. As I have written previuosly, in fact a much more sophisticated landscape generator would be desirable, but is clearly out of scope for this patch. So, it´s up to you (devs) wether you use the 240 patch, or do simply nothing about desert height, leaving the hardcoded value of 4.

Devs (not only frosh) have a very strong feeling towards not going for a gui and having it configurable by the player.
The gui and everything else can easily be done afterwards as a separate patch.
I understand this logic and I can agree with that.

They can agree on "_settings_game.construction.max_heightlevel / 4" for desert amount (landscape.cpp), at least for now as far as mhl is concerned.
I mentioned before that this patch could be rewritten as a oneliner and as you had most code available ... I do not feel too guilty about this.

Both 155 and 240 are stored in the savegame but not in the config file.
-> should be stored in both places (like snowline)

So what did I miss technically?

src/table/gameopt_settings.ini in patch 155.
src/table/settings.ini in patch 240.

It is not "both in the same location" as I said before but one in the savegame and not in the config file and the other patch the other way round, sorry. I had trouble waking up properly this morning and posted a bit too soon.

Gui clamping of selecting desertline.
->The actual clamping of the desert line height in the gui, when creating a new game, could be based on the selected max height?
¿ "upper clamp = _settings_game.construction.max_heightlevel / 4 * 3" ? instead of MAX_DESERT_HEIGHT. see previous comment.

You mean, forbidding a desert height higher than max. heightlevel?

Yes indeed I meant that

Well, one might implement this, makes code a bit more complicate as then for any change of these two values, the respective other value has to be checked. On the other hand, both are maximal values, if you choose max. desert height 100 on a max. heightlevel 50 map, you simply won´t get any rainforest. So, it´s a defined result, and thus I decided on not implementing additional checks for this. But this can be changed if we want it to be changed.

If you clamp the selection of desert line height to be lower than _settings_game.construction.max_heightlevel (with a gui redraw if needed) the scenario you mention above will never happen (maybe mhl should do this for the snowline to avoid the same problem there.)
But we can/could/should discuss this further as a separate patch ¿and in a separate topic? (-> even now already and based on clean trunk if you want that.)

As mentioned above ... devs would rather not implement 240 as is for now but replace it by a oneliner. (except for saving the value in the savegame maybe, I did not ask this)
"generated height / 4" for now.

12:41:24 <frosch123> also, i think the max-height must be fixed at map generation, any may not be altered in game

Can we agree on making this an option in the cheat window?

I.e. on the same level as changes to money, time, climate, etc.

I don´t care about where and how, but I think there should really be a way to change this. I, and I think many other players too, play this game in single player mode, without even AI or network players. Thus, I want to be able to form the game (of course within the technically possible limits) the way I like it. And this involves corrections of this kind if after starting a game, I realize that some aspect of the game is not yet in the state I want it.

Devs even have stronger feelings about this not changing in game then over desert line (see previous posts in this thread).
Maybe this could be done as a cheat but I can/should/will not speak in Devs name.
My previous post with the quotes I posted after asking permission.

Again, I quoted frosh but he was not the only one of the Devs to share the same view as you can read on the previous pages.

[HackaLittleBit]

Maybe this is a soluton:

Code: Select all

minimal map         variable
length / width  |  max-height
  64            |      16
 128            |      32
 256            |      64
 512            |     128
1024            |     256
2048            |     256

[ChillCore]

[Disclaimer end]
From now on all my posts will be solely my own opinion again. Unless I quote again like before.



Thank you for the feedback hackalittlebit.

The suggestion you make has been the case from the very start.
That is, the max possible generated height was clamped this way but the effective result (heighest tile of the map) was unpredictable.

With the new introduced setting you can select the effectively generated height (If I understood correctly that is, I still have to test).
Based on this *now* known max_height, at game start, the rest of the game can take predictable actions (AIs, industry placement and maybe more) which would have been be a lot harder with previous code.
And if I am not mistaking that predictable behaviour is also the reason Devs do not want it to be altered ingame.

[ic111]

Ok, I will fix the technical issues. For the desert line, I will use the max_heightlevel / 4 rule.

Concerning max. heightlevel: Ok, maybe I´ll write a patch for a cheat, might be useful as my private patch if at some time I feel the need for it was well...

May it be altered in scenario editor, or should altering it be forbidden there as well?

But I have to ask one meta-question: Do I use the right communication channel? You, ChillCore, seem to be much better informed than I...