Patch: The Rainfall River Generator

[pi1985]

Ok. I noticed no difference in file lists, bit difference in patch parameters: you used only -p1, I used -F0 and -p1 - install patch exactly in that place where it taken. This cause rejects. But without it some hunks applied with "fuzz = 2":

Code: Select all

...
patching file src/lang/english.txt
Hunk #1 succeeded at 2928 with fuzz 2 (offset -3 lines).
patching file src/script/api/game/game_window.hpp.sq
Hunk #1 succeeded at 559 with fuzz 2 (offset -2 lines).
patching file src/script/api/script_window.hpp
Hunk #1 succeeded at 1428 with fuzz 2 (offset -2 lines).
...

Anyway, I can't make this because of:

Code: Select all

[SRC] Compiling genworld.cpp
/home/vanya/Documents/Программы/OpenTTD/patches/r27474-rivers/src/genworld.cpp:383:40: error: ‘string’ is not a member of ‘std’
/home/vanya/Documents/Программы/OpenTTD/patches/r27474-rivers/src/genworld.cpp:383:53: error: ‘s’ was not declared in this scope
/home/vanya/Documents/Программы/OpenTTD/patches/r27474-rivers/src/genworld.cpp:383:56: error: expected primary-expression before ‘char’
/home/vanya/Documents/Программы/OpenTTD/patches/r27474-rivers/src/genworld.cpp:383:75: error: expected primary-expression before ‘char’
/home/vanya/Documents/Программы/OpenTTD/patches/r27474-rivers/src/genworld.cpp:383:95: error: expected primary-expression before ‘int’
/home/vanya/Documents/Программы/OpenTTD/patches/r27474-rivers/src/genworld.cpp:383:107: error: expression list treated as compound expression in initializer [-fpermissive]
/home/vanya/Documents/Программы/OpenTTD/patches/r27474-rivers/src/genworld.cpp:384:1: error: expected ‘,’ or ‘;’ before ‘{’ token
/home/vanya/Documents/Программы/OpenTTD/patches/r27474-rivers/src/genworld.cpp:383:15: warning: ‘FindMatchingCloseBracket’ defined but not used [-Wunused-variable]

Ok, in include section in genworld.cpp adding

Code: Select all

#include <string>

But:

Code: Select all

[SRC] Compiling genworld.cpp
In file included from /usr/lib64/gcc/x86_64-alt-linux/4.7.2/../../../../include/c++/4.7.2/string:54:0,
                 from /home/vanya/Documents/Программы/OpenTTD/patches/r27474-rivers/src/genworld.cpp:40:
/usr/lib64/gcc/x86_64-alt-linux/4.7.2/../../../../include/c++/4.7.2/bits/basic_string.h: In function ‘std::string std::to_string(int)’:
/usr/lib64/gcc/x86_64-alt-linux/4.7.2/../../../../include/c++/4.7.2/bits/basic_string.h:2864:45: error: ‘SAFEGUARD_DO_NOT_USE_THIS_METHOD’ is not a member of ‘std’
/usr/lib64/gcc/x86_64-alt-linux/4.7.2/../../../../include/c++/4.7.2/bits/basic_string.h: In function ‘std::string std::to_string(unsigned int)’:
/usr/lib64/gcc/x86_64-alt-linux/4.7.2/../../../../include/c++/4.7.2/bits/basic_string.h:2869:45: error: ‘SAFEGUARD_DO_NOT_USE_THIS_METHOD’ is not a member of ‘std’
/usr/lib64/gcc/x86_64-alt-linux/4.7.2/../../../../include/c++/4.7.2/bits/basic_string.h: In function ‘std::string std::to_string(long int)’:
/usr/lib64/gcc/x86_64-alt-linux/4.7.2/../../../../include/c++/4.7.2/bits/basic_string.h:2875:45: error: ‘SAFEGUARD_DO_NOT_USE_THIS_METHOD’ is not a member of ‘std’
/usr/lib64/gcc/x86_64-alt-linux/4.7.2/../../../../include/c++/4.7.2/bits/basic_string.h: In function ‘std::string std::to_string(long unsigned int)’:
/usr/lib64/gcc/x86_64-alt-linux/4.7.2/../../../../include/c++/4.7.2/bits/basic_string.h:2880:45: error: ‘SAFEGUARD_DO_NOT_USE_THIS_METHOD’ is not a member of ‘std’
/usr/lib64/gcc/x86_64-alt-linux/4.7.2/../../../../include/c++/4.7.2/bits/basic_string.h: In function ‘std::string std::to_string(long long int)’:
/usr/lib64/gcc/x86_64-alt-linux/4.7.2/../../../../include/c++/4.7.2/bits/basic_string.h:2886:45: error: ‘SAFEGUARD_DO_NOT_USE_THIS_METHOD’ is not a member of ‘std’
/usr/lib64/gcc/x86_64-alt-linux/4.7.2/../../../../include/c++/4.7.2/bits/basic_string.h: In function ‘std::string std::to_string(long long unsigned int)’:
/usr/lib64/gcc/x86_64-alt-linux/4.7.2/../../../../include/c++/4.7.2/bits/basic_string.h:2892:45: error: ‘SAFEGUARD_DO_NOT_USE_THIS_METHOD’ is not a member of ‘std’
/usr/lib64/gcc/x86_64-alt-linux/4.7.2/../../../../include/c++/4.7.2/bits/basic_string.h: In function ‘std::string std::to_string(float)’:
/usr/lib64/gcc/x86_64-alt-linux/4.7.2/../../../../include/c++/4.7.2/bits/basic_string.h:2901:45: error: ‘SAFEGUARD_DO_NOT_USE_THIS_METHOD’ is not a member of ‘std’
/usr/lib64/gcc/x86_64-alt-linux/4.7.2/../../../../include/c++/4.7.2/bits/basic_string.h: In function ‘std::string std::to_string(double)’:
/usr/lib64/gcc/x86_64-alt-linux/4.7.2/../../../../include/c++/4.7.2/bits/basic_string.h:2910:45: error: ‘SAFEGUARD_DO_NOT_USE_THIS_METHOD’ is not a member of ‘std’
/usr/lib64/gcc/x86_64-alt-linux/4.7.2/../../../../include/c++/4.7.2/bits/basic_string.h: In function ‘std::string std::to_string(long double)’:
/usr/lib64/gcc/x86_64-alt-linux/4.7.2/../../../../include/c++/4.7.2/bits/basic_string.h:2919:45: error: ‘SAFEGUARD_DO_NOT_USE_THIS_METHOD’ is not a member of ‘std’

What I'm doing wrong

[cirdan]

Ok, in include section in genworld.cpp adding

Code: Select all

#include <string>

Where did you add this? Try placing it before any other include, and certainly before

Code: Select all

#include "safeguards.h"

[pi1985]

This is it Thanks. Now it works fine!
Thanks to author for this great patch!

[pi1985]

I have been added this patch to my patchpack and I had some issues when compiling in minwg:

Code: Select all

[SRC] Compiling genworld_gui.cpp
In file included from C:/MinGW/msys/1.0/home/vanya/2.3/src/genworld_gui.cpp:34:0:
C:/MinGW/msys/1.0/home/vanya/2.3/src/rivers_rainfall.h:248:32: sorry, unimplemented: non-static data member initializers
C:/MinGW/msys/1.0/home/vanya/2.3/src/rivers_rainfall.h:248:32: error: ISO C++ forbids in-class initialization of non-const static member 'number_of_tiles_so_far'
C:/MinGW/msys/1.0/home/vanya/2.3/src/rivers_rainfall.h:482:32: sorry, unimplemented: non-static data member initializers
C:/MinGW/msys/1.0/home/vanya/2.3/src/rivers_rainfall.h:482:32: error: ISO C++ forbids in-class initialization of non-const static member 'number_of_tiles_so_far'
C:/MinGW/msys/1.0/home/vanya/2.3/src/rivers_rainfall.h:485:42: sorry, unimplemented: non-static data member initializers
C:/MinGW/msys/1.0/home/vanya/2.3/src/rivers_rainfall.h:485:42: error: ISO C++ forbids in-class initialization of non-const static member 'number_of_generated_lake_centers'
C:/MinGW/msys/1.0/home/vanya/2.3/src/rivers_rainfall.h:886:25: sorry, unimplemented: non-static data member initializers
C:/MinGW/msys/1.0/home/vanya/2.3/src/rivers_rainfall.h:886:25: error: ISO C++ forbids in-class initialization of non-const static member 'create_lake_runs'
make[1]: *** [genworld_gui.o] Error 1
make[1]: Leaving directory `/usr/home/vanya/2.3/objs/release'
make: *** [all] Error 1

and MS VS2010:

Code: Select all

c:\users\123\desktop\ottd_dev\2.3\src\rivers_rainfall.h(32): fatal error C1083: Cannot open include file: 'sys/time.h': No such file or directory

I made some changes to make it compile correctly.

[ic111]

A very small bugfix release - basically, I needed to fix one small reject to make the patch queue apply against current trunk.

[Wahazar]

Any chance to update patch against current version?
There is lot of rejections, and I have no clue how to fix it (for example, there is no GenerateRivers function in landscape.cpp)

[SteveF2]

I've had a go at rebasing this patch against current master (trunk). You can find the code here: https://github.com/ZornsLemma/OpenTTD/tree/rivers-v61d

It's only been very lightly tested but it seems to work OK. Most of the code didn't need to be touched, there were just a few tweaks around the edges; this is good, but it also means I'm not that familiar with how most of the code works as I wasn't forced to touch it. If you hit any problems please post them here and maybe I'll be able to do something, but no promises.

I'm afraid I can't build a Windows binary of this - I did the work on Linux - but perhaps someone else will be able to do that.

If you look at the rivers-v61 branch in that repository you can see the individual patches from ic111's v61 patch posted on 23 Apr 2017 on a branch off the 1.7.2 release; this required no tweaking and might prove a useful reference for future debugging to see if a problem was present in the original patch or has been introduced in my updated version. I had hoped to keep that structure with multiple commits building up to the final result when I rebased the branch against current trunk to give rivers-v61d but my git-wrangling skills weren't up to it.

Cheers.

Steve

[Aegir]

I've had a go at rebasing this patch against current master (trunk). You can find the code here: https://github.com/ZornsLemma/OpenTTD/tree/rivers-v61d

It's only been very lightly tested but it seems to work OK. Most of the code didn't need to be touched, there were just a few tweaks around the edges; this is good, but it also means I'm not that familiar with how most of the code works as I wasn't forced to touch it. If you hit any problems please post them here and maybe I'll be able to do something, but no promises.

I'm afraid I can't build a Windows binary of this - I did the work on Linux - but perhaps someone else will be able to do that.

If you look at the rivers-v61 branch in that repository you can see the individual patches from ic111's v61 patch posted on 23 Apr 2017 on a branch off the 1.7.2 release; this required no tweaking and might prove a useful reference for future debugging to see if a problem was present in the original patch or has been introduced in my updated version. I had hoped to keep that structure with multiple commits building up to the final result when I rebased the branch against current trunk to give rivers-v61d but my git-wrangling skills weren't up to it.

Cheers.

Steve

Had a play with this in Visual Studio 2019 and came up blanks. First major problem was dependencies on "sys\time.h" which wasn't present on Windows. Dealt with that issue but still couldn't coax a successful compile out of it for other reasons I couldn't deduce. Kudos on the effort to bring this patch back up to scratch again though! I'll have to give it a bash next time I'm on a Linux machine.

[SteveF2]

Thanks for trying anyway! If you feel like giving it another go, it might be worth trying to build unpatched OpenTTD first - if that doesn't work you may be able to get advice elsewhere on the forums. If you can build unpatched OpenTTD but have errors building with the RRG patch, let me know what they are and I might be able to advise. I just can't help with general Windows build problems, since I've never tried it.

[Aegir]

Thanks for trying anyway! If you feel like giving it another go, it might be worth trying to build unpatched OpenTTD first - if that doesn't work you may be able to get advice elsewhere on the forums. If you can build unpatched OpenTTD but have errors building with the RRG patch, let me know what they are and I might be able to advise. I just can't help with general Windows build problems, since I've never tried it.

Yup, I can already build openttd master and JGR's fork.

Visual Studio was pretty detailed with its complaints, so when I get a minute I'll collect some logs and see if you can make anything of it.

Edit: output from a fresh compile attempt is attached. Looks like its the linker stuff it finally choked on. Way over my head .

[SteveF2]

Thanks, that's helpful. I hadn't amended the Visual Studio project files to include the new C++ files added by the patch. I think all I had to do was run the 'generate' script in the projects directory, so I've done that and pushed a new commit to the branch. Please have another go with that and if it fails I hope it will fail with a different error at least.

[Aegir]

Thanks, that's helpful. I hadn't amended the Visual Studio project files to include the new C++ files added by the patch. I think all I had to do was run the 'generate' script in the projects directory, so I've done that and pushed a new commit to the branch. Please have another go with that and if it fails I hope it will fail with a different error at least.

Sure thing, I'll let you know shortly.

Edit: Here's the updated output

[Aegir]

Got a feeling though it might have something to do with the sys\time.h includes I had to cobble together on Windows. I've attached them below for giggles. Well aware I'm coloring outside the lines here.

8>rivers_rainfall.obj : error LNK2001: unresolved external symbol "int __fastcall gettimeofday(struct timeval *,void *)" (?gettimeofday@@YIHPAUtimeval@@PAX@Z)
8>D:\Users\Richard\Desktop\OpenTTD-rivers-v61d\projects\..\objs\Win32\Release\openttd.exe : fatal error LNK1120: 1 unresolved externals

[jfs]

Thanks, that's helpful. I hadn't amended the Visual Studio project files to include the new C++ files added by the patch. I think all I had to do was run the 'generate' script in the projects directory, so I've done that and pushed a new commit to the branch. Please have another go with that and if it fails I hope it will fail with a different error at least.

It looks like the only use of gettimeofday() is to display some performance debugging. A while ago some additional macros were added to debug.h to capture/print timing in microseconds, TICC() and TOCC(). Maybe you can replace PrintRunningTimeToDebug() with something based on those, or otherwise switch to using std::chrono instead for the timing, as that is definitely portable.

[SteveF2]

jfs - good idea, I've had a go at changing the code to use std::chrono.

Aegir - looks like you diagnosed the problem correctly. I've pushed the new code using std::chrono out to my repository so please have another go and let us know how you get on with the build. This time for sure!

[Aegir]

jfs - good idea, I've had a go at changing the code to use std::chrono.

Aegir - looks like you diagnosed the problem correctly. I've pushed the new code using std::chrono out to my repository so please have another go and let us know how you get on with the build. This time for sure!

All good, I'll give it a crack!

Edit: Winner winner chicken dinner! Still a few warnings in the build that aren't present in a build of master, but I've got OpenTTD generating rivers on one of my hell-hole messes of a heightmap right now.

[SteveF2]

Nice one, thanks! I'll take a look at those warnings later and see if any of them can/should be fixed by tweaking the code. The C4018 signed/unsigned mismatch warning seems to crop up on all sorts of probably-untouched files, so I'll check the new cases of it but probably not go out of my way to avoid it, but the other warnings might indicate genuine bugs or potential bugs. I generally prefer to tweak code to eliminate compiler warnings where possible, but I'll probably not try to do this for C4018.

[SteveF2]

I've pushed some commits to the branch which I hope will fix the Visual Studio warnings; if you'd like to retry (absolutely no rush) please let me know how I did.

There's a slim chance I broke something with these commits but they're fairly unintrusive so it's probably OK. I gave it a very quick test afterwards and it seems fine.

[Aegir]

I've pushed some commits to the branch which I hope will fix the Visual Studio warnings; if you'd like to retry (absolutely no rush) please let me know how I did.

There's a slim chance I broke something with these commits but they're fairly unintrusive so it's probably OK. I gave it a very quick test afterwards and it seems fine.

I'll give it a crack when I get a minute

[GuilhermeJK]

Hello there,

I'm trying to put The Rainfall River Generator into JGR patch pack and only one error remained to be fixed that I just can't find why.

The error is in file "rivers_path.cpp"

Code: Select all

Error	C2440	'=': cannot convert from 'int32 (__cdecl *)(AyStar *,OpenListNode *)' to 'AyStar_EndNodeCheck (__cdecl *)'	openttd	D:\OpenTTD-patches-jgrpp-rainfall\src\rivers_path.cpp	295	

in function:

Code: Select all

void PathRiverGenerator::BuildRiver(TileIndex begin, TileIndex end)
{
	AyStar finder;
	MemSetT(&finder, 0);
	finder.CalculateG = PathRiverGenerator::River_CalculateG;
	finder.CalculateH = PathRiverGenerator::River_CalculateH;
	finder.GetNeighbours = PathRiverGenerator::River_GetNeighbours;
	finder.EndNodeCheck = PathRiverGenerator::River_EndNodeCheck;
	finder.FoundEndNode = PathRiverGenerator::River_FoundEndNode;
	finder.user_target = &end;

	finder.Init(PathRiverGenerator::River_Hash, 1 << RIVER_HASH_SIZE);

	AyStarNode start;
	start.tile = begin;
	start.direction = INVALID_TRACKDIR;
	finder.AddStartNode(&start, 0);
	finder.Main();
	finder.Free();
}

This line:

Code: Select all

finder.EndNodeCheck = PathRiverGenerator::River_EndNodeCheck;

I went to aystar.h and PathRiverGenerator struct at "rivers_path.h" and both are set as "int32"

Any idea?

If you need extra info, just ask. I'm using Visual Studio 2019 to compile it. I have already compiled successfully both Vanilla and JGR packs from source.