Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Airport entry incorrect calculation #6341

Closed
DorpsGek opened this issue Jul 13, 2015 · 5 comments
Closed

Airport entry incorrect calculation #6341

DorpsGek opened this issue Jul 13, 2015 · 5 comments
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)

Comments

@DorpsGek
Copy link
Member

marcole opened the ticket and wrote:

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 :-).

Attachments

Reported version: trunk
Operating system: All


This issue was imported from FlySpray: https://bugs.openttd.org/task/6341
@DorpsGek
Copy link
Member Author

marcole wrote:

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.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/6341#comment14009

@DorpsGek
Copy link
Member Author

planetmaker wrote:

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 :) )


This comment was imported from FlySpray: https://bugs.openttd.org/task/6341#comment14010

@DorpsGek
Copy link
Member Author

marcole wrote:

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 :-)

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/6341#comment14011

@DorpsGek
Copy link
Member Author

frosch wrote:

I prefer the first version. The explicit DiagDirDifference is better than some DiagDir->DiagDirDiff cast.


This comment was imported from FlySpray: https://bugs.openttd.org/task/6341#comment14075

@DorpsGek
Copy link
Member Author

frosch closed the ticket.

Reason for closing: Fixed

in r27422. Thanks for the patch!


This comment was imported from FlySpray: https://bugs.openttd.org/task/6341

@DorpsGek DorpsGek added Core flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) labels Apr 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)
Projects
None yet
Development

No branches or pull requests

1 participant