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

Some Aiprot functions doesn't take layouts/views into account #4764

Closed
DorpsGek opened this issue Sep 7, 2011 · 4 comments
Closed

Some Aiprot functions doesn't take layouts/views into account #4764

DorpsGek opened this issue Sep 7, 2011 · 4 comments
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)

Comments

@DorpsGek
Copy link
Member

DorpsGek commented Sep 7, 2011

Zuu opened the ticket and wrote:

Trunk version: r22893

------------------------

station_cmd.cpp:
GetMinimalAirportDistanceToTile(const AirportSpec *as, TileIndex town_tile, TileIndex airport_tile)

The function will produce wrong results for

* A 90 or 270 degree rotated non-irregular airport
* An irregular airport (unless all irregularities exist in the inside - eg. a hole in the middle)

To fix this the function will need to take an extra argument of which layout of the airport to use. (note that this function can operate on yet to be built airports)

------------------------

station_cmd.cpp:
AirportGetNearestTown(const AirportSpec *as, TileIndex airport_tile)

This function also need another argument to take layout into account.

------------------------

station_cmd.cpp:
GetAirportNoiseLevelForTown(const AirportSpec *as, TileIndex town_tile, TileIndex tile)

This function needs an extra argument to take layout into account.

------------------------

Above, mostly only extra arguments have been mentioned. On the implementation side, there is an issue that size_x and size_y are attached to the AirportSpec and not to the different layouts. While it is not a problem to rotate them for 90 and 270 degree rotations, the bigger issue is that different layouts can have different tile layouts and thus different sizes.

Another question is if GetMinimalAirportDistanceToTile should take irregular airports into account or still use a bounding box of the airport for speed reasons. For a large L-shaped airport, using a square bounding box to get eg. noise level could be quite wrong compared to really computing the distance between the airport and town. On the other hand a bounding box would be much faster, but will require a size_x and size_y for each layout.

Reported version: trunk
Operating system: All


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

DorpsGek commented Sep 8, 2011

Zuu wrote:

In an attempt to see how performance critical these functions are I have looked up which functions that call these functions:

GetMinimalAirportDistanceToTile is called by:
- GetAirportNoiseLevelForTown
- AirportGetNearestTown

GetAirportNoiseLevelForTown is called by:
- UpdateAirportsNoise // called only by DoCreateTown
- CmdBuildAirport
- RemoveAirport (station_cmd.cpp)
- AIAirportType::GetNoiseLevelIncrease

AirportGetNearestTown is called by:
- UpdateAirportsNoise
- CmdBuildAirport
- RemoveAirport
- AIAirportType::GetNoiseLevelIncrease
- AIAirportType::GetNearestTown


This comment was imported from FlySpray: https://bugs.openttd.org/task/4764#comment10333

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 8, 2011

Zuu wrote:

This patch inserts a new 'layout' argument after the first argument on the three functions mentioned in the bug report. The implementation of GetMinimalAirportDistanceToTile has been changed so that it iterates through all tiles of the airport and compute the manhattan distance to the tile in the last function argument.

Improvements over current trunk implementation:
- Works good with irregular airports
- Works good even if two layouts of the same airport type have different sizes
- Supports rotated layouts

Negative:
- Lower performance than trunk implementation

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/4764#comment10334

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 8, 2011

Zuu wrote:

Fixed issues stated by Yexo on IRC:
in GetMinimalAirportDistanceToTile there is some extra whitespace (the line after mindist=dist;
and an "assert(layout < as->num_table);" at the top of that function would be nice imo
or maybe that assert is better in GetAirportNoiseLevelForTown and/or AirportGetNearestTown
Putting the assert in both callers is perhaps better as it can be done once and not inside any loop.
/any loop/town loop/

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/4764#comment10335

@DorpsGek
Copy link
Member Author

DorpsGek commented Dec 6, 2011

Rubidium closed the ticket.

Reason for closing: Fixed

In r23441. Thanks for the patch


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

@DorpsGek DorpsGek closed this as completed Dec 6, 2011
@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