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

BASE: Don't check for config key presence when loading gfx mode #3360

Merged
merged 1 commit into from Sep 18, 2021

Conversation

@lotharsm
Copy link
Member

@lotharsm lotharsm commented Sep 11, 2021

I noticed that while bilinear filtering is disabled by default with RegisterDefault(); at startup, bilinear filtering is always applied when starting a game (at least on the OPENGLSDL backend) unless you explicitly set filtering=false in the configuration file by simply opening the options dialog, applying the settings and closing it again.

While researching this, I think the culprit is that main.cpp checks for ConfMan.hasKey("filtering"), but registerDefault() technically won't set a configuration key. This means that this checks override our defaults.

I think that this check is not necessary. With the checks for hasKey removed, it will first try to load the values set in the configuration file and otherwise fall back to the values set by registerDefault() instead.

Since we only deal with bool values here, it shouldn't even break in case we don't use registerDefault for those values since in this case, ConfMan.getBool will simply return false as well (which are the default values we set with registerDefault.

@Die4Ever
Copy link
Member

@Die4Ever Die4Ever commented Sep 18, 2021

for reference

bool ConfigManager::getBool(const String &key, const String &domName) const {
    String value(get(key, domName));
    bool val;
    if (parseBool(value, val))
        return val;

    error("ConfigManager::getBool(%s,%s): '%s' is not a valid bool",
          key.c_str(), domName.c_str(), value.c_str());
}
bool parseBool(const String &val, bool &valAsBool) {
    if (val.equalsIgnoreCase("true") ||
        val.equalsIgnoreCase("yes") ||
        val.equals("1")) {
        valAsBool = true;
        return true;
    }
    if (val.equalsIgnoreCase("false") ||
        val.equalsIgnoreCase("no") ||
        val.equals("0") ||
        val.empty()) {
        valAsBool = false;
        return true;
    }

    return false;
}

the parseBool function checks for val.empty() and will set valAsBool to false, and return true for success
so this code is good

Copy link
Member

@Die4Ever Die4Ever left a comment

looks good, tested it out real quick with

    if (ConfMan.getBool("fake_config_option"))
        error("BAD!");

@Die4Ever Die4Ever merged commit 74d4380 into scummvm:master Sep 18, 2021
8 checks passed
@lotharsm lotharsm deleted the fix-register-default-filtering branch Sep 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants