Patch: The Rainfall River Generator

[ic111]

A small release, with no functional changes, just moving some code formerly placed in the 999_Temp patch to the right place.

[wallyweb]

A small release, ...

Here is the Windows 32bit binary:

[ic111]

The next release. This time, I improved city / town generation.

First, the answer to an obvious question: Why generating cities in a river generator? The answer is, that I use the information I gather during river generation anyway for deciding what might be the appropriate places for a city. E.g., directly after river generation, given some river tile, I can simply ask an array for the amount of water that is flowing there. Without that information, I would need to inspect a lot of neighbor tiles to get a rough picture wether there is a huge river (maybe a good place for a big city?) or some small lake or river.

Note that currently, when the Rainfall River Generator is active, its city generator automatically is also active. Also, there is no GUI for parametrizing the city generator, you might however have a look into the CityPlacers defined in rivers_rainfall.h, and the way they are used in rivers_rainfall.cpp. There, in code, some rather obvious parameters might be changed, and in fact, people playing with them would be quite a good test.
My test case mostly was a heightmap of a rather alpine region, with clearly visible valleys.

How does it work?
(1) Generate a bunch of aggregated values (a CityScore) for each 16x16 grid on map. The probably most important one is the river_size_score, which is the biggest river flow in that 16x16 grid section, divided by the biggest river flow in the adjacent 10*16x10*16 tiles region, scaled on a range 0..1000.
(2) Set up some CityPlacers. A CityPlacer is called for some 16x16 grid section, and then decides based on its CityScore wether it is interested in placing a city or town there. If yes, it tries to find an appropriate tile in that section. CityPlacers typically have just a few lines of code, as their whole job is inspecting one or a few aggregated values. E.g. the HugeRiverCityPlacer only accepts grid sections with a river_size_score >= 300, which indicate a rather big river.
(3) Each CityPlacer is assigned a probability with which it is used, again on a scale 0..1000. Probability 500 e.g. means that it is used (in average) for every second attempt to find a grid section for a city.
(4) First use a rather big city appropriate configuration for placing 1/3 cities, and 2/3 towns, then use another configuration for placing just towns.

This is the rough picture how it generally works. As I have said, there are quite a lot of parameters with which one can play, also of course other CityPlacers with different ideas based on that concepts are possible.

What is currently missing is a GUI for configuring all those values. A good strategy for testing it is, temporarily removing all but one CityPlacer, and having a look what exactly that CityPlacer does. E.g. a RiverBranchCityPlacer is supposed to place city at positions where rivers intersect.

[ic111]

As an additional note, if you want to (permanently) turn off the new city generation in the current version, you can do this by not applying the last patch in the queue.

[wallyweb]

The next release. This time, I improved city / town generation.

make failed when compiling for Win32 binary using MinGW.

[ic111]

Thank you for testing. However, to be honest, I don´t understand any of the error messages. In my source code (with all patches until including 320 applied),

- class RainfallRiverGenerator exists, but has no destructor
- RG_ORIGINAL and RG_RAINFALL are declared as part of enum RiverGenerator in genworld.h

To be honest, beside "make clean" and "make" or checking out a new, clean, copy of the source tree (if you didn´t do that anyway) I have no clue what you or I might do about that.

Can anyone help with advice, or tell me what mistake I might have done (if any)?

[Alberth]

Maybe "on-1_5_2" means Wallyweb uses 1.5.2?

Trunk compiles here (at Linux), with a few warnings:

Code: Select all

src/rivers_rainfall.cpp: In member function 'void NumberOfLowerConnectedComponentCalculator::DiscardTilesEdgeUp(TileIndex*, Slope*, int*, Direction, Slope, Direction, Direction, Direction, Direction)':
src/rivers_rainfall.cpp:200:83: warning: comparison of constant '10' with boolean expression is always true [-Wbool-compare]
    invalidate_mask[diagonal_neighbor_one] = ((slope_one != SLOPE_EW && slope_one) != SLOPE_NS);
                                                                                   ^
src/rivers_rainfall.cpp:201:83: warning: comparison of constant '10' with boolean expression is always true [-Wbool-compare]
    invalidate_mask[diagonal_neighbor_two] = ((slope_two != SLOPE_EW && slope_two) != SLOPE_NS);
                                                                                   ^
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)':
src/rivers_rainfall.cpp:2703:182: warning: format '%u' expects argument of type 'unsigned int', but argument 4 has type 'std::vector<TileWithValue>::size_type {aka long unsigned int}' [-Wformat=]
src/rivers_rainfall.cpp: In member function 'void RainfallRiverGenerator::LowerHigherWaterTilesUntilValid(std::set<unsigned int>&, int*, byte*)':
src/rivers_rainfall.cpp:3364:180: warning: format '%u' expects argument of type 'unsigned int', but argument 5 has type 'std::set<unsigned int>::size_type {aka long unsigned int}' [-Wformat=]

The "always true" warnings look serious to me.

[wallyweb]

Can anyone help with advice, or tell me what mistake I might have done (if any)?

The mistake might be mine.
Considering:

As an additional note, if you want to (permanently) turn off the new city generation in the current version, you can do this by not applying the last patch in the queue.

I assumed the last patch to be 320_GenerateCities.patch
My goal was to create two binaries ... one with new city generation turned off and one with new city generation turned on.
I set up two folders, one for off and one for on.
I copied and applied your patches, except for 320, to the off folder.
I then, before compiling, copied the contents of the off folder into the on folder where I then applied your 320 patch.
I then tried to compile each folder independently.
I just now looked into the v12 and v13 zips and I see that they both have the 320 patch.
My off folder was corrupted because it did not include a 320 patch.
The corruption followed the copy into the on folder and applying the 320 patch did not fix it.
I will now do a clean patching and compile with 320 in place to verify that this was indeed the source of the problem.

[Alberth]

I tried trunk r27396 + patch without 320, and + patch with 320. Both worked for me (with the same warnings).

[wallyweb]

Maybe "on-1_5_2" means Wallyweb uses 1.5.2?

Indeed it does. The "on" indicates that the last patch (320...) was applied.
A clean patch application (including 320...) and compile succeeded:

OpenTTD 1_5_2 Rainfall_v14_cityOn.7z

Windows 32bit binary
city generator on

(4.37 MiB) Downloaded 87 times

Later this evening I will try the "off" version without 320... DONE

A clean patch application (without 320...) and compile succeeded:

OpenTTD 1_5_2 Rainfall_v14_cityOff.7z

Windows 32bit binary
city generator off

(4.37 MiB) Downloaded 82 times

And now a new issue ...
With both versions in hand, I tested in the Scenario Editor.
The "Off" (No cities) worked fine.
The "On" (Build cities) crashed in Windows (no crash log) immediately after clicking on Generate in the World Generation Window.

generate_crash.png (40.6 KiB) Viewed 3131 times

[ic111]

Ok, thank you, I will have a look on these warnings (I didn't get them...), and on the crash you report.

[ic111]

A bugfix release, thanks for all bug reports above.

Fixed:
- The warning about a senseless comparison was indeed triggered by a bug: The brackets were wrongly placed. This shouldn´t have triggered crashes, but it might have lead to in some circumstances wrong calculations (one case of the decision where water from a tile can flow to was affected)
- The harmless warnings about format specifiers in debug messages are hopefully gone (didn´t reproduce them myself, but I already fixed similar ones in the past)
- The reported crash was a division by zero. You tried to generated a 64x64 map. The generator generated no river and lake tiles at all. And the CityPlacer divided by the total amount of lake tiles...

[wallyweb]

A bugfix release, thanks for all bug reports above.

- The reported crash was a division by zero. You tried to generated a 64x64 map. The generator generated no river and lake tiles at all. And the CityPlacer divided by the total amount of lake tiles...

I tested with this release v.15, and the minimun safe map size is 256x256.
The game will crash at 64x64 or 128x128.

Here are the new Windows 32bit binaries:

City generation = OFF

OpenTTD 1_5_2 Rainfall_v15_cityOff.7z

Windows 32bit
City generation = OFF

(4.37 MiB) Downloaded 86 times

City generation = ON
Set Minimum Map Size 256x256

OpenTTD 1_5_2 Rainfall_v15_cityOn.7z

Windows 32bit
City generation = ON
Set Minimum Map Size 256x256

(4.37 MiB) Downloaded 86 times

Enjoy.

[ic111]

Fixed a bug that could occur only on small map sizes

(for getting a notion of "how big a river is", the CityPlacer inspects a region of five of the 16x16 grid sections into each direction. Near the map edge, that region is considerably smaller than in the middle of the map, simply because you cannot step 5 * 16 tiles to the north if you already are at the northern corner of the map. This can lead to the effect, that rivers near the map edge look bigger than they are using that criterium, which results in significantly more cities near the map edge than in the middle of the map.

Thus, in such cases I expand the region accordingly towards the middle of the map, and that calculation needs an extra case for the case that 10 * 16 tiles southeast of the current grid, the other map edge is passed because the map is so small.

That calculation had a wrong cast to uint...)

I guess you ran into that issue, but I can´t be sure, since I have no stacktrace, on my system the crashes didn´t occur.

[Comm Cody]

The main issue I run into with this patch is that on my current laptop, maps bigger than 1024x1024 wonk up on me.

[wallyweb]

Fixed a bug that could occur only on small map sizes

Compiled and tested and all is well:

City generation = OFF

OpenTTD 1_5_2 Rainfall_v16_cityOff_Win32.7z

City generation = OFF

(4.37 MiB) Downloaded 82 times

City generation = ON

OpenTTD 1_5_2 Rainfall_v16_cityOn_Win32.7z

City generation = ON

(4.37 MiB) Downloaded 77 times

The main issue I run into with this patch is that on my current laptop, maps bigger than 1024x1024 wonk up on me.

I can confirm this with both city = ON and city = OFF.
The largest map I can get is 512x512.
Larger maps crash the game in Windows7 without a crash log.

[Eddi]

you probably want to turn the city generation thing into a setting.

[ic111]

Yes, indeed. Something similar than I did for the concept of a river generator (before I started with this patch, there wasn't any infrastructure for choosing a river generator either) would probably be needed. Although the fact that my city generator only works together with my river generator makes things a bit complicate.

And a separate settings dialog for the city settings would also be desirable.

It's a question of (a) having time for it and (b) learning how settings should work at all...

[Eddi]

i'd use the settings that control the indiviual water edges as a starting point, they also get disabled when the main setting overrrides it.

as for choosing the river generator, there is already infrastructure for choosing a tree generator.

[ic111]

For river generators, I solved this already a long time ago - patches 002 and 005 in the queue, which (at least the 002) can be used independent of the remaining queue.

Here is the next, rather intermediate, release.

You, and I also, noticed that in some cases, the river beds were digged to a surprisingly low level, compared with the original heightmap state. E.g., in my heightmap test case, I have prepared a vally for a huge river, gradually ascending from sea level to level 8.

I frequently noticed, that this river for a very long time stayed at height 2, and in the end ascended quite quickly to level 8.

The reason for this was, that in the last calculation step, function LowerHigherWaterTilesUntilValid and functions called from there, the condition for generating inclined slopes were too hard. For example, I took the flow direction into account, which turned out to be a rather bad idea (at least in that implementation) for wide rivers, as the process of making a river wider is not based on flow directions.

And if that function isn´t able to generate a inclined slope towards the next height level, it generates a flat slope at the current height level. This process, performed in an iterative manner, can lower quite a long section of a river, when the condition for ascending to the next level is too hard.

The drawback, and the reason why I regard this version as intermediate, is that now sometimes, rivers flow upwards on the small scale, because that fine tuning step decided wrong. It happens not too often, so the maps are usable, but you can find locations were the water ignores physics, when you search them...