diff --git a/res/skins/LateNight/effect_unit.xml b/res/skins/LateNight/effect_unit.xml index d8527fe00ad1..1d1ff1b8e1f7 100644 --- a/res/skins/LateNight/effect_unit.xml +++ b/res/skins/LateNight/effect_unit.xml @@ -93,11 +93,8 @@ ON - ,enabled - LeftButton - - - ,enabled + + [EffectRack_EffectUnit_Effect1],enabled diff --git a/src/control/control.cpp b/src/control/control.cpp index 8c7329eb79ba..e1791c5db385 100644 --- a/src/control/control.cpp +++ b/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::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::getControl( if (pCreatorCO) { pControl = QSharedPointer( 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); diff --git a/src/control/control.h b/src/control/control.h index c76c16dbac19..edc876f5eac2 100644 --- a/src/control/control.h +++ b/src/control/control.h @@ -36,7 +36,7 @@ class ControlDoublePrivate : public QObject { static QSharedPointer 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 >* 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; diff --git a/src/control/controlobject.cpp b/src/control/controlobject.cpp index e61146242a88..a4a29f5cddca 100644 --- a/src/control/controlobject.cpp +++ b/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. diff --git a/src/control/controlobject.h b/src/control/controlobject.h index 20d6c92a2c2c..f5e6396fc173 100644 --- a/src/control/controlobject.h +++ b/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; } diff --git a/src/control/controlpushbutton.cpp b/src/control/controlpushbutton.cpp index 9c160b7ca81e..6fe0486d56b8 100644 --- a/src/control/controlpushbutton.cpp +++ b/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) { diff --git a/src/control/controlpushbutton.h b/src/control/controlpushbutton.h index b4483c994e6e..1666bb1fdb97 100644 --- a/src/control/controlpushbutton.h +++ b/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 { diff --git a/src/effects/effect.cpp b/src/effects/effect.cpp index dfab7f107a40..0eef4287cdb8 100644 --- a/src/effects/effect.cpp +++ b/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); diff --git a/src/effects/effectchainslot.cpp b/src/effects/effectchainslot.cpp index f0ab27de6d6c..5560ca5e10c9 100644 --- a/src/effects/effectchainslot.cpp +++ b/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,8 +264,16 @@ 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); @@ -272,6 +281,14 @@ void EffectChainSlot::registerChannel(const ChannelHandleAndGroup& handle_group) 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) { diff --git a/src/effects/effectrack.cpp b/src/effects/effectrack.cpp index ec3a0637d105..72b77d2cd585 100644 --- a/src/effects/effectrack.cpp +++ b/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; } diff --git a/src/effects/effectslot.cpp b/src/effects/effectslot.cpp index 43c861c2eefd..c0116ecc4e29 100644 --- a/src/effects/effectslot.cpp +++ b/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); } diff --git a/src/engine/effects/engineeffect.cpp b/src/engine/effects/engineeffect.cpp index a73cdc056afa..f0658801621e 100644 --- a/src/engine/effects/engineeffect.cpp +++ b/src/engine/effects/engineeffect.cpp @@ -6,7 +6,7 @@ EngineEffect::EngineEffect(const EffectManifest& manifest, const QSet& registeredChannels, EffectInstantiatorPointer pInstantiator) : m_manifest(manifest), - m_enableState(EffectProcessor::ENABLING), + m_enableState(EffectProcessor::DISABLED), m_parameters(manifest.parameters().size()) { const QList& parameters = m_manifest.parameters(); for (int i = 0; i < parameters.size(); ++i) { diff --git a/src/test/controlobjecttest.cpp b/src/test/controlobjecttest.cpp index ab55c48e187a..0945b1e72c85 100644 --- a/src/test/controlobjecttest.cpp +++ b/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()); +} + } diff --git a/src/test/effectslottest.cpp b/src/test/effectslottest.cpp index 2079f3f47be6..1346e32ad433 100644 --- a/src/test/effectslottest.cpp +++ b/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); diff --git a/src/test/mixxxtest.cpp b/src/test/mixxxtest.cpp index 3ee1df9166fa..eda31fe31153 100644 --- a/src/test/mixxxtest.cpp +++ b/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() {