Patch: The Rainfall River Generator

Forum for technical discussions regarding development. If you have a general suggestion, problem or comment, please use one of the other forums.

Moderator: OpenTTD Developers

Post Reply
ic111
Director
Director
Posts: 608
Joined: 17 Jul 2007 17:56

Patch: The Rainfall River Generator

Post by ic111 »

I hereby publish the Rainfall River Generator.

It is a completely new, sophisticated river generator. Features / the basic idea at a glance (more information in the attached manual, and if you want to dive a level deeper, in extensive source code comments):

- Each tile gains one unit of water
- Water flows downwards, sums up, generates rivers once a certain flow is exceeded
- Lake basins fill with water
- All rivers are connected, i.e. there are no gaps between the spring and a destination lake or the ocean.
- Islands in lakes
- Optionally wider rivers, if flow exceeds certain bounds, a river gets e.g. two or three tiles wide.
- Optionally wider valleys, to get usable space if the small valleys of a heightmap would otherwise be completely consumed by a river
- Extensive configuration options, e.g. the flow needed to actually form a river can be chosen freely; choosing 201 gives a bit less rivers than 200
- Adds a dropdown for the to be used river generator to the map generation windows
- Maintains the "Few", "Moderate", "Lot of" simple settings, and transforms chosen values into the configuration world of the Rainfall River Generator
- NEW since version v9: Generate cities preferable somewhere near rivers and lakes

Look at the attached screenshot for a first impression of what it does.

I publish this patch after about two months of occasional evening-work in an already quite mature state, the calculation should be finished, things like lake generation (which was probably the most complicated thing in terms of algorithms) - hopefully - simply work. Basically, beside some small flaws I have currently no TODOs with it left.

Of course I wouldn´t have done that work if I wouldn´t aim at something that can finally be a second river generator beside the existing one in trunk. Thus, any feedback is welcome, as long as you don´t ask me to completely switch algorithms...

The primary aim of the generator is producing qualitatively good, real-world-looking rivers and lakes. What should be clear is, this comes at some performance cost, especially because several of the calculation steps (e.g. calculating number of lower tiles, calculating flow) don´t actually depend on the number of rivers the player finally sees, but just scale with the map size. But in terms of landscape generation I personally regard it as quite ok, if it runs for some dozens of seconds and finally produces a good result; after all I then typically play that result for days or weeks of real-world-time.

Especially, people who like 2048x2084 or 4096x4096 maps will wait relatively long when using that generator. But my aim are more the 512 or 1024 maps anyway; I would never play a 4096 map simply because I cannot fill it with railroads etc.

Anyway, it does what I want, have fun with it.
Sample screenshot of the Rainfall River Generator
Sample screenshot of the Rainfall River Generator
Rivers.png (890.84 KiB) Viewed 2316 times
manual.pdf
Manual for the rainfall river generator
(1.48 MiB) Downloaded 1129 times
Attachments
rivers_v61.zip
(269.08 KiB) Downloaded 318 times
Last edited by ic111 on 23 Apr 2017 18:33, edited 8 times in total.
ic111
Director
Director
Posts: 608
Joined: 17 Jul 2007 17:56

Re: Patch: The Rainfall River Generator

Post by ic111 »

Some technical issues / questions:

My generator frequently terraforms landscape at the small scale. For being able to do so, I added support for calling the terraform code from landscape generation. This works perfectly, I only wasn´t able to figure out how I can make the code correctly treat the case "Ocean is raised", or "Shore is lowered to flat Ocean". I noticed this because occasionally an assertion in water_map "Shore with wrong slope" was triggered.

Patch 063 is a workaround that fixes this issue by post-processing after terraforming, but there must be a cleaner way, since exactly the same thing works in-game without problems, I just didn´t manage to find out how.

Then: To make the number of lower tiles and the flow calculation work properly, I need to calculate a connected component of all tiles of some height, once I have found one such tile (for background information about this sentence, read the manual and the source code comments, your keyword especially is the height index). Anyway, on flat maps, in worst case you suddenly have nearly the whole map in your connected component, either a std::map or a std::set. On my machine, I got segfaults as soon as the container grow beyond something between 200000 and 300000 entries.

I regard this a bit surprising, since even if the container needs 100 bytes per entry (I hope it is less, does someone know about the memory footprint of std::map / std::set?), I still am just in the order of 20 or 30 MB, which shouldn´t cause out-of-memory symptoms.

