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

Airports: Improper aircraft movement when the northernmost airport tile is not part of the layout #8437

Closed
JGRennison opened this issue Dec 26, 2020 · 5 comments

Comments

@JGRennison
Copy link
Contributor

Version of OpenTTD

Current master (2c8c6d4).

Expected result

Aircraft movement is as expected when the northernmost airport tile (lowest tile index corner of the airport layout bounding rectangle) is not part of the airport layout.

Actual result

Aircraft movement is not as expected when the northernmost airport tile (lowest tile index corner of the airport layout bounding rectangle) is not part of the airport layout.

An airport layout with this property is OpenGFX Airports 0.5->Hub->Intercontinental->Rotated 180°.
See attached savegame.

Observed issues include:

  1. If the height of the aforementioned tile is not equal to that of the rest of the airport, the aircraft will land and subsequently taxi/etc. at that height. This may be above or below the ground level.
  2. Helicopters landing at helipad 2 in the OpenGFX Airports 0.5->Hub->Intercontinental->Rotated 180° airport layout trigger an assertion failure due to Vehicle::tile being set to the aforementioned tile upon landing, where a MP_STATION tile is required.

Steps to reproduce

See attached savegame.
For sub-issue 1, observe the movement of aircraft 7 and 8.
For sub-issue 2, press the "go" button for aircraft 1 to 6, and observe the subsequent assertion failure when the aircraft next come to land.
Airport Testing Ltd, 2010-02-28.sav.zip

@James103
Copy link
Contributor

James103 commented Dec 26, 2020

I can't open the save file when downloading from Google Chrome 87, as the browser forces you to delete the file because "it is dangerous" with no way to keep the file.

Edit: The above behavior was because I had Safe Browsing on. Affected users may need to disable Safe Browsing for a brief moment in order to download the save game.

@JGRennison
Copy link
Contributor Author

I can't open the save file when downloading from Google Chrome 87, as the browser forces you to delete the file because "it is dangerous" with no way to keep the file.

I'd suggest using a different browser if the behaviour can't be disabled.

LordAro added a commit to LordAro/OpenTTD that referenced this issue Dec 27, 2020
… drawn at the wrong height

Only the rotated intercontinental airport, don't get excited
@LordAro
Copy link
Member

LordAro commented Dec 27, 2020

Making some progress on this

Bug 1 is fixed here - LordAro@b427f30

Bug 2 is proving more tricky. Some notes of things I've found so far:

  • Bug actually exists for all second (and third?) heliports, just only matters for the rotated airport with a missing tile at the top
  • Helicopter has to actually descend to the heliport to trigger the bug - a helicopter taxiing to the heliport isn't enough.

@LordAro
Copy link
Member

LordAro commented Dec 27, 2020

I'm by no means certain of this, but I think this boils down to the "reuse" of AMED_SLOWTURN for flight and turning on the ground.

The first helipad of the intercontinental has some sort of offset - so the helicopter has to move a bit before loading/unloading, and its tile attribute is set to its actual tile position.

The second does not have any offset and it lands directly on the helipad. This means that for some reason that I'm not clear on, the vehicle's tile is set to the tile of the airport, rather than its actual position.

This appears to be the case for the majority of helipads (I've not tested them all), it's just that this one for the intercontinental is the exception. This only matters for the (rotated) intercontinental airport due to it being non-rectangular

@JGRennison
Copy link
Contributor Author

From doing a bit of printf debugging, the assignment of the airport tile to the vehicle is here: https://github.com/OpenTTD/OpenTTD/blob/master/src/aircraft_cmd.cpp#L952-L953

tile is assigned from st->airport.tile at the start of the function.

The landing altitude in z could also be wrong for the same reason as the first sub-issue.

If it's not an oil rig, setting x and y from v->x_pos and v->y_pos and recalculating tile from that looks like it would work.

		if (st->airport.type != AT_OILRIG) {
			x = v->x_pos;
			y = v->y_pos;
			tile = TileVirtXY(x, y);
		}

		/* Vehicle is now at the airport. */
		v->tile = tile;

stops the crash, but I'm not sure whether that's really valid/sensible.

LordAro added a commit to LordAro/OpenTTD that referenced this issue Dec 28, 2020
… drawn at the wrong height

Only the rotated intercontinental airport, don't get excited
LordAro added a commit to LordAro/OpenTTD that referenced this issue Dec 28, 2020
LordAro added a commit to LordAro/OpenTTD that referenced this issue Dec 28, 2020
… drawn at the wrong height

Only the rotated intercontinental airport, don't get excited
LordAro added a commit to LordAro/OpenTTD that referenced this issue Dec 28, 2020
JGRennison added a commit to JGRennison/OpenTTD-patches that referenced this issue Dec 29, 2020
(Part 2: helicopter landings)
LordAro added a commit to LordAro/OpenTTD that referenced this issue Jan 3, 2021
… drawn at the wrong height

Only the rotated intercontinental airport, don't get excited
LordAro added a commit to LordAro/OpenTTD that referenced this issue Jan 3, 2021
LordAro added a commit to LordAro/OpenTTD that referenced this issue Jan 3, 2021
LordAro added a commit to LordAro/OpenTTD that referenced this issue Jan 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants