Updated extra-large-maps patch, version 12 (for r23992)

[CARST]

Yes, i know that ...

But i only play with friends of mine and we all use one version then. I always send them a zip containing the latest version, my selfmade scenarios and the needed grfs. Their not so much into these stuff, but they all go mad about the game...

[Starbud]

I have updated my patch to 11849. See first post for updated patch. As per CARSTs request, I have also combined my patch with "close airport" patch and "2 airports per player per town" patch.

The result (combined patch + windows binaries) is attached to this post.

I think i misunderstood the patch, i thought that the airports would be placed next to each other and in effect doubling the number of runways.
I noticed i was wrong lol.

But i managed to make a nice pattern of way too many airports in one region.

[Bilbo]

I think i misunderstood the patch, i thought that the airports would be placed next to each other and in effect doubling the number of runways.
I noticed i was wrong lol.

But i managed to make a nice pattern of way too many airports in one region. :)

No. It only raises the airport limit to be more fair for multiplayer games (in which you stick two heliports somewhere in the city and nobody can build any airport in that city anymore. At least not without using tricks like stationwalking ... :)

Also, these airport patches are not mine. My patch is only the large map patch and I merely combined the two airport patches with mine patch, producing one larger patch and one corresponding binary :)

[Starbud]

Also, these airport patches are not mine. My patch is only the large map patch and I merely combined the two airport patches with mine patch, producing one larger patch and one corresponding binary

Nevertheless i apreciate that you made the game better

[Bilbo]

Updated patch and new windows binaries (r12388) are in the first post.

[Bilbo]

Hmm .. I tried playing with the "large" version on 8192 x 8192 map, I built one normal and one very long (cca 4000-5000 tiles long) train route and it seems that I encountered a bug:

Train 3 with head at 1313x541x3 (0x43c523) crashed with train 7 (head at 6556x4643x3 (0x44799c)) - these trains were at least 4 thousand tiles apart!
Train 4 with head at 1313x541x3 (0x43a521) then crashed into one of these two, but when I looked at them, all of them were on separate track

I am not attaching the savegame, cause it is 89Mb large and that its somewhat over size attachment limit on forums.

I guess there is some overflow in the FindTrainCollideEnum in train_cmd.cpp.

There is:

int x_diff = v->x_pos - tcc->v->x_pos;
int y_diff = v->y_pos - tcc->v->y_pos;

foloowed later by:

if (x_diff * x_diff + y_diff * y_diff > 25) return NULL;

that might be the cause. Once x_diff * x_diff + y_diff * y_diff gets over 0x7fffffff we get an ugly overflow
the x_pos is coordinate in some "small units", where TILE_SIZE units is one tile. TILE_SIZE=16
For safety, we must assume that x_diff < sqrt(0x7fffffff/2), therefore x_diff<32767. This "safe" distance is 2047 tiles.

But with large map patch, larger X and Y distances are possible (8191 in the "normal" version and even more in the "no-limits" version) and these would trigger an overflow here.

To devs:
I thought about two ways of fixing this. First would be converting x_diff and y_diff, so the overflow would no longer occur (unless we play with one of dimension larger than 134 million tiles, which is currently unsupported and once you get that large map, all int coordinates will overflow anyway :)

Seconds would be adding extra checks before this line, like:
if (abs(x_diff) > 5 || abs(y_diff) > 5) return NULL;
or
if (x_diff > 5 || x_diff < 5 || y_diff > 5 || y_diff < 5) return NULL;
(what is usually cheaper in CPU time when compiled, extra check or abs()?)

Which of the fixes should I incorporate in my patch? I guess the first is better, as on 64 bit archoitectures the performance decrease of the extra check may be neglibigle or zero and on 32bits only neglibigle.

[DaleStan]

(what is usually cheaper in CPU time when compiled, extra check or abs()?)

Unless abs is inlined, go for the extra checks, and avoid the function call overhead.
If abs is inlined, the most expensive operations are probably the x_pos and y_pos memory reads (can't be avoided) and the conditional jumps. I get the same number of memory reads (4) and the same number of conditional jumps (also 4) both ways. However, with the extra checks, the minimum number of conditional jumps is 1; an inlined abs requires a minimum of 2.

Dunno how much of that is significant.

[Yexo]

I have absolutely no idea what that code is supposed to do, but\

Code: Select all

if (x_diff * x_diff + y_diff * y_diff > 25) return NULL;

is certainly different from

Code: Select all

if (abs(x_diff) > 5 || abs(y_diff) > 5) return NULL;

Try with x_diff=4 and y_diff=4.

[DaleStan]

I'm guessing it's a cheap guard for some expensive code, so the expensive code only runs when it might actually do something interesting.

[Yexo]

Just looked at the code. I'm still unsure what is does, but it's not a cheap guard for expensive code.

Code: Select all

	/* can't collide with own wagons && can't crash in depot && the same height level */
	if (coll != tcc->v && v->u.rail.track != TRACK_BIT_DEPOT && abs(v->z_pos - tcc->v->z_pos) < 6) {
		int x_diff = v->x_pos - tcc->v->x_pos;
		int y_diff = v->y_pos - tcc->v->y_pos;

		/* needed to disable possible crash of competitor train in station by building diagonal track at its end */
		if (x_diff * x_diff + y_diff * y_diff > 25) return NULL;

		if (!(tcc->v->vehstatus & VS_CRASHED)) {
			/* two drivers + passengers killed in train tcc->v (if it was not crashed already) */
			tcc->num += 2 + CountPassengersInTrain(tcc->v);
			SetVehicleCrashed(tcc->v);
		}

[Bilbo]

Well, the line:

if (abs(x_diff) > 5 || abs(y_diff) > 5) return NULL;

would be stuffed before the line with:

if (x_diff * x_diff + y_diff * y_diff > 25) return NULL;

So both checks would be present. The second (curent one) check can overflow when trains are more than 2047 tiles in either dimension - therefore these train could crash.

This check was changed in revision 11877 by smatz. Before it the check was:

Code: Select all

	/* can't collide with own wagons && can't crash in depot && not too far */
	if (coll != tcc->v && v->u.rail.track != TRACK_BIT_DEPOT &&
			abs(v->z_pos - tcc->v->z_pos) < 6 &&
			abs(v->x_pos - tcc->v->x_pos) < 6 &&
			abs(v->y_pos - tcc->v->y_pos) < 6 ) {

which looks to be immune to overflows

So ... smatz broke it in r11877. :)

I am now testing the fix where x_diff and y_diff are int64 on large map. So far it goes well, unless I see another crash, I'll post the patch with the fix.

[Bilbo]

Fixed patch and windows binaries (no more train crashes) are in the first post.

[leno4]

I think this game would be extremly pointless at the highest size. Also would run very slow on old comps, but thats not a prob with me have tried full size and runs good.

Dont know if much people will use but good patch anyway

[Bilbo]

They don't have to use the highest size. There are many reasonable smaller sizes, I guess most used would be probably something like 1024x4096 or 512x4096. The Überlarge sizer are for enthusiasts, experimentators and for testing. Enabling them won't do anything bad :)

Now I am trying to run it on 2.4 Ghz quadcore with 4 GB ram and the largest size possible (8192 x 8192) is well playable. Still, I don't have too much vehicles there, only 84 trains and 89 aircrafts, so I'll have still to fill the map :)

Perhaps I'll try to turn on the AI and see what happens :)

[leno4]

They don't have to use the highest size. There are many reasonable smaller sizes, I guess most used would be probably something like 1024x4096 or 512x4096. The Überlarge sizer are for enthusiasts, experimentators and for testing. Enabling them won't do anything bad

Now I am trying to run it on 2.4 Ghz quadcore with 4 GB ram and the largest size possible (8192 x 8192) is well playable. Still, I don't have too much vehicles there, only 84 trains and 89 aircrafts, so I'll have still to fill the map

Perhaps I'll try to turn on the AI and see what happens

Hmm i got 2.4 ghz with 4gb ram aswell, biggest size and about 100 trains, works fine.

[CARST]

Hi Bilbo ... as you updated your patch with that fix for trains not crashing on the ultra-large-maps could you also update the version you made for me with the two airport patches (2ap per player per town & ap close)?


Regards,
Carst



Edit: FORGET ABOUT IT! Okay sorry for posting here, but i managed to do it myself, i discovered BuildOTTD isn't working on Vista64, so i used my second PC with WinXP32 for building my own version of OTTD. I made a new plain empty "carst.patch"-file in WordPad, pasted the code of 2airportsPerTownPerPlayer, your updated extra-large-map-patch, close airport and update airport into it and it compiled without error; just pax-destinations didn't compile when i added it to my "carst.patch". Thx anyway for your work...

[CARST]

Hi Bilbo, i just want to report the following things:

I won't be able to actively test things myself in the long-run till semester ends at end of July, but i now got results from tests of the ultra-version of the extra-large-maps-patch on 9 computer systems.

I think after your last update fixing the error with the trains i can savely make the statement that the patch seems stable and is working fine and in my opion is even trunk-worthy.


Tested on three 1GB systems, one 2GB system and two 4GB system:
- Maps with 8192x4096 tiles work and can be saved and loaded again without restrictions on any systems with 1 GB of RAM (or more).

Tested on two 512 MB systems:
- Maps with 8192x4096 tiles DO NOT work and can NOT be saved and loaded again on any systems with less than 1 GB of RAM.

I will tell my friends with the 1GB systems to also test maps with 8192x8192 tiles.



So what are the odds to get this patch with the 8192x8192 tiles-limitation into trunk? If it gets into trunk a warning sign could be useful to advice people what are the maximum map sizes at which level of RAM. (>=1GB = 8192x8192; <1GB = 4096x4096; <512MB 2048x2048)


Regards, CARST

[Bilbo]

So what are the odds to get this patch with the 8192x8192 tiles-limitation into trunk? If it gets into trunk a warning sign could be useful to advice people what are the maximum map sizes at which level of RAM. (>=1GB = 8192x8192; <1GB = 4096x4096; <512MB 2048x2048)

8192*8192 = 600MB while running, 1.2GB while saving/loading (plus you get like 100Mb save file :)
4096*4096 = 150MB while running, 300MB while saving/loading

So, on 1 GB RAM 8192*8192 will work, but swapping will occur during save/load.

I think that approximate momory consumption for selected map size (the save/load consumption) could be printed in the "new map" window for people to decide whether they have strong computer enough to handle that or not.
Also, 8192x8192 is maximum you can get reasonably working on any 32bit system (without quirks like PAE and such)

I'll test it also on 64bit system with even higher limit (I have now computer with 4 GB ram, I guess 16K * 16K may work :) to see how it works :)

There is "special" version for inclusion in trunk, with significantly lower limits (max 2048*2048 tiles, max dimension 8192) - http://bugs.openttd.org/task/1059 - since devs wished for lower limits.

However, once it is in trunk, raising the limit "sky high" is just matter of changing two constants in one header and recompile :)

I have now updated the version to current trunk, both here in this thread and on the bug tracker. So perhaps devs will notice and include this in trunk. Maybe :)

[wozzar]

Sorry if this has been mention before. But i can only go up to map size 16384 by 4096 I cant go any higher and the higher map size numbers are in red.
And i am using the binary

[Starbud]

Is it that you reach the limit of 67Mtiles?
Are you able to first decrease 4096 to 2048 and then increase 16384 to 32768?
That should work i think.