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 #6507: Don't try to load invalid depots from older savegames #7546

Merged
merged 4 commits into from Apr 29, 2019

Conversation

@LordAro
Copy link
Member

LordAro commented Apr 27, 2019

And also another bug spotted along the way
And another cleanup

Fixes #6605 as well, as it appears to be duplicated

LordAro added 4 commits Apr 26, 2019
GroupStatistics pool was not initialised before trying to delete vehicles (specifically, trams with no tram track)
…r bridges losing their type
@LordAro LordAro requested a review from PeterN Apr 27, 2019
@PeterN

This comment has been minimized.

Copy link
Member

PeterN commented Apr 28, 2019

Is there any possibility of vehicles being inside one of these invalid depots?

@LordAro

This comment has been minimized.

Copy link
Member Author

LordAro commented Apr 28, 2019

Given we don't properly understand what caused the invalid depots, I can't be entirely sure. However, given all known "invalid depots" have been next to rails, and there aren't any connecting rails, I don't think so. As best as I can tell, they were depots that existed at some point, but were removed prior to saving, but somehow were still saved anyway

@nielsmh

This comment has been minimized.

Copy link
Contributor

nielsmh commented Apr 28, 2019

Is there a way to check if any vehicles are (supposed to be) inside the depot before deleting it, and fail loading in that case?

@LordAro

This comment has been minimized.

Copy link
Member Author

LordAro commented Apr 28, 2019

Wouldn't it fail in some other way if you try to delete a depot with vehicles in it?

@PeterN

This comment has been minimized.

Copy link
Member

PeterN commented Apr 28, 2019

Probably overthinking it. Deal with other brokenness if it occurs, let's not preempt it.

@PeterN
PeterN approved these changes Apr 29, 2019
@PeterN PeterN merged commit 63a7df0 into OpenTTD:master Apr 29, 2019
8 checks passed
8 checks passed
OpenTTD CI Build #20190427.2 succeeded
Details
OpenTTD CI (Linux commit-checker) Linux commit-checker succeeded
Details
OpenTTD CI (Linux linux-amd64-clang-3.8) Linux linux-amd64-clang-3.8 succeeded
Details
OpenTTD CI (Linux linux-amd64-gcc-6) Linux linux-amd64-gcc-6 succeeded
Details
OpenTTD CI (Linux linux-i386-gcc-6) Linux linux-i386-gcc-6 succeeded
Details
OpenTTD CI (MacOS) MacOS succeeded
Details
OpenTTD CI (Windows Win32) Windows Win32 succeeded
Details
OpenTTD CI (Windows Win64) Windows Win64 succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.