Anyway, to circumven that problem, I abort connected component calculation once I calculated 100000 tiles. This makes calculation a bit less "correct", but keeps the memory footprint under control.

The generation map progress is another topic: For example the number of lower tiles metrics is calculated heightlevel for heightlevel. I increase progress at each heightlevel step (actually, up to three times per heightlevel as I have to separate the calculations of different slope kinds). On big, flat maps this is not enough, it freezes calculation ways too long. On the other hand, I have no clear measure below the heightlevel scheme I can print to the user. What would be convenient in such cases would be a message like "x tiles processed so far", "x lakes processed so far", etc., without upper bound since I don´t have an upper bound in advance, but as far as I know this kind of progress isn´t supported by current map progress scheme.

EDIT: Another question: Do I really need a savegame bump if I only add config variables for map generation? Or is there a way to live without? After all, the games generated with this patch will be savegame-incompatible, but only because generation was different, not because anything in-game is different. If there would be a way to do so, I would like to avoid this.
zooks
Transport Coordinator
Transport Coordinator
Posts: 262
Joined: 29 Jun 2006 08:36

Re: Patch: The Rainfall River Generator

Post by zooks »

Brilliant stuff! Can't wait to see how this works in combination with my fractal heightmap generator
Kogut
Tycoon
Tycoon
Posts: 2493
Joined: 26 Aug 2009 06:33
Location: Poland

Re: Patch: The Rainfall River Generator

Post by Kogut »

- Extensive configuration options, e.g. the flow needed to actually form a river can be chosen freely; choosing 201 gives a bit more rivers than 200
Is it a typo? I would expect that higher flow needed to actually form a river would reduce number of rivers.
Correct me If I am wrong - PM me if my English is bad
AIAI - AI for OpenTTD
ic111
Director
Director
Posts: 608
Joined: 17 Jul 2007 17:56

Re: Patch: The Rainfall River Generator

Post by ic111 »

Yes, it´s a typo, you are perfectly right. More flow needed gives less rivers.
Eddi
Tycoon
Tycoon
Posts: 8258
Joined: 17 Jan 2007 00:14

Re: Patch: The Rainfall River Generator

Post by Eddi »

back when i dabbled in river generation (that was even before the current river generator was implemented) i found the river placement too restrictive. rivers should be able to be placed on any slope.
oberhümer
Tycoon
Tycoon
Posts: 1283
Joined: 23 Oct 2009 19:35
Location: Here and there, sometime or another

Re: Patch: The Rainfall River Generator

Post by oberhümer »

Crashes on my end (Ubuntu 14.04) if I select Rainfall Rivers in the map generator window (not with "No rivers", of course). Maybe I didn't apply the patch queue correctly/against the wrong version? I used HG revision 21915/SVN revision 27069. It's certainly strange that OpenTTD doesn't report itself as modified, but it seems that's just the way MQ works.

And there are a few of these compiler warnings:
/home/***/Documents/code/ottd-rainfallrivers/trunk.hg/src/rivers_rainfall.cpp:393:2: note: in expansion of macro ‘DEBUG’
DEBUG(map, RAINFALL_NUMBER_OF_LOWER_LOG_LEVEL, "CONNECTED_COMPONENT: Processing connected component of %i tiles and height %i, starting with tile (%i, %i)",
^
/home/***/Documents/code/ottd-rainfallrivers/trunk.hg/src/debug.h:39:116: warning: format ‘%i’ expects argument of type ‘int’, but argument 3 has type ‘std::map<unsigned int, int>::size_type {aka long unsigned int}’ [-Wformat=]
#define DEBUG(name, level, ...) if ((level) == 0 || _debug_ ## name ## _level >= (level)) debug(#name, __VA_ARGS__) ^
Attachments
crash.zip
(54.38 KiB) Downloaded 233 times
--- Licenses: GNU LGPL, version 2 or newer, code and graphics. CC-By-SA, graphics, alternatively. If you're using any, I'd like to hear about it --- Call them "track types" ---
--- Mostly inactive developer for: NuTracks - Central European Train Set --- Running/compiling for: Linux (x86) - Android - Windows (32/64 bit) ---

--- Need a file packer? 7-Zip --- BOINC - use your computing power to benefit science --- Block trackers, not ads --- Unix in dispersible pellets, the formula for the future. ---
Eddi
Tycoon
Tycoon
Posts: 8258
Joined: 17 Jan 2007 00:14

Re: Patch: The Rainfall River Generator

Post by Eddi »

that rather smells like a subtle difference between compilers used (unsigned vs signed in map::size_type), but it should be safe to just comment out that line.

the version detection will say "modified" if there are uncommited changes in the repo, otherwise it will just use the revision hash as version (which should be safe enough for multiplayer and other purposes that need the version)
ic111
Director
Director
Posts: 608
Joined: 17 Jul 2007 17:56

Re: Patch: The Rainfall River Generator

Post by ic111 »

Regarding the crash: Could this also be a symptom of the issue causing the warnings above?

The crash occur at a point in code I didn´t have problems with for a long time, so I suspect some problem that hits you but not me.

Probably, a stacktrace with line numbers would be convenient, I don´t know wether I can figure something out without that information...


And regarding rivers on all slopes: Well, their presence would have saved me quite a bit of work...
oberhümer
Tycoon
Tycoon
Posts: 1283
Joined: 23 Oct 2009 19:35
Location: Here and there, sometime or another

Re: Patch: The Rainfall River Generator

Post by oberhümer »

Full GDB backtrace attached, debug binary of course.
Attachments
backtrace.txt
(13.52 KiB) Downloaded 413 times
--- Licenses: GNU LGPL, version 2 or newer, code and graphics. CC-By-SA, graphics, alternatively. If you're using any, I'd like to hear about it --- Call them "track types" ---
--- Mostly inactive developer for: NuTracks - Central European Train Set --- Running/compiling for: Linux (x86) - Android - Windows (32/64 bit) ---

--- Need a file packer? 7-Zip --- BOINC - use your computing power to benefit science --- Block trackers, not ads --- Unix in dispersible pellets, the formula for the future. ---
User avatar
planetmaker
OpenTTD Developer
OpenTTD Developer
Posts: 9432
Joined: 07 Nov 2007 22:44
Location: Sol d

Re: Patch: The Rainfall River Generator

Post by planetmaker »

I like very much what I see here.

The full series gives a few easy-to-fix compile warnings on debian x64 (gcc 4.7.2):

Code: Select all

ottdtrunk/src/rivers_rainfall.cpp: In member function ‘void NumberOfLowerHeightIterator::ProcessNonSteepTile(TileIndex, HeightLevelIterationMode)’:
ottdtrunk/src/rivers_rainfall.cpp:393:2: warning: format ‘%i’ expects argument of type ‘int’, but argument 3 has type ‘std::map<unsigned int, int>::size_type {aka long unsigned int}’ [-Wformat]
ottdtrunk/src/rivers_rainfall.cpp:398:3: warning: format ‘%i’ expects argument of type ‘int’, but argument 3 has type ‘std::map<unsigned int, int>::size_type {aka long unsigned int}’ [-Wformat]
ottdtrunk/src/rivers_rainfall.cpp:430:5: warning: format ‘%i’ expects argument of type ‘int’, but argument 3 has type ‘std::vector<TileWithValue>::size_type {aka long unsigned int}’ [-Wformat]
ottdtrunk/src/rivers_rainfall.cpp:456:4: warning: format ‘%i’ expects argument of type ‘int’, but argument 3 has type ‘std::vector<TileWithValue>::size_type {aka long unsigned int}’ [-Wformat]
ottdtrunk/src/rivers_rainfall.cpp:458:4: warning: format ‘%i’ expects argument of type ‘int’, but argument 3 has type ‘std::vector<TileWithValue>::size_type {aka long unsigned int}’ [-Wformat]
ottdtrunk/src/rivers_rainfall.cpp: In member function ‘void Lake::PrintToDebug(int, int)’:
ottdtrunk/src/rivers_rainfall.cpp:927:2: warning: format ‘%i’ expects argument of type ‘int’, but argument 6 has type ‘std::set<unsigned int>::size_type {aka long unsigned int}’ [-Wformat]
ottdtrunk/src/rivers_rainfall.cpp:927:2: warning: format ‘%i’ expects argument of type ‘int’, but argument 7 has type ‘std::set<unsigned int>::size_type {aka long unsigned int}’ [-Wformat]
ottdtrunk/src/rivers_rainfall.cpp: In member function ‘int DefineLakesIterator::ConsumeLake(Lake*, TileIndex, LakeDefinitionState&, LakeDefinitionRun&)’:
ottdtrunk/src/rivers_rainfall.cpp:1136:2: warning: format ‘%i’ expects argument of type ‘int’, but argument 3 has type ‘std::set<unsigned int>::size_type {aka long unsigned int}’ [-Wformat]
ottdtrunk/src/rivers_rainfall.cpp: In member function ‘void DefineLakesIterator::CreateLake(TileIndex, LakeDefinitionState&, int)’:
ottdtrunk/src/rivers_rainfall.cpp:1455:5: warning: format ‘%i’ expects argument of type ‘int’, but argument 3 has type ‘std::vector<unsigned int>::size_type {aka long unsigned int}’ [-Wformat]
ottdtrunk/src/rivers_rainfall.cpp: In member function ‘void LakeModificator::DiscardRegion(TileIndex, int, std::set<unsigned int>*, std::set<unsigned int>&, std::set<unsigned int>&, bool)’:
ottdtrunk/src/rivers_rainfall.cpp:2518:3: warning: format ‘%i’ expects argument of type ‘int’, but argument 3 has type ‘std::set<unsigned int>::size_type {aka long unsigned int}’ [-Wformat]
ottdtrunk/src/rivers_rainfall.cpp:2518:3: warning: format ‘%i’ expects argument of type ‘int’, but argument 4 has type ‘std::vector<TileWithValue>::size_type {aka long unsigned int}’ [-Wformat]
ottdtrunk/src/rivers_rainfall.cpp: In member function ‘void RainfallRiverGenerator::PrepareLake(TileIndex, int*, byte*, DefineLakesIterator*, std::set<unsigned int>&)’:
ottdtrunk/src/rivers_rainfall.cpp:2741:3: warning: format ‘%i’ expects argument of type ‘int’, but argument 5 has type ‘std::map<unsigned int, int>::size_type {aka long unsigned int}’ [-Wformat]
ottdtrunk/src/rivers_rainfall.cpp:2779:5: warning: format ‘%i’ expects argument of type ‘int’, but argument 3 has type ‘std::vector<unsigned int>::size_type {aka long unsigned int}’ [-Wformat]
ottdtrunk/src/rivers_rainfall.cpp:2844:2: warning: format ‘%i’ expects argument of type ‘int’, but argument 5 has type ‘std::set<unsigned int>::size_type {aka long unsigned int}’ [-Wformat]
ottdtrunk/src/rivers_rainfall.cpp: In member function ‘void RainfallRiverGenerator::LowerHigherWaterTilesUntilValid(std::set<unsigned int>&, int*, byte*)’:
ottdtrunk/src/rivers_rainfall.cpp:3087:3: warning: format ‘%i’ expects argument of type ‘int’, but argument 4 has type ‘std::set<unsigned int>::size_type {aka long unsigned int}’ [-Wformat]
ottdtrunk/src/rivers_rainfall.cpp:3162:3: warning: format ‘%i’ expects argument of type ‘int’, but argument 4 has type ‘std::set<unsigned int>::size_type {aka long unsigned int}’ [-Wformat]
ottdtrunk/src/rivers_rainfall.cpp:3162:3: warning: format ‘%i’ expects argument of type ‘int’, but argument 5 has type ‘std::set<unsigned int>::size_type {aka long unsigned int}’ [-Wformat]
ottdtrunk/src/rivers_rainfall.cpp: In member function ‘void RainfallRiverGenerator::FineTuneTilesForWater(int*, byte*, std::vector<TileWithHeightAndFlow>&, std::set<unsigned int>&)’:
ottdtrunk/src/rivers_rainfall.cpp:3195:2: warning: format ‘%i’ expects argument of type ‘int’, but argument 3 has type ‘std::set<unsigned int>::size_type {aka long unsigned int}’ [-Wformat]
In file included from ottdtrunk/src/rivers_rainfall.cpp:16:0:
ottdtrunk/src/landscape_util.h: In instantiation of ‘void ConnectedComponentCalculator<T>::StoreConnectedComponent(T&, TileIndex, int) [with T = std::map<unsigned int, int>; TileIndex = unsigned int]’:
ottdtrunk/src/rivers_rainfall.cpp:391:97:   required from here
ottdtrunk/src/landscape_util.h:199:4: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
ottdtrunk/src/landscape_util.h: In instantiation of ‘void ConnectedComponentCalculator<T>::StoreConnectedComponent(T&, TileIndex, int) [with T = std::set<unsigned int>; TileIndex = unsigned int]’:
ottdtrunk/src/rivers_rainfall.cpp:817:98:   required from here
ottdtrunk/src/landscape_util.h:199:4: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
In the detailed configuration window: the buttons do not scale with UI zoom and the text might not fit anymore for all items in the 'river shape' section. The numbers in the other sections don't seem to have that problem (for reference: sprite font 2x UI zoom, zBase).

And indeed it crashes for me upon map generation with the default settings:

Code: Select all

Stacktrace (probably similar to oberhümer's):
 [00] bin/openttd(_ZNK12CrashLogUnix13LogStacktraceEPcPKc+0x37) [0x71bc27]
 [01] bin/openttd(_ZNK8CrashLog12FillCrashLogEPcPKc+0xeb) [0x5e9c2b]
 [02] bin/openttd(_ZNK8CrashLog12MakeCrashLogEv+0x4c) [0x5e9e8c]
 [03] bin/openttd() [0x71bb95]
 [04] /lib/x86_64-linux-gnu/libc.so.6(+0x35250) [0x7ff731313250]
 [05] bin/openttd(_ZN41NumberOfLowerConnectedComponentCalculator23DiscardTilesThreeRaisedEPjP5SlopePi9DirectionS1_S4_S4_S1_S1_+0x10e) [0x7628fe]
 [06] bin/openttd(_ZN41NumberOfLowerConnectedComponentCalculator29GetNumberOfLowerNeighborTilesEj+0x424) [0x762fc4]
 [07] bin/openttd(_ZN41NumberOfLowerConnectedComponentCalculator16StoreInContainerERSt3mapIjiSt4lessIjESaISt4pairIKjiEEEj+0x6f) [0x7721cf]
 [08] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x67) [0x7701c7]
 [09] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x168) [0x7702c8]
 [10] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0xa2) [0x770202]
 [11] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x37e) [0x7704de]
 [12] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x37e) [0x7704de]
 [13] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x37e) [0x7704de]
 [14] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x37e) [0x7704de]
 [15] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x37e) [0x7704de]
 [16] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x168) [0x7702c8]
 [17] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x37e) [0x7704de]
 [18] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x37e) [0x7704de]
 [19] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x168) [0x7702c8]
 [20] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x168) [0x7702c8]
 [21] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x37e) [0x7704de]
 [22] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x37e) [0x7704de]
 [23] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x37e) [0x7704de]
 [24] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x37e) [0x7704de]
 [25] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x168) [0x7702c8]
 [26] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0xa2) [0x770202]
 [27] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0xa2) [0x770202]
 [28] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x37e) [0x7704de]
 [29] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x34b) [0x7704ab]
 [30] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x37e) [0x7704de]
 [31] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x37e) [0x7704de]
 [32] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x37e) [0x7704de]
 [33] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x37e) [0x7704de]
 [34] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x168) [0x7702c8]
 [35] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0xa2) [0x770202]
 [36] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x168) [0x7702c8]
 [37] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x37e) [0x7704de]
 [38] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x37e) [0x7704de]
 [39] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x34b) [0x7704ab]
 [40] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x34b) [0x7704ab]
 [41] bin/openttd(_ZN28ConnectedComponentCalculatorISt3mapIjiSt4lessIjESaISt4pairIKjiEEEE23StoreConnectedComponentERS7_ji+0x34b) [0x7704ab]
 [42] bin/openttd(_ZN27NumberOfLowerHeightIterator19ProcessNonSteepTileEj24HeightLevelIterationMode+0x8c) [0x76be5c]
 [43] bin/openttd(_ZN19HeightLevelIterator18CalculateRecursiveEi24HeightLevelIterationModeiii+0x251) [0x6732d1]
 [44] bin/openttd(_ZN19HeightLevelIterator18CalculateRecursiveEi24HeightLevelIterationModeiii+0x1f5) [0x673275]
 [45] bin/openttd(_ZN19HeightLevelIterator18CalculateRecursiveEi24HeightLevelIterationModeiii+0x1a6) [0x673226]
 [46] bin/openttd(_ZN19HeightLevelIterator18CalculateRecursiveEi24HeightLevelIterationModeiii+0x1dc) [0x67325c]
 [47] bin/openttd(_ZN19HeightLevelIterator18CalculateRecursiveEi24HeightLevelIterationModeiii+0x1dc) [0x67325c]
 [48] bin/openttd(_ZN19HeightLevelIterator18CalculateRecursiveEi24HeightLevelIterationModeiii+0x1dc) [0x67325c]
 [49] bin/openttd(_ZN19HeightLevelIterator18CalculateRecursiveEi24HeightLevelIterationModeiii+0x1a6) [0x673226]
 [50] bin/openttd(_ZN19HeightLevelIterator18CalculateRecursiveEi24HeightLevelIterationModeiii+0x1dc) [0x67325c]
 [51] bin/openttd(_ZN19HeightLevelIterator18CalculateRecursiveEi24HeightLevelIterationModeiii+0x1a6) [0x673226]
 [52] bin/openttd(_ZN19HeightLevelIterator18CalculateRecursiveEi24HeightLevelIterationModeiii+0x1a6) [0x673226]
 [53] bin/openttd(_ZN19HeightLevelIterator9CalculateEi24HeightLevelIterationMode+0x83) [0x6733a3]
 [54] bin/openttd(_ZN22RainfallRiverGenerator14GenerateRiversEv+0x6b) [0x76facb]
 [55] bin/openttd(_Z17GenerateLandscapeh+0x286) [0x6723c6]
 [56] bin/openttd() [0x632727]
 [57] bin/openttd(_ZN20ThreadObject_pthread10ThreadProcEv+0xb) [0x829b0b]
 [58] bin/openttd(_ZN20ThreadObject_pthread12stThreadProcEPv+0x9) [0x829b89]
 [59] /lib/x86_64-linux-gnu/libpthread.so.0(+0x7e0e) [0x7ff733ca9e0e]
 [60] /lib/x86_64-linux-gnu/libc.so.6(clone+0x6d) [0x7ff7313c70fd]
ic111
Director
Director
Posts: 608
Joined: 17 Jul 2007 17:56

Re: Patch: The Rainfall River Generator

Post by ic111 »

planetmaker wrote:I like very much what I see here.

The full series gives a few easy-to-fix compile warnings on debian x64 (gcc 4.7.2):
Thanks; I suspect that the reason for them is that I printed the size() of the containers with %i, I exchanged that by %u. If this doesn´t help, please tell me again.
In the detailed configuration window: the buttons do not scale with UI zoom and the text might not fit anymore for all items in the 'river shape' section. The numbers in the other sections don't seem to have that problem (for reference: sprite font 2x UI zoom, zBase).
I could reproduce that problem for the dropdowns; I fixed it by adding the padding I didn´t recognize in UpdateWidgetSize before.
And indeed it crashes for me upon map generation with the default settings:
I suspect the reason was a missing check for != INVALID_TILE.

As this didn´t lead to crashes on my machine, I can just hope that I fixed the problem, please test and report wether in works now. I hope I didn´t introduce too many of this kind of bugs, but lets see ;-)

Here is the second version, fixing the three issues described above.
Attachments
rivers_v2.zip
Second version: Bugfixing compiler warnings, scaling in options window, missing check for != INVALID_TILE.
(160.74 KiB) Downloaded 228 times
oberhümer
Tycoon
Tycoon
Posts: 1283
Joined: 23 Oct 2009 19:35
Location: Here and there, sometime or another

Re: Patch: The Rainfall River Generator

Post by oberhümer »

Warnings are still there, but slightly modified:

Code: Select all

/home/****/Documents/code/ottd-rainfallrivers/trunk.hg/src/debug.h:39:116: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 3 has type ‘std::set<unsigned int>::size_type {aka long unsigned int}’ [-Wformat=]
  #define DEBUG(name, level, ...) if ((level) == 0 || _debug_ ## name ## _level >= (level)) debug(#name, __VA_ARGS__)
                                                                                                            ^
Still crashes too, but proceeds further before doing so. New backtrace attached.
Attachments
backtrace-new.txt
(17.97 KiB) Downloaded 347 times
--- Licenses: GNU LGPL, version 2 or newer, code and graphics. CC-By-SA, graphics, alternatively. If you're using any, I'd like to hear about it --- Call them "track types" ---
--- Mostly inactive developer for: NuTracks - Central European Train Set --- Running/compiling for: Linux (x86) - Android - Windows (32/64 bit) ---

--- Need a file packer? 7-Zip --- BOINC - use your computing power to benefit science --- Block trackers, not ads --- Unix in dispersible pellets, the formula for the future. ---
ic111
Director
Director
Posts: 608
Joined: 17 Jul 2007 17:56

Re: Patch: The Rainfall River Generator

