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

ControllerEmu: Split the Setting class #4001

Merged
merged 2 commits into from Jul 13, 2016

Conversation

leoetlino
Copy link
Member

@leoetlino leoetlino commented Jul 11, 2016

The Setting class was used for both numeric values and booleans, and
other parts of the code had "hacks" to make it work with booleans.

By splitting Setting into NumericSetting and BooleanSetting, it is
clearer which settings are numeric, and which are boolean, so there is
no need to guess by checking the default values or anything like that.
Also, booleans are stored as booleans in config files, instead of 1.0.


This change is Reviewable

@delroth
Copy link
Member

delroth commented Jul 11, 2016

Will this keep compatibility with old settings files?

@leoetlino
Copy link
Member Author

Hmm, I hadn't thought about that. Most settings should work just fine, but I'll check for the 1.0 => True change.

The Setting class was used for both numeric values and booleans, and
other parts of the code had hacks to make it work with booleans.

By splitting Setting into NumericSetting and BooleanSetting, it is
clear which settings are numeric, and which are boolean, so there is
no need to guess by checking the default values or anything like that.
Also, booleans are stored as booleans in config files, instead of 1.0.
This is needed to keep compatibility with old configuration files which
didn't store booleans as booleans, but as floats/doubles.
@leoetlino
Copy link
Member Author

This should now be able to parse 1.0000000000000000 (which is how True was stored previously) as true.

@delroth delroth merged commit 89a0317 into dolphin-emu:master Jul 13, 2016
@leoetlino leoetlino deleted the split-controller-setting branch July 13, 2016 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants