From d52a1fe6f925e448b67a4b997690514389f5ca28 Mon Sep 17 00:00:00 2001
From: Bradley Austin Davis <bdavis@saintandreas.org>
Date: Mon, 13 Jun 2016 20:26:27 -0700
Subject: [PATCH] Fix persistence for global scope settings handles, clean up
 invalid variants in settings

---
 libraries/shared/src/SettingHandle.h      | 39 +++++++++++++++++++----
 libraries/shared/src/SettingInterface.cpp |  4 +--
 libraries/shared/src/SettingInterface.h   | 11 ++++---
 libraries/shared/src/SettingManager.cpp   | 10 +++---
 4 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/libraries/shared/src/SettingHandle.h b/libraries/shared/src/SettingHandle.h
index f19fc5875b..74587dcdcb 100644
--- a/libraries/shared/src/SettingHandle.h
+++ b/libraries/shared/src/SettingHandle.h
@@ -77,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 (_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 1ebaa5cf82..95c6bc1efc 100644
--- a/libraries/shared/src/SettingInterface.cpp
+++ b/libraries/shared/src/SettingInterface.cpp
@@ -119,9 +119,9 @@ namespace Setting {
     }
 
     
-    void Interface::maybeInit() {
+    void Interface::maybeInit() const {
         if (!_isInitialized) {
-            init();
+            const_cast<Interface*>(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> _manager;
+        mutable QWeakPointer<Manager> _manager;
     };
 }
 
diff --git a/libraries/shared/src/SettingManager.cpp b/libraries/shared/src/SettingManager.cpp
index bacec2ee0c..ed8565d6c3 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);
+            }
         });
     }
 
@@ -101,7 +103,7 @@ namespace Setting {
                 if (newValue == savedValue) {
                     continue;
                 }
-                if (newValue == UNSET_VALUE) {
+                if (newValue == UNSET_VALUE || !newValue.isValid()) {
                     remove(key);
                 } else {
                     setValue(key, newValue);