Patch: The Rainfall River Generator
Moderator: OpenTTD Developers
Re: Patch: The Rainfall River Generator
Do I understand it right, your problem is that instead of the startup game, you see a blue area?
If yes, I have no clue what might cause that. It doesn´t smell like anything I remember to have changed.
Anyway, thank you for the binary.
If yes, I have no clue what might cause that. It doesn´t smell like anything I remember to have changed.
Anyway, thank you for the binary.
Re: Patch: The Rainfall River Generator
Yes. It can't be seen in the png but the blue area is water tiles.ic111 wrote:Do I understand it right, your problem is that instead of the startup game, you see a blue area?
Anyway, thank you for the binary.
wallyweb on tt-forums: Screenshots - Wallyweb World - Projects & Releases
wallyweb on Simuscape: Projects - Releases
Other Stuff: TTDPatch 2.6 "Nightly" download - cirdan's OpenTTD branch (New Map Features)
Screenshot Of The Month Contest Winner: August 2015 - Tied May 2016 - January 2018 - December 2018 - May 2019
wallyweb on Simuscape: Projects - Releases
Other Stuff: TTDPatch 2.6 "Nightly" download - cirdan's OpenTTD branch (New Map Features)
Screenshot Of The Month Contest Winner: August 2015 - Tied May 2016 - January 2018 - December 2018 - May 2019
Re: Patch: The Rainfall River Generator
a blue starting area means one of two things:
- it couldn't find the opntitle.dat file, or it is corrupted
- loading the opntitle.dat file failed due to savegame conversion errors
Re: Patch: The Rainfall River Generator
Here is version 43. Changes:
- Improved documentation around patch 40 (number of lower tiles)
- Removed the concept of a HeightLevelIterationMode. It is no longer necessary, as the improved code written in the last few weeks always used HLIM_ALL.
- Removed some unnecessary code regarding terraforming for rivers
Generally, I don´t think providing windows binary for this sort of change is necessary - a player shouldn´t notice that kind of change, they are purely improvements in the background.
Regarding the startup game: I only add config options with SGF_NEWGAME_ONLY, and recently removed the savegame bump. Maybe I somehow affected the ability to load the intro game by doing the latter?
Wallyweb: If you would give as that file (opntitle.dat), then I could try wether I can reproduce the problem.
- Improved documentation around patch 40 (number of lower tiles)
- Removed the concept of a HeightLevelIterationMode. It is no longer necessary, as the improved code written in the last few weeks always used HLIM_ALL.
- Removed some unnecessary code regarding terraforming for rivers
Generally, I don´t think providing windows binary for this sort of change is necessary - a player shouldn´t notice that kind of change, they are purely improvements in the background.
Regarding the startup game: I only add config options with SGF_NEWGAME_ONLY, and recently removed the savegame bump. Maybe I somehow affected the ability to load the intro game by doing the latter?
Wallyweb: If you would give as that file (opntitle.dat), then I could try wether I can reproduce the problem.
- Attachments
-
- rivers_v43.zip
- (283.24 KiB) Downloaded 114 times
Re: Patch: The Rainfall River Generator
I just ran a diff for opntitle.dat in trunk versus v.42 and ExamDiff tells me they are exactly the same so the file has not been corrupted.ic111 wrote:Wallyweb: If you would give as that file (opntitle.dat), then I could try wether I can reproduce the problem.
Here is the file: I will compile v43 later this evening (after my dinner ) to verify if the problem followed your changes.
wallyweb on tt-forums: Screenshots - Wallyweb World - Projects & Releases
wallyweb on Simuscape: Projects - Releases
Other Stuff: TTDPatch 2.6 "Nightly" download - cirdan's OpenTTD branch (New Map Features)
Screenshot Of The Month Contest Winner: August 2015 - Tied May 2016 - January 2018 - December 2018 - May 2019
wallyweb on Simuscape: Projects - Releases
Other Stuff: TTDPatch 2.6 "Nightly" download - cirdan's OpenTTD branch (New Map Features)
Screenshot Of The Month Contest Winner: August 2015 - Tied May 2016 - January 2018 - December 2018 - May 2019
Re: Patch: The Rainfall River Generator
Great patch. It now works with large maps, very useful (there is no more need to import all rivers from txt file into scenario editor).
One small improvement suggestion: is it possible to carve out river, if there is small terrain obstacle?
For example: (or maybe I miss something, I have lake outflow probability 500, maybe other options count?)
EDIT: I had 'flow consumed per lake volume' 100, because I misunderstand this option.
EDIT2: unfortunately, 1.5.2 RFRG map does not work with regular 1.5.2 version - impossible to load map (for example, it is impossible to join 1.5.2RFRG server with normal 1.5.2 client).
Is it possible to fix that?
One small improvement suggestion: is it possible to carve out river, if there is small terrain obstacle?
For example: (or maybe I miss something, I have lake outflow probability 500, maybe other options count?)
EDIT: I had 'flow consumed per lake volume' 100, because I misunderstand this option.
EDIT2: unfortunately, 1.5.2 RFRG map does not work with regular 1.5.2 version - impossible to load map (for example, it is impossible to join 1.5.2RFRG server with normal 1.5.2 client).
Is it possible to fix that?
Formerly known as: McZapkie
Projects: Reproducible Map Generation patch, NewGRFs: Manpower industries, PolTrams, Polroad, 600mm narrow gauge, wired, ECS industry extension, V4 CEE train set, HotHut.
Another favorite games: freeciv longturn, OHOL/2HOL.
Projects: Reproducible Map Generation patch, NewGRFs: Manpower industries, PolTrams, Polroad, 600mm narrow gauge, wired, ECS industry extension, V4 CEE train set, HotHut.
Another favorite games: freeciv longturn, OHOL/2HOL.
Re: Patch: The Rainfall River Generator
It reports "Unexpected end of chunk".wallyweb wrote: Here is the file:
So, maybe I indeed do something which is forbidden unless one distributes a new savegame version (which I don´t do, and actually don´t want to do if I can avoid it). Are you (or someone else) able to load your favorite savegame (of unpatched trunk or an official OpenTTD release)? (question is basically answered below).
Indeed, this is the option you need. If you set that value to zero, all lakes will fill up, until they find an outflow. The bigger the value is, the more probable it is that flow gets exhausted before the lake overflows.EDIT: I had 'flow consumed per lake volume' 100, because I misunderstand this option.
Seems that this answers my question from above. Yes, of course this is a bug I want to fix...EDIT2: unfortunately, 1.5.2 RFRG map does not work with regular 1.5.2 version - impossible to load map (for example, it is impossible to join 1.5.2RFRG server with normal 1.5.2 client).
Is it possible to fix that?
Re: Patch: The Rainfall River Generator
Do you remember in which version you did this?ic111 wrote: and recently removed the savegame bump.
wallyweb on tt-forums: Screenshots - Wallyweb World - Projects & Releases
wallyweb on Simuscape: Projects - Releases
Other Stuff: TTDPatch 2.6 "Nightly" download - cirdan's OpenTTD branch (New Map Features)
Screenshot Of The Month Contest Winner: August 2015 - Tied May 2016 - January 2018 - December 2018 - May 2019
wallyweb on Simuscape: Projects - Releases
Other Stuff: TTDPatch 2.6 "Nightly" download - cirdan's OpenTTD branch (New Map Features)
Screenshot Of The Month Contest Winner: August 2015 - Tied May 2016 - January 2018 - December 2018 - May 2019
Re: Patch: The Rainfall River Generator
Two hours of debugging in the innermost sections of OpenTTD settings & saveload handling... Below, I describe the facts I found so far, if anyone gets a clue what might be wrong based on my findings below, hints are quite welcome.
First of all, here is a new release, I adjusted some of the flags. Now, I define my settings in settings.ini using
This seems to fix a part of the problem, but not all. Now, I run into "Invalid chunk size" in SlLoadChunk, in this code:
If I get it right, then len is the chunk length actually stored in the savegame, whereas the reader increases its position based on the SaveLoad definitions. I definitely know that the error is thrown in Load_PATS, i.e. while loading the settings. Not surprising, as I don´t have any savegame-related changes beside new settings.
Here, I have (based on gdb):
i.e., I seem to read more than I´m allowed to read based on the contents of the savegame. The difference is 73 bytes, the sum of my new config variables (except the length of the char*) is 34 bytes, i.e. this is no clear hint.
The SaveLoad definition is defined in the generated file objs/setting/table/settings.h, e.g.
i.e., the flags SGF_NEWGAME_ONLY, SLF_NOT_IN_SAVE, and SLF_NO_NETWORK_SYNC are propagated. Looking at the definition of SDT_VAR, you end up at SDT_GENERAL, i.e.
Note: The flags are passed as fourth parameter. Now look at the definition of SLE_GENERAL, which if I get it right is supposed to produce a SaveLoad instance:
... The fourth parameter (type) is passed to the third parameter of the resulting struct, which is...
i.e. passed to conv. And indeed, in SlIsObjectValidInSavegame, called from SlObjectMember, we have
... SlObjectMember is implemented as follows:
Everything seems fine, nevertheless I seem to read something I shouldn´t try to read.
Now, if anyone has a clue what might be going on here, this would be a quite good message, otherwise, this will probably become a second debugging session...
Revision 41, here: http://www.tt-forums.net/viewtopic.php? ... 0#p1159349Do you remember in which version you did this?
First of all, here is a new release, I adjusted some of the flags. Now, I define my settings in settings.ini using
Code: Select all
SGF_NEWGAME_ONLY | SLF_NOT_IN_SAVE | SLF_NO_NETWORK_SYNC
Code: Select all
/* Read length */
len = (SlReadByte() << 16) | ((m >> 4) << 24);
len += SlReadUint16();
_sl.obj_len = len;
endoffs = _sl.reader->GetSize() + len;
ch->load_proc();
if (_sl.reader->GetSize() != endoffs) {
SlErrorCorrupt("Invalid chunk size");
}
Here, I have (based on gdb):
Code: Select all
_sl.reader->GetSize() = 656114
endoffs = 656041
The SaveLoad definition is defined in the generated file objs/setting/table/settings.h, e.g.
Code: Select all
SDT_VAR(GameSettings, game_creation.rainfall.flow_for_river, SLE_INT32, 0, SGF_NEWGAME_ONLY | SLF_NOT_IN_SAVE |
SLF_NO_NETWORK_SYNC, DEF_MODERATE_RIVERS_FLOW_FOR_RIVER, MIN_FLOW_FOR_RIVER, MAX_FLOW_FOR_RIVER, 0,
STR_CONFIG_SETTING_RAINFALL_FLOW_FOR_RIVER, STR_CONFIG_SETTING_RAINFALL_FLOW_FOR_RIVER_HELPTEXT, STR_NULL,
NULL, 194, SL_MAX_VERSION, SC_ADVANCED),
Code: Select all
#define SDT_GENERAL(name, sdt_cmd, sle_cmd, type, flags, guiflags, base, var, length, def, min, max, interval, full, str, strhelp,
strval, proc, load, from, to, cat)\
{NSD_GENERAL(name, def, sdt_cmd, guiflags, min, max, interval, full, str, strhelp, strval, proc, load, cat),
SLE_GENERAL(sle_cmd, base, var, type | flags, length, from, to)}
Code: Select all
#define SLE_GENERAL(cmd, base, variable, type, length, from, to) {false, cmd, type, length, from, to, (void*)cpp_offsetof(base, variable),
cpp_sizeof(base, variable)}
Code: Select all
struct SaveLoad {
bool global; ///< should we load a global variable or a non-global one
SaveLoadType cmd; ///< the action to take with the saved/loaded type, All types need different action
VarType conv; ///< type of the variable to be saved, int
uint16 length; ///< (conditional) length of the variable (eg. arrays) (max array size is 65536 elements)
(...)
Code: Select all
static inline bool SlIsObjectValidInSavegame(const SaveLoad *sld)
{
if (_sl_version < sld->version_from || _sl_version > sld->version_to) return false;
if (sld->conv & SLF_NOT_IN_SAVE) return false;
return true;
}
Code: Select all
VarType conv = GB(sld->conv, 0, 8);
switch (sld->cmd) {
case SL_VAR:
case SL_REF:
case SL_ARR:
case SL_STR:
case SL_LST:
/* CONDITIONAL saveload types depend on the savegame version */
if (!SlIsObjectValidInSavegame(sld)) return false;
if (SlSkipVariableOnLoad(sld)) return false;
(...)
Now, if anyone has a clue what might be going on here, this would be a quite good message, otherwise, this will probably become a second debugging session...
- Attachments
-
- rivers_v44a.zip
- (237.92 KiB) Downloaded 94 times
Re: Patch: The Rainfall River Generator
Your additional settings all include the line:
SLF_NOT_IN_SAVE and SLF_NO_NETWORK_SYNC are defined in and have the type of enum VarTypes, and should go in the flags field, not the guiflags field. Putting them in the guiflags will not have the desired effect.
Unfortunately C++ enums (before C++11 typed enums) are not type safe, and so you need to check manually that you are in fact matching the types correctly.
Have a look at the existing game_creation settings for what the fields should look like.
Code: Select all
guiflags = SGF_MULTISTRING | SGF_NEWGAME_ONLY | SLF_NOT_IN_SAVE | SLF_NO_NETWORK_SYNC
Unfortunately C++ enums (before C++11 typed enums) are not type safe, and so you need to check manually that you are in fact matching the types correctly.
Have a look at the existing game_creation settings for what the fields should look like.
Ex TTDPatch Coder
Patch Pack, Github
Patch Pack, Github
Re: Patch: The Rainfall River Generator
Thank you very much, this indeed was the problem.
This release should fix the issues with loading savegames.
This release should fix the issues with loading savegames.
- Attachments
-
- rivers_v45.zip
- (255.64 KiB) Downloaded 101 times
Re: Patch: The Rainfall River Generator
Indeed! The intro window now displays properly.ic111 wrote:This release should fix the issues with loading savegames.
For those who would like to test this release in Windows, here is the Windows 32 bit binary:
Enjoy
wallyweb on tt-forums: Screenshots - Wallyweb World - Projects & Releases
wallyweb on Simuscape: Projects - Releases
Other Stuff: TTDPatch 2.6 "Nightly" download - cirdan's OpenTTD branch (New Map Features)
Screenshot Of The Month Contest Winner: August 2015 - Tied May 2016 - January 2018 - December 2018 - May 2019
wallyweb on Simuscape: Projects - Releases
Other Stuff: TTDPatch 2.6 "Nightly" download - cirdan's OpenTTD branch (New Map Features)
Screenshot Of The Month Contest Winner: August 2015 - Tied May 2016 - January 2018 - December 2018 - May 2019
Re: Patch: The Rainfall River Generator
And another release. Changes:
- I removed the fan delta lake manipulation, as that setting didn´t have the desired effect.
- Improved source code comments
- I made the factor used for removing tiny oceans, and for removing tiny basins configurable. But only in the config file. If considered important, the GUI might be adjusted at a later time.
The idea behind that step is:
- Tiny oceans especially in tgp tend to consume huge rivers, which doesn´t look realistic at all. Thus, I remove all oceans smaller than MapSize() / small_oceans_removal_factor
- Small basins tend to trigger lakes too often, and additionally cost performance while setting up lakes. Thus, eliminating those small basins is a good idea before starting.
Removing in both cases means raising land. The default values are MapSize() / 1000 for tiny oceans, and 13 for tiny basins.
- I removed the fan delta lake manipulation, as that setting didn´t have the desired effect.
- Improved source code comments
- I made the factor used for removing tiny oceans, and for removing tiny basins configurable. But only in the config file. If considered important, the GUI might be adjusted at a later time.
The idea behind that step is:
- Tiny oceans especially in tgp tend to consume huge rivers, which doesn´t look realistic at all. Thus, I remove all oceans smaller than MapSize() / small_oceans_removal_factor
- Small basins tend to trigger lakes too often, and additionally cost performance while setting up lakes. Thus, eliminating those small basins is a good idea before starting.
Removing in both cases means raising land. The default values are MapSize() / 1000 for tiny oceans, and 13 for tiny basins.
- Attachments
-
- rivers_v46.zip
- (262.09 KiB) Downloaded 102 times
Re: Patch: The Rainfall River Generator
And another binary ...ic111 wrote:And another release.
Enjoy
wallyweb on tt-forums: Screenshots - Wallyweb World - Projects & Releases
wallyweb on Simuscape: Projects - Releases
Other Stuff: TTDPatch 2.6 "Nightly" download - cirdan's OpenTTD branch (New Map Features)
Screenshot Of The Month Contest Winner: August 2015 - Tied May 2016 - January 2018 - December 2018 - May 2019
wallyweb on Simuscape: Projects - Releases
Other Stuff: TTDPatch 2.6 "Nightly" download - cirdan's OpenTTD branch (New Map Features)
Screenshot Of The Month Contest Winner: August 2015 - Tied May 2016 - January 2018 - December 2018 - May 2019
Re: Patch: The Rainfall River Generator
I found some graphical glitches:
river going on the top of small hill (and many others with rivers traversing instead of flowing down the mountain gradient).
Is it possible to lower terrain 1 tile (except the lowest one) before river placement?
I know you have some erosion included, but maybe it need to be tweaked. What about flow speed?
High gradient should make much higher erosion than flat area.
it looks like river going up and down.river going on the top of small hill (and many others with rivers traversing instead of flowing down the mountain gradient).
Is it possible to lower terrain 1 tile (except the lowest one) before river placement?
I know you have some erosion included, but maybe it need to be tweaked. What about flow speed?
High gradient should make much higher erosion than flat area.
Formerly known as: McZapkie
Projects: Reproducible Map Generation patch, NewGRFs: Manpower industries, PolTrams, Polroad, 600mm narrow gauge, wired, ECS industry extension, V4 CEE train set, HotHut.
Another favorite games: freeciv longturn, OHOL/2HOL.
Projects: Reproducible Map Generation patch, NewGRFs: Manpower industries, PolTrams, Polroad, 600mm narrow gauge, wired, ECS industry extension, V4 CEE train set, HotHut.
Another favorite games: freeciv longturn, OHOL/2HOL.
Re: Patch: The Rainfall River Generator
Thank you, this is indeed a case my river-may-not-flow-upwards-detection misses so far. The problem here is that two rivers merge, but my detection works river-wise so far. It will be a bit tricky to improve, but I will give it a try.McZapkie wrote: it looks like river going up and down.
On the one hand, I would to some extent consider that an artefact of OpenTTDs abstraction of the world.river going on the top of small hill (and many others with rivers traversing instead of flowing down the mountain gradient).
On the other hand, the generator (very) roughly works in the following steps:
(1) Calculate flow based on number of lower tiles measure
(2) Wider rivers, wider valleys (involves random terraforming)
(3) Fine tune terrain to make slopes valid against rivers, while keeping them connected
The difficult thing about (3) is that I need to do terraforming that does damage landscape as little as possible. Thus, one of my strategies is, perform (among a number of possible terraformings) that one that fixes the greatest number of planned river tiles with respect to their slope.
If I would not do that, then I could easily run into situations where one accidentally lowers half of a lake, and similar things. Short: Situations that look considerably worse than anything you presented here as screenshots.
So, to conclude, such things trigger the question: Should one add another postprocessing step with even more code, or should one live with the result one sees here?
No, I would not like to do that. I think rivers flowing through a valley, being at the same heightlevel as the valley, fit more into the scale of OpenTTD than if the rivers would be one tile lower (and, I also think they look better).Is it possible to lower terrain 1 tile (except the lowest one) before river placement?
Would be a new concept. Erosion doesn´t exist as concept so far.I know you have some erosion included, but maybe it need to be tweaked. What about flow speed?
High gradient should make much higher erosion than flat area.
Re: Patch: The Rainfall River Generator
Another post, as I want to write some sentences about the general state of this patch.
My goal during the last few weeks was improving it such that I can regard it more or less finished. Of course with the goal to make it (at least from my side) possible that it eventually becomes a / the river generator of OpenTTD. Which, obviously, is not my decision.
Thus, to summarize its contents and state:
- The patches until 019 deal with the genworld / GUI infrastructure (config settings & dialogs, etc.)
- Patches 020 to 105 deal with deciding where rivers and lakes are supposed to be created, and with placing them on landscape, including some basic terraforming.
- Patch 130 makes rivers look less straight
- Patches 140 to 190 modify the shape of lakes
- Patches 200 introduces wider rivers and valleys
- Patches 220 to 270 sets up a less landscape-damaging terraforming scheme than patch 105 does (which nevertheless is necessary as those later steps are not applicable to 100% of the tiles, just for, say, 98%), and fine tunes some river situations
- Patch 300 is the generation progress.
Here, the river generator is complete. Everything after this patch is for town placing, in detail
- Patch 310 sets up an API for that
- Patch 312 collects the information, often river-related
- Patch 314 introduces the concrete town placement strategies
- Patches 320 to 348 introduce the town placer config GUI
- Patches 360 and 370 finally builds things together.
- Side note: Patches 000, 035, 045, 255 are purely for debugging.
To summarize, a potential review of the river generator would need to cover the patches until 300. Due to the size of the queue, probably a reasonable approach would be, start at the first patch(es), and discuss them in increasing order, one-by-one or group-by-group, but not all at a time.
Maybe after a look on the general picture.
In fact, most of that queue is quite self-contained, and should not affect the remaining game. E.g., making the terraforming API accessible from landscape generation is not meant to change anything functional regarding the existing terraforming.
A few of them, e.g. patch 020, are quite library-oriented either, in that they provide general code that is not exactly river-oriented, but might be reused elsewhere when needed.
My primary goal now is, take that piece of work into a stable state. For this, people generating their favorite heightmap with this river generator and reporting on that (as McZapkie did) are needed.
For me, the most important question is, does it run stable (no crashes), and in a reasonable time?
Compared to that, I´m quite reluctant regarding more and more fine-tuning steps, which always have the danger of introducing new bugs (and, it also is a question of real world time).
So far some general words about that patch queue, have fun with it.
My goal during the last few weeks was improving it such that I can regard it more or less finished. Of course with the goal to make it (at least from my side) possible that it eventually becomes a / the river generator of OpenTTD. Which, obviously, is not my decision.
Thus, to summarize its contents and state:
- The patches until 019 deal with the genworld / GUI infrastructure (config settings & dialogs, etc.)
- Patches 020 to 105 deal with deciding where rivers and lakes are supposed to be created, and with placing them on landscape, including some basic terraforming.
- Patch 130 makes rivers look less straight
- Patches 140 to 190 modify the shape of lakes
- Patches 200 introduces wider rivers and valleys
- Patches 220 to 270 sets up a less landscape-damaging terraforming scheme than patch 105 does (which nevertheless is necessary as those later steps are not applicable to 100% of the tiles, just for, say, 98%), and fine tunes some river situations
- Patch 300 is the generation progress.
Here, the river generator is complete. Everything after this patch is for town placing, in detail
- Patch 310 sets up an API for that
- Patch 312 collects the information, often river-related
- Patch 314 introduces the concrete town placement strategies
- Patches 320 to 348 introduce the town placer config GUI
- Patches 360 and 370 finally builds things together.
- Side note: Patches 000, 035, 045, 255 are purely for debugging.
To summarize, a potential review of the river generator would need to cover the patches until 300. Due to the size of the queue, probably a reasonable approach would be, start at the first patch(es), and discuss them in increasing order, one-by-one or group-by-group, but not all at a time.
Maybe after a look on the general picture.
In fact, most of that queue is quite self-contained, and should not affect the remaining game. E.g., making the terraforming API accessible from landscape generation is not meant to change anything functional regarding the existing terraforming.
A few of them, e.g. patch 020, are quite library-oriented either, in that they provide general code that is not exactly river-oriented, but might be reused elsewhere when needed.
My primary goal now is, take that piece of work into a stable state. For this, people generating their favorite heightmap with this river generator and reporting on that (as McZapkie did) are needed.
For me, the most important question is, does it run stable (no crashes), and in a reasonable time?
Compared to that, I´m quite reluctant regarding more and more fine-tuning steps, which always have the danger of introducing new bugs (and, it also is a question of real world time).
So far some general words about that patch queue, have fun with it.
Re: Patch: The Rainfall River Generator
I compiled v46 and got a few compiler warnings of note.
Most significantly was in patch 348, in genworld.cpp. std::string::npos is negative, and the result of std::string.find and the return type of FindMatchingCloseBracket are stored in a size_t, which is unsigned, so the tests for std::string::npos may be broken, and things may not won't work if the close bracket is actually missing. std::string::size_type should probably be used instead.
There were also a number of warnings about format strings in DEBUG macros. On a number of platforms, size_t is not the same size or signedness as int, so the compiler is liable to complain if using %i for size_t arguments. PRINTF_SIZE (%zu) should probably be used instead.
I had a (very) quick skim through the patches up to 300, when I get some more time I might have a closer look. Aside from whitespace, one thing I did notice is in patch 105, the variable names of clause_one to clause_eleven are a bit mystifying.
Most significantly was in patch 348, in genworld.cpp. std::string::npos is negative, and the result of std::string.find and the return type of FindMatchingCloseBracket are stored in a size_t, which is unsigned, so the tests for std::string::npos may be broken, and things may not won't work if the close bracket is actually missing. std::string::size_type should probably be used instead.
There were also a number of warnings about format strings in DEBUG macros. On a number of platforms, size_t is not the same size or signedness as int, so the compiler is liable to complain if using %i for size_t arguments. PRINTF_SIZE (%zu) should probably be used instead.
I had a (very) quick skim through the patches up to 300, when I get some more time I might have a closer look. Aside from whitespace, one thing I did notice is in patch 105, the variable names of clause_one to clause_eleven are a bit mystifying.
Ex TTDPatch Coder
Patch Pack, Github
Patch Pack, Github
Re: Patch: The Rainfall River Generator
But terraforming is performed before river placement, if I understand correctly?ic111 wrote: If I would not do that, then I could easily run into situations where one accidentally lowers half of a lake, and similar things.
Therefore any terraforming would not destroy any river or lake, because there is none yet.
After flow calculations and terraforming with use of flow erosion, you can just perform standard river flow procedure
(of course with your width/curvature modifications, and with starting points calculated from flow calculations).
BTW, currently some rivers are very short, even 1 tile short. If I remember correctly, original procedure cancel river creation if length is below 'min_river_length'.
I agree if speaking about rivers flowing trough plain lowland (in case of openttd, usually level 1), but in case of any hills, there are usually canyons/narrower valleys. Despite of personal aesthetic thoughts, such valleys are practical, because allow to place nice flat bridges instead of bumpy ones.I think rivers flowing through a valley, being at the same height level as the valley, fit more into the scale of OpenTTD than if the rivers would be one tile lower (and, I also think they look better).
P.S.
A general note - I'm not sure, if flow direction is necessary to store for each tile. Maybe only flow amount is important to store?
Flow direction is just a local flow gradient. Just in case, if you need some memory allocation optimisation.
Re: Patch: The Rainfall River Generator
Thanks, I´ll have a look at them. Do you use standard compiler warning settings, or do you use somewhat more restrictive settings?JGR wrote:I compiled v46 and got a few compiler warnings of note.
I admit I never had a close look on those settings, but maybe I should, as this is not the first time people report about compiler warnings I never saw...
Indeed. And is already used at most points of code, what you found are the exceptions...PRINTF_SIZE (%zu) should probably be used instead.
Thank you. Probably, at this point of time concentrating on those up to about patch 35 would be best, in a second step those including number of lower tiles, flow, and lake definition (i.e. up to patch 70).I had a (very) quick skim through the patches up to 300, when I get some more time I might have a closer look.
The later patches hopefully won't change in the big scale, but might be affected if people find flaws, or suggest improvements during testing. E.g. what McZapkie found regarding upwards flow will trigger some changes in the patch determining logical rivers.
In contrast to that, things like the height index did not change for a very long time now.
I'll have a look at it, at some point in time I needed those clauses in a separate way in the debug statements...Aside from whitespace, one thing I did notice is in patch 105, the variable names of clause_one to clause_eleven are a bit mystifying.
The general picture is:But terraforming is performed before river placement, if I understand correctly?
(1) Decide which tiles are supposed to become rivers and lakes (compare functions like IsOrdinaryLakeTile).
(2) As those tiles can have any slope, perform terraforming such that all planned river/lake tiles have valid slope
(3) Actually make the tiles River tiles in the OpenTTD sense.
So, this is not true. The whole point is just, that terraforming things such that slopes are ok, and you not as a side effect destroy the river three tiles downwards or upwards some slope is a quite hard problem.Therefore any terraforming would not destroy any river or lake, because there is none yet.
I have my helper functions for this, so I might even try to terraform lower land near a river, but it's just that such processing costs me at least two or three evenings, and leaves the patch by again some hundred lines of code bigger.
Yes, that's the case. I create a river if flow exceeds some limit, say 200, and if a flow patch exceeds that limit just one or two tiles before the ocean, you get such a river.BTW, currently some rivers are very short, even 1 tile short. If I remember correctly, original procedure cancel river creation if length is below 'min_river_length'.
Maybe misunderstanding: None of those values is meant to be stored permanently. I only have some debug patches in the queue that show those values in the land info window as long as you did not exit the game after generation.A general note - I'm not sure, if flow direction is necessary to store for each tile. Maybe only flow amount is important to store?
Needed during calculation are all of them. In fact, flow amount are 4 bytes, and in a fifth byte, flow direction is stored as 3 bit, and the remaining 5 bits are used for storing predicates like "planned river tile", or "planned ordinary lake tile".
Who is online
Users browsing this forum: No registered users and 60 guests