Merge pull request #10859 from ronso0/effect-chain-clear-empty-preset

Effects: allow clearing chains with empty '---' preset

6 files changed, 104 insertions, 9 deletions

diff --git a/src/effects/effectchain.cpp b/src/effects/effectchain.cpp
index 378746518d..69124a40c6 100644
--- a/src/effects/effectchain.cpp
+++ b/src/effects/effectchain.cpp
@@ -356,13 +356,13 @@ void EffectChain::setControlLoadedPresetIndex(uint index) {
 
 void EffectChain::slotControlNextChainPreset(double value) {
     if (value > 0) {
-        loadChainPreset(presetAtIndex(presetIndex() + 1));
+        slotControlChainPresetSelector(1);
     }
 }
 
 void EffectChain::slotControlPrevChainPreset(double value) {
     if (value > 0) {
-        loadChainPreset(presetAtIndex(presetIndex() - 1));
+        slotControlChainPresetSelector(-1);
     }
 }
 
diff --git a/src/effects/effectsmanager.cpp b/src/effects/effectsmanager.cpp
index 23c20cf83b..5a373de674 100644
--- a/src/effects/effectsmanager.cpp
+++ b/src/effects/effectsmanager.cpp
@@ -177,6 +177,8 @@ void EffectsManager::readEffectsXml() {
     }
     file.close();
 
+    // Note: QuickEffect chains are created only for existing main decks
+    // thus only for those the configured presets are requested
     QStringList deckStrings;
     for (auto it = m_quickEffectChains.begin(); it != m_quickEffectChains.end(); it++) {
         deckStrings << it.key();
diff --git a/src/effects/effectsmanager.h b/src/effects/effectsmanager.h
index d6efe32f8e..d11275b56a 100644
--- a/src/effects/effectsmanager.h
+++ b/src/effects/effectsmanager.h
@@ -88,6 +88,7 @@ class EffectsManager {
 
     QList<StandardEffectChainPointer> m_standardEffectChains;
     OutputEffectChainPointer m_outputEffectChain;
+    // These two store <deck group, effect chain pointer>
     QHash<QString, EqualizerEffectChainPointer> m_equalizerEffectChains;
     QHash<QString, QuickEffectChainPointer> m_quickEffectChains;
 
diff --git a/src/effects/presets/effectchainpreset.cpp b/src/effects/presets/effectchainpreset.cpp
index 69eb59a2be..0d3dda3250 100644
--- a/src/effects/presets/effectchainpreset.cpp
+++ b/src/effects/presets/effectchainpreset.cpp
@@ -7,7 +7,8 @@
 EffectChainPreset::EffectChainPreset()
         : m_name(kNoEffectString),
           m_mixMode(EffectChainMixMode::DrySlashWet),
-          m_dSuper(0.0) {
+          m_dSuper(0.0),
+          m_readOnly(false) {
 }
 
 EffectChainPreset::EffectChainPreset(const QDomElement& chainElement) {
@@ -28,6 +29,8 @@ EffectChainPreset::EffectChainPreset(const QDomElement& chainElement) {
 
     m_dSuper = XmlParse::selectNodeDouble(chainElement, EffectXml::kChainSuperParameter);
 
+    m_readOnly = false;
+
     QDomElement effectsElement = XmlParse::selectElement(chainElement, EffectXml::kEffectsRoot);
     QDomNodeList effectList = effectsElement.childNodes();
 
diff --git a/src/effects/presets/effectchainpreset.h b/src/effects/presets/effectchainpreset.h
index 38c3095501..890e380923 100644
--- a/src/effects/presets/effectchainpreset.h
+++ b/src/effects/presets/effectchainpreset.h
@@ -40,6 +40,12 @@ class EffectChainPreset {
     double superKnob() const {
         return m_dSuper;
     }
+    void setReadOnly() {
+        m_readOnly = true;
+    }
+    bool isReadOnly() const {
+        return m_readOnly;
+    }
 
     const QList<EffectPresetPointer>& effectPresets() const {
         return m_effectPresets;
@@ -50,4 +56,5 @@ class EffectChainPreset {
     EffectChainMixMode::Type m_mixMode;
     double m_dSuper;
     QList<EffectPresetPointer> m_effectPresets;
+    bool m_readOnly;
 };
diff --git a/src/effects/presets/effectchainpresetmanager.cpp b/src/effects/presets/effectchainpresetmanager.cpp
index ff637695d0..6f132bbeef 100644
--- a/src/effects/presets/effectchainpresetmanager.cpp
+++ b/src/effects/presets/effectchainpresetmanager.cpp
@@ -123,9 +123,15 @@ void EffectChainPresetManager::importPreset() {
         EffectChainPresetPointer pPreset(
                 new EffectChainPreset(doc.documentElement()));
         if (!pPreset->isEmpty() && !pPreset->name().isEmpty()) {
+            // Don't allow '---' because that's the name of the internal empty preset
+            if (pPreset->name() == kNoEffectString) {
+                pPreset->setName(pPreset->name() +
+                        QStringLiteral(" (") + tr("imported") + QStringLiteral(")"));
+            }
+
             while (m_effectChainPresets.contains(pPreset->name())) {
                 pPreset->setName(pPreset->name() +
-                        QLatin1String(" (") + tr("duplicate") + QLatin1String(")"));
+                        QStringLiteral(" (") + tr("duplicate") + QStringLiteral(")"));
             }
 
             // An imported chain preset might contain an LV2 plugin that the user does not
@@ -227,10 +233,22 @@ void EffectChainPresetManager::renamePreset(const QString& oldName) {
     VERIFY_OR_DEBUG_ASSERT(m_effectChainPresets.contains(oldName)) {
         return;
     }
+    if (m_effectChainPresets.value(oldName)->isReadOnly()) {
+        QMessageBox msgBox;
+        msgBox.setText(tr("Effect chain preset can not be renamed"));
+        msgBox.setInformativeText(
+                tr("Effect chain preset \"%1\" is read-only and can not be renamed.")
+                        .arg(oldName));
+        msgBox.setIcon(QMessageBox::Warning);
+        msgBox.exec();
+        return;
+    }
 
     QString newName;
     QString errorText;
-    while (newName.isEmpty() || m_effectChainPresets.contains(newName)) {
+    // Don't allow '---' as new name either
+    while (newName.isEmpty() || m_effectChainPresets.contains(newName) ||
+            newName == kNoEffectString) {
         bool okay = false;
         newName = QInputDialog::getText(nullptr,
                 tr("Rename effect chain preset"),
@@ -246,6 +264,8 @@ void EffectChainPresetManager::renamePreset(const QString& oldName) {
 
         if (newName.isEmpty()) {
             errorText = tr("Effect chain preset name must not be empty.") + QStringLiteral("\n");
+        } else if (newName == kNoEffectString) {
+            errorText = tr("Invalid name \"%1\"").arg(newName) + QStringLiteral("\n");
         } else if (m_effectChainPresets.contains(newName)) {
             errorText =
                     tr("An effect chain preset named \"%1\" already exists.")
@@ -301,6 +321,16 @@ bool EffectChainPresetManager::deletePreset(const QString& chainPresetName) {
     VERIFY_OR_DEBUG_ASSERT(m_effectChainPresets.contains(chainPresetName)) {
         return false;
     }
+    if (m_effectChainPresets.value(chainPresetName)->isReadOnly()) {
+        QMessageBox msgBox;
+        msgBox.setText(tr("Effect chain preset can not be deleted"));
+        msgBox.setInformativeText(
+                tr("Effect chain preset \"%1\" is read-only and can not be deleted.")
+                        .arg(chainPresetName));
+        msgBox.setIcon(QMessageBox::Warning);
+        msgBox.exec();
+        return false;
+    }
     auto pressedButton = QMessageBox::question(nullptr,
             tr("Remove effect chain preset"),
             tr("Are you sure you want to delete the effect chain preset "
@@ -360,6 +390,18 @@ void EffectChainPresetManager::setQuickEffectPresetOrder(
                 m_effectChainPresets.value(chainPresetName));
     }
 
+    // Ensure empty '---' preset is the first list item
+    const auto& pEmptyPreset = m_effectChainPresets.value(kNoEffectString);
+    VERIFY_OR_DEBUG_ASSERT(pEmptyPreset) {
+        return;
+    }
+    int index = m_quickEffectChainPresetsSorted.indexOf(pEmptyPreset);
+    if (index == -1) { // not in list, re-add it
+        m_quickEffectChainPresetsSorted.prepend(pEmptyPreset);
+    } else if (index != 0) { // not first item, move to top
+        m_quickEffectChainPresetsSorted.move(index, 0);
+    }
+
     emit quickEffectChainPresetListUpdated();
 }
 
@@ -373,13 +415,19 @@ void EffectChainPresetManager::savePresetAndReload(EffectChainPointer pChainSlot
 bool EffectChainPresetManager::savePreset(EffectChainPresetPointer pPreset) {
     QString name;
     QString errorText;
-    while (name.isEmpty() || m_effectChainPresets.contains(name)) {
+    // Don't allow '---' because that's the name of the internal empty preset
+    // Clear initial name to avoid confusion.
+    QString presetName;
+    if (pPreset->name() != kNoEffectString) {
+        presetName = pPreset->name();
+    }
+    while (name.isEmpty() || m_effectChainPresets.contains(name) || name == kNoEffectString) {
         bool okay = false;
         name = QInputDialog::getText(nullptr,
                 tr("Save preset for effect chain"),
                 errorText + "\n" + tr("Name for new effect chain preset:"),
                 QLineEdit::Normal,
-                pPreset->name(),
+                presetName,
                 &okay)
                        .trimmed();
         if (!okay) {
@@ -388,6 +436,8 @@ bool EffectChainPresetManager::savePreset(EffectChainPresetPointer pPreset) {
 
         if (name.isEmpty()) {
             errorText = tr("Effect chain preset name must not be empty.") + QStringLiteral("\n");
+        } else if (name == kNoEffectString) {
+            errorText = tr("Invalid name \"%1\"").arg(name) + QStringLiteral("\n");
         } else if (m_effectChainPresets.contains(name)) {
             errorText =
                     tr("An effect chain preset named \"%1\" already exists.")
@@ -447,6 +497,11 @@ void EffectChainPresetManager::importUserPresets() {
         EffectChainPresetPointer pEffectChainPreset = loadPresetFromFile(
                 savedPresetsPath + kFolderDelimiter + filePath);
         if (pEffectChainPreset && !pEffectChainPreset->isEmpty()) {
+            // Don't allow '---' because that's the name of the internal empty preset
+            if (pEffectChainPreset->name() == kNoEffectString) {
+                pEffectChainPreset->setName(pEffectChainPreset->name() +
+                        QLatin1String(" (") + tr("imported") + QLatin1String(")"));
+            }
             m_effectChainPresets.insert(
                     pEffectChainPreset->name(), pEffectChainPreset);
         }
@@ -542,6 +597,11 @@ void EffectChainPresetManager::resetToDefaults() {
 }
 
 bool EffectChainPresetManager::savePresetXml(EffectChainPresetPointer pPreset) {
+    // Don't store the empty '---' preset in effects/chains.
+    // Shouldn't be possible via GUI anyway because the 'Update Preset' button
+    // is not shown in WEffectChainPresetButton if this preset is loaded.
+    DEBUG_ASSERT(pPreset->name() != kNoEffectString);
+
     QString path(m_pConfig->getSettingsPath() + kEffectChainPresetDirectory);
     QDir effectsChainsDir(path);
     if (!effectsChainsDir.exists()) {
@@ -591,7 +651,7 @@ EffectsXmlData EffectChainPresetManager::readEffectsXml(
         quickEffectPresets.insert(deckString, defaultQuickEffectChainPreset);
     }
 
-    // Reload state of standard chains
+    // Read state of standard chains
     QDomElement root = doc.documentElement();
     QDomElement rackElement = XmlParse::selectElement(root, EffectXml::kRack);
     QDomElement chainsElement =
@@ -668,10 +728,26 @@ EffectsXmlData EffectChainPresetManager::readEffectsXml(
 
     prependRemainingPresetsToLists();
 
+    // Create the empty '---' chain preset on each start.
+    // Its sole purpose is to eject the current QuickEffect chain presets via GUI.
+    // It will not be saved to effects/chains nor written to effects.xml
+    // except as identifier for QuickEffect chains.
+    // It will not be visible in the effects preferences.
+    EffectManifestPointer pEmptyChainManifest(new EffectManifest());
+    pEmptyChainManifest->setName(kNoEffectString);
+    // Required for the QuickEffect selector in DlgPrefEQ
+    pEmptyChainManifest->setShortName(kNoEffectString);
+    auto pEmptyChainPreset =
+            EffectChainPresetPointer(new EffectChainPreset(pEmptyChainManifest));
+    pEmptyChainPreset->setReadOnly();
+
+    m_effectChainPresets.insert(pEmptyChainPreset->name(), pEmptyChainPreset);
+    m_quickEffectChainPresetsSorted.prepend(pEmptyChainPreset);
+
     emit effectChainPresetListUpdated();
     emit quickEffectChainPresetListUpdated();
 
-    // Reload presets that were loaded into QuickEffects on last shutdown
+    // Read names of presets that were loaded into QuickEffects on last shutdown
     QDomElement quickEffectPresetsElement =
             XmlParse::selectElement(root, EffectXml::kQuickEffectChainPresets);
     QDomNodeList quickEffectNodeList =
@@ -683,6 +759,8 @@ EffectsXmlData EffectChainPresetManager::readEffectsXml(
             QString deckGroup = presetNameElement.attribute(QStringLiteral("group"));
             auto pPreset = m_effectChainPresets.value(presetNameElement.text());
             if (pPreset != nullptr) {
+                // Replace defaultQuickEffectChainPreset with pPreset
+                // for this deck group
                 quickEffectPresets.insert(deckGroup, pPreset);
             }
         }
@@ -717,6 +795,10 @@ void EffectChainPresetManager::saveEffectsXml(QDomDocument* pDoc, const EffectsX
     QDomElement quickEffectChainPresetListElement =
             pDoc->createElement(EffectXml::kQuickEffectList);
     for (const auto& pPreset : std::as_const(m_quickEffectChainPresetsSorted)) {
+        // Don't store the empty '---' in the QuickEffect preset list
+        if (pPreset->name() == kNoEffectString) {
+            continue;
+        }
         XmlParse::addElement(*pDoc,
                 quickEffectChainPresetListElement,
                 EffectXml::kChainPresetName,