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: NewGRF feature cleanup. #7385

Merged
merged 2 commits into from Mar 21, 2019
Merged

Fix: NewGRF feature cleanup. #7385

merged 2 commits into from Mar 21, 2019

Conversation

PeterN
Copy link
Member

@PeterN PeterN commented Mar 18, 2019

  • NewGRF features are documented in hex, so display as hex
  • Crash when loading NewGRF with out of range feature.

@PeterN
Copy link
Member Author

@PeterN PeterN commented Mar 18, 2019

Possible issue, unknown features within a NewGRF get ignored, rather than disabling the file, so it effectively silently fails (unless you had debugging on). It may be preferable to disable the file.

@planetmaker
Copy link
Contributor

@planetmaker planetmaker commented Mar 18, 2019

Possible issue, unknown features within a NewGRF get ignored, rather than disabling the file, so it effectively silently fails (unless you had debugging on). It may be preferable to disable the file.

That is more a feature IMHO than an issue / bug as it makes OpenTTD (somewhat) forward compatible with future NewGRFs

@PeterN
Copy link
Member Author

@PeterN PeterN commented Mar 18, 2019

That is also what I was thinking. It's certainly better than OpenTTD hard-crashing :-)

@PeterN PeterN added the backport requested label Mar 20, 2019
src/newgrf.cpp Show resolved Hide resolved
@PeterN PeterN merged commit 3357cac into OpenTTD:master Mar 21, 2019
8 checks passed
@PeterN PeterN deleted the feature-cleanup branch Mar 21, 2019
@TrueBrain TrueBrain added backported and removed backport requested labels Mar 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants