Rivers development

[gerw]

Looks promising
Which pakset are you using? Does this pakset have canals?

[jamespetts]

Looks promising
Which pakset are you using? Does this pakset have canals?

I was using Pak128; I am not sure whether Pak128 has canals. I can try it again with Pak64, if that would help? But there needs to be a solution for paksets without canals...

Edit: Same result with Pak64. Have you tried this in a debugger...?

Edit 2: No access violation with PakGerman (which does have canals), but also no rivers (of any type)...

Edit 3: Pak96.Comic produced no access violation, and did indeed produce a "river": made entirely of what it describes as an "extended field path", which looks like a thin, brown, muddy track...

[gerw]

It works fine for me with pak.german, pak64 and pak.comic. But it fails with pak.HAJO. Can you try this patch?

Edit: changed patch file

[jamespetts]

I still get an access violation with Pak128 and Pak64. PakGerman now produces a river that is canal-based for its entire length, but all the slopes are represented as locks. Pak96.Comic produced another long country path based river. PakBritain gave an access violation.

Is it not possible to have a happy medium between a sea-based river for the entire length, and a canal based river for the entire length? Some rivers do indeed form in deep gorges, whereas some are small streams...

Edit: Incidentally, have you made any changes other than to the create_rivers method? I ask because I have to apply your patch manually - TortoiseSVN isn't working well for me, because my code is heavily modified.

[gerw]

I still get an access violation with Pak128 and Pak64. PakGerman now produces a river that is canal-based for its entire length, but all the slopes are represented as locks. Pak96.Comic produced another long country path based river. PakBritain gave an access violation.

Can you debug this?

Is it not possible to have a happy medium between a sea-based river for the entire length, and a canal based river for the entire length? Some rivers do indeed form in deep gorges, whereas some are small streams...

This is desired. But it is a long way to this

Edit: Incidentally, have you made any changes other than to the create_rivers method? I ask because I have to apply your patch manually - TortoiseSVN isn't working well for me, because my code is heavily modified.

Please use a 'fresh' version for testing and debugging!

[VS]

You can do another checkout. Or copy your current one and revert.

[jamespetts]

Debugging:

the access violation occurs here:

Code Select Expand


uint8 karte_t::sp2num(spieler_t *sp)
{
if(  sp==NULL  ) {
return PLAYER_UNOWNED;
}
for(int i=0; i<MAX_PLAYER_COUNT; i++) {
if(spieler[i] == sp) {                          <<<<<<<<<<<<<<THIS LINE
return i;
}
}
dbg->fatal( "karte_t::sp2num()", "called with an invalid player!" );
}


(The line one on which it occurs is marked). "Sp" is a valid value, but "Speiler" is empty. That is called from void ding_t::set_besitzer(spieler_t *sp), which in turn is called from the line "weg->calc_bild();" in long grund_t::neuen_weg_bauen(weg_t *weg, ribi_t::ribi ribi, spieler_t *sp), which in turn is called from the line "cost = -gr->neuen_weg_bauen(sch, ribi, sp)-besch->get_preis();" in voidwegbauer_t::baue_schiene(), which in turn is called from the line "baue_schiene();" in void wegbauer_t::baue(), which is called by riverbuilder.baue(); in your method.

As to the checkouts and versions - I am currently trying to set up a DVCS for Simutrans-Experimental, so that I can revert the SVN to the default Simutrans to be able to test individual small changes, but I am having considerable trouble making DVCS work at all.

[VS]

Well, "this" seems to be NULL, so you probably have invalid karte to begin with.

EDIT: Oh yes, it seems the ding (weg?) does not have set a local "welt", so it can not look up the player, that's why it dies there.

EDIT2: Gotcha. wegbauer.cc, line 1956 is where the weg (here called "sch") is created; going into it, suddenly, static member "welt" is nothing... why?

EDIT3: Mwahahahaha. I may be wrong, but the weg_t class does initialize its static welt (through ancestor ding_t) only once it is created for first time with this parameter. My theory is that it does not happen before creation of rivers and thus the fake weg_t instance used for estimating costs of destroying trees(?) does not know its welt. I will try to create an object "just for the sake of it" and see if it fixes that.

EDIT4: Har har. I am the winner, until we discover the running pointers everywhere

Code Select Expand

if(  true  ) { // Generating of rivers.
baum_t b = baum_t(this, koord3d());
create_rivers(1);
}

[gerw]

Thank you VS, for figuring this out. Maybe a line like this will fix it?

Code Select Expand

schiene_t dummy(this);

By the way, simutrans got a new feature: lake in a river

[attachment deleted by admin]

[VS]

Ah, sure, see my edit #5. Any convenient object will do.

[jamespetts]

There are only 4 edits...

Edit: In any event, VS's patch has fixed the access violations, although we now still have only canal type rivers...

[VS]

To be honest, gerw got there first.

And there really are just 4 edits, I meant the last one.

It might be nice to walk the "route" backwards and instead of laying canals lower the first, say, 1/10 tiles.

[gerw]

