Larger Airports

[Darkvater]

GRRrrrrrrrrr I have almost finished my post when Explorer crashed (I know, don't use IE ) and I lost all of it. So I'll keep it short: I have tested it extensively on my machine (WinXP mostly, some Linux (Mandrake10)), so it should work. I encourage you though to test it yourself to find any hidden defects so that it can be added to svn as soon as possible.

I have uploaded patch for larger airports to sourceforge, get it here:
http://sourceforge.net/tracker/index.ph ... tid=636367

There are 2 issues:

1. Saving loading with large airports gives strange results. To solve this, either look at option 2. here or send all aircraft to hangar, stop them, then save game. On loading, all will be fine.
2. I have added code to solve compatibility with the old format (if you don't run this game will crash with assert). It is commented out in aircraft_cmd.c at the bottom. That is because it doesn't belong there, but somewhere in saveload.c. Anyways, if you uncomment it and make it static; eg

   Code: Select all

   static bool only_once = true;
   if (only_once) {
     ....do all of that
     only_once = false;
   }
   

   it will be run the first time an airplane moves, and all will be ok. *NOTE* If you dont' do this, game will crash with assert at the title screen because of optntitle.dat which has airplanes!!

Oops, I forgot something in the airport-update-compatible code:

Code: Select all

if (v_oldstyle->vehstatus & VS_STOPPED) {continue;}

Should be:

Code: Select all

if ((v_oldstyle->vehstatus & VS_STOPPED) && (v_oldstyle->u.air.state == HANGAR)) {continue;}

Since only the aircraft stopped AND in hangar should be left alone, all other stopped have to move.
And add below AircraftLeaveHangar:

Code: Select all

v_oldstyle->vehstatus &= ~VS_STOPPED; // make airplane moving

Sorry

[pasky]

Ok I admit a lot of this is quite minor, but if this stuff cummulates in the
source it eventually becomes unhackable ;-).

• * What about consistent naming conventions? _airport_map5_tiles_big and then suddenly _metropolitan_airport_map5_tiles_big? I'd say rather _airport_map5_tiles_metropolitan.
  
  * Please take care about indentation - use tabs consistently, otherwise it looks like crap with tab width != 2. Example:
  

  Code: Select all

  -               if (_current_player == _local_player && p1 != 2) {
  -                       _last_built_aircraft_depot_tile = tile + _airport_depot_offs[p1];
  -               }
  +               if (_current_player == _local_player && p1 <= AT_INTERNATIONAL) {
  +      _last_built_aircraft_depot_tile = tile + GetAirport(p1)->airport_depots[0];
  +    }
  

  In aircraft_cmd.c you seem to use spaces exclusively - bad thing.
  
  * Please use either hex constants or decimal constants consistently. Currently it's a mix. I don't know about you but converting numbers between bases takes excess brainpower for me. So I'd say keep them all in hex for now.
  
  * It helps to have an empty lines between declarations and the code if the block is big enough.
  
  * I think the code for fixing aircrafts could live in vehicle.c's load handler for now. I put the wagon override stuff there as well. Regarding airports flags reset, that should live in station_cmd.c?
  
  * I think you should use uint32 instead of uint, so the size is obvious and ensured.
  
  * What's the etymology of AirportFTAbduildup?
  
  * Perhaps AirportFTAInit()/AirportFTADone() would be better than AirportFTAClass_{Constructor,Destructor}() ?
  
  * I'm not sure if the *BITS() macros don't in reality obfuscate the code instead. Aren't the bit operations obvious enough?

[Darkvater]

Ok I admit a lot of this is quite minor, but if this stuff cummulates in the
source it eventually becomes unhackable .

I admit to your concern. Let me try to motivate my decisions and admit my failures

• * What about consistent naming conventions? _airport_map5_tiles_big and then suddenly _metropolitan_airport_map5_tiles_big? I'd say rather _airport_map5_tiles_metropolitan.

  well, _airport_map5_tiles_big was supposed to be _city_airport_map5_tiles_big, but since that was an original airport I left it as it is. My mistake. Would probably change it to _airport_map5_tiles_...

  * Please take care about indentation - use tabs consistently, otherwise it looks like crap with tab width != 2. Example:
  
  In aircraft_cmd.c you seem to use spaces exclusively - bad thing.

  I use a tabwidth of 2 spaces and have the editor convert all tabs to spaces. Hence the spaces. Of course not everyone uses the same thing, so we should have some general code-layout set in a sticky on this forum to get a more general look. Good point! The leader has spoken in favor of tabs of size 2. Most fortunately ultraedit has a featue of changing leading spaces into tabs. Phew

  * Please use either hex constants or decimal constants consistently. Currently it's a mix. I don't know about you but converting numbers between bases takes excess brainpower for me. So I'd say keep them all in hex for now.

  Whereever the code was touched, I used decimal numbers. I left Hex as it is, not wanting to screw up anything there. I prefer decimal though instead of hex, since it kills me to convert to decimal (since output of grfcodec is decimal too)

  * It helps to have an empty lines between declarations and the code if the block is big enough.

  Again, coding conventions we should agree on. You are probably right though

  * I think the code for fixing aircrafts could live in vehicle.c's load handler for now. I put the wagon override stuff there as well. Regarding airports flags reset, that should live in station_cmd.c?

  Fixing aircraft and airports reset is ONE thing and be called at every game loaded. I'd rather put it in saveload...but actually if it is run only once (with static if) aircraft_cmd.c also works, since you only fix it when aircraft moves . But it should not belong there

  * I think you should use uint32 instead of uint, so the size is obvious and ensured.

  Agreed and changed

  * What's the etymology of AirportFTAbduildup?

  http://darkvater.no-ip.info/ottd/airports.html

  * Perhaps AirportFTAInit()/AirportFTADone() would be better than AirportFTAClass_{Constructor,Destructor}() ?

  Yes, could be, altough AirportInitialize and AirportUninitialize are used to set up all the airports

  * I'm not sure if the *BITS() macros don't in reality obfuscate the code instead. Aren't the bit operations obvious enough?

  It should stay in *BITS. Right now it would be for example:

  Code: Select all

  CLRBITS(target, bitscheck)

  Without macro:

  Code: Select all

  target &= ~bitscheck

  Apart from this being ugly code to do over and over again, it gives rise to lot of errors, and confusion.
  Or For example CLRBIT is:

  Code: Select all

  CLRBIT(x,y) ((x) &= ~(1 << (y)))

  You DON'T want to write that every time in the code

But it works? I'd be pissed if it didn't.

[pasky]

Please take care about indentation - use tabs consistently, otherwise it looks like crap with tab width != 2. Example:

In aircraft_cmd.c you seem to use spaces exclusively - bad thing.

I use a tabwidth of 2 spaces and have the editor convert all tabs to spaces. Hence the spaces. Of course not everyone uses the same thing, so we should have some general code-layout set in a sticky on this forum to get a more general look. Good point!

I think using tabs for indentation is best because everyone can then happily use whatever indentation size he likes most.

Please use either hex constants or decimal constants consistently. Currently it's a mix. I don't know about you but converting numbers between bases takes excess brainpower for me. So I'd say keep them all in hex for now.

Whereever the code was touched, I used decimal numbers. I left Hex as it is, not wanting to screw up anything there. I prefer decimal though instead of hex, since it kills me to convert to decimal (since output of grfcodec is decimal too)

Huh? Output of grfcodec is hexadecimal. Or what output do you mean?

I think we should follow the current conventions when adding new code, that means using hex constants. Then we can agree upon converting them all to decimal if it'll make sense. (Similarly to how did we do it with the hex XY coordinates of dialogs.) But please keep it of one style.

* I think the code for fixing aircrafts could live in vehicle.c's load handler for now. I put the wagon override stuff there as
well. Regarding airports flags reset, that should live in station_cmd.c?

Fixing aircraft and airports reset is ONE thing and be called at every game loaded. I'd rather put it in saveload...but actually if it is run only once (with static if) aircraft_cmd.c also works, since you only fix it when aircraft moves :). But it should not belong there

Currently compatibility hacks for older savegames are put at the place where the relevant data is loaded, and I don't think that's a bad convention. Alternately you could call kinda FixupLoadedAircrafts() from Load_VEHS(). Or you could do it the way you do it now, fixing it up at the first run of a function, I don't think it's _that_ bad (but eventually when making the various vehicles more modular I think such hooks into Load_VEHS() will be better approach, but we could always change this later without any hassles.

* What's the etymology of AirportFTAbduildup?

http://darkvater.no-ip.info/ottd/airports.html

I'm sorry but I didn't found there what does FTA mean, neither whether 'bduildup' has some special meaning or you just manage to make the same typo each time you wrote this identifier.

* I'm not sure if the *BITS() macros don't in reality obfuscate the code instead. Aren't the bit operations obvious enough?

It should stay in *BITS. Right now it would be for example:

Code: Select all

CLRBITS(target, bitscheck)

Without macro:

Code: Select all

target &= ~bitscheck

Apart from this being ugly code to do over and over again, it gives rise to lot of errors, and confusion.
Or For example CLRBIT is:

Code: Select all

CLRBIT(x,y) ((x) &= ~(1 << (y)))

You DON'T want to write that every time in the code

[/list]

I agree with *BIT() macros and I have nothing against them. The *BITS() macros seem rather *too* trivial to me, though. But if anything please keep these at one place - put them to macros.h.

So it works? :?:

Yes it does. Very impressive! :) Although I tested it with only a single aircraft so far...

I think the new airports aren't very balanced from the gameplay viewpoint, though. I mean, why should you ever bother building the city airport anymore when you can build a metropolitan airport which costs the same, has the same size and better throughput? Even if it would cost slightly more, I think the city airport is pretty unfeasible. Either the metropolitan airport should cost considerably more or it should be slightly larger so that you might not fit it somewhere. Ie. it could be 6x7 (compared to 6x6 city airport) and by 1/2 more expensive. Then I wouldn't be afraid of making the international airport even much more expensive (5/3 of international airport cost, perhaps) - after all it requires a lot of underground corridors etc.

BTW the metropolitan airport is missing the transmitter. Too bad :-(. But when you'd increase its size to 6x7... ;-)

[pasky]

I have nothing to tabs of size 2 as long as they are _tabs_... ;)

[Darkvater]

Huh? Output of grfcodec is hexadecimal. Or what output do you mean?

GRFcodec crates an nfo and a pcx file. Most of the hex values in aircraft_cmd.c refer to the numbers that are above each subpicture in the nfo file. And those are in decimal format. It is really sucky to always convert the hex code to decimal or back when you want to change a graphic, or add a new one. But indeed, should agree on an update some day

* I think the code for fixing aircrafts could live in vehicle.c's load handler for now. I put the wagon override stuff there as
well. Regarding airports flags reset, that should live in station_cmd.c?

Currently compatibility hacks for older savegames are put at the place where the relevant data is loaded, and I don't think that's a bad convention. Alternately you could call kinda FixupLoadedAircrafts() from Load_VEHS(). Or you could do it the way you do it now, fixing it up at the first run of a function, I don't think it's _that_ bad (but eventually when making the various vehicles more modular I think such hooks into Load_VEHS() will be better approach, but we could always change this later without any hassles.

First run is not that nice imho, it would need to reset the static variable on each load of a game obviously. It is okay to me to seperate the two, one in vehicles and the other in airports, as long as they get called both on loading an oldformat game

I'm sorry but I didn't found there what does FTA mean, neither whether 'bduildup' has some special meaning or you just manage to make the same typo each time you wrote this identifier.

FTA would be a finite state machine. Well it should be FSM since it stands for Finite State Machine. It's just silly cause I had a class about 'Formele Talen en Automaten' which would stand for 'Formal Languages and Automatas'. The abbriviation for that is FTA. But it fits though Finite sTate mAchine. The airports movement go through this machine deciding where to go next. 'Buildup' would be the input, or structure of these machines. 'dbuildup' is a typo .This is then copied into an internal format (almost the same). This way, the framework is already there for the implementation of more airports.

I agree with *BIT() macros and I have nothing against them. The *BITS() macros seem rather *too* trivial to me, though. But if anything please keep these at one place - put them to macros.h.

Yes, BITS are a bit trivial. I thought since not everyone is into bit juggling, doing a CLR/SET/HAS is a much easier and more understandeable way to do then then doing ^= ~. (I am one of those people )

I think the new airports aren't very balanced from the gameplay viewpoint, though. I mean, why should you ever bother building the city airport anymore when you can build a metropolitan airport which costs the same, has the same size and better throughput? Even if it would cost slightly more, I think the city airport is pretty unfeasible. Either the metropolitan airport should cost considerably more or it should be slightly larger so that you might not fit it somewhere. Ie. it could be 6x7 (compared to 6x6 city airport) and by 1/2 more expensive. Then I wouldn't be afraid of making the international airport even much more expensive (5/3 of international airport cost, perhaps) - after all it requires a lot of underground corridors etc.

To tell you the truth I haven't even looked at prices, never even thought about it. But the other side of the balance: who would ever build a small airport if you have large airports? Anyways, prices need to be upped greatly. I already mentioned in the readme that introduction of airports should be done based on years. Currently you get the city airport when the first jet-plane is availble. Needs a change imho.

BTW the metropolitan airport is missing the transmitter. Too bad . But when you'd increase its size to 6x7...

It has a radar tough

[pasky]

Huh? Output of grfcodec is hexadecimal. Or what output do you mean?

GRFcodec crates an nfo and a pcx file. Most of the hex values in aircraft_cmd.c refer to the numbers that are above each subpicture in the nfo file. And those are in decimal format. It is really sucky to always convert the hex code to decimal or back when you want to change a graphic, or add a new one. But indeed, should agree on an update some day :)

Aha, that makes sense.

What about making an enum listing the sprite numbers (I don't care about the base then, but it would be nice to write commented out hex value for the first and last item of the enum, so you have an idea where does it fit into the map of other sprite numbers) and then using the symbolic names?

* I think the code for fixing aircrafts could live in vehicle.c's load handler for now. I put the wagon override stuff there as
well. Regarding airports flags reset, that should live in station_cmd.c?

Currently compatibility hacks for older savegames are put at the place where the relevant data is loaded, and I don't think that's a bad convention. Alternately you could call kinda FixupLoadedAircrafts() from Load_VEHS(). Or you could do it the way you do it now, fixing it up at the first run of a function, I don't think it's _that_ bad (but eventually when making the various vehicles more modular I think such hooks into Load_VEHS() will be better approach, but we could always change this later without any hassles.

First run is not that nice imho, it would need to reset the static variable on each load of a game obviously. It is okay to me to seperate the two, one in vehicles and the other in airports, as long as they get called both on loading an oldformat game :)

You have a point about resetting the static variable. That's evil, then I'd vote for separating the hooks and putting them to the relevant Load_*() functions.

I think the new airports aren't very balanced from the gameplay viewpoint, though. I mean, why should you ever bother building the city airport anymore when you can build a metropolitan airport which costs the same, has the same size and better throughput? Even if it would cost slightly more, I think the city airport is pretty unfeasible. Either the metropolitan airport should cost considerably more or it should be slightly larger so that you might not fit it somewhere. Ie. it could be 6x7 (compared to 6x6 city airport) and by 1/2 more expensive. Then I wouldn't be afraid of making the international airport even much more expensive (5/3 of international airport cost, perhaps) - after all it requires a lot of underground corridors etc.

To tell you the truth I haven't even looked at prices, never even thought about it. But the other side of the balance: who would ever build a small airport if you have large airports? Anyways, prices need to be upped greatly. I already mentioned in the readme that introduction of airports should be done based on years. Currently you get the city airport when the first jet-plane is availble. Needs a change imho.

Sometimes you just cannot fit anything but a small airport to that space, or you have money trouble (very early stage of game). Otherwise I'd propose a timetable of: 1920 - Country airports, 1960 (1955?) - City airports, 1970 - Metropolitan airports, 1985 - International airports. But I think metropolitan airports should be really wider by one tile. :P

BTW, in the GUI as well as in the code I think 'small' should be renamed to 'country' and 'big' to 'city'. But it'd be fine to do this in a separate patch later, no high priority.

BTW the metropolitan airport is missing the transmitter. Too bad :-(. But when you'd increase its size to 6x7... ;-)

It has a radar tough :P

[/quote]

But I love transmitters! :)

[Darkvater]

Sometimes you just cannot fit anything but a small airport to that space, or you have money trouble (very early stage of game). Otherwise I'd propose a timetable of: 1920 - Country airports, 1960 (1955?) - City airports, 1970 - Metropolitan airports, 1985 - International airports. But I think metropolitan airports should be really wider by one tile.

BTW, in the GUI as well as in the code I think 'small' should be renamed to 'country' and 'big' to 'city'. But it'd be fine to do this in a separate patch later, no high priority.

Yes, see http://i-want-a-website.com/about-tt/ttfaq13.html#13-8

[pasky]

Sometimes you just cannot fit anything but a small airport to that space, or you have money trouble (very early stage of game). Otherwise I'd propose a timetable of: 1920 - Country airports, 1960 (1955?) - City airports, 1970 - Metropolitan airports, 1985 - International airports. But I think metropolitan airports should be really wider by one tile. :P

BTW, in the GUI as well as in the code I think 'small' should be renamed to 'country' and 'big' to 'city'. But it'd be fine to do this in a separate patch later, no high priority.

Yes, see http://i-want-a-website.com/about-tt/ttfaq13.html#13-8

Are you going to exactly follow that proposal?

[Darkvater]

Sometimes you just cannot fit anything but a small airport to that space, or you have money trouble (very early stage of game). Otherwise I'd propose a timetable of: 1920 - Country airports, 1960 (1955?) - City airports, 1970 - Metropolitan airports, 1985 - International airports. But I think metropolitan airports should be really wider by one tile.

BTW, in the GUI as well as in the code I think 'small' should be renamed to 'country' and 'big' to 'city'. But it'd be fine to do this in a separate patch later, no high priority.

Yes, see http://i-want-a-website.com/about-tt/ttfaq13.html#13-8

Are you going to exactly follow that proposal?

Well not exactly, since metropolitan should be much better then. But I'd like to follow the introduction dates listed there.