diff --git a/libraries/networking/src/AccountManager.cpp b/libraries/networking/src/AccountManager.cpp index bac031885f..26b3801ec1 100644 --- a/libraries/networking/src/AccountManager.cpp +++ b/libraries/networking/src/AccountManager.cpp @@ -173,6 +173,7 @@ void AccountManager::setAuthURL(const QUrl& authURL) { << "from previous settings file"; } } + settings.endGroup(); if (_accountInfo.getAccessToken().token.isEmpty()) { qCWarning(networking) << "Unable to load account file. No existing account settings will be loaded."; diff --git a/libraries/script-engine/src/ScriptEngines.cpp b/libraries/script-engine/src/ScriptEngines.cpp index 29c223f4b3..beddc21787 100644 --- a/libraries/script-engine/src/ScriptEngines.cpp +++ b/libraries/script-engine/src/ScriptEngines.cpp @@ -334,6 +334,7 @@ void ScriptEngines::clearScripts() { Settings settings; settings.beginWriteArray(SETTINGS_KEY); settings.remove(""); + settings.endArray(); } void ScriptEngines::saveScripts() { diff --git a/libraries/shared/src/SettingHandle.cpp b/libraries/shared/src/SettingHandle.cpp index b2f23f5a04..13f9ea48ce 100644 --- a/libraries/shared/src/SettingHandle.cpp +++ b/libraries/shared/src/SettingHandle.cpp @@ -18,6 +18,7 @@ const QString Settings::firstRun { "firstRun" }; + Settings::Settings() : _manager(DependencyManager::get()), _locker(&(_manager->getLock())) @@ -25,6 +26,9 @@ Settings::Settings() : } Settings::~Settings() { + if (_prefixes.size() != 0) { + qFatal("Unstable Settings Prefixes: You must call endGroup for every beginGroup and endArray for every begin*Array call"); + } } void Settings::remove(const QString& key) { @@ -50,14 +54,17 @@ bool Settings::contains(const QString& key) const { } int Settings::beginReadArray(const QString & prefix) { + _prefixes.push(prefix); return _manager->beginReadArray(prefix); } void Settings::beginWriteArray(const QString& prefix, int size) { + _prefixes.push(prefix); _manager->beginWriteArray(prefix, size); } void Settings::endArray() { + _prefixes.pop(); _manager->endArray(); } @@ -66,10 +73,12 @@ void Settings::setArrayIndex(int i) { } void Settings::beginGroup(const QString& prefix) { + _prefixes.push(prefix); _manager->beginGroup(prefix); } void Settings::endGroup() { + _prefixes.pop(); _manager->endGroup(); } diff --git a/libraries/shared/src/SettingHandle.h b/libraries/shared/src/SettingHandle.h index e83c563036..8e07d28dad 100644 --- a/libraries/shared/src/SettingHandle.h +++ b/libraries/shared/src/SettingHandle.h @@ -58,8 +58,10 @@ public: void setQuatValue(const QString& name, const glm::quat& quatValue); void getQuatValueIfValid(const QString& name, glm::quat& quatValue); +private: QSharedPointer _manager; QWriteLocker _locker; + QStack _prefixes; }; namespace Setting { @@ -75,15 +77,40 @@ namespace Setting { virtual ~Handle() { deinit(); } // Returns setting value, returns its default value if not found - T get() { return get(_defaultValue); } + T get() const { + return get(_defaultValue); + } + // Returns setting value, returns other if not found - T get(const T& other) { maybeInit(); return (_isSet) ? _value : other; } - T getDefault() const { return _defaultValue; } + T get(const T& other) const { + maybeInit(); + return (_isSet) ? _value : other; + } + + const T& getDefault() const { + return _defaultValue; + } - void set(const T& value) { maybeInit(); _value = value; _isSet = true; } - void reset() { set(_defaultValue); } - - void remove() { maybeInit(); _isSet = false; } + void reset() { + set(_defaultValue); + } + + void set(const T& value) { + maybeInit(); + if ((!_isSet && (value != _defaultValue)) || _value != value) { + _value = value; + _isSet = true; + save(); + } + } + + void remove() { + maybeInit(); + if (_isSet) { + _isSet = false; + save(); + } + } protected: virtual void setVariant(const QVariant& variant); diff --git a/libraries/shared/src/SettingInterface.cpp b/libraries/shared/src/SettingInterface.cpp index 630d52bf7d..95c6bc1efc 100644 --- a/libraries/shared/src/SettingInterface.cpp +++ b/libraries/shared/src/SettingInterface.cpp @@ -98,6 +98,8 @@ namespace Setting { // Register Handle manager->registerHandle(this); _isInitialized = true; + } else { + qWarning() << "Settings interface used after manager destroyed"; } // Load value from disk @@ -117,9 +119,9 @@ namespace Setting { } - void Interface::maybeInit() { + void Interface::maybeInit() const { if (!_isInitialized) { - init(); + const_cast(this)->init(); } } diff --git a/libraries/shared/src/SettingInterface.h b/libraries/shared/src/SettingInterface.h index 2b32e8a3b4..5e23d42223 100644 --- a/libraries/shared/src/SettingInterface.h +++ b/libraries/shared/src/SettingInterface.h @@ -39,19 +39,20 @@ namespace Setting { virtual ~Interface() = default; void init(); - void maybeInit(); + void maybeInit() const; void deinit(); void save(); void load(); - - bool _isInitialized = false; + bool _isSet = false; const QString _key; + + private: + mutable bool _isInitialized = false; friend class Manager; - - QWeakPointer _manager; + mutable QWeakPointer _manager; }; } diff --git a/libraries/shared/src/SettingManager.cpp b/libraries/shared/src/SettingManager.cpp index bacec2ee0c..abb8525b03 100644 --- a/libraries/shared/src/SettingManager.cpp +++ b/libraries/shared/src/SettingManager.cpp @@ -33,8 +33,8 @@ namespace Setting { void Manager::customDeleter() { } - void Manager::registerHandle(Setting::Interface* handle) { - QString key = handle->getKey(); + void Manager::registerHandle(Interface* handle) { + const QString& key = handle->getKey(); withWriteLock([&] { if (_handles.contains(key)) { qWarning() << "Setting::Manager::registerHandle(): Key registered more than once, overriding: " << key; @@ -58,7 +58,9 @@ namespace Setting { } else { loadedValue = value(key); } - handle->setVariant(loadedValue); + if (loadedValue.isValid()) { + handle->setVariant(loadedValue); + } }); } @@ -94,6 +96,7 @@ namespace Setting { } void Manager::saveAll() { + bool forceSync = false; withWriteLock([&] { for (auto key : _pendingChanges.keys()) { auto newValue = _pendingChanges[key]; @@ -101,15 +104,21 @@ namespace Setting { if (newValue == savedValue) { continue; } - if (newValue == UNSET_VALUE) { + if (newValue == UNSET_VALUE || !newValue.isValid()) { + forceSync = true; remove(key); } else { + forceSync = true; setValue(key, newValue); } } _pendingChanges.clear(); }); + if (forceSync) { + sync(); + } + // Restart timer if (_saveTimer) { _saveTimer->start();