Patch: Close airports

[cirdan]

hi
looking to other patches, i changed your code making it buildOTTD-friendly.
It's a simple change, when you declare the file you want patch you must code in this way:

Code: Select all

Index: src/aircraft_cmd.cpp
===================================================================
--- src/aircraft_cmd.cpp	(revision 10765)
+++ src/aircraft_cmd.cpp	(working copy)

I've changed the lines and now i've compiled it.

Ah, ok, thank you.

I found this patch very intresting, but i think i found a bug: when there are a lot of aircraft in queue, it's quite impossible to bulldoze the airport, becouse it says "aircraft in the way" also if the airpot is empty. maybe becouse everytime there's an aircraft trying to land?

Yes, that's a bug, but it isn't the patch, it's already in the code. I think the culprit is this piece of code (file src/station_cmd.cpp, function RemoveAirport):

Code: Select all

        BEGIN_TILE_LOOP(tile_cur, w, h, tile) {
                if (!EnsureNoVehicle(tile_cur)) return CMD_ERROR;

                if (flags & DC_EXEC) {
                        DeleteAnimatedTile(tile_cur);
                        DoClearSquare(tile_cur);
                }
        } END_TILE_LOOP(tile_cur, w, h, tile)

The code tries to ensure that there aren't any vehicles on any tile of the airport, but in doing so it doesn't care whether it's a plane just overflying the airport. The attached savegame and screenshot demonstrate this: a plane is flying from airport A (to the northwest) to airport C (to the southeast); while it overflies airport B (which is not even on the schedule for the plane), the airport can't be demolished (the game complains "Aircraft in the way").

Closing an airport just makes this happen more often, as there can be lots of aircraft overflying the airport (as in your case).

Perhaps EnsureNoVehicle() should be changed to EnsureNoVehicleOnGround()? Any devs out there?

[RainierWatcher]

AFAIK, that problem has been in trunk for ages, possibly since the original TTD. No idea why it does that. I could be wrong though, and it might be a lesser bug that was introduced when the crash following demolishing an airport was fixed.

[richk67]

AFAIK, that problem has been in trunk for ages, possibly since the original TTD. No idea why it does that. I could be wrong though, and it might be a lesser bug that was introduced when the crash following demolishing an airport was fixed.

Might also be that the aircraft shadow (which is an aircraft vehicle anyway) is currently on the airport. Just a thought.

You may want to check whether if the aircraft is not over the airport, if you can delete it then.

[RainierWatcher]

I think it's only with the top square of the airport. I think that square also determines the town the airport is in.

I'm sure I've demolished an airport when it's over other squares.

[Youri219]

I think it's only with the top square of the airport. I think that square also determines the town the airport is in.

I'm sure I've demolished an airport when it's over other squares.

If you look carefully at the above screenshot, you will indeed see the shadow in that square.

[cirdan]

I think it's only with the top square of the airport. I think that square also determines the town the airport is in.

I'm sure I've demolished an airport when it's over other squares.

Instead of speculating, we could as well test it.

Try and load the savegame I attached in my previous post, and try to demolish the airport while the plane overflies it. The game won't let you as soon as the plane is over the topmost tile and until it has left the rightmost tile. (Hint: Enable the build in pause mode cheat; it's easy to quickly unpause-pause so that the plane just moves a little.) So it isn't only the topmost tile, or that with the station sign.

[RainierWatcher]

Well this was the first square I could demolish it on.

I've never demolished an Intercontinental before, but smaller airports seem less affected.

[cirdan]

I've rebased the patch against the latest nightlies, as there seems to be some interest in this patch in the ChrisIN thread.

The new patch applies cleanly against r11152 and r11159 (at least). I've tried to make the patch svn-friendly, but I'm not sure if it will work with svn as I don't use it.

[doghousedean]

Thanks, much appreciated.

If you dont use SVN what do you use for patches?

Thanks again

[cirdan]

If you dont use SVN what do you use for patches?

I untar the sources into a directory (say ottd-r11159) and keep it clean, I make a working copy into another directory (say ottd-r11159-airport-close) and when I'm done with the changes I make the patch with 'diff -ur ottd-r11159 ottd-r11159-airport-close'.

[Snuk the Great]

Just a quick thought on this patch. The only reason why I would need to close an airport with this patch, is when I want my airfield upgraded (or maybe moved), so wouldn't it be a nice idea to just get an 'upgrade' button instead?

Now the way I see this working is:
1) You click upgrade
2) Select a new airfield and place it the way you want it, like a normal airport, but with some range limitations or something.
3) Use a graphic like the bought land sign to reserver the land.
4) Wait untill all aircrafts are gone from the airport.
5) Let the system delete the old airport and create the new one like we requested.
6) Open up again

I think this would just be a bit easyer for the end user. Tell me what you guys think.

[different]

But I'm sure trains would be a totally different thing.

Maybye bouncing off station just like if there were clear tiles (no railway)?
However, this would require change in code not related to stations I think

IMO it should be like 'skip order'. So if you have three stations on a mainline and close the middle one, the trains will try and find a bypass - otherwise they could pass through the station without stopping. Or, alternately, they would wait at the entrance until it was opened.

