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 #7847: Use ViewportSign coordinates for sign Kdtree coordinates #7849
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…d() and ViewportSignKdtreeItem::MakeStation()
SamuXarick
reviewed
Dec 1, 2019
…nates for stations Keeping track of whether the ViewportSign data are valid for Kdtree usage allows moving more housekeeping to Station::UpdateVirtCoord and Waypoint::UpdateVirtCoord and helps simplify a lot of other code.
…ions It's the more correct approach, it was only not usable before due to inconsistencies with when sign positions were updated.
…e it to viewport_type.h
nielsmh
changed the title
Fix #7847: Use ViewportSign coordinates for sign Kdtree coordinates for stations
Fix #7847: Use ViewportSign coordinates for sign Kdtree coordinates
Dec 1, 2019
glx22
approved these changes
Dec 1, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
douiwby
pushed a commit
to douiwby/OpenTTD
that referenced
this pull request
Apr 16, 2020
…nates (OpenTTD#7849) Ensure the same coordinates are used for station/town/player signs regardless of how the landscape changes below it after the coordinates were first determined. By keeping track of whether each ViewportSign is valid for Kdtree use (and only ever registering the viewport sign when the object is valid) a lot of code can be simplified and become more robust at the same time.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #7847 by ensuring the same coordinates are used for a station sign regardless of how the landscape changes below it after the coordinates were first determined.
By keeping track of whether the ViewportSign is valid for Kdtree use (and only ever registering the station viewport sign when the sign object is valid) a lot of code can be simplified, and it should prevent a lot of similar bugs too.
I think this is the real solution to all Kdtree bugs regarding station signs. I think something similar needs to be done for town names and player signs too.