Codechange: differentiate in SettingDescBase implementations for different classes of settings #9288
Conversation
|
Oh and there's a nice regression failure, probably incorrect parsing of the settings chunk. |
Today I learned the default save game is so old it does not even contain the PATS chunk, but yeah I broke save game compatibility somewhere somehow. Though the first push was mostly to get a feeling whether the other compilers would accept the code, as I have been battling with that mostly for the last week. |
rubidium42
force-pushed
the
setting-classes
branch
5 times, most recently
from
ebbd2b5 to
07eed85
Compare
rubidium42
force-pushed
the
setting-classes
branch
2 times, most recently
from
ec2fb22 to
cbaa8f1
Compare
There was a problem hiding this comment.
This PR is rather difficult to review. I am not blaming you or anything, as I think it is needed and awesome work. I also have no suggestions how to make reviewing easier .. just mentioning that I gave up half-way through, as I was going blind :D Sorry .. I did my best :)
rubidium42
force-pushed
the
setting-classes
branch
2 times, most recently
from
7e47f55 to
3b2030e
Compare
…istinction between (non)company
The comments for SettingDescType; it is a byte, so not 4 bytes and since it is not a flag there are about 250 other possibilities left instead of 9. SettingGuiFlag is uint16 so has 2 bytes allocated. SettingDescGlobVarList and related comments imply that global vars cannot be used elsewhere, but they are used for settings just fine. Even then the type is not used anywhere else but the definition of the table.
Not using vectors as those require copying from the initializer list and that makes unique_ptrs to the actual SettingDesc objects later impossible.
…w for sub classes
And by doing so remove the hack where ints were put into pointers so "def" could either be an int or a string
rubidium42
force-pushed
the
setting-classes
branch
from
3b2030e to
0fbbf7d
Compare
Motivation / Problem
String settings cannot be validated before they are getting set. To make this possible different implementations of
OnChangewith different types are needed. This is the first step into getting to a place where that distinction can be made, and proper validations can be performed on settings.Description
Split the SettingDescBase into a number of distinct sub classes with their own implementations of some common functionality.
Put the setting arrays into initializer_lists, so iterating over them can be done more easily.
Replaced the custom string decoding for the one-to-many lookups by actually using a vector of the allowed values and comparing against that.
These change will be followed by rubidium42/OpenTTD@setting-classes...rubidium42:setting-prechange-check to make the pre checks an actual reality, but I'm trying to limit the PRs in size so they will be separate.
Limitations
None as far as I know.
Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.