From 35f29a794b157ef1ff820b553e3a2ab7f48f91a7 Mon Sep 17 00:00:00 2001
From: Dan Paulat <dpaulat@gmail.com>
Date: Wed, 28 Feb 2024 22:37:22 -0600
Subject: [PATCH] Unregister settings callbacks on layer destruction

Fixes #151
---
 .../scwx/qt/map/overlay_product_layer.cpp     | 43 ++++++++-----
 .../scwx/qt/settings/settings_variable.cpp    | 61 +++++++++++++------
 .../scwx/qt/settings/settings_variable.hpp    | 22 ++++++-
 3 files changed, 89 insertions(+), 37 deletions(-)

diff --git a/scwx-qt/source/scwx/qt/map/overlay_product_layer.cpp b/scwx-qt/source/scwx/qt/map/overlay_product_layer.cpp
index d7e19b2f..3e5d09b6 100644
--- a/scwx-qt/source/scwx/qt/map/overlay_product_layer.cpp
+++ b/scwx-qt/source/scwx/qt/map/overlay_product_layer.cpp
@@ -31,28 +31,38 @@ public:
    {
       auto& productSettings = settings::ProductSettings::Instance();
 
-      productSettings.sti_forecast_enabled().RegisterValueStagedCallback(
-         [=, this](const bool& value)
-         {
-            stiForecastEnabled_ = value;
-            stiNeedsUpdate_     = true;
+      stiForecastEnabledCallbackUuid_ =
+         productSettings.sti_forecast_enabled().RegisterValueStagedCallback(
+            [=, this](const bool& value)
+            {
+               stiForecastEnabled_ = value;
+               stiNeedsUpdate_     = true;
 
-            Q_EMIT self_->NeedsRendering();
-         });
-      productSettings.sti_past_enabled().RegisterValueStagedCallback(
-         [=, this](const bool& value)
-         {
-            stiPastEnabled_ = value;
-            stiNeedsUpdate_ = true;
+               Q_EMIT self_->NeedsRendering();
+            });
+      stiPastEnabledCallbackUuid_ =
+         productSettings.sti_past_enabled().RegisterValueStagedCallback(
+            [=, this](const bool& value)
+            {
+               stiPastEnabled_ = value;
+               stiNeedsUpdate_ = true;
 
-            Q_EMIT self_->NeedsRendering();
-         });
+               Q_EMIT self_->NeedsRendering();
+            });
 
       stiForecastEnabled_ =
          productSettings.sti_forecast_enabled().GetStagedOrValue();
       stiPastEnabled_ = productSettings.sti_past_enabled().GetStagedOrValue();
    }
-   ~Impl() = default;
+   ~Impl()
+   {
+      auto& productSettings = settings::ProductSettings::Instance();
+
+      productSettings.sti_forecast_enabled().UnregisterValueStagedCallback(
+         stiForecastEnabledCallbackUuid_);
+      productSettings.sti_past_enabled().UnregisterValueStagedCallback(
+         stiPastEnabledCallbackUuid_);
+   }
 
    void UpdateStormTrackingInformation();
 
@@ -87,6 +97,9 @@ public:
 
    OverlayProductLayer* self_;
 
+   boost::uuids::uuid stiForecastEnabledCallbackUuid_;
+   boost::uuids::uuid stiPastEnabledCallbackUuid_;
+
    bool stiForecastEnabled_ {true};
    bool stiPastEnabled_ {true};
 
diff --git a/scwx-qt/source/scwx/qt/settings/settings_variable.cpp b/scwx-qt/source/scwx/qt/settings/settings_variable.cpp
index 1f7661c0..276c9070 100644
--- a/scwx-qt/source/scwx/qt/settings/settings_variable.cpp
+++ b/scwx-qt/source/scwx/qt/settings/settings_variable.cpp
@@ -4,6 +4,8 @@
 #include <scwx/util/logger.hpp>
 
 #include <boost/json.hpp>
+#include <boost/unordered/unordered_flat_map.hpp>
+#include <boost/uuid/random_generator.hpp>
 #include <fmt/ostream.h>
 
 namespace scwx
@@ -30,8 +32,10 @@ public:
    std::optional<T>              maximum_ {};
    std::function<bool(const T&)> validator_ {nullptr};
 
-   std::vector<ValueCallbackFunction> valueChangedCallbackFunctions_ {};
-   std::vector<ValueCallbackFunction> valueStagedCallbackFunctions_ {};
+   boost::unordered_flat_map<boost::uuids::uuid, ValueCallbackFunction>
+      valueChangedCallbackFunctions_ {};
+   boost::unordered_flat_map<boost::uuids::uuid, ValueCallbackFunction>
+      valueStagedCallbackFunctions_ {};
 };
 
 template<class T>
@@ -80,11 +84,11 @@ bool SettingsVariable<T>::SetValue(const T& value)
 
       for (auto& callback : p->valueChangedCallbackFunctions_)
       {
-         callback(p->value_);
+         callback.second(p->value_);
       }
       for (auto& callback : p->valueStagedCallbackFunctions_)
       {
-         callback(p->value_);
+         callback.second(p->value_);
       }
    }
 
@@ -128,11 +132,11 @@ bool SettingsVariable<T>::SetValueOrDefault(const T& value)
 
    for (auto& callback : p->valueChangedCallbackFunctions_)
    {
-      callback(p->value_);
+      callback.second(p->value_);
    }
    for (auto& callback : p->valueStagedCallbackFunctions_)
    {
-      callback(p->value_);
+      callback.second(p->value_);
    }
 
    return validated;
@@ -145,11 +149,11 @@ void SettingsVariable<T>::SetValueToDefault()
 
    for (auto& callback : p->valueChangedCallbackFunctions_)
    {
-      callback(p->value_);
+      callback.second(p->value_);
    }
    for (auto& callback : p->valueStagedCallbackFunctions_)
    {
-      callback(p->value_);
+      callback.second(p->value_);
    }
 }
 
@@ -167,7 +171,7 @@ void SettingsVariable<T>::StageDefault()
 
    for (auto& callback : p->valueStagedCallbackFunctions_)
    {
-      callback(p->default_);
+      callback.second(p->default_);
    }
 }
 
@@ -191,7 +195,7 @@ bool SettingsVariable<T>::StageValue(const T& value)
 
       for (auto& callback : p->valueStagedCallbackFunctions_)
       {
-         callback(value);
+         callback.second(value);
       }
    }
 
@@ -211,11 +215,11 @@ bool SettingsVariable<T>::Commit()
 
       for (auto& callback : p->valueChangedCallbackFunctions_)
       {
-         callback(p->value_);
+         callback.second(p->value_);
       }
       for (auto& callback : p->valueStagedCallbackFunctions_)
       {
-         callback(p->value_);
+         callback.second(p->value_);
       }
    }
 
@@ -229,7 +233,7 @@ void SettingsVariable<T>::Reset()
 
    for (auto& callback : p->valueStagedCallbackFunctions_)
    {
-      callback(p->value_);
+      callback.second(p->value_);
    }
 }
 
@@ -327,11 +331,11 @@ bool SettingsVariable<T>::ReadValue(const boost::json::object& json)
 
    for (auto& callback : p->valueChangedCallbackFunctions_)
    {
-      callback(p->value_);
+      callback.second(p->value_);
    }
    for (auto& callback : p->valueStagedCallbackFunctions_)
    {
-      callback(p->value_);
+      callback.second(p->value_);
    }
 
    return validated;
@@ -344,17 +348,34 @@ void SettingsVariable<T>::WriteValue(boost::json::object& json) const
 }
 
 template<class T>
-void SettingsVariable<T>::RegisterValueChangedCallback(
+boost::uuids::uuid SettingsVariable<T>::RegisterValueChangedCallback(
    ValueCallbackFunction callback)
 {
-   p->valueChangedCallbackFunctions_.push_back(std::move(callback));
+   boost::uuids::uuid uuid = boost::uuids::random_generator()();
+   p->valueChangedCallbackFunctions_.emplace(uuid, std::move(callback));
+   return uuid;
 }
 
 template<class T>
-void SettingsVariable<T>::RegisterValueStagedCallback(
-   ValueCallbackFunction callback)
+void SettingsVariable<T>::UnregisterValueChangedCallback(
+   boost::uuids::uuid uuid)
 {
-   p->valueStagedCallbackFunctions_.push_back(std::move(callback));
+   p->valueChangedCallbackFunctions_.erase(uuid);
+}
+
+template<class T>
+boost::uuids::uuid
+SettingsVariable<T>::RegisterValueStagedCallback(ValueCallbackFunction callback)
+{
+   boost::uuids::uuid uuid = boost::uuids::random_generator()();
+   p->valueStagedCallbackFunctions_.emplace(uuid, std::move(callback));
+   return uuid;
+}
+
+template<class T>
+void SettingsVariable<T>::UnregisterValueStagedCallback(boost::uuids::uuid uuid)
+{
+   p->valueStagedCallbackFunctions_.erase(uuid);
 }
 
 template<class T>
diff --git a/scwx-qt/source/scwx/qt/settings/settings_variable.hpp b/scwx-qt/source/scwx/qt/settings/settings_variable.hpp
index c05c543f..bf461e8a 100644
--- a/scwx-qt/source/scwx/qt/settings/settings_variable.hpp
+++ b/scwx-qt/source/scwx/qt/settings/settings_variable.hpp
@@ -5,6 +5,8 @@
 #include <functional>
 #include <optional>
 
+#include <boost/uuid/uuid.hpp>
+
 class QAbstractButton;
 class QWidget;
 
@@ -194,7 +196,15 @@ public:
     *
     * @param callback Function to be called
     */
-   void RegisterValueChangedCallback(ValueCallbackFunction callback);
+   boost::uuids::uuid
+   RegisterValueChangedCallback(ValueCallbackFunction callback);
+
+   /**
+    * Unregisters a function previously registered for change callback.
+    *
+    * @param uuid Uuid of the callback registration
+    */
+   void UnregisterValueChangedCallback(boost::uuids::uuid uuid);
 
    /**
     * Registers a function to be called when the staged value changes (or a
@@ -204,7 +214,15 @@ public:
     *
     * @param callback Function to be called
     */
-   void RegisterValueStagedCallback(ValueCallbackFunction callback);
+   boost::uuids::uuid
+   RegisterValueStagedCallback(ValueCallbackFunction callback);
+
+   /**
+    * Unregisters a function previously registered for value staging callback.
+    *
+    * @param uuid Uuid of the callback registration
+    */
+   void UnregisterValueStagedCallback(boost::uuids::uuid uuid);
 
 protected:
    virtual bool Equals(const SettingsVariableBase& o) const override;