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 #11438: Crash when decreasing map size #13665

Merged
merged 1 commit into from Jan 12, 2021

Conversation

Gymnasiast
Copy link
Member

@Gymnasiast Gymnasiast commented Dec 30, 2020

The issue was caused by the code trying to remove a park entrance element that was not one that it knew about. It therefore refused to remove the tile element, caused code further upstream to get into an infinite loop.

Since clear_element_at is only called from functions that either clear the map completely or clear all the out of range elements, I figured that forcibly removing them was not too much of a problem.

@IntelOrca
Copy link
Contributor

If calling tile_element_remove works, why do we need to execute game actions at all?

@Gymnasiast
Copy link
Member Author

Because they also do stuff like cleaning up banner entries and removing park entrances from the global list. tile_element_remove() is used as a last resort.

@ZehMatt
Copy link
Member

ZehMatt commented Dec 31, 2020

How does it not know that the entrance exists? Seems like an issue that is not handled correctly elsewhere.

@Gymnasiast
Copy link
Member Author

There are countless reasons why the tile elements can be incorrect. One of them is simply copying stuff using the Tile Inspector.

@ZehMatt
Copy link
Member

ZehMatt commented Jan 2, 2021

I'm not a big fan about having a forced removal for all the other elements, given the entrance is not registered is the fault of the tile inspector, the removal logic is correct.

@Gymnasiast
Copy link
Member Author

As I said, there are more reasons why it might be incorrect. Corruption of tile elements is very common and I'm frankly a bit surprised that you seem unaware of this.

@ZehMatt
Copy link
Member

ZehMatt commented Jan 2, 2021

I'm aware that things can get corrupted but that is exactly my point, fighting the code that corrupts the state rather than hiding it. I understand to why the park entrance can get into this situation but the cause is apparently known so we should fix it there. As for the other corruptions we need to identify the cause. In my experience attempts like this usually shift the issue elsewhere, that is why I think the fix is fine for the entrance only but not for the rest.

@IntelOrca
Copy link
Contributor

@ZehMatt it needs to handle all edge cases regardless of how they may have been introduced

@ZehMatt
Copy link
Member

ZehMatt commented Jan 3, 2021

I think it would be better to adjust the game actions in that regard, one thing that comes to my mind is have a flag that skips most of the things in query, the code needs to check of course for this specific flag. Its somewhat close to the clearance check. This is a cleaner solution and the plugins could also benefit to apply the specific flag to skip certain checks.

@Gymnasiast Gymnasiast linked an issue Jan 8, 2021 that may be closed by this pull request
@Gymnasiast Gymnasiast merged commit bf63a56 into OpenRCT2:develop Jan 12, 2021
@Gymnasiast Gymnasiast deleted the fix/11438 branch January 12, 2021 21:04
@Gymnasiast Gymnasiast added this to the v0.3.3 milestone Jan 12, 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

Successfully merging this pull request may close these issues.

Shrinking map freezes game
3 participants