More height levels (in trunk since r27010)

[ic111]

You come at the right moment.
I have a very busy week ahead...
Please,
feel free to make all changes you feel are needed without considering me.
(I will adapt and learn all over if need to be).
The easiest for you to see what i did is pull a diff with v4.1, i think
Good bughunting to you.

Thanks

Just one question regarding the reduction of the max. possible height to 255: I agree that you will hardly ever exceed this range. So, I can live with this change and don't think that I will reach it in my scenarios.

However, I would like the patch being that flexible that you can easily switch to another decision. After all, I would not say that considering future ideas like flexible sea level we may not under any circumstances touch the boundary at 255 height levels. Altough I think it's not very probable.

So, did you (not you personally, I read the forum posts for that change) just alter the type of the extended heightlevel type and the savegame handling code, or did you switch to byte as data type at more places? After having a look at some source files, I suppose you did the former. (no, I didn't take a diff between 4.1 and 7.6...).

Considering the work such a patch involves, I just don't want to take the risk of introducing a bottleneck one reaches in some months or years, so except for the real saving I want to write as much code as possible using nothing smaller than int16 as type of the z value.

[ChillCore]

Some values have been reduced to int16 from int32 by me.
I can not recall returning some to byte however...

If you want i can return all z values I changed back to int32.
(as they were before v 6.1)(+ the latest fixed bugs)
That will not me take that long to do.

I do have some time left this week, but not as much as i would like.

ps:
If you have more questions , just shoot.
I understand you not being able to read my mind.

[ChillCore]

Hello,
I was wondering how the code behaves with higher levels then 255.
Highest tile level is, thanks to CommanderZ, based on mapsize.

What I did is patch my latest posted patch on top of Alain his patchpak.
I changed MAX_TILE_HEIGHT_EXTENDED to 1023 and started testing.(scenario editor)
Map size 4096 * 4096 gives level 511 no problem, but when testing 8192 * 8192 I run out of memory...

Can somebody please test this ?

I have attached a win build too just in case somebody wants to help who cannot compile for himself.
Also may i add that this build IS buggy... i only wanted to test mapgeneration and resulting height...

Edit:
Typos.
Note to self: re-read * 3, before hitting submit.

Edit 2:
New bug: NoAi is doing something outside the map causing assert on mapsize.
(Height patch alone too.)
Grab the binaries while you can, if you want it...
I will remove it sometime tomorrow,(3rd before going to bed...)
I am not building a patchpak, just needed bigger maps...

Edit3:
Further testing shows that the above mentioned bug is related to edges not being water.
I let a game run ff for about ten years now with 4 ai ... still running. (edges are set to water.)

[Bilbo]

Hello,
I was wondering how the code behaves with higher levels then 255.
Highest tile level is, thanks to CommanderZ, based on mapsize.

What I did is patch my latest posted patch on top of Alain his patchpak.
I changed MAX_TILE_HEIGHT_EXTENDED to 1023 and started testing.(scenario editor)
Map size 4096 * 4096 gives level 511 no problem, but when testing 8192 * 8192 I run out of memory...

Can somebody please test this ?

I have attached a win build too just in case somebody wants to help who cannot compile for himself.
Also may i add that this build IS buggy... i only wanted to test mapgeneration and resulting height...

Edit:
Typos.
Note to self: re-read * 3, before hitting submit.

Edit 2:
New bug: NoAi is doing something outside the map causing assert on mapsize.
(Height patch alone too.)
Grab the binaries while you can, if you want it...
I will remove it sometime tomorrow,(3rd before going to bed...)
I am not building a patchpak, just needed bigger maps...

Edit3:
Further testing shows that the above mentioned bug is related to edges not being water.
I let a game run ff for about ten years now with 4 ai ... still running. (edges are set to water.)

With the height patch you need 12 bytes per square -> 805Mb for map array + few Mb for vehicles and other stuff. Double the amount (1610Mb) when saving/loading game -> so if you do not save/load at all, you need just half the ram :). If you have less RAM than that ... you have trouble :)

Also, note that even if you have enough ram, if you compile 32bit OpenTTD (and not 64bit), you will have only 4gb of virtual memory, of which 2 gb is usally available for user processes - since the tile array need large CONTIGUOUS blocks of memory, if the free memory within that 2gb block become fragmented (there are also libraries, stack, code and such stuff in that space), you can run out of memory before even depleting the 2gb limit (or physical RAM+swap, whatever is sooner).

You can try to compile and run openTTD on 64bit architecture to avoid this, there are no more nasty "maybe 2gb, but usually less" limits per process. (yes, you have PAE, you can compile kernel to use 3g/1g memory layout ... but usually its max. 2gb/process)

[ChillCore]

I have only 1024mb on this machine, so i went the other way:
Tweak tgp a little to test mapsize 4096 * 4096 and 1023 high.

Reaching higher levels is no problem,
Maps become rather unplayable with above mentioned settings, to look good it would at least take 8192 * 8192 to make height 1023 look good at middle height levels.

I do not see the advantages at this point in time.(considering memory usage)
In the far future with superservers that can handle 32000*32000 maps and 512 players connected and 1000 spectators maybe...
By that time i will be doing something not related to tiles.

So it is possible later if somebody feels like doing so. Tgp would need tweaking, and more smallmap colours would need to be found, snowline from byte to int ...
but apart from that i think the code is holding up fine.
I did not test sprite z-offset yet at this height.

Oh yeah that bug when clicking planes...(not buying)->Crash in gfx.cpp
Gone...
Thank you, Rubidium.

@ Bilbo
Thank you for explaining that.
That stopped me from trying impossible things.

While i have your attention...
Do you need credit for parts of the code in the logfile?
I ask because i have not yet looked at your patches more closely, so i do not know what parts are yours, if any, and which are not.

[Bilbo]

@ Bilbo
Thank you for explaining that.
That stopped me from trying impossible things.

While i have your attention...
Do you need credit for parts of the code in the logfile?
I ask because i have not yet looked at your patches more closely, so i do not know what parts are yours, if any, and which are not.

I only come with "pseudopatch" that just raised max level to 127 as I wanted to see if it could work and if not, what parts of code will be broken by this. This patch raises the limit in a bit different way and actually solves the problems I did not take care of. I do not see any "my" code in this patch, so only thing you can credit me for is that you got inspiration to write your patch when you saw my "patch" :).

[ic111]

Hello,

I am at revision 15633 plus version 7.6 of the more heightlevel patch. If I try to generate a new game, the assertion

assert(log_frequency == TGP_FREQUENCY_MAX)

in HeightMapGenerate (tgp.cpp) is violated, generation fails completely. Using the debugger, I found out that at this place, log_frequency is 7, whereas TGP_FREQUENCY is 8.

This seems to be reproducable.

Does anyone know more / is able to find the bug?

[CommanderZ]

I will look at it tomorrow. I hope I will finally make building the executable with squirrel work

[ChillCore]

Can you please try with a mapsize 256 * 256 minimum or bigger.
Smaller maps are known to crash with tgp frequency set to 8.
original is 6 -> 2, 4, 8, 16 , 32, 64, problem.
If that fails there is something else wrong.

[CommanderZ]

I have been working mostly with 512^2 maps, smaller ones were tested only very ocassionally.

[ChillCore]

I have been tuning tgp noise on 2048 * 2048 when you posted tgp_adaptor.v2.
My attention shifted to z offset, I noticed the crasches on smallmaps later and did not get back to it yet.
I will get to it while finetuning the smallmapcolors.
I am just not sure what i will do yet... get back to six, use an if else, something else.
I have not yet tested tgp adaptor 2 with frequency set at 6... It could be we can make it look just as good as now.

Edit:
@ CommanderZ:
Or you may look at it tomorow... if you wish to do so.
Very rough and really rough smoothness settings are yours, with a little tweak

[ic111]

Ok, generating a map of 256x256 works - my bug report was for generating a map of 128x128.

What I also notice (and was already reported before as well): The allow_more_heightlevels setting seems to be broken in some way.

If I load an old savegame (i.e. older savegame version), trying to turn the patch on does not work, although the callback function is executed according to gdb.

If I generate a new game with the patch being turned on, turning the patch off (which should work if like in this case no mountain higher than height 16 is on the map) results in a segfault indicating that the setting didn't actually changed. (the callback emptied the extended array, but the allow_more_heightlevel - setting was not set to false, so the code tries to read the extended array).

I noticed that there where some changes to the handling of settings since December - at least the settings window looks quite differently now. Maybe one has to do something with the settings (maybe extend some data structure) we don't know about?

So, I summarize, I suspect that for some odd reason the setting is not actually altered, although the callback is called. Does anyone of you have an idea, why this happens? Where do I have to look at?

[CommanderZ]

I have been tuning tgp noise on 2048 * 2048 when you posted tgp_adaptor.v2.
My attention shifted to z offset, I noticed the crasches on smallmaps later and did not get back to it yet.
I will get to it while finetuning the smallmapcolors.
I am just not sure what i will do yet... get back to six, use an if else, something else.
I have not yet tested tgp adaptor 2 with frequency set at 6... It could be we can make it look just as good as now.

Edit:
@ CommanderZ:
Or you may look at it tomorow... if you wish to do so.
Very rough and really rough smoothness settings are yours, with a little tweak

If you wish to continue improving the TGP I will be glad to let you. This school semester is somehow far more demanding than I expected

[ChillCore]

@ ic111:

Ok, generating a map of 256x256 works

I am glad it is the known bug.

About the button not functioning...
A little after devs made changes to the settings, they also did a cleanup.
They removed a lot of #include "blahblah" in the headers, maybe the problem lies there somewhere.
I have put #include "../tile_map.h" back in map_sl.cpp just now.
¿Now the button changes color according to the game that has been loaded?
Also on/off status is updated.(Choosing before starting a game and in game does not toggle yet.)

IMHO, I do not want to push in any direction,
first priority is edges not being water.
They cause higher levels to fall outside the map causing invalid tiles.
(can you change map border invalid tiles starting at -64, -64 ?)

@ CommanderZ:
whoever gets there first ... a little of yours and a little of mine ...
It is only a matter of deciding what to do with tgp frequency max,
and then finetuning some more the noise...

@ Devs:
If this patch is ever to hit trunk...
I suppose you guys would like to keep mapsize 64 * 64 possible, right?

[CommanderZ]

@ Devs:
If this patch is ever to hit trunk...
I suppose you guys would like to keep mapsize 64 * 64 possible, right?

I asked this some time ago and the anwer was clear YES. And I understand it - sometimes it is good to have as small map as possible.

[ic111]

Did you ask this here, in this thread, or somewhere else?

(I just want to find out wether I should ask about the button at some other place also)

[CommanderZ]

Did you ask this here, in this thread, or somewhere else?

(I just want to find out wether I should ask about the button at some other place also)

No, I think it was in this thread.

[Hirundo]

If I generate a new game with the patch being turned on, turning the patch off (which should work if like in this case no mountain higher than height 16 is on the map) results in a segfault indicating that the setting didn't actually changed. (the callback emptied the extended array, but the allow_more_heightlevel - setting was not set to false, so the code tries to read the extended array).

The function that is called when a setting changes should return true if the setting should actually be changed. As far as I could see, this doesn't happen here.

[ic111]

The function that is called when a setting changes should return true if the setting should actually be changed. As far as I could see, this doesn't happen here.

Ok, thanks, this was the bug, now it works

I always thought, I return the new value of the setting I change here (e.g. if I change from true to false, but false is not possible, I return true), and not just a boolean flag success (e.g. if I change from true to false and false is not possible, I return false).

The only thing I don't understand is why this worked three months ago...

[Hirundo]

The only thing I don't understand is why this worked three months ago...

r15334