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

Fix f5381798: Docking station tile area was being misused #8014

Merged
merged 1 commit into from Feb 22, 2020

Conversation

SamuXarick
Copy link
Contributor

No description provided.

@LordAro
Copy link
Member

LordAro commented Feb 19, 2020

What makes you think this is wrong? It certainly wasn't caused by f538179 - the function structure predates that

@SamuXarick
Copy link
Contributor Author

SamuXarick commented Feb 19, 2020

Before docking tiles, there was only 1 tile, and the area had to be set manually to 1 width, 1 height.
f538179#diff-249239106fd58103249d1178388d72feL404

The way it works now it puts in a rectangle area the possible locations of docking tiles.
*ta = this->docking_station already includes values for ta->tile, ta->w and ta->h
The break; was causing ta->w and ta->h to be redefined to only the docking tile at the top corner. For a single docking tile, that is still correct, but for multiple docking tiles for the same station, that was only partially correct.

With return; instead of break; the problem is solved. This in turn also makes ta->w = 1; and ta->h = 1; obsolete, because all other StationType types all have a return;, making that part of the code unreachable. Therefore I cleared it.

@LordAro LordAro requested a review from PeterN February 19, 2020 22:06
@PeterN
Copy link
Member

PeterN commented Feb 19, 2020

This change looks correct, however I'd like to know how this problem manifests. You've stated this fixes the problem without actually saying what problem it fixes other than it was "misused".

@SamuXarick
Copy link
Contributor Author

SamuXarick commented Feb 21, 2020

My explanation was actually wrong, the problem is worse than I thought.

Charningstone Transport, 15th Jul 1950 (2)

On this screenshot, ta->tile is the tile at the top corner of the white recangle area. ta->w is 5 and ta->h is 8. The area encompasses the two docking tiles of the same multi-dock station.

The break; was causing ta->w and ta->h to be redefined to only the tile at the top corner. In this case it's not even a docking tile.

Station::GetTileArea is called by YAPF and NPF, via CalcClosestStationTile.

Savegame
Charningstone Transport, 29th Jun 1950.zip

@SamuXarick
Copy link
Contributor Author

SamuXarick commented Feb 21, 2020

Charningstone Transport, 5th Aug 1954.zip
With the fix, pathfinder sends ships towards the right corner. Without it, it sends towards the north corner.

image

@LordAro LordAro added the backport requested This PR should be backport to current release (RC / stable) label Feb 22, 2020
@frosch123 frosch123 merged commit ea7044a into OpenTTD:master Feb 22, 2020
@SamuXarick SamuXarick deleted the docking-station-tile-area branch February 22, 2020 15:52
LordAro pushed a commit to LordAro/OpenTTD that referenced this pull request Feb 29, 2020
Berbe pushed a commit to Berbe/OpenTTD that referenced this pull request Mar 6, 2020
Berbe pushed a commit to Berbe/OpenTTD that referenced this pull request Mar 6, 2020
LordAro pushed a commit to LordAro/OpenTTD that referenced this pull request Mar 30, 2020
@LordAro LordAro added backported This PR is backported to a current release (RC / stable) and removed backport requested This PR should be backport to current release (RC / stable) labels Mar 31, 2020
douiwby pushed a commit to douiwby/OpenTTD that referenced this pull request Apr 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported This PR is backported to a current release (RC / stable)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants