From e1a68d8cf598e25c5348a7b91b05dbe7ea854b69 Mon Sep 17 00:00:00 2001 From: elsid Date: Wed, 15 Nov 2023 08:14:16 +0100 Subject: [PATCH] Ignore absent default setting value --- apps/opencs/model/prefs/state.cpp | 35 +------------------ apps/opencs/model/prefs/state.hpp | 2 -- .../openmw_test_suite/settings/testvalues.cpp | 34 ++++++++++++++++++ components/settings/settings.hpp | 9 +++++ components/settings/settingvalue.hpp | 4 +-- 5 files changed, 46 insertions(+), 38 deletions(-) diff --git a/apps/opencs/model/prefs/state.cpp b/apps/opencs/model/prefs/state.cpp index d55913fb3c..364f7b3320 100644 --- a/apps/opencs/model/prefs/state.cpp +++ b/apps/opencs/model/prefs/state.cpp @@ -463,8 +463,6 @@ CSMPrefs::IntSetting& CSMPrefs::State::declareInt(const std::string& key, const if (mCurrentCategory == mCategories.end()) throw std::logic_error("no category for setting"); - setDefault(key, std::to_string(default_)); - CSMPrefs::IntSetting* setting = new CSMPrefs::IntSetting(&mCurrentCategory->second, &mMutex, key, label, *mIndex); mCurrentCategory->second.addSetting(setting); @@ -477,10 +475,6 @@ CSMPrefs::DoubleSetting& CSMPrefs::State::declareDouble(const std::string& key, if (mCurrentCategory == mCategories.end()) throw std::logic_error("no category for setting"); - std::ostringstream stream; - stream << default_; - setDefault(key, stream.str()); - CSMPrefs::DoubleSetting* setting = new CSMPrefs::DoubleSetting(&mCurrentCategory->second, &mMutex, key, label, *mIndex); @@ -494,8 +488,6 @@ CSMPrefs::BoolSetting& CSMPrefs::State::declareBool(const std::string& key, cons if (mCurrentCategory == mCategories.end()) throw std::logic_error("no category for setting"); - setDefault(key, default_ ? "true" : "false"); - CSMPrefs::BoolSetting* setting = new CSMPrefs::BoolSetting(&mCurrentCategory->second, &mMutex, key, label, *mIndex); mCurrentCategory->second.addSetting(setting); @@ -508,8 +500,6 @@ CSMPrefs::EnumSetting& CSMPrefs::State::declareEnum(const std::string& key, cons if (mCurrentCategory == mCategories.end()) throw std::logic_error("no category for setting"); - setDefault(key, default_.mValue); - CSMPrefs::EnumSetting* setting = new CSMPrefs::EnumSetting(&mCurrentCategory->second, &mMutex, key, label, *mIndex); mCurrentCategory->second.addSetting(setting); @@ -522,8 +512,6 @@ CSMPrefs::ColourSetting& CSMPrefs::State::declareColour(const std::string& key, if (mCurrentCategory == mCategories.end()) throw std::logic_error("no category for setting"); - setDefault(key, default_.name().toUtf8().data()); - CSMPrefs::ColourSetting* setting = new CSMPrefs::ColourSetting(&mCurrentCategory->second, &mMutex, key, label, *mIndex); @@ -538,9 +526,6 @@ CSMPrefs::ShortcutSetting& CSMPrefs::State::declareShortcut( if (mCurrentCategory == mCategories.end()) throw std::logic_error("no category for setting"); - std::string seqStr = getShortcutManager().convertToString(default_); - setDefault(key, seqStr); - // Setup with actual data QKeySequence sequence; @@ -560,8 +545,6 @@ CSMPrefs::StringSetting& CSMPrefs::State::declareString( if (mCurrentCategory == mCategories.end()) throw std::logic_error("no category for setting"); - setDefault(key, default_); - CSMPrefs::StringSetting* setting = new CSMPrefs::StringSetting(&mCurrentCategory->second, &mMutex, key, label, *mIndex); @@ -575,9 +558,6 @@ CSMPrefs::ModifierSetting& CSMPrefs::State::declareModifier(const std::string& k if (mCurrentCategory == mCategories.end()) throw std::logic_error("no category for setting"); - std::string modStr = getShortcutManager().convertToString(default_); - setDefault(key, modStr); - // Setup with actual data int modifier; @@ -600,33 +580,20 @@ void CSMPrefs::State::declareSubcategory(const QString& label) new CSMPrefs::Subcategory(&mCurrentCategory->second, &mMutex, label, *mIndex)); } -void CSMPrefs::State::setDefault(const std::string& key, const std::string& default_) -{ - Settings::CategorySetting fullKey(mCurrentCategory->second.getKey(), key); - - Settings::CategorySettingValueMap::iterator iter = Settings::Manager::mDefaultSettings.find(fullKey); - - if (iter == Settings::Manager::mDefaultSettings.end()) - Settings::Manager::mDefaultSettings.insert(std::make_pair(fullKey, default_)); -} - CSMPrefs::State::State(const Files::ConfigurationManager& configurationManager) : mConfigFile("openmw-cs.cfg") , mDefaultConfigFile("defaults-cs.bin") , mConfigurationManager(configurationManager) , mCurrentCategory(mCategories.end()) , mIndex(std::make_unique()) + , mValues(std::make_unique(*mIndex)) { if (sThis) throw std::logic_error("An instance of CSMPRefs::State already exists"); sThis = this; - Values values(*mIndex); - declare(); - - mValues = std::make_unique(std::move(values)); } CSMPrefs::State::~State() diff --git a/apps/opencs/model/prefs/state.hpp b/apps/opencs/model/prefs/state.hpp index 97d657ddaf..018a81a8d9 100644 --- a/apps/opencs/model/prefs/state.hpp +++ b/apps/opencs/model/prefs/state.hpp @@ -80,8 +80,6 @@ namespace CSMPrefs void declareSubcategory(const QString& label); - void setDefault(const std::string& key, const std::string& default_); - public: State(const Files::ConfigurationManager& configurationManager); diff --git a/apps/openmw_test_suite/settings/testvalues.cpp b/apps/openmw_test_suite/settings/testvalues.cpp index 81af308795..236417b559 100644 --- a/apps/openmw_test_suite/settings/testvalues.cpp +++ b/apps/openmw_test_suite/settings/testvalues.cpp @@ -59,6 +59,33 @@ namespace Settings EXPECT_EQ(values.mCamera.mFieldOfView.get(), 1); } + TEST_F(SettingsValuesTest, constructorWithDefaultShouldDoLookup) + { + Manager::mUserSettings[std::make_pair("category", "value")] = "13"; + Index index; + SettingValue value{ index, "category", "value", 42 }; + EXPECT_EQ(value.get(), 13); + value.reset(); + EXPECT_EQ(value.get(), 42); + } + + TEST_F(SettingsValuesTest, constructorWithDefaultShouldSanitize) + { + Manager::mUserSettings[std::make_pair("category", "value")] = "2"; + Index index; + SettingValue value{ index, "category", "value", -1, Settings::makeClampSanitizerInt(0, 1) }; + EXPECT_EQ(value.get(), 1); + value.reset(); + EXPECT_EQ(value.get(), 0); + } + + TEST_F(SettingsValuesTest, constructorWithDefaultShouldFallbackToDefault) + { + Index index; + const SettingValue value{ index, "category", "value", 42 }; + EXPECT_EQ(value.get(), 42); + } + TEST_F(SettingsValuesTest, moveConstructorShouldSetDefaults) { Index index; @@ -79,6 +106,13 @@ namespace Settings EXPECT_EQ(values.mCamera.mFieldOfView.get(), 1); } + TEST_F(SettingsValuesTest, moveConstructorShouldThrowOnMissingSetting) + { + Index index; + SettingValue defaultValue{ index, "category", "value", 42 }; + EXPECT_THROW([&] { SettingValue value(std::move(defaultValue)); }(), std::runtime_error); + } + TEST_F(SettingsValuesTest, findShouldThrowExceptionOnTypeMismatch) { Index index; diff --git a/components/settings/settings.hpp b/components/settings/settings.hpp index c061755bc1..bcdcbb16d1 100644 --- a/components/settings/settings.hpp +++ b/components/settings/settings.hpp @@ -77,6 +77,15 @@ namespace Settings static osg::Vec2f getVector2(std::string_view setting, std::string_view category); static osg::Vec3f getVector3(std::string_view setting, std::string_view category); + template + static T getOrDefault(std::string_view setting, std::string_view category, const T& defaultValue) + { + const auto key = std::make_pair(category, setting); + if (!mUserSettings.contains(key) && !mDefaultSettings.contains(key)) + return defaultValue; + return get(setting, category); + } + template static T get(std::string_view setting, std::string_view category) { diff --git a/components/settings/settingvalue.hpp b/components/settings/settingvalue.hpp index 392e5b646f..8183e8c1ac 100644 --- a/components/settings/settingvalue.hpp +++ b/components/settings/settingvalue.hpp @@ -339,8 +339,8 @@ namespace Settings std::unique_ptr>&& sanitizer = nullptr) : BaseSettingValue(getSettingValueType(), category, name, index) , mSanitizer(std::move(sanitizer)) - , mDefaultValue(defaultValue) - , mValue(defaultValue) + , mDefaultValue(sanitize(defaultValue)) + , mValue(sanitize(Settings::Manager::getOrDefault(mName, mCategory, mDefaultValue))) { }