Effect enable switches changes

res/skins/LateNight/effect_unit.xml

@@ -93,11 +93,8 @@
                   <Text>ON</Text>
                 </State>
                 <Connection>
-                  <ConfigKey persist="true"><Variable name="group"/>,enabled</ConfigKey>
-                  <ButtonState>LeftButton</ButtonState>
-                </Connection>
-                <Connection>
-                  <ConfigKey persist="true"><Variable name="group"/>,enabled</ConfigKey>
+                  <!-- FIXME: Quick hack until effect chains are implemented. -->
+                  <ConfigKey persist="true">[EffectRack<Variable name="EffectRack"/>_EffectUnit<Variable name="EffectUnit"/>_Effect1],enabled</ConfigKey>
                 </Connection>
               </PushButton>
             </Children>

src/control/control.cpp

@@ -32,7 +32,7 @@ ControlDoublePrivate::ControlDoublePrivate()
 ControlDoublePrivate::ControlDoublePrivate(ConfigKey key,
                                            ControlObject* pCreatorCO,
                                            bool bIgnoreNops, bool bTrack,
-                                           bool bPersist)
+                                           bool bPersist, double defaultValue)
         : m_key(key),
           m_bPersistInConfiguration(bPersist),
           m_bIgnoreNops(bIgnoreNops),
@@ -42,19 +42,18 @@ ControlDoublePrivate::ControlDoublePrivate(ConfigKey key,
                        Stat::SAMPLE_VARIANCE | Stat::MIN | Stat::MAX),
           m_confirmRequired(false),
           m_pCreatorCO(pCreatorCO) {
-    initialize();
+    initialize(defaultValue);
 }
 
