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 #8108, c01a2e2: possible crash on loading TTD savegames with phantom oil rigs #8109

Merged
merged 2 commits into from May 4, 2020

Conversation

SamuXarick
Copy link
Contributor

Need help with commit message. Also, I'm unsure if the iterator issue originated from ab711e6. Plz someone check.

@Yexo
Copy link
Contributor

Yexo commented May 4, 2020

c01a2e is the actual commit that swapped out SmallVector with std::vector, and changed the semantics of the erase function which causes this crash.

* It was 3 (till 2.2) and later 5 (till 5.1).
* Setting it unconditionally does not hurt.
*/
Station::GetByTile(t)->airport.type = AT_OILRIG;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you modify the last sentence of the comment to something like this which explains why it's moved out of the if/else block like before:
DeleteOilRig asserts on the correct type, and setting it unconditionally does not hurt.

@@ -2203,7 +2202,8 @@ bool AfterLoadGame()
}

if (remove) {
DeleteAnimatedTile(*tile);
tile = _animated_tiles.erase(tile);
MarkTileDirtyByTile(*tile);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you even need to mark dirty tiles in afterlolad?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied over from DeleteAnimatedTile. But if it doesn't need that, I can remove this line.

@SamuXarick SamuXarick force-pushed the TTD-oilrig-and-iterator-crash branch from 7d5635a to 2a2f080 Compare May 4, 2020 18:52
@SamuXarick SamuXarick changed the title Fix #8108, ab711e6: Convert TTD oil rigs encoding, and fix incompatible iterator crash Fix #8108, c01a2e2: Convert TTD oil rigs encoding, and fix incompatible iterator crash May 4, 2020
@SamuXarick SamuXarick force-pushed the TTD-oilrig-and-iterator-crash branch from 2a2f080 to 4bdfe8e Compare May 4, 2020 18:57
@Yexo
Copy link
Contributor

Yexo commented May 4, 2020

Suggested commit message: Fix #8108, c01a2e2: possible crash on loading TTD savegames with phantom oil rigs

@SamuXarick SamuXarick force-pushed the TTD-oilrig-and-iterator-crash branch from 4bdfe8e to 5f13a08 Compare May 4, 2020 19:00
@SamuXarick SamuXarick changed the title Fix #8108, c01a2e2: Convert TTD oil rigs encoding, and fix incompatible iterator crash Fix #8108, c01a2e2: possible crash on loading TTD savegames with phantom oil rigs May 4, 2020
@SamuXarick SamuXarick force-pushed the TTD-oilrig-and-iterator-crash branch from 5f13a08 to b4e35fa Compare May 4, 2020 19:13
@Yexo Yexo merged commit 8edbb42 into OpenTTD:master May 4, 2020
@SamuXarick SamuXarick deleted the TTD-oilrig-and-iterator-crash branch May 4, 2020 19:31
@LordAro LordAro added the backport requested This PR should be backport to current release (RC / stable) label May 4, 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 Jul 27, 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