It might be nice to walk the "route" backwards and instead of laying canals lower the first, say, 1/10 tiles.

Of course, I want to do something like this.

[jamespetts]

The flat version that I accidentally got when trying to make the original version work produced lovely valleys, but one wouldn't want them everywhere that there are rivers. It would be useful to have code that does some flattening (and puts a canal at the bottom), but not flattening everything to sea level. Near the mouth of the river, it would be flattened to sea-level, and create an estuary.

[gerw]

You two are very impatient

I'll type the patch as fast as I can 

[jamespetts]

Sorry for being impatient - it's just that we love the rivers patch!

[VS]

I can wait. It was a mere suggestion

[jamespetts]

I can wait.

Speak for yourself

[gerw]

Now, rivers have a river bed all over their length (is this nice?) and the last 1/4 is lowered to sea level.

[sojo]

Can you put a screenshot to the patch? So the not-programmer can see what is new.

[Fabio]

Now, rivers have a river bed all over their length (is this nice?)

i think it's just perfect

and the last 1/4 is lowered to sea level.

great!

Can you put a screenshot to the patch? So the not-programmer can see what is new.

pleeeeeeeeeeeeeeease!!!!!! we common sithaners can't wait!!!

[IgorEliezer]

pleeeeeeeeeeeeeeease!!!!!! we common sithaners can't wait!!!

*Igor waves his hands *

[jamespetts]

Some screenshots of the new rivers code (with my change to allow multiple river generation). Looking very good so far, but still needs some slight tweaking, I think: (1) there are some graphical glitches with canals in places (some can be seen in the screenshot); (2) the canals used for rivers should somehow be renamed to "river" rather than "canal", without stopping people from using canals called "canals" in the game (this will require a pakset implementation, I think); (3) it would be good if rivers could have wider estuaries, and also, if the size (length and width) of their estuary could depend on the river's total length, so as to have a mixture of small and large rivers, just as in reality, and to provide good visual variety; (4) rivers can currently be removed (!) using the remove tool, which is not ideal; and (5) there needs to be a way of setting the number of rivers produced at map generation in the "new map" GUI.

Having said that, though, the rivers patch is a very significant advance for Simutrans. Excellent work!

[sojo]

This looks nice. But the land should more flat. Mostly is the waterside only a little higher than the water.

[gauthier]

I'm just waiting until the implementation of this patch in nightly version 

...
...

WONDERFUL ! AWESOME ! MAGNIFICAL ! SUPER !

[wernieman]

When you think, that the path is ready for testing, can you send me a PM?

[vilvoh]

Simply awesome!!...

[The Hood]

This is one of the best additions I have seen for ages - well done!  I hope you can iron out all the last bits of problems without too much hassle!

[jamespetts]

Another suggestion for refinement: it would be good if there was a way of detecting when the rivers joined each other. Currently, one river does not take account of the presence of another (I know I am using my own hacked code to implement multiple rivers in a very simple way - perhaps this was planned for multiple rivers in any event). But it would be rather lovely if, not only did multiple rivers join into one whenever their paths crossed, but if the resulting estuary was twice (or three or four times, etc., depending on the number of tributaries) as big as an estuary from a single river (that is, both longer and wider at the sea end).

But, I should repeat, excellent work. This is a very important and exciting advance for Simutrans.

[Magic_Gorter]

Never thought that this development would go this fast. Great job. I really hope it will be part of simutrans soon.

[Fabio]

(2) the canals used for rivers should somehow be renamed to "river" rather than "canal", without stopping people from using canals called "canals" in the game (this will require a pakset implementation, I think);

For pak 128 i could take care of it, as i did for canals, once i know what it's exactly needed. I would suggest a canal named River and with NO icon, as cityroads used to be, or an icon only visible in map editing menu of public player (this would be indeed the best option).

Never thought that this development would go this fast.

In a way i knew that the basics were already there, but i never hoped to see such a GREAT result in such a short time!
Congratulation, buddies!

[Spike]

Great

Rivers have been something that I wanted in Simutrans since a very long time. Thanks for the addition!

[gerw]

Again: Multiple rivers aren't supported yet.

And of course, we need new graphics for the rivers. Maybe even waterfalls which can't be used by ships? I will talk to prissi, how to do this.

[prissi]

I had a look at the patch: Some comments would help to figure out, what is actually going on.

The river length should most likely depend on the map size, imho. Also a binary heap is overkill, if you are doing a random walk in the cost function. (I am not even sure this a a valid way to do the way search.) Maybe just a much more simple random walk would suffice too ...

[gerw]

I had a look at the patch: Some comments would help to figure out, what is actually going on.

Indeed. I will comment the patch, if it is already complete.

The river length should most likely depend on the map size, imho.

That is a good idea.

Also a binary heap is overkill, if you are doing a random walk in the cost function. (I am not even sure this a a valid way to do the way search.) Maybe just a much more simple random walk would suffice too ...

It's not a simple random walk. It's a route search on a graph, where the weights have a random addend and therefore it is valid. This is needed to get curved rivers.