MAP REWRITE patch 0.9

mkxx · Post by **mkxx** » 30 Nov 2005 01:15

Hi all,

I can prepare OTTD sourcecode for new Tile structure. In the following picture is explained, how I'll do it. See the image and my next post in next pages.

The main idea is to replace direct access to map array with two raw-get/set-functions with which you only access the map array. These functions access to map array directly and behave to other source code like old map array (with vars type_height, m1, m2, ...). There is very easy to rewrite data structure, 'cause you don't need to rewrite whole source code but only these two functions.

I have the patch at home so I can post it here, if it will be usefull.

(Main patch was here before, but I deleted it. I think I have not explained it good :))

CobraA1 · Post by **CobraA1** » 30 Nov 2005 02:41

Why begin with "TT_"?

MeusH · Post by **MeusH** » 30 Nov 2005 09:08

CobraA1 wrote:Why begin with "TT_"?

TileType?

Before it was MP_ which was really strange name, I like TT much more.

mkxx · Post by **mkxx** » 30 Nov 2005 09:15

Yes, it means TileType
-> there are TileTypes same as in TileType enum (so, TT_Clear struct for Tile where Tile.type == MP_CLEAR)

mkxx · Post by **mkxx** » 03 Dec 2005 11:53

One week ago and no reply. Could you write me what you think about this work? Is it usefull or useless?

prissi · Post by **prissi** » 03 Dec 2005 11:58

Even if I did only small patches, I think such a structure like this is essential for good support. It would be a great healp and would also have made the small patches I made much easier.

Dextro · Post by **Dextro** » 03 Dec 2005 12:01

ok now I'm curious... Wasn't the dev team doing a complete map rewrite? The first version was scrapped it's true but I though they had restarted the project...

So the question is: did you talk to the devs before starting this mkxx? I bet the code's great and all but it's really bad if you had all this trouble for nothing

mkxx · Post by **mkxx** » 03 Dec 2005 12:54

I know that devs are doing it. But you can never do rewrite of data structures if you do not replace direct access to map array with getters and setters (one function for setting and one for getting values from map array). This is the main idea of this patch. You can rewrite map array only if you can control all acesses to map array.

If devs would be interested, I can post the script, which rewrites accesses to map array to getter and setter functions.

bociusz · Post by **bociusz** » 03 Dec 2005 16:09

I suggest to go to IRC (irc://irc.freenode.net #openttd) and talk with the devs

ajy170 · Post by **ajy170** » 04 Dec 2005 09:26

I can't belive, after, how long has it been, we are getting a new map array

Me ponders on the possabilities and thinks that life isn't so bad after all

webfreakz.nl · Post by **webfreakz.nl** » 04 Dec 2005 10:54

wow! this patch is huge!

does it affect the working of the game (the pathfinding) or just the way you can program?

mkxx · Post by **mkxx** » 04 Dec 2005 19:53

ajy170: wait a minute before your luck :o) after minute look into http://svn.openttd.org and look into /branch/map directory - open tile.h - you'll see quite different structure :)) this patch is nice but is not compatible with THEIR ideas - you understand? you'll have to wait more more more more time to get new map array work :)

webfreakz.nl: it does not affect the game, only the style of programming - it was made like an temporary step between "original" OTTD sourcecode and new - with map array rewritten

prissi · Post by **prissi** » 05 Dec 2005 22:01

I really think this may bring OTTD forward and makes managing and rewriting things much easier. But did a developer commented here or did I just overlooked it?

LordOfThePigs · Post by **LordOfThePigs** » 05 Dec 2005 22:27

Sorry for saying this so bluntly, but this patch is pretty much useless. What it currently does is only replace direct access to map array by using some accessors, but does not abstract the code from the underlying data structure.

check the following lines in the diff to see what I mean:

Code: Select all

-			if ((_m[tile].m5&1)     != (p->ai.cur_dir_a&1))
+			if ((MRW_GET(tile,5)&1) != (p->ai.cur_dir_a&1))

-		b =     _m[tile].m5 & 0x21;
+		b = MRW_GET(tile,5) & 0x21;

It is basically exactly the same code. No abstraction is brought from this patch.

What we need is code like that:

Code: Select all

This ugly, uncommented, incomprehensible, bit-for-bit manipulating code with direct access to the map array:

-			m = _m[tile].m1;
-			m = (m & 3) + 1;
-			n = _m[tile].m5;
-			if (m == 4 && (m=0,++n) == 32+1 && (n=30,b)) {
-				_m[tile].m1 = 0x83;
-				_m[tile].m5 = 29;
-				DeleteAnimatedTile(tile);
-			} else {
-				SB(_m[tile].m1, 0, 2, m);
-				_m[tile].m5 = n;

Gets transformed into this:

+			bool wrapAnimation = false;
+			state = GetOilWellsAnimationState(tile);
+			state ++;
+			tileType = GetIndustryTileType(tile);
+
+			if(state == 4){
+				state = 0;
+				tileType ++;
+				if(tileType == 32 + 1){ // animation is wrapping
+					tileType = 30;
+					if(b){
+						wrapAnimation = true;
+						SetOilWellsAnimationState(tile, 3);
+						SetTileIndustryType(tile, 29);
+						DeleteAnimatedTile(tile);
+					}
+				}
+			}
+			if(!wrapAnimation){
+				SetOilWellsAnimationState(tile, state);
+				SetTileIndustryType(tile, tileType);

The new code is much more readable, and brings the required abstraction level. All that is needed to change the underlying data structure after having rewritten the code in this way is to change the accessor functions.

charlieg · Post by **charlieg** » 08 Dec 2005 12:44

LordOfThePigs: you are wrong. It provides enough abstraction to mean you can change the way maps are represented without completely breaking the game.

I can't provide specific examples but I can make one up.

Say you have property *foobar in the old representation. In the new one, it gets split into *foo and *bar.

With the getter/setter functions you only need update 1 place - change the functions to get/set the *foo and *bar instead of just *foobar - and the rest of the game will work fine. However, without getter/setter functions you need to change every place *foobar is referenced throughout the codebase.

This will greatly ease the transition to a new data structure.

Dextro · Post by **Dextro** » 08 Dec 2005 12:54

charlieg wrote:LordOfThePigs: you are wrong. It provides enough abstraction to mean you can change the way maps are represented without completely breaking the game.

I can't provide specific examples but I can make one up.

Say you have property *foobar in the old representation. In the new one, it gets split into *foo and *bar.

With the getter/setter functions you only need update 1 place - change the functions to get/set the *foo and *bar instead of just *foobar - and the rest of the game will work fine. However, without getter/setter functions you need to change every place *foobar is referenced throughout the codebase.

This will greatly ease the transition to a new data structure.

it makes sense but won't that limit the amount of possibilities? I mean right now it's acessing directly if I understand it righ so when it's changes it will improve almost imediatly. But if you use getter/setter functions by changing the map array the game is still limited like it was before because the getter/setter function is limiting the access to new features the map array provides am I right? Sort of like taking a ferrari engine and placing it inside a fiat chasis: it may be a great engine but the chasis isn't able to use it.

Brianetta · Post by **Brianetta** » 08 Dec 2005 13:43

Dextro: No, your car analogy isn't adequate. Nobody's putting in a new engine. They're putting in basic controls.

Say you have a Fiat. It has no accelerator pedal, but it also has no engine cover. To go faster, you lean into the engine and open the butterfly valve on the carburettor. If a different engine (the Ferrari one, perhaps, or even a Citroen if you think it might actually work) is put into the car, everybody who drives the car has to find the valve on the new engine, which probably works differently.

If you had an accelerator pedal, you wouldn't have to worry - wherever the valve is, the accelerator is connected somehow. There's an engine cover. You don't care where that valve is, or which way it turns, unless you're a mechanic.

The engine is the map array. The various drivers of the car are the various functions. The accelerator is a setter. The mechanic would be a developer.

That was a really dodgy analogy, but I wanted to modify yours rather than use a new one. Encapsulating the map array is not just a good idea, it's an absolutely vital prerequisite to coming up with a new one. Getters and setters don't limit access to new features, because any new features will come with getters and setters of their own.

Doing it by direct access means that everything that accesses the array has to be rewritten when the array changes. That's what killed development of the new array in the first place. The new array isn't an expansion of the old, it's a rewrite. The getters and setters will have their methods rewritten, but the interface used by various functions throughout the codebase will be the same or expanded.

Post by **peter1138** » 08 Dec 2005 13:56

We need to get the get/set accessors implemented before any changes to the map array itself.

Changing the map array first (as this patch does) is doing it the wrong way around.

Brianetta · Post by **Brianetta** » 08 Dec 2005 14:03

peter1138 wrote:We need to get the get/set accessors implemented before any changes to the map array itself.

That's what I thought this patch was.

peter1138 wrote:Changing the map array first (as this patch does) is doing it the wrong way around.

Guess I should have read it.

Dextro · Post by **Dextro** » 08 Dec 2005 14:20

Thanks for clearing it up then, guess I got it wrong

Still won't there be risks that by using get/set functions you might block access to parts of the new array that might show up in the future?

Transport Tycoon Forums

MAP REWRITE patch 0.9

MAP REWRITE patch 0.9

Who is online