control: Refactor ControlProxy and require flag to allow empty keys

9 files changed, 45 insertions, 32 deletions

diff --git a/src/control/control.cpp b/src/control/control.cpp
index 9256d148e0..275e1b64e3 100644
--- a/src/control/control.cpp
+++ b/src/control/control.cpp
@@ -115,6 +115,10 @@ QSharedPointer<ControlDoublePrivate> ControlDoublePrivate::getControl(
         return QSharedPointer<ControlDoublePrivate>();
     }
 
+    // Setting this flag makes no sense here and indicates that the programmer
+    // is confused and should consider taking a little break.
+    DEBUG_ASSERT(!flags.testFlag(ControlFlag::InitializeLater));
+
     QSharedPointer<ControlDoublePrivate> pControl;
     // Scope for MMutexLocker.
     {
diff --git a/src/control/control.h b/src/control/control.h
index dbf6183a89..e7747a9c85 100644
--- a/src/control/control.h
+++ b/src/control/control.h
@@ -15,8 +15,10 @@ class ControlObject;
 
 enum class ControlFlag {
     None = 0,
-    NoWarnIfMissing = 1,
-    NoAssertIfMissing = 2,
+    InitializeLater = 1,
+    AllowEmptyKey = 1 << 1,
+    NoAssertIfMissing = 1 << 2,
+    NoWarnIfMissing = (1 << 3) | NoAssertIfMissing,
 };
 
 Q_DECLARE_FLAGS(ControlFlags, ControlFlag)
diff --git a/src/control/controlmodel.cpp b/src/control/controlmodel.cpp
index 0ebfbad2be..04ecd863b7 100644
--- a/src/control/controlmodel.cpp
+++ b/src/control/controlmodel.cpp
@@ -21,8 +21,8 @@ void ControlModel::addControl(const ConfigKey& key,
     info.key = key;
     info.title = title;
     info.description = description;
-    info.pControl = new ControlProxy(this);
-    info.pControl->initialize(info.key);
+    info.pControl = new ControlProxy(info.key, this);
+    info.pControl->initialize();
 
     beginInsertRows(QModelIndex(), m_controls.size(),
                     m_controls.size());
diff --git a/src/control/controlproxy.cpp b/src/control/controlproxy.cpp
index d450b9ee2b..83c3840a5c 100644
--- a/src/control/controlproxy.cpp
+++ b/src/control/controlproxy.cpp
@@ -3,35 +3,41 @@
 #include "control/controlproxy.h"
 #include "control/control.h"
 
-ControlProxy::ControlProxy(QObject* pParent)
-        : QObject(pParent),
-          m_pControl(NULL) {
-}
-
 ControlProxy::ControlProxy(const QString& g, const QString& i, QObject* pParent, ControlFlags flags)
-        : QObject(pParent) {
-    initialize(ConfigKey(g, i), flags);
+        : ControlProxy(ConfigKey(g, i), pParent, flags) {
 }
 
 ControlProxy::ControlProxy(const char* g, const char* i, QObject* pParent, ControlFlags flags)
-        : QObject(pParent) {
-    initialize(ConfigKey(g, i), flags);
+        : ControlProxy(ConfigKey(g, i), pParent, flags) {
 }
 
 ControlProxy::ControlProxy(const ConfigKey& key, QObject* pParent, ControlFlags flags)
-        : QObject(pParent) {
-    initialize(key, flags);
+        : QObject(pParent),
+          m_pControl(nullptr) {
+    DEBUG_ASSERT(!key.isNull() || flags.testFlag(ControlFlag::AllowEmptyKey));
+    m_key = key;
+
+    if (!flags.testFlag(ControlFlag::InitializeLater)) {
+        flags.setFlag(ControlFlag::InitializeLater, false);
+        initialize(flags);
+    }
 }
 
-void ControlProxy::initialize(const ConfigKey& key, ControlFlags flags) {
-    m_key = key;
-    // Don't bother looking up the control if key is NULL. Prevents log spew.
-    if (!key.isNull()) {
-        m_pControl = ControlDoublePrivate::getControl(key, flags);
+void ControlProxy::initialize(ControlFlags flags) {
+    // Prevent double initialization
+    DEBUG_ASSERT(!m_pControl);
+
+    // Prevent empty keys
+    if (m_key.isNull()) {
+        DEBUG_ASSERT(flags.testFlag(ControlFlag::AllowEmptyKey));
+        return;
     }
+
+    m_pControl = ControlDoublePrivate::getControl(m_key, flags);
+    DEBUG_ASSERT(flags.testFlag(ControlFlag::NoAssertIfMissing) || m_pControl);
+    DEBUG_ASSERT(flags.testFlag(ControlFlag::NoAssertIfMissing) || valid());
 }
 
 ControlProxy::~ControlProxy() {
     //qDebug() << "ControlProxy::~ControlProxy()";
 }
-
diff --git a/src/control/controlproxy.h b/src/control/controlproxy.h
index daf15121ab..3da065fdbf 100644
--- a/src/control/controlproxy.h
+++ b/src/control/controlproxy.h
@@ -19,7 +19,6 @@
 class ControlProxy : public QObject {
     Q_OBJECT
   public:
-    ControlProxy(QObject* pParent = NULL);
     ControlProxy(const QString& g,
             const QString& i,
             QObject* pParent = NULL,
@@ -33,7 +32,7 @@ class ControlProxy : public QObject {
             ControlFlags flags = ControlFlag::None);
     virtual ~ControlProxy();
 
-    void initialize(const ConfigKey& key, ControlFlags flags = ControlFlag::None);
+    void initialize(ControlFlags flags = ControlFlag::None);
 
     const ConfigKey& getKey() const {
         return m_key;
diff --git a/src/mixer/playermanager.cpp b/src/mixer/playermanager.cpp
index dcca65e325..fe483c1e34 100644
--- a/src/mixer/playermanager.cpp
+++ b/src/mixer/playermanager.cpp
@@ -604,7 +604,7 @@ void PlayerManager::slotLoadTrackToPlayer(TrackPointer pTrack, QString group, bo
     // AutoDJProcessor is initialized after PlayerManager, so check that the
     // ControlProxy is pointing to the real ControlObject.
     if (!m_pAutoDjEnabled->valid()) {
-        m_pAutoDjEnabled->initialize(ConfigKey("[AutoDJ]", "enabled"));
+        m_pAutoDjEnabled->initialize();
     }
     bool autoDjSkipClone = m_pAutoDjEnabled->get() && (pPlayer == m_decks.at(0) || pPlayer == m_decks.at(1));
 
diff --git a/src/skin/legacyskinparser.cpp b/src/skin/legacyskinparser.cpp
index 6d2919c336..d1d923f4e3 100644
--- a/src/skin/legacyskinparser.cpp
+++ b/src/skin/legacyskinparser.cpp
@@ -102,7 +102,7 @@ ControlObject* controlFromConfigKey(const ConfigKey& key, bool bPersist,
     }
     // Don't warn if the control doesn't exist. Skins use this to create
     // controls.
-    ControlObject* pControl = ControlObject::getControl(key, ControlFlag::NoWarnIfMissing);
+    ControlObject* pControl = ControlObject::getControl(key, ControlFlag::NoAssertIfMissing);
 
     if (pControl) {
         if (pCreated) {
diff --git a/src/widget/wmainmenubar.cpp b/src/widget/wmainmenubar.cpp
index 623779447a..b9a951e88f 100644
--- a/src/widget/wmainmenubar.cpp
+++ b/src/widget/wmainmenubar.cpp
@@ -690,8 +690,7 @@ void VisibilityControlConnection::slotClearControl() {
 }
 
 void VisibilityControlConnection::slotReconnectControl() {
-    m_pControl.reset(new ControlProxy(this));
-    m_pControl->initialize(m_key, ControlFlag::NoAssertIfMissing);
+    m_pControl.reset(new ControlProxy(m_key, this, ControlFlag::NoAssertIfMissing));
     m_pControl->connectValueChanged(this, &VisibilityControlConnection::slotControlChanged);
     m_pAction->setEnabled(m_pControl->valid());
     slotControlChanged();
diff --git a/src/widget/wwidgetstack.cpp b/src/widget/wwidgetstack.cpp
index 8aa9eb25f3..db910e8a9a 100644
--- a/src/widget/wwidgetstack.cpp
+++ b/src/widget/wwidgetstack.cpp
@@ -28,13 +28,16 @@ void WidgetStackControlListener::onCurrentWidgetChanged(int index) {
     }
 }
 
-WWidgetStack::WWidgetStack(QWidget* pParent, const ConfigKey& nextConfigKey,
-        const ConfigKey& prevConfigKey, const ConfigKey& currentPageConfigKey)
+WWidgetStack::WWidgetStack(QWidget* pParent,
+        const ConfigKey& nextConfigKey,
+        const ConfigKey& prevConfigKey,
+        const ConfigKey& currentPageConfigKey)
         : QStackedWidget(pParent),
           WBaseWidget(this),
-          m_nextControl(nextConfigKey, this),
-          m_prevControl(prevConfigKey, this),
-          m_currentPageControl(currentPageConfigKey, this) {
+          m_nextControl(nextConfigKey, this, ControlFlag::AllowEmptyKey),
+          m_prevControl(prevConfigKey, this, ControlFlag::AllowEmptyKey),
+          m_currentPageControl(
+                  currentPageConfigKey, this, ControlFlag::AllowEmptyKey) {
     m_nextControl.connectValueChanged(this, &WWidgetStack::onNextControlChanged);
     m_prevControl.connectValueChanged(this, &WWidgetStack::onPrevControlChanged);
     m_currentPageControl.connectValueChanged(this, &WWidgetStack::onCurrentPageControlChanged);