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 #7847: Use ViewportSign coordinates for sign Kdtree coordinates #7849

Merged
merged 6 commits into from Dec 1, 2019

Conversation

nielsmh
Copy link
Contributor

@nielsmh nielsmh commented Nov 30, 2019

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.

…d() and ViewportSignKdtreeItem::MakeStation()
…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.
@nielsmh 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
@nielsmh
Copy link
Contributor Author

nielsmh commented Dec 1, 2019

Tested #7371 report, PSG 201 save remains loadable without crashing.

Tested #7440 report, is still able to create and then remove a town in scenario editor.

Tested #7481 and #7530 via the save provided in the latter, seems to pass the oil rig deletion.

Copy link
Contributor

@glx22 glx22 left a 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

@nielsmh nielsmh merged commit 9900af3 into OpenTTD:master Dec 1, 2019
@nielsmh nielsmh deleted the fix-7847 branch December 1, 2019 22:17
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash - Assertion failed at line 213 of src\core\kdtree.hpp: next != INVALID_NODE
3 participants