FS#1562 - patch: upgrade airports

Attached to Project: OpenTTD
Opened by Círdan (cirdan) - Sunday, 16 December 2007, 21:35 GMT
Last edited by andythenorth (andythenorth) - Monday, 14 August 2017, 20:09 GMT
Type Patch
Category Core
Status Closed
Assigned To andythenorth (andythenorth)
Operating System All
Severity Low
Priority Normal
Reported Version 0.5.3
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 4
  • Miraceti (miraceti) (2012-09-09)
  • Opperpanter (Opperpanter) (2010-01-17)
  • David Jaša (dejv) (2009-12-26)
  • Iain McFarlane (imcfarla) (2009-01-11)
Private No


I would like to submit my upgrade-airport patch for inclusion in trunk.

The patch allows an airport to be built over another one, much like it can already be done with railroad stations. When doing so, the old airport is destroyed and the new one is built in an atomical way. The advantages of this are:

- First of all, it simplifies upgrading airports (replacing an airport type with another one, usually bigger). You just select the new airport and build it over the old one.

- Being an atomical operation means that there will be no "window of opportunity" between removing the old airport and placing the new one, so the town can't suddenly build a road or, worse, a building in the area in the interim.

- Also, town authority ratings are checked before execution starts, so you won't find you've removed an airport and then you can't build the new one, nor rebuild the old one.

Requirements for the airport remain the same, except that the 2-airports-per-town rule is not checked if replacing an airport.

This patch addresses the following feature request:

Of course, this patch is most useful coupled with the close-airport patch. Together they make upgrading an airport really easy. :-)
This task depends upon

Closed by  andythenorth (andythenorth)
Monday, 14 August 2017, 20:09 GMT
Reason for closing:  Won't implement
Additional comments about closing:  Mass closure of patch tickets with no commentary for >5 years. Goal is to reduce patch queue as an experiment to see if it aids faster reviewing and rejection/acceptance (it may not). If this offends you and the patch is maintained and compiles with current trunk, discuss with andythenorth in irc. (andythenorth has no ability to review patches but can re-open tickets).
Comment by Jean-Francois Claeys (Belugas) - Monday, 17 December 2007, 00:22 GMT
Could you comment some of your work a little more?
CanRemoveAirport, for example.
Comment by Círdan (cirdan) - Monday, 17 December 2007, 22:32 GMT
> Could you comment some of your work a little more?

Sure. Is this new version better?

> CanRemoveAirport, for example.

CanRemoveAirport is code extracted from RemoveAirport. Now it is also needed in CmdBuildAirport, so I factored it out.

By the way, the new version also changes CMD_ERROR in CanRemoveAirport into more meaningful error messages ("Aircraft in the way").
Comment by Círdan (cirdan) - Sunday, 13 January 2008, 12:16 GMT
Rebased against r11818, to make up for the new handling of EXPENSES_*.

I still don't know if the comments I added are enough or more are needed...

Also, could someone change the 'reported version' from 0.5.3 to 'devel'? I failed to set it properly in the original post.
Comment by Círdan (cirdan) - Sunday, 20 January 2008, 11:35 GMT
I've updated the patch to fix a nasty bug that would crash the game if an airport was upgraded with CTRL pressed (to build an airport adjacent to another).
Comment by Círdan (cirdan) - Thursday, 03 April 2008, 18:53 GMT
Rebased against 0.6.0.
Comment by Zdeněk Sojka (SmatZ) - Thursday, 03 April 2008, 23:20 GMT
When you move the airport, the sign doesn't move
Comment by Círdan (cirdan) - Saturday, 05 April 2008, 19:00 GMT
Oops, good catch. Fixed, and updated against r12565.
Comment by Círdan (cirdan) - Sunday, 25 May 2008, 14:56 GMT
I've updated the patch against r13232, per a request in the forums, and I thought that I could as well post it here, too.
Comment by Círdan (cirdan) - Saturday, 09 August 2008, 12:04 GMT
Rebased against r14013.
Comment by Círdan (cirdan) - Saturday, 04 October 2008, 10:03 GMT
Rebased against r14432.

May I ask what the devs' opinion about this patch is? I have been keeping it up to date for nearly a year now, and periodically posting it here, but I still do not know if you just do not like it, if it has a chance but there are issues with it, if it happens to touch things in the code which are currently unsettled... Any comment would be appreciated.
Comment by David Jaša (dejv) - Saturday, 26 December 2009, 12:27 GMT
Wiki says this feature is 90 % complete:
What needs to be done to merge the patch to trunk?
Comment by Alberth (Alberth) - Saturday, 20 August 2011, 10:53 GMT
 FS#1292  (Out-of-service stations (proposal)) is a more generic description of closing & upgrading stations in general
