compat/options: always do a full check of changes

Get rid of state variable and compare transient/saved state exactly.

Marking the "modified" bit caused IO when changing and then changing
back to the original value, then saving.

2 files changed, 73 insertions, 57 deletions

diff --git a/opentrack-compat/options.cpp b/opentrack-compat/options.cpp
index 785698de..813e39ef 100644
--- a/opentrack-compat/options.cpp
+++ b/opentrack-compat/options.cpp
@@ -33,7 +33,6 @@ void group::save() const
     for (auto& i : kvs)
         s->setValue(i.first, i.second);
     s->endGroup();
-    s->sync();
 }
 
 void group::put(const QString &s, const QVariant &d)
@@ -88,25 +87,44 @@ const mem<QSettings> group::ini_file()
     return std::make_shared<QSettings>();
 }
 
+bool group::operator==(const group& other) const
+{
+    for (const auto& kv : kvs)
+    {
+        const QVariant val = other.get<QVariant>(kv.first);
+        if (!other.contains(kv.first) || kv.second != val)
+        {
+            qDebug() << "bundle" << name << "modified" << "key" << kv.first << "to" << kv.second << "from" << val;
+            return false;
+        }
+    }
+
+    for (const auto& kv : other.kvs)
+    {
+        const QVariant val = get<QVariant>(kv.first);
+        if (!contains(kv.first) || kv.second != val)
+        {
+            qDebug() << "bundle" << name << "modified" << "key" << kv.first << "to" << kv.second << "from" << val;
+            return false;
+        }
+    }
+    return true;
+}
+
 impl_bundle::impl_bundle(const QString& group_name)
     :
       mtx(QMutex::Recursive),
       group_name(group_name),
       saved(group_name),
-      transient(saved),
-      modified(false)
+      transient(saved)
 {}
 
 void impl_bundle::reload()
 {
     {
         QMutexLocker l(&mtx);
-        if (modified)
-        {
-            saved = group(group_name);
-            transient = saved;
-            modified = false;
-        }
+        saved = group(group_name);
+        transient = saved;
     }
     emit reloading();
 }
@@ -115,14 +133,7 @@ void impl_bundle::store_kv(const QString& name, const QVariant& datum)
 {
     QMutexLocker l(&mtx);
 
-    auto old = transient.get<QVariant>(name);
-    if (!transient.contains(name) || datum != old)
-    {
-        if (!modified)
-            qDebug() << "bundle" << group_name << "modified" << "key" << name << "to" << datum << "from" << old;
-        modified = true;
-        transient.put(name, datum);
-    }
+    transient.put(name, datum);
 }
 
 bool impl_bundle::contains(const QString &name) const
@@ -134,30 +145,55 @@ bool impl_bundle::contains(const QString &name) const
 void impl_bundle::save()
 {
     bool modified_ = false;
+
     {
         QMutexLocker l(&mtx);
-        if (modified)
+        if (saved != transient)
         {
-            qDebug() << "bundle" << group_name << "saved";
+            qDebug() << "bundle" << group_name << "changed, saving";
             modified_ = true;
-            modified = false;
             saved = transient;
-            transient.save();
+            saved.save();
         }
     }
+
     if (modified_)
         emit saving();
 }
 
-bool impl_bundle::modifiedp() const
+bool impl_bundle::modifiedp() const // XXX unused
 {
     QMutexLocker l(const_cast<QMutex*>(&mtx));
-    return modified;
+    return transient != saved;
 }
 
 namespace detail
 {
 
+void opt_singleton::bundle_decf(const opt_singleton::k& key)
+{
+    QMutexLocker l(&implsgl_mtx);
+
+    if (--std::get<0>(implsgl_data[key]) == 0)
+    {
+        qDebug() << "bundle -" << key;
+
+        implsgl_data.erase(key);
+    }
+}
+
+opt_singleton::opt_singleton() : implsgl_mtx(QMutex::Recursive) {}
+
+opt_bundle::opt_bundle(const QString& group_name)
+    : impl_bundle(group_name)
+{
+}
+
+opt_bundle::~opt_bundle()
+{
+    detail::singleton().bundle_decf(group_name);
+}
+
 pbundle opt_singleton::bundle(const opt_singleton::k &key)
 {
     QMutexLocker l(&implsgl_mtx);
@@ -176,30 +212,12 @@ pbundle opt_singleton::bundle(const opt_singleton::k &key)
     return shr;
 }
 
-void opt_singleton::bundle_decf(const opt_singleton::k &key)
-{
-    QMutexLocker l(&implsgl_mtx);
-
-    if (--std::get<0>(implsgl_data[key]) == 0)
-        implsgl_data.erase(key);
 }
 
-opt_singleton::opt_singleton() : implsgl_mtx(QMutex::Recursive) {}
-
-}
-
-opt_bundle::opt_bundle(const QString &group_name)
-    : impl_bundle(group_name)
-{
-}
-
-opt_bundle::~opt_bundle()
-{
-    qDebug() << "bundle -" << group_name;
-    detail::singleton().bundle_decf(group_name);
-}
-
-base_value::base_value(pbundle b, const QString &name) : b(b), self_name(name) {}
+base_value::base_value(pbundle b, const QString &name) :
+    b(b),
+    self_name(name)
+{}
 
 opts::~opts()
 {
diff --git a/opentrack-compat/options.hpp b/opentrack-compat/options.hpp
index 40a4b0a6..76f36146 100644
--- a/opentrack-compat/options.hpp
+++ b/opentrack-compat/options.hpp
@@ -106,6 +106,8 @@ namespace options {
         static QString ini_pathname();
         static const QStringList ini_list();
         static const mem<QSettings> ini_file();
+        bool operator==(const group& other) const;
+        bool operator!=(const group& other) const { return !(*this == other); }
 
         template<typename t>
         t get(const QString& k) const
@@ -125,7 +127,6 @@ namespace options {
         const QString group_name;
         group saved;
         group transient;
-        bool modified;
         impl_bundle(const impl_bundle&) = delete;
         impl_bundle& operator=(const impl_bundle&) = delete;
     signals:
@@ -148,10 +149,15 @@ namespace options {
         }
     };
 
-    class opt_bundle;
-
     namespace detail
     {
+        class OPENTRACK_COMPAT_EXPORT opt_bundle final : public impl_bundle
+        {
+        public:
+            opt_bundle(const QString& group_name);
+            ~opt_bundle();
+        };
+
         struct OPENTRACK_COMPAT_EXPORT opt_singleton
         {
         public:
@@ -172,18 +178,10 @@ namespace options {
         OPENTRACK_COMPAT_EXPORT opt_singleton& singleton();
     }
 
-    using pbundle = std::shared_ptr<opt_bundle>;
+    using pbundle = std::shared_ptr<detail::opt_bundle>;
 
     pbundle bundle(const QString& name);
 
-    class OPENTRACK_COMPAT_EXPORT opt_bundle final : public impl_bundle
-    {
-    public:
-        opt_bundle() : impl_bundle("i-have-no-name") {}
-        opt_bundle(const QString& group_name);
-        ~opt_bundle();
-    };
-
 #define DEFINE_SLOT(t) void setValue(t datum) { store(datum); }
 #define DEFINE_SIGNAL(t) void valueChanged(t)