control: Use assertions in ControlDoublePrivate if warn is true

14 files changed, 109 insertions, 73 deletions

diff --git a/src/control/control.cpp b/src/control/control.cpp
index 78f6484eb8..9256d148e0 100644
--- a/src/control/control.cpp
+++ b/src/control/control.cpp
@@ -102,17 +102,19 @@ void ControlDoublePrivate::insertAlias(const ConfigKey& alias, const ConfigKey&
 
 // static
 QSharedPointer<ControlDoublePrivate> ControlDoublePrivate::getControl(
-        const ConfigKey& key, bool warn, ControlObject* pCreatorCO,
-        bool bIgnoreNops, bool bTrack, bool bPersist, double defaultValue) {
-    if (key.isEmpty()) {
-        if (warn) {
-            qWarning() << "ControlDoublePrivate::getControl returning NULL"
-                       << "for empty ConfigKey.";
-        }
+        const ConfigKey& key,
+        ControlFlags flags,
+        ControlObject* pCreatorCO,
+        bool bIgnoreNops,
+        bool bTrack,
+        bool bPersist,
+        double defaultValue) {
+    VERIFY_OR_DEBUG_ASSERT(!key.isEmpty()) {
+        qWarning() << "ControlDoublePrivate::getControl returning NULL"
+                   << "for empty ConfigKey.";
         return QSharedPointer<ControlDoublePrivate>();
     }
 
-
     QSharedPointer<ControlDoublePrivate> pControl;
     // Scope for MMutexLocker.
     {
@@ -120,9 +122,8 @@ QSharedPointer<ControlDoublePrivate> ControlDoublePrivate::getControl(
         auto it = s_qCOHash.constFind(key);
         if (it != s_qCOHash.constEnd()) {
             if (pCreatorCO) {
-                if (warn) {
-                    qDebug() << "ControlObject" << key.group << key.item << "already created";
-                }
+                qDebug() << "ControlObject" << key.group << key.item << "already created";
+                DEBUG_ASSERT(!"ControlObject already created");
             } else {
                 pControl = it.value();
             }
@@ -137,9 +138,12 @@ QSharedPointer<ControlDoublePrivate> ControlDoublePrivate::getControl(
             MMutexLocker locker(&s_qCOHashMutex);
             //qDebug() << "ControlDoublePrivate::s_qCOHash.insert(" << key.group << "," << key.item << ")";
             s_qCOHash.insert(key, pControl);
-        } else if (warn) {
+        } else if (!flags.testFlag(ControlFlag::NoWarnIfMissing)) {
             qWarning() << "ControlDoublePrivate::getControl returning NULL for ("
                        << key.group << "," << key.item << ")";
+            if (!flags.testFlag(ControlFlag::NoAssertIfMissing)) {
+                DEBUG_ASSERT(!"ControlObject not found");
+            }
         }
     }
     return pControl;
diff --git a/src/control/control.h b/src/control/control.h
index f0d87ec6eb..dbf6183a89 100644
--- a/src/control/control.h
+++ b/src/control/control.h
@@ -13,6 +13,15 @@
 
 class ControlObject;
 
+enum class ControlFlag {
+    None = 0,
+    NoWarnIfMissing = 1,
+    NoAssertIfMissing = 2,
+};
+
+Q_DECLARE_FLAGS(ControlFlags, ControlFlag)
+Q_DECLARE_OPERATORS_FOR_FLAGS(ControlFlags)
+
 class ControlDoublePrivate : public QObject {
     Q_OBJECT
   public:
@@ -33,10 +42,13 @@ class ControlDoublePrivate : public QObject {
     // Gets the ControlDoublePrivate matching the given ConfigKey. If pCreatorCO
     // is non-NULL, allocates a new ControlDoublePrivate for the ConfigKey if
     // one does not exist.
-    static QSharedPointer<ControlDoublePrivate> getControl(
-            const ConfigKey& key, bool warn = true,
-            ControlObject* pCreatorCO = NULL, bool bIgnoreNops = true, bool bTrack = false,
-            bool bPersist = false, double defaultValue = 0.0);
+    static QSharedPointer<ControlDoublePrivate> getControl(const ConfigKey& key,
+            ControlFlags flags = ControlFlag::None,
+            ControlObject* pCreatorCO = NULL,
+            bool bIgnoreNops = true,
+            bool bTrack = false,
+            bool bPersist = false,
+            double defaultValue = 0.0);
 
     // Adds all ControlDoublePrivate that currently exist to pControlList
     static void getControls(QList<QSharedPointer<ControlDoublePrivate> >* pControlsList);
diff --git a/src/control/controlobject.cpp b/src/control/controlobject.cpp
index db41618534..2de1c38683 100644
--- a/src/control/controlobject.cpp
+++ b/src/control/controlobject.cpp
@@ -33,8 +33,13 @@ ControlObject::ControlObject(ConfigKey key, bool bIgnoreNops, bool bTrack,
         : m_key(key) {
     // Don't bother looking up the control if key is NULL. Prevents log spew.
     if (!m_key.isNull()) {
-        m_pControl = ControlDoublePrivate::getControl(m_key, true, this,
-                bIgnoreNops, bTrack, bPersist, defaultValue);
+        m_pControl = ControlDoublePrivate::getControl(m_key,
+                ControlFlag::None,
+                this,
+                bIgnoreNops,
+                bTrack,
+                bPersist,
+                defaultValue);
     }
 
     // getControl can fail and return a NULL control even with the create flag.
@@ -62,9 +67,9 @@ void ControlObject::privateValueChanged(double dValue, QObject* pSender) {
 }
 
 // static
-ControlObject* ControlObject::getControl(const ConfigKey& key, bool warn) {
+ControlObject* ControlObject::getControl(const ConfigKey& key, ControlFlags flags) {
     //qDebug() << "ControlObject::getControl for (" << key.group << "," << key.item << ")";
-    QSharedPointer<ControlDoublePrivate> pCDP = ControlDoublePrivate::getControl(key, warn);
+    QSharedPointer<ControlDoublePrivate> pCDP = ControlDoublePrivate::getControl(key, flags);
     if (pCDP) {
         return pCDP->getCreatorCO();
     }
diff --git a/src/control/controlobject.h b/src/control/controlobject.h
index 75bbd39418..fd3f34247f 100644
--- a/src/control/controlobject.h
+++ b/src/control/controlobject.h
@@ -42,10 +42,12 @@ class ControlObject : public QObject {
     virtual ~ControlObject();
 
     // Returns a pointer to the ControlObject matching the given ConfigKey
-    static ControlObject* getControl(const ConfigKey& key, bool warn = true);
-    static inline ControlObject* getControl(const QString& group, const QString& item, bool warn = true) {
+    static ControlObject* getControl(const ConfigKey& key, ControlFlags flags = ControlFlag::None);
+    static inline ControlObject* getControl(const QString& group,
+            const QString& item,
+            ControlFlags flags = ControlFlag::None) {
         ConfigKey key(group, item);
-        return getControl(key, warn);
+        return getControl(key, flags);
     }
 
     QString name() const {
diff --git a/src/control/controlobjectscript.cpp b/src/control/controlobjectscript.cpp
index cb6ced8715..d95d0c0176 100644
--- a/src/control/controlobjectscript.cpp
+++ b/src/control/controlobjectscript.cpp
@@ -3,7 +3,7 @@
 #include "control/controlobjectscript.h"
 
 ControlObjectScript::ControlObjectScript(const ConfigKey& key, QObject* pParent)
-        : ControlProxy(key, pParent) {
+        : ControlProxy(key, pParent, ControlFlag::NoAssertIfMissing) {
 }
 
 bool ControlObjectScript::addScriptConnection(const ScriptConnection& conn) {
diff --git a/src/control/controlproxy.cpp b/src/control/controlproxy.cpp
index 9434ed6e71..d450b9ee2b 100644
--- a/src/control/controlproxy.cpp
+++ b/src/control/controlproxy.cpp
@@ -8,26 +8,26 @@ ControlProxy::ControlProxy(QObject* pParent)
           m_pControl(NULL) {
 }
 
-ControlProxy::ControlProxy(const QString& g, const QString& i, QObject* pParent)
+ControlProxy::ControlProxy(const QString& g, const QString& i, QObject* pParent, ControlFlags flags)
         : QObject(pParent) {
-    initialize(ConfigKey(g, i));
+    initialize(ConfigKey(g, i), flags);
 }
 
-ControlProxy::ControlProxy(const char* g, const char* i, QObject* pParent)
+ControlProxy::ControlProxy(const char* g, const char* i, QObject* pParent, ControlFlags flags)
         : QObject(pParent) {
-    initialize(ConfigKey(g, i));
+    initialize(ConfigKey(g, i), flags);
 }
 
-ControlProxy::ControlProxy(const ConfigKey& key, QObject* pParent)
+ControlProxy::ControlProxy(const ConfigKey& key, QObject* pParent, ControlFlags flags)
         : QObject(pParent) {
-    initialize(key);
+    initialize(key, flags);
 }
 
-void ControlProxy::initialize(const ConfigKey& key, bool warn) {
+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, warn);
+        m_pControl = ControlDoublePrivate::getControl(key, flags);
     }
 }
 
diff --git a/src/control/controlproxy.h b/src/control/controlproxy.h
index 4e70c3d974..daf15121ab 100644
--- a/src/control/controlproxy.h
+++ b/src/control/controlproxy.h
@@ -20,12 +20,20 @@ class ControlProxy : public QObject {
     Q_OBJECT
   public:
     ControlProxy(QObject* pParent = NULL);
-    ControlProxy(const QString& g, const QString& i, QObject* pParent = NULL);
-    ControlProxy(const char* g, const char* i, QObject* pParent = NULL);
-    ControlProxy(const ConfigKey& key, QObject* pParent = NULL);
+    ControlProxy(const QString& g,
+            const QString& i,
+            QObject* pParent = NULL,
+            ControlFlags flags = ControlFlag::None);
+    ControlProxy(const char* g,
+            const char* i,
+            QObject* pParent = NULL,
+            ControlFlags flags = ControlFlag::None);
+    ControlProxy(const ConfigKey& key,
+            QObject* pParent = NULL,
+            ControlFlags flags = ControlFlag::None);
     virtual ~ControlProxy();
 
-    void initialize(const ConfigKey& key, bool warn = true);
+    void initialize(const ConfigKey& key, ControlFlags flags = ControlFlag::None);
 
     const ConfigKey& getKey() const {
         return m_key;
diff --git a/src/controllers/controllerengine.cpp b/src/controllers/controllerengine.cpp
index cc298e4f07..9ca07d0cc9 100644
--- a/src/controllers/controllerengine.cpp
+++ b/src/controllers/controllerengine.cpp
@@ -676,14 +676,14 @@ void ControllerEngine::setValue(QString group, QString name, double newValue) {
     ControlObjectScript* coScript = getControlObjectScript(group, name);
 
     if (coScript != nullptr) {
-        ControlObject* pControl = ControlObject::getControl(coScript->getKey());
+        ControlObject* pControl = ControlObject::getControl(
+                coScript->getKey(), ControlFlag::NoAssertIfMissing);
         if (pControl && !m_st.ignore(pControl, coScript->getParameterForValue(newValue))) {
             coScript->slotSet(newValue);
         }
     }
 }
 
-
 /* -------- ------------------------------------------------------
    Purpose: Returns the normalized value of a Mixxx control (for scripts)
    Input:   Control group (e.g. [Channel1]), Key name (e.g. [filterHigh])
@@ -713,7 +713,8 @@ void ControllerEngine::setParameter(QString group, QString name, double newParam
     ControlObjectScript* coScript = getControlObjectScript(group, name);
 
     if (coScript != nullptr) {
-        ControlObject* pControl = ControlObject::getControl(coScript->getKey());
+        ControlObject* pControl = ControlObject::getControl(
+                coScript->getKey(), ControlFlag::NoAssertIfMissing);
         if (pControl && !m_st.ignore(pControl, newParameter)) {
           coScript->setParameter(newParameter);
         }
@@ -1440,7 +1441,8 @@ bool ControllerEngine::isScratching(int deck) {
     Output:  -
     -------- ------------------------------------------------------ */
 void ControllerEngine::softTakeover(QString group, QString name, bool set) {
-    ControlObject* pControl = ControlObject::getControl(ConfigKey(group, name));
+    ControlObject* pControl = ControlObject::getControl(
+            ConfigKey(group, name), ControlFlag::NoAssertIfMissing);
     if (!pControl) {
         return;
     }
@@ -1460,8 +1462,10 @@ void ControllerEngine::softTakeover(QString group, QString name, bool set) {
      Input:   ControlObject group and key values
      Output:  -
      -------- ------------------------------------------------------ */
-void ControllerEngine::softTakeoverIgnoreNextValue(QString group, const QString name) {
-    ControlObject* pControl = ControlObject::getControl(ConfigKey(group, name));
+void ControllerEngine::softTakeoverIgnoreNextValue(
+        QString group, const QString name) {
+    ControlObject* pControl = ControlObject::getControl(
+            ConfigKey(group, name), ControlFlag::NoAssertIfMissing);
     if (!pControl) {
         return;
     }
diff --git a/src/controllers/midi/midioutputhandler.cpp b/src/controllers/midi/midioutputhandler.cpp
index 6c3a045f65..f2ad6aac90 100644
--- a/src/controllers/midi/midioutputhandler.cpp
+++ b/src/controllers/midi/midioutputhandler.cpp
@@ -14,10 +14,10 @@
 #include "control/controlobject.h"
 
 MidiOutputHandler::MidiOutputHandler(MidiController* controller,
-                                     const MidiOutputMapping& mapping)
+        const MidiOutputMapping& mapping)
         : m_pController(controller),
           m_mapping(mapping),
-          m_cos(mapping.controlKey, this),
+          m_cos(mapping.controlKey, this, ControlFlag::NoAssertIfMissing),
           m_lastVal(-1) { // -1 = virgin
     m_cos.connectValueChanged(this, &MidiOutputHandler::controlChanged);
 }
diff --git a/src/mixer/playermanager.cpp b/src/mixer/playermanager.cpp
index 347fc5cb31..dcca65e325 100644
--- a/src/mixer/playermanager.cpp
+++ b/src/mixer/playermanager.cpp
@@ -42,30 +42,31 @@ QAtomicPointer<ControlProxy> PlayerManager::m_pCOPNumSamplers;
 QAtomicPointer<ControlProxy> PlayerManager::m_pCOPNumPreviewDecks;
 
 PlayerManager::PlayerManager(UserSettingsPointer pConfig,
-                             SoundManager* pSoundManager,
-                             EffectsManager* pEffectsManager,
-                             VisualsManager* pVisualsManager,
-                             EngineMaster* pEngine) :
-        m_mutex(QMutex::Recursive),
-        m_pConfig(pConfig),
-        m_pSoundManager(pSoundManager),
-        m_pEffectsManager(pEffectsManager),
-        m_pVisualsManager(pVisualsManager),
-        m_pEngine(pEngine),
-        // NOTE(XXX) LegacySkinParser relies on these controls being Controls
-        // and not ControlProxies.
-        m_pCONumDecks(new ControlObject(
-                ConfigKey("[Master]", "num_decks"), true, true)),
-        m_pCONumSamplers(new ControlObject(
-                ConfigKey("[Master]", "num_samplers"), true, true)),
-        m_pCONumPreviewDecks(new ControlObject(
-                ConfigKey("[Master]", "num_preview_decks"), true, true)),
-        m_pCONumMicrophones(new ControlObject(
-                ConfigKey("[Master]", "num_microphones"), true, true)),
-        m_pCONumAuxiliaries(new ControlObject(
-                ConfigKey("[Master]", "num_auxiliaries"), true, true)),
-        m_pAutoDjEnabled(make_parented<ControlProxy>("[AutoDJ]", "enabled", this)),
-        m_pTrackAnalysisScheduler(TrackAnalysisScheduler::NullPointer()) {
+        SoundManager* pSoundManager,
+        EffectsManager* pEffectsManager,
+        VisualsManager* pVisualsManager,
+        EngineMaster* pEngine)
+        : m_mutex(QMutex::Recursive),
+          m_pConfig(pConfig),
+          m_pSoundManager(pSoundManager),
+          m_pEffectsManager(pEffectsManager),
+          m_pVisualsManager(pVisualsManager),
+          m_pEngine(pEngine),
+          // NOTE(XXX) LegacySkinParser relies on these controls being Controls
+          // and not ControlProxies.
+          m_pCONumDecks(new ControlObject(
+                  ConfigKey("[Master]", "num_decks"), true, true)),
+          m_pCONumSamplers(new ControlObject(
+                  ConfigKey("[Master]", "num_samplers"), true, true)),
+          m_pCONumPreviewDecks(new ControlObject(
+                  ConfigKey("[Master]", "num_preview_decks"), true, true)),
+          m_pCONumMicrophones(new ControlObject(
+                  ConfigKey("[Master]", "num_microphones"), true, true)),
+          m_pCONumAuxiliaries(new ControlObject(
+                  ConfigKey("[Master]", "num_auxiliaries"), true, true)),
+          m_pAutoDjEnabled(make_parented<ControlProxy>(
+                  "[AutoDJ]", "enabled", this, ControlFlag::NoWarnIfMissing)),
+          m_pTrackAnalysisScheduler(TrackAnalysisScheduler::NullPointer()) {
     m_pCONumDecks->connectValueChangeRequest(this,
             &PlayerManager::slotChangeNumDecks, Qt::DirectConnection);
     m_pCONumSamplers->connectValueChangeRequest(this,
diff --git a/src/mixxx.cpp b/src/mixxx.cpp
index a6dc42563a..0fd9dde2a6 100644
--- a/src/mixxx.cpp
+++ b/src/mixxx.cpp
@@ -804,7 +804,7 @@ void MixxxMainWindow::finalize() {
                 // A deletion early in the list may trigger a destructor
                 // for a control later in the list, so we check for a null
                 // pointer each time.
-                ControlObject* pCo = ControlObject::getControl(key, false);
+                ControlObject* pCo = ControlObject::getControl(key, ControlFlag::NoAssertIfMissing);
                 if (pCo) {
                     delete pCo;
                 }
diff --git a/src/skin/legacyskinparser.cpp b/src/skin/legacyskinparser.cpp
index f6db4add2f..6d2919c336 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, false);
+    ControlObject* pControl = ControlObject::getControl(key, ControlFlag::NoWarnIfMissing);
 
     if (pControl) {
         if (pCreated) {
diff --git a/src/test/beatstranslatetest.cpp b/src/test/beatstranslatetest.cpp
index f2a183eba4..1352a8f1a1 100644
--- a/src/test/beatstranslatetest.cpp
+++ b/src/test/beatstranslatetest.cpp
@@ -26,8 +26,8 @@ TEST_F(BeatsTranslateTest, SimpleTranslateMatch) {
     EXPECT_TRUE(m_pChannel1->getEngineBuffer()->getVisualPlayPos() > 0);
 
     // Make both decks playing.
-    ControlObject::getControl(m_sGroup1, "play", true)->set(1.0);
-    ControlObject::getControl(m_sGroup2, "play", true)->set(1.0);
+    ControlObject::getControl(m_sGroup1, "play", ControlFlag::None)->set(1.0);
+    ControlObject::getControl(m_sGroup2, "play", ControlFlag::None)->set(1.0);
     ProcessBuffer();
     // Manually set the "bpm" control... I would like to figure out why this
     // doesn't get set naturally, but this will do for now.
diff --git a/src/widget/wmainmenubar.cpp b/src/widget/wmainmenubar.cpp
index 4014382b48..623779447a 100644
--- a/src/widget/wmainmenubar.cpp
+++ b/src/widget/wmainmenubar.cpp
@@ -691,7 +691,7 @@ void VisibilityControlConnection::slotClearControl() {
 
 void VisibilityControlConnection::slotReconnectControl() {
     m_pControl.reset(new ControlProxy(this));
-    m_pControl->initialize(m_key, false);
+    m_pControl->initialize(m_key, ControlFlag::NoAssertIfMissing);
     m_pControl->connectValueChanged(this, &VisibilityControlConnection::slotControlChanged);
     m_pAction->setEnabled(m_pControl->valid());
     slotControlChanged();