Comment by Círdan (cirdan) - Sunday, 21 August 2011, 14:08 GMT
Err, sorry, no, this patch is completely independent of  FS#1292 . I think you have posted the same comment in  FS#1497  and here without noting the difference.

 FS#1497  is indeed similar to (or a particular case of)  FS#1292 . This patch, on the other hand, addresses a different problem, namely that of atomically overbuilding an (empty) airport, and has nothing to do with routing vehicles and the like. Think of it as follows: To upgrade a station (road, rail or airport--docks are different since ships do not enter the dock tiles), first you must ensure that there are no vehicles on the station, then you do the actual replacing. The first part is where airport closure or out-of-service stations help, to tell vehicles to "stay away" from the station (temporarily or permanently) so that they do not interfere. But this patch addresses the second part, removing and rebuilding the station, by making the operation atomic and more UI-friendly (fewer clicks, less hassle). In fact, this patch brings airports in line with road and rail stations, which can already be overbuilt in the current code (this makes me wonder if airports are so special that they do not deserve this feature...).

Anyway, now that someone is actually looking at this, can I please have this patch reviewed? I am providing an up-to-date patch which should apply against current trunk, and I can also provide the same patch split into three smaller patches if desired (this is the way I maintain it).
Comment by Alberth (Alberth) - Sunday, 21 August 2011, 18:16 GMT
My main motivation for posting issue  FS#1292  in both your issues is to make sure all related issues can be found easily.
I do understand that the three issues aim for different things (otherwise I'd merged them).

Unfortunately, I am not enough familiar with airport buy/sell code to decide about your patch at code level.
However, I can see some other potential problems.

- This patch assumes the airport is empty. In a normal game that never happens. As such, the current patch does not provide an upgrade path for a user.

- Adding the 'close airports' patch  FS#1497  would give the user a more useful way. That however means that the 'close airports' patch needs to be added to trunk before this one can be considered.
- The 'close airports' patch to me does not go far enough. I don't see a convincing reason to not allow closing of other types of station, even if they are easier to modify while in use. Other devs may disagree here.

- In the NewGRF airports proposal, a different way described to upgrade. That means a decision needs to be made first which of both proposals is better.
Comment by Círdan (cirdan) - Sunday, 21 August 2011, 21:08 GMT
> - This patch assumes the airport is empty.

I am not sure I understand what you mean by "assumes the airport is empty". If you mean that the patch blindly upgrades an airport without checking whether there are aircraft on it or not, this is not the case. If you mean that the patch is only useful when the airport is empty, then that is evidently true: The aim of the patch is to simplify the double user action "demolish old airport+build new airport" into a single one, and to demolish an airport it has to be empty. There is no semantic change with this patch.

> In a normal game that never happens.

In my games it does happen... In fact, I cannot see another way of turning an existing airport into a better one at the same station.

> As such, the current patch does not provide an upgrade path for a user.

Right. The patch does not provide an upgrade path, it simplifies the existing one. You may claim that this is not a big win, but it is a win nevertheless. And I remember reading several posts in the forum asking precisely for this simplification.

> - Adding the 'close airports' patch  FS#1497  would give the user a more useful way. [...]
> - The 'close airports' patch to me does not go far enough. [...]

You seem to think that this patch is only useful if the airport-close patch is applied first, but I have to disagree. While the airport-close patch greatly enhances its usefulness, this patch remains useful (albeit not as much) on its own, for the reasons above. Plus it makes airports behave like road and rail stations, which can already be overbuilt. Apparently, in  FS#1497 , coherency with other station types was important...

> - In the NewGRF airports proposal, a different way described to upgrade. That means a decision needs to be made first which of both proposals is better.

Why? Whichever proposal is better, why cannot we have both? The airport GRF spec is currently... vague, to say the least, about overbuilding airports. It gives some hints about what should happen to aircraft, but does not address all possible issues. Will it be possible to overbuild an airport in such a way that a terminal building ends up right on top of a currently loading airplane? As I see it, the upgrade spec can only make sense for certain overlap configurations, at best, while this patch would apply to the demolish-the-whole-thing-and-start-over case (at the cost of requiring the airport to be empty). They would complement each other rather nicely, in fact.

Perhaps part of the problem is that I called this patch "airport upgrade" instead of "airport overbuild", which would better convey what it does...