FS#6341 - Airport entry incorrect calculation

Attached to Project: OpenTTD
Opened by Marek (marcole) - Monday, 13 July 2015, 18:05 GMT
Last edited by frosch (frosch) - Friday, 30 October 2015, 16:27 GMT
Type Bug
Category Core
Status Closed
Assigned To Marek (marcole)
Operating System All
Severity Low
Priority Normal
Reported Version trunk
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


Looking at the aircraft and airport movement schemes I have found this issue.
I think the code related to airport entry is not correct.
Specifically, aircraft_cmd.cpp, line 812:
dir = ChangeDiagDir(dir, (DiagDirDiff)ReverseDiagDir(DirToDiagDir(rotation)));

It doesn't seem alright to me.
For example, the default rotation yields the opposite direction as desired and the aircraft has to do 180 degree turns.
I was always wondering why the aircraft does these crazy turns on arrival to the airport.
Now I have found that also the code comment specifically says it is undesirable and was meant to deal with this.
Unfortunately I don't think it works and airport entry values confirm that.

So here are my suggested patches (movement looks much better now :-).
This task depends upon

Closed by  frosch (frosch)
Friday, 30 October 2015, 16:27 GMT
Reason for closing:  Fixed
Additional comments about closing:  in r27422. Thanks for the patch!
Comment by Marek (marcole) - Tuesday, 14 July 2015, 17:33 GMT
I have thought about this a bit more and I think the current implementation might have sense but needs to see the rotation as a difference, not a direction.
That means it might be something like:
dir = ChangeDiagDir(dir, ReverseDiagDirDiff((DiagDirDiff)DirToDiagDir(rotation)));
which needs one new direction function.

Here is the additional patch with several new functions to have a more complete set, although some are not used yet (but might be in the future ;-)
It may have sense to also switch the rotation parameter from Direction to DirDiff completely or even to DiagDirDiff, but it needs to be discussed.

Similarly, there is this one piece of code (airport.cpp, line 86) that basically says the same thing, and there are others for sure:
amd.direction = ChangeDir(orig->direction, (DirDiff)rotation);

I can provide complete patches for that if necessary and if seen reasonable.
Comment by Ingo von Borstel (planetmaker) - Wednesday, 15 July 2015, 13:05 GMT
I'm not on my OpenTTD computer currently; however can you either describe more in detail where the unreasonable aircraft rotation happens or maybe provide a screenshot with an aircraft at that place (that makes it easier to verify :) )
Comment by Marek (marcole) - Wednesday, 15 July 2015, 21:01 GMT
Sure, although visualisation of movement (attached) should be enough and you can check the behaviour in the trunk title save anytime (city & international).

The entry points of city airport are 26, 29, 27, 28 (obviously correctly meant for NE, SE, SW, NW).
However, the aircraft approaching the airport choose the entry points as 27, 28, 26, 29 (for the same approaching directions).
That means that generally all the aircraft choose the worst possible entry point and are forced to do 180 degree turn right away.
The aircraft approaching from NE goes to 27 instead of 26 and that is unreasonable and I think not intended.

In case of e.g. international airport the entry points are 38, 37, 40, 39, but the chosen ones for approaching directions from NE to NW are 40, 39, 38, 37.
Although a bit simpler scheme it yields the same undesirable movement pattern.

All this applies to the default (i.e. 0) airport rotation, but is incorrect in the same way in case of 180 degree rotation.
It actually works for 90 and -90 degree airport rotation but that is just a coincidental effect I would say :-)
Comment by frosch (frosch) - Friday, 30 October 2015, 16:27 GMT
I prefer the first version. The explicit DiagDirDifference is better than some DiagDir->DiagDirDiff cast.