[chrissicom]

This patch line

Code: Select all

SetWindowWidgetLoweredState(w, 14, st->airport_flags & AIRPORT_CLOSED_block);

gives a performance warning when compiling

3>..\src\station_gui.cpp(723) : warning C4800: 'const uint64' : forcing value to bool 'true' or 'false' (performance warning)

any idea how to fix this?

btw... I know lots of games where

4) Wait untill all aircrafts are gone from the airport.

this never happens, i.e. there's always an aircraft on the airport.

[glx]

Use

Code: Select all

SetWindowWidgetLoweredState(w, 14, st->airport_flags & AIRPORT_CLOSED_block != 0);

[cirdan]

This patch line

Code: Select all

SetWindowWidgetLoweredState(w, 14, st->airport_flags & AIRPORT_CLOSED_block);

gives a performance warning when compiling

3>..\src\station_gui.cpp(723) : warning C4800: 'const uint64' : forcing value to bool 'true' or 'false' (performance warning)

any idea how to fix this?

Use

Code: Select all

SetWindowWidgetLoweredState(w, 14, st->airport_flags & AIRPORT_CLOSED_block != 0);

I don't get that warning, as gcc has no such things as "performance warnings". But yes, this should fix it.

Something that could also be done to further ease upgrading an airport would be to allow an airport to be built over another, implying demolish-then-build, much like you can do with railroad stations. It just saves a few clicks, but it is convenient anyway, and it would prevent "I've already demolished my old airport and now the local authority won't let me build a new one". I may give it a try.

[gule]

Nice patch.

But could you hide "Close Airport" button when station type is not airport, not just disable it.
You can do this with function

Code: Select all

static inline void SetWindowWidgetHiddenState(Window *w, byte widget_index, bool hidden_stat)

[cirdan]

Nice patch.

But could you hide "Close Airport" button when station type is not airport, not just disable it.
You can do this with function

Code: Select all

static inline void SetWindowWidgetHiddenState(Window *w, byte widget_index, bool hidden_stat)

Certainly, it wouldn't be difficult, but I'm not sure if it would be better that way, much like the 'aircraft' button (show aircraft with this station on their schedule) is just disabled, and not hidden, when the station has no airport facility.

[gule]

I know. It is only my opinion that it would look better. It is your call.

[DaleStan]

Certainly, it wouldn't be difficult, but I'm not sure if it would be better that way, much like the 'aircraft' button (show aircraft with this station on their schedule) is just disabled, and not hidden, when the station has no airport facility.

The decision between disabled and hidden basically amounts to: "If the designers were half-smart about UI issues, they'd ... omit the impossible entries. If they were really smart, they'd leave the impossible choices in but gray them out, signifying that if your system were configured a bit differently you really could [use them]" -- ESR, TAoUU.

I think the important thing to note there is "a bit differently". Does adding an airport count as "a bit different"? IMO, options that depend on major changes should be hidden, not disabled, sort of like how I know XP can support multiple monitors, but there's no way for me to configure this box to do so, since it doesn't have multiple video cards/outputs.

The show-$TYPE-vehicles buttons should definitely be disabled, not hidden. Partially because adding especially a bus or truck station is a little change, but also because the five of them together are far more obvious than only one or two.

[cirdan]

Certainly, it wouldn't be difficult, but I'm not sure if it would be better that way, much like the 'aircraft' button (show aircraft with this station on their schedule) is just disabled, and not hidden, when the station has no airport facility.

The decision between disabled and hidden basically amounts to: "If the designers were half-smart about UI issues, they'd ... omit the impossible entries. If they were really smart, they'd leave the impossible choices in but gray them out, signifying that if your system were configured a bit differently you really could [use them]" -- ESR, TAoUU.

I think the important thing to note there is "a bit differently". Does adding an airport count as "a bit different"? IMO, options that depend on major changes should be hidden, not disabled, sort of like how I know XP can support multiple monitors, but there's no way for me to configure this box to do so, since it doesn't have multiple video cards/outputs.

The show-$TYPE-vehicles buttons should definitely be disabled, not hidden. Partially because adding especially a bus or truck station is a little change, but also because the five of them together are far more obvious than only one or two.

I have just reread your post three times and I still do not get what point you are trying to make, if any, regarding the particular question of whether to disable or hide the "close airport" button. If I understand correctly, you are supporting the quote in your first paragraph, under the interpretation that things that would be usable if the system were configured "a bit differently" should be disabled, but that major differences should imply hiding. However, I fail to see whether you consider adding an airport to be a major difference, although the last paragraph seems to suggest that you do not. Perhaps you were not trying to make that point and I am just being thick.

Anyway, back on topic, the main reason I see to have the button disabled, and not hidden, is that if more buttons are later added, then hidden buttons either leave gaps (which I find aesthetically unpleasant) or force rearrangement of the other buttons (which is bad UI).

I know. It is only my opinion that it would look better. It is your call.

I don't think it is my call. I consider this a community project, so anyone can give their opinion, and if enough people prefer the button hidden, we can make it so, or better yet, give the user the choice.