-void ControlDoublePrivate::initialize() {
-    double value = 0;
+void ControlDoublePrivate::initialize(double defaultValue) {
+    double value = defaultValue;
     if (m_bPersistInConfiguration) {
         UserSettingsPointer pConfig = ControlDoublePrivate::s_pUserConfig;
-        if (pConfig != NULL) {
-            // Assume toDouble() returns 0 if conversion fails.
-            value = pConfig->getValueString(m_key).toDouble();
+        if (pConfig != nullptr) {
+            value = pConfig->getValue(m_key, defaultValue);
         }
     }
-    m_defaultValue.setValue(0);
+    m_defaultValue.setValue(defaultValue);
     m_value.setValue(value);
 
     //qDebug() << "Creating:" << m_trackKey << "at" << &m_value << sizeof(m_value);
@@ -106,7 +105,7 @@ 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) {
+        bool bIgnoreNops, bool bTrack, bool bPersist, double defaultValue) {
     if (key.isEmpty()) {
         if (warn) {
             qWarning() << "ControlDoublePrivate::getControl returning NULL"
@@ -137,7 +136,7 @@ QSharedPointer<ControlDoublePrivate> ControlDoublePrivate::getControl(
         if (pCreatorCO) {
             pControl = QSharedPointer<ControlDoublePrivate>(
                     new ControlDoublePrivate(key, pCreatorCO, bIgnoreNops,
-                                             bTrack, bPersist));
+                                             bTrack, bPersist, defaultValue));
             MMutexLocker locker(&s_qCOHashMutex);
             //qDebug() << "ControlDoublePrivate::s_qCOHash.insert(" << key.group << "," << key.item << ")";
             s_qCOHash.insert(key, pControl);

src/control/control.h

@@ -36,7 +36,7 @@ class ControlDoublePrivate : public QObject {
     static QSharedPointer<ControlDoublePrivate> getControl(
             const ConfigKey& key, bool warn = true,
             ControlObject* pCreatorCO = NULL, bool bIgnoreNops = true, bool bTrack = false,
-            bool bPersist = false);
+            bool bPersist = false, double defaultValue = 0.0);
 
     // Adds all ControlDoublePrivate that currently exist to pControlList
     static void getControls(QList<QSharedPointer<ControlDoublePrivate> >* pControlsList);
@@ -126,8 +126,9 @@ class ControlDoublePrivate : public QObject {
 
   private:
     ControlDoublePrivate(ConfigKey key, ControlObject* pCreatorCO,
-                         bool bIgnoreNops, bool bTrack, bool bPersist);
-    void initialize();
+                         bool bIgnoreNops, bool bTrack, bool bPersist,
+                         double defaultValue);
+    void initialize(double defaultValue);
     void setInner(double value, QObject* pSender);
 
     ConfigKey m_key;

src/control/controlobject.cpp

@@ -29,8 +29,8 @@ ControlObject::ControlObject() {
 }
 
 ControlObject::ControlObject(ConfigKey key, bool bIgnoreNops, bool bTrack,
-                             bool bPersist) {
-    initialize(key, bIgnoreNops, bTrack, bPersist);
+                             bool bPersist, double defaultValue) {
+    initialize(key, bIgnoreNops, bTrack, bPersist, defaultValue);
 }
 
 ControlObject::~ControlObject() {
@@ -40,14 +40,14 @@ ControlObject::~ControlObject() {
 }
 
 void ControlObject::initialize(ConfigKey key, bool bIgnoreNops, bool bTrack,
-                               bool bPersist) {
+                               bool bPersist, double defaultValue) {
     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);
+                                                      bPersist, defaultValue);
     }
 
     // getControl can fail and return a NULL control even with the create flag.

src/control/controlobject.h

@@ -34,9 +34,11 @@ class ControlObject : public QObject {
     // bIgnoreNops: Don't emit a signal if the CO is set to its current value.
     // bTrack: Record statistics about this control.
     // bPersist: Store value on exit, load on startup.
+    // defaultValue: default value of CO. If CO is persistent and there is no valid
+    //               value found in the config, this is also the initial value.
     ControlObject(ConfigKey key,
-                  bool bIgnoreNops=true, bool bTrack=false,
-                  bool bPersist=false);
+                  bool bIgnoreNops = true, bool bTrack = false,
+                  bool bPersist = false, double defaultValue = 0.0);
     virtual ~ControlObject();
 
     // Returns a pointer to the ControlObject matching the given ConfigKey
@@ -176,7 +178,7 @@ class ControlObject : public QObject {
 
   private:
     void initialize(ConfigKey key, bool bIgnoreNops, bool bTrack,
-                    bool bPersist);
+                    bool bPersist, double defaultValue);
     inline bool ignoreNops() const {
         return m_pControl ? m_pControl->ignoreNops() : true;
     }

src/control/controlpushbutton.cpp

@@ -21,8 +21,8 @@
    Purpose: Creates a new simulated latching push-button.
    Input:   key - Key for the configuration file
    -------- ------------------------------------------------------ */
-ControlPushButton::ControlPushButton(ConfigKey key, bool bPersist)
-        : ControlObject(key, false, false, bPersist),
+ControlPushButton::ControlPushButton(ConfigKey key, bool bPersist, double defaultValue)
+        : ControlObject(key, false, false, bPersist, defaultValue),
           m_buttonMode(PUSH),
           m_iNoStates(2) {
     if (m_pControl) {

src/control/controlpushbutton.h

@@ -53,7 +53,7 @@ class ControlPushButton : public ControlObject {
         }
     }
 
-    ControlPushButton(ConfigKey key, bool bPersist=false);
+    ControlPushButton(ConfigKey key, bool bPersist = false, double defaultValue = 0.0);
     virtual ~ControlPushButton();
 
     inline ButtonMode getButtonMode() const {

src/effects/effect.cpp

@@ -16,7 +16,7 @@ Effect::Effect(EffectsManager* pEffectsManager,
           m_pInstantiator(pInstantiator),
           m_pEngineEffect(NULL),
           m_bAddedToEngine(false),
-          m_bEnabled(true) {
+          m_bEnabled(false) {
     foreach (const EffectManifestParameter& parameter, m_manifest.parameters()) {
         EffectParameter* pParameter = new EffectParameter(
             this, pEffectsManager, m_parameters.size(), parameter);

src/effects/effectchainslot.cpp

@@ -3,6 +3,7 @@
 #include "effects/effectrack.h"
 #include "control/controlpotmeter.h"
 #include "control/controlpushbutton.h"
+#include "mixer/playermanager.h"
 #include "util/math.h"
 
 EffectChainSlot::EffectChainSlot(EffectRack* pRack, const QString& group,
@@ -263,15 +264,31 @@ void EffectChainSlot::registerChannel(const ChannelHandleAndGroup& handle_group)
                    << handle_group.name();
         return;
     }
+
+    double initialValue = 0.0;
+    int deckNumber;
+    if (PlayerManager::isDeckGroup(handle_group.name(), &deckNumber) &&
+        (m_iChainSlotNumber + 1) == (unsigned) deckNumber) {
+        initialValue = 1.0;
+    }
     ControlPushButton* pEnableControl = new ControlPushButton(
-            ConfigKey(m_group, QString("group_%1_enable").arg(handle_group.name())));
+            ConfigKey(m_group, QString("group_%1_enable").arg(handle_group.name())),
+            true, initialValue);
     pEnableControl->setButtonMode(ControlPushButton::POWERWINDOW);
 
     ChannelInfo* pInfo = new ChannelInfo(handle_group, pEnableControl);
     m_channelInfoByName[handle_group.name()] = pInfo;
     m_channelStatusMapper.setMapping(pEnableControl, handle_group.name());
     connect(pEnableControl, SIGNAL(valueChanged(double)),
             &m_channelStatusMapper, SLOT(map()));
+
+    if (m_pEffectChain != nullptr) {
+        if (pEnableControl->toBool()) {
+            m_pEffectChain->enableForChannel(handle_group);
+        } else {
+            m_pEffectChain->disableForChannel(handle_group);
+        }
+    }
 }
 
 void EffectChainSlot::slotEffectLoaded(EffectPointer pEffect, unsigned int slotNumber) {

src/effects/effectrack.cpp

@@ -333,6 +333,10 @@ bool QuickEffectRack::loadEffectToGroup(const QString& groupName,
         pEffectSlot->onChainSuperParameterChanged(
                 pChainSlot->getSuperParameter(), true);
     }
+
+    if (pEffect != nullptr) {
+        pEffect->setEnabled(true);
+    }
     return true;
 }
 
@@ -366,6 +370,9 @@ bool EqualizerRack::loadEffectToGroup(const QString& groupName,
     }
 
     pChain->replaceEffect(0, pEffect);
+    if (pEffect != nullptr) {
+        pEffect->setEnabled(true);
+    }
     return true;
 }
 

src/effects/effectslot.cpp

@@ -29,11 +29,10 @@ EffectSlot::EffectSlot(const QString& group,
     m_pControlNumButtonParameterSlots = new ControlObject(ConfigKey(m_group, "num_button_parameterslots"));
     m_pControlNumButtonParameterSlots->setReadOnly();
 
+    // Default to disabled to prevent accidental activation of effects
+    // at the beginning of a set.
     m_pControlEnabled = new ControlPushButton(ConfigKey(m_group, "enabled"));
     m_pControlEnabled->setButtonMode(ControlPushButton::POWERWINDOW);
-    // Default to enabled. The skin might not show these buttons.
-    m_pControlEnabled->setDefaultValue(true);
-    m_pControlEnabled->set(true);
     connect(m_pControlEnabled, SIGNAL(valueChanged(double)),
             this, SLOT(slotEnabled(double)));
 
@@ -146,9 +145,10 @@ void EffectSlot::loadEffect(EffectPointer pEffect) {
         m_pControlNumParameters->forceSet(pEffect->numKnobParameters());
         m_pControlNumButtonParameters->forceSet(pEffect->numButtonParameters());
 
-        // Enabled is a persistent property of the effect slot, not of the
-        // effect. Propagate the current setting to the effect.
-        pEffect->setEnabled(m_pControlEnabled->get() > 0.0);
+        // The enabled status persists in the EffectSlot when loading a new
+        // EffectPointer to the EffectSlot. Effects and EngineEffects default to
+        // disabled, so if this EffectSlot was enabled, enable the Effect and EngineEffect.
+        pEffect->setEnabled(m_pControlEnabled->toBool());
 
         connect(pEffect.data(), SIGNAL(enabledChanged(bool)),
                 this, SLOT(slotEffectEnabledChanged(bool)));
@@ -161,11 +161,11 @@ void EffectSlot::loadEffect(EffectPointer pEffect) {
             addEffectButtonParameterSlot();
         }
 
-        foreach (EffectParameterSlotPointer pParameter, m_parameters) {
+        for (const auto& pParameter : m_parameters) {
             pParameter->loadEffect(pEffect);
         }
 
-        foreach (EffectButtonParameterSlotPointer pParameter, m_buttonParameters) {
+        for (const auto& pParameter : m_buttonParameters) {
             pParameter->loadEffect(pEffect);
         }
 

src/engine/effects/engineeffect.cpp

@@ -6,7 +6,7 @@ EngineEffect::EngineEffect(const EffectManifest& manifest,
                            const QSet<ChannelHandleAndGroup>& registeredChannels,
                            EffectInstantiatorPointer pInstantiator)
         : m_manifest(manifest),
-          m_enableState(EffectProcessor::ENABLING),
+          m_enableState(EffectProcessor::DISABLED),
           m_parameters(manifest.parameters().size()) {
     const QList<EffectManifestParameter>& parameters = m_manifest.parameters();
     for (int i = 0; i < parameters.size(); ++i) {

src/test/controlobjecttest.cpp

@@ -49,4 +49,36 @@ TEST_F(ControlObjectTest, aliasRetrieval) {
     EXPECT_EQ(ControlObject::getControl(ckAlias), co.get());
 }
 
+TEST_F(ControlObjectTest, persistence) {
+    ConfigKey ck("[Test]", "key");
+    ControlObject* testCo1 = new ControlObject(ck, true, false, true, 3.0);
+    // Should be initialized to default value with no valid value in config
+    EXPECT_DOUBLE_EQ(3.0, testCo1->get());
+
+    testCo1->set(5.0);
+
+    // simulate restarting Mixxx
+    delete testCo1;
+    m_pConfig->save();
+    m_pConfig.clear();
+    m_pConfig = UserSettingsPointer(
+                    new UserSettings(getTestDataDir().filePath("test.cfg")));
+    ControlDoublePrivate::setUserConfig(m_pConfig);
+
+    ControlObject* testCo2 = new ControlObject(ck, true, false, true, 3.0);
+    EXPECT_DOUBLE_EQ(5.0, testCo2->get());
+
+    // simulate restarting Mixxx and
+    // a user editing the stored CO value to an invalid value
+    delete testCo2;
+    m_pConfig->set(ck, QString("NotANumber"));
+    m_pConfig->save();
+    m_pConfig.clear();
+    m_pConfig = UserSettingsPointer(
+                    new UserSettings(getTestDataDir().filePath("test.cfg")));
+    ControlDoublePrivate::setUserConfig(m_pConfig);
+    ControlObject* testCo3 = new ControlObject(ck, true, false, true, 3.0);
+    EXPECT_DOUBLE_EQ(3.0, testCo3->get());
+}
+
 }

src/test/effectslottest.cpp

@@ -56,16 +56,18 @@ TEST_F(EffectSlotTest, ControlsReflectSlotState) {
 
     // Check the controls reflect the state of their loaded effect.
     EffectPointer pEffect = m_pEffectsManager->instantiateEffect(manifest.id());
-
-    // Enabled defaults to true in both effects and the slot.
-    pEffect->setEnabled(false);
-    EXPECT_DOUBLE_EQ(1.0, ControlObject::get(ConfigKey(group, "enabled")));
+    // Enabled defaults to false in effect, slot, and engine effect.
+    EXPECT_DOUBLE_EQ(0, ControlObject::get(ConfigKey(group, "enabled")));
     EXPECT_DOUBLE_EQ(0, ControlObject::get(ConfigKey(group, "num_parameters")));
 
     pEffectSlot->loadEffect(pEffect);
-    EXPECT_LE(0, ControlObject::get(ConfigKey(group, "enabled")));
+    EXPECT_DOUBLE_EQ(0, ControlObject::get(ConfigKey(group, "enabled")));
     EXPECT_DOUBLE_EQ(1, ControlObject::get(ConfigKey(group, "num_parameters")));
+
+    pEffect->setEnabled(true);
     EXPECT_TRUE(pEffect->enabled());
+    EXPECT_DOUBLE_EQ(1, ControlObject::get(ConfigKey(group, "enabled")));
+    EXPECT_DOUBLE_EQ(1, ControlObject::get(ConfigKey(group, "num_parameters")));
 
     // loaded is read-only.
     ControlObject::set(ConfigKey(group, "loaded"), 0.0);

src/test/mixxxtest.cpp

@@ -65,6 +65,7 @@ MixxxTest::MixxxTest()
         : m_testDataDir(makeTestDir()),
           m_pConfig(new UserSettings(makeTestConfigFile(
               m_testDataDir.filePath("test.cfg")))) {
+    ControlDoublePrivate::setUserConfig(m_pConfig);
 }
 
 MixxxTest::~MixxxTest() {