Post by ic111 »

oberhümer wrote:Warnings are still there, but slightly modified:
Ok, has anyone a clue what format specifier your compiler expects for outputting the size of a container? Or should I simply cast it?
Still crashes too, but proceeds further before doing so. New backtrace attached.
Hm, looks like the same file. Are you sure you attached the new stacktrace? If yes, then the problem persists, and I have to have another look at it today in the evening.
User avatar
planetmaker
OpenTTD Developer
OpenTTD Developer
Posts: 9432
Joined: 07 Nov 2007 22:44
Location: Sol d

Re: Patch: The Rainfall River Generator

Post by planetmaker »

The compile warnings did not change (much) and it's still crashy at the same place.
The right print formatter for unsigned long is %lu (see the attached diff which also fixes the unsigned - signed comparison in landscape_utils.h)

Code: Select all

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffcccc1700 (LWP 9167)]
0x00000000009bb64a in GetFlowDirection (water_info=0x7fffc023ab00 "", tile=4294967295) at /home/ingo/ottd/trunk/src/rivers_rainfall.h:306
306	inline Direction GetFlowDirection(byte *water_info, TileIndex tile) { return (Direction)GB(water_info[tile], 0, 3); }
(gdb) bt
#0  0x00000000009bb64a in GetFlowDirection (water_info=0x7fffc023ab00 "", tile=4294967295) at /home/ingo/ottd/trunk/src/rivers_rainfall.h:306
#1  0x00000000009b1ac6 in CurveFlowModificator::FindStartTile (this=0x7fffc0136e20, min_equal_directions=0) at /home/ingo/ottd/trunk/src/rivers_rainfall.cpp:1819
#2  0x00000000009b1c8e in CurveFlowModificator::ModifyFlow (this=0x7fffc0136e20, min_equal_directions=0) at /home/ingo/ottd/trunk/src/rivers_rainfall.cpp:1875
#3  0x00000000009baadc in RainfallRiverGenerator::GenerateRivers (this=0x7fffc00008c0) at /home/ingo/ottd/trunk/src/rivers_rainfall.cpp:3645
#4  0x000000000085d0d1 in CreateRivers () at /home/ingo/ottd/trunk/src/landscape.cpp:948
#5  0x000000000085d488 in GenerateLandscape (mode=0 '\000') at /home/ingo/ottd/trunk/src/landscape.cpp:1037
#6  0x000000000080c827 in _GenerateWorld () at /home/ingo/ottd/trunk/src/genworld.cpp:133
#7  0x0000000000ace880 in ThreadObject_pthread::ThreadProc (this=0x1b5a350) at /home/ingo/ottd/trunk/src/thread/thread_pthread.cpp:75
#8  0x0000000000ace854 in ThreadObject_pthread::stThreadProc (thr=0x1b5a350) at /home/ingo/ottd/trunk/src/thread/thread_pthread.cpp:63
#9  0x00007ffff78bfe0e in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0
#10 0x00007ffff4fdd0fd in clone () from /lib/x86_64-linux-gnu/libc.so.6
Attachments
fix_warnings.diff
(10.86 KiB) Downloaded 349 times
ic111
Director
Director
Posts: 608
Joined: 17 Jul 2007 17:56

Re: Patch: The Rainfall River Generator

Post by ic111 »

planetmaker wrote:The compile warnings did not change (much) and it's still crashy at the same place.
Hm, your stacktrace tells me that it no longer crashes in the number of lower tiles calculation, but now in the calculate flow calculation, which is indeed one step further.

The last stacktrace of Oberhümer was still in the number of lower tiles calculation.

I´ll take a look...
The right print formatter for unsigned long is %lu (see the attached diff which also fixes the unsigned - signed comparison in landscape_utils.h)
Thanks!
ic111
Director
Director
Posts: 608
Joined: 17 Jul 2007 17:56

Re: Patch: The Rainfall River Generator

Post by ic111 »

planetmaker wrote:The compile warnings did not change (much) and it's still crashy at the same place.
The right print formatter for unsigned long is %lu (see the attached diff which also fixes the unsigned - signed comparison in landscape_utils.h)
With %lu, I´m the one who gets the warnings. Probably because this machine is still 32bit.

I read a bit, and came across this link: http://stackoverflow.com/questions/2125 ... 9#22114959

The issue doesn´t seem to be trivial, according to what I read there, and elsewhere, %zu might be the thing we search. In the v3 I hereby release, I use that one.

If that isn´t ok either, then I´ll probably switch to explicitely casting those to unsigned long or similar.

Code: Select all

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffcccc1700 (LWP 9167)]
0x00000000009bb64a in GetFlowDirection (water_info=0x7fffc023ab00 "", tile=4294967295) at /home/ingo/ottd/trunk/src/rivers_rainfall.h:306
306	inline Direction GetFlowDirection(byte *water_info, TileIndex tile) { return (Direction)GB(water_info[tile], 0, 3); }
(gdb) bt
Was easy to fix, another instance of not testing for != INVALID_TILE. Strange that my system doesn´t segfault on those errors, makes testing not exactly easier...

Regarding the last stacktrace from Oberhümer, I read another time over the function where it happens, I don´t see any potential cause there beside those I already fixed in v2; and Planetmakers stacktrace was definitely later in terms of execution.
Attachments
rivers_v3.zip
Another attempt to fix the format specifiers; fixed an instance of not testing for INVALID_TILE.
(149.46 KiB) Downloaded 213 times
oberhümer
Tycoon
Tycoon
Posts: 1283
Joined: 23 Oct 2009 19:35
Location: Here and there, sometime or another

Re: Patch: The Rainfall River Generator

Post by oberhümer »

Still crashes for me, but at "Defining lakes" this time - same problem again? New stack trace attached.
I also noticed a small glitch: During map generation, the progress bar first advances to 10% and then jumps back to 5% when the first river generation step is reached.

Edit: Fixed by adding an INVALID_TILE check to RainfallRiverGenerator::IsPlannedAsWater.
Attachments
backtrace-v3.txt
(6.6 KiB) Downloaded 343 times
--- Licenses: GNU LGPL, version 2 or newer, code and graphics. CC-By-SA, graphics, alternatively. If you're using any, I'd like to hear about it --- Call them "track types" ---
--- Mostly inactive developer for: NuTracks - Central European Train Set --- Running/compiling for: Linux (x86) - Android - Windows (32/64 bit) ---

--- Need a file packer? 7-Zip --- BOINC - use your computing power to benefit science --- Block trackers, not ads --- Unix in dispersible pellets, the formula for the future. ---
ic111
Director
Director
Posts: 608
Joined: 17 Jul 2007 17:56

Re: Patch: The Rainfall River Generator

Post by ic111 »

Thanks, you are perfectly right, the test was missing also here.

Does the generator complete now, or did you just find the next segfault?
Attachments
rivers_v4.zip
Added the INVALID_TILE check to IsPlannedAsWater
(151.06 KiB) Downloaded 205 times
oberhümer
Tycoon
Tycoon
Posts: 1283
Joined: 23 Oct 2009 19:35
Location: Here and there, sometime or another

Re: Patch: The Rainfall River Generator

Post by oberhümer »

It works now, but in some cases OpenTTD hangs when defining lakes (viewport doesn't update or react to interaction, window can only be force-closed). Tested with the "East Africa (High Terrain)" height map at 1024x1024 and "Moderate" rivers.
This patch also tends to generate clustered short rivers, at least on OpenTTD-generated maps. On height maps, some rivers do become nice and long, but there still tend to be multiple rivers in the same area. Rivers really ought to be merged if they run parallel and relatively close to each other.
I also think the default river width increase is a bit too conservative, and does it really make sense for the flow required to add another tile of width to increase exponentially?
--- Licenses: GNU LGPL, version 2 or newer, code and graphics. CC-By-SA, graphics, alternatively. If you're using any, I'd like to hear about it --- Call them "track types" ---
--- Mostly inactive developer for: NuTracks - Central European Train Set --- Running/compiling for: Linux (x86) - Android - Windows (32/64 bit) ---

--- Need a file packer? 7-Zip --- BOINC - use your computing power to benefit science --- Block trackers, not ads --- Unix in dispersible pellets, the formula for the future. ---
Post Reply

Return to “OpenTTD Development”

Who is online

Users browsing this forum: No registered users and 11 guests