Better approach for forwarding parameter stability (#346)

* New approach for forwarding parameter stability

* More work

* More comprehensive tests

* Trying to fix test failures

* Apply clang-format

* More test tweaking

* CI changes (for now)

* More logging for test failures

* Smarter root directory determination for tests

* Resetting CI

* Don't save forwarding param slots for processors when saving presets

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

.github/workflows/cmake.yml

@@ -75,7 +75,7 @@ jobs:
       run: cmake --build build --config Release --parallel 3 --target BYOD_Standalone BYOD_VST3 BYOD_CLAP BYOD_headless
 
     - name: Unit Tests
-      if: runner.os == 'Linux'
+      if: runner.os == 'Linux' || runner.os == 'MacOS'
       run: build/BYOD --unit-tests --all
 
     - name: Validate

CMakeLists.txt

@@ -3,7 +3,7 @@ set(CMAKE_OSX_DEPLOYMENT_TARGET "10.12" CACHE STRING "Minimum OS X deployment ta
 if(WIN32)
     set(CMAKE_SYSTEM_VERSION 7.1 CACHE STRING INTERNAL FORCE) # Windows SDK for Windows 7 and up
 endif()
-project(BYOD VERSION 1.2.1)
+project(BYOD VERSION 1.2.2)
 set(CMAKE_CXX_STANDARD 20)
 
 # Useful for testing with address sanitizer:

src/BYOD.cpp

@@ -131,7 +131,7 @@ void BYOD::getStateInformation (MemoryBlock& destData)
 
 void BYOD::setStateInformation (const void* data, int sizeInBytes)
 {
-    stateManager->loadState (getXmlFromBinary (data, sizeInBytes).get());
+    stateManager->loadState (getXmlFromBinary (data, sizeInBytes).get(), *paramForwarder);
 
     if (wrapperType == WrapperType::wrapperType_AudioUnitv3)
     {

src/headless/CMakeLists.txt

@@ -57,4 +57,9 @@ target_link_libraries(BYOD_headless PUBLIC
     BYOD
 )
 
+target_compile_definitions(BYOD_headless
+    PRIVATE
+        BYOD_ROOT_DIR="${CMAKE_SOURCE_DIR}"
+)
+
 set_target_properties(BYOD_headless PROPERTIES CXX_VISIBILITY_PRESET hidden)

src/headless/tests/AmpIRsSaveLoadTest.cpp

@@ -36,9 +36,7 @@ class AmpIRsSaveLoadTest : public UnitTest
 
         const auto testIRFile = []
         {
-            auto rootDir = File::getSpecialLocation (File::currentExecutableFile);
-            while (rootDir.getFileName() != "BYOD")
-                rootDir = rootDir.getParentDirectory();
+            const auto rootDir = juce::File { BYOD_ROOT_DIR };
             return rootDir.getChildFile ("src/headless/tests/test_ir.wav");
         }();
 

src/headless/tests/ForwardingParamStabilityTest.cpp

@@ -2,6 +2,70 @@
 #include "processors/chain/ProcessorChainActionHelper.h"
 #include "state/ParamForwardManager.h"
 
+#include "processors/drive/BlondeDrive.h"
+#include "processors/drive/RangeBooster.h"
+#include "processors/drive/Warp.h"
+#include "processors/drive/big_muff/BigMuffDrive.h"
+#include "processors/drive/diode_circuits/DiodeClipper.h"
+#include "processors/drive/diode_circuits/DiodeRectifier.h"
+#include "processors/drive/flapjack/Flapjack.h"
+#include "processors/drive/zen_drive/ZenDrive.h"
+#include "processors/modulation/Chorus.h"
+#include "processors/modulation/Rotary.h"
+#include "processors/modulation/Tremolo.h"
+#include "processors/modulation/phaser/Phaser4.h"
+#include "processors/modulation/phaser/Phaser8.h"
+#include "processors/modulation/scanner_vibrato/ScannerVibrato.h"
+#include "processors/modulation/uni_vibe/UniVibe.h"
+#include "processors/other/Compressor.h"
+#include "processors/other/Delay.h"
+#include "processors/other/EnvelopeFilter.h"
+#include "processors/other/spring_reverb/SpringReverbProcessor.h"
+#include "processors/tone/BassCleaner.h"
+#include "processors/tone/BigMuffTone.h"
+#include "processors/tone/BlondeTone.h"
+#include "processors/tone/GraphicEQ.h"
+#include "processors/tone/HighCut.h"
+#include "processors/tone/LofiIrs.h"
+#include "processors/tone/ladder_filter/LadderFilterProcessor.h"
+#include "processors/tone/tube_screamer_tone/TubeScreamerTone.h"
+
+template <typename ProcType>
+static std::unique_ptr<BaseProcessor> processorFactory (UndoManager* um)
+{
+    return std::make_unique<ProcType> (um);
+}
+
+ProcessorStore::StoreMap minimalStore = {
+    { "Blonde Drive", { &processorFactory<BlondeDrive>, { ProcessorType::Drive, 1, 1 } } },
+    { "Diode Clipper", { &processorFactory<DiodeClipper>, { ProcessorType::Drive, 1, 1 } } },
+    { "Diode Rectifier", { &processorFactory<DiodeRectifier>, { ProcessorType::Drive, 1, 1 } } },
+    { "Flapjack", { &processorFactory<Flapjack>, { ProcessorType::Drive, 1, 1 } } },
+    { "Muff Drive", { &processorFactory<BigMuffDrive>, { ProcessorType::Drive, 1, 1 } } },
+    { "Range Booster", { &processorFactory<RangeBooster>, { ProcessorType::Drive, 1, 1 } } },
+    { "Warp", { &processorFactory<Warp>, { ProcessorType::Drive, 1, 1 } } },
+    { "Yen Drive", { &processorFactory<ZenDrive>, { ProcessorType::Drive, 1, 1 } } },
+    { "Bass Cleaner", { &processorFactory<BassCleaner>, { ProcessorType::Tone, 1, 1 } } },
+    { "Blonde Tone", { &processorFactory<BlondeTone>, { ProcessorType::Tone, 1, 1 } } },
+    { "Graphic EQ", { &processorFactory<GraphicEQ>, { ProcessorType::Tone, 1, 1 } } },
+    { "High Cut", { &processorFactory<HighCut>, { ProcessorType::Tone, 1, 1 } } },
+    { "LoFi IRs", { &processorFactory<LofiIrs>, { ProcessorType::Tone, 1, 1 } } },
+    { "Muff Tone", { &processorFactory<BigMuffTone>, { ProcessorType::Tone, 1, 1 } } },
+    { "TS-Tone", { &processorFactory<TubeScreamerTone>, { ProcessorType::Tone, 1, 1 } } },
+    { "Ladder Filter", { &processorFactory<LadderFilterProcessor>, { ProcessorType::Tone, 1, 1 } } },
+    { "Chorus", { &processorFactory<Chorus>, { ProcessorType::Modulation, Chorus::numInputs, Chorus::numOutputs } } },
+    { "Phaser4", { &processorFactory<Phaser4>, { ProcessorType::Modulation, Phaser4::numInputs, Phaser4::numOutputs } } },
+    { "Phaser8", { &processorFactory<Phaser8>, { ProcessorType::Modulation, Phaser8::numInputs, Phaser8::numOutputs } } },
+    { "Rotary", { &processorFactory<Rotary>, { ProcessorType::Modulation, Rotary::numInputs, Rotary::numOutputs } } },
+    { "Scanner Vibrato", { &processorFactory<ScannerVibrato>, { ProcessorType::Modulation, ScannerVibrato::numInputs, ScannerVibrato::numOutputs } } },
+    { "Solo-Vibe", { &processorFactory<UniVibe>, { ProcessorType::Modulation, UniVibe::numInputs, UniVibe::numOutputs } } },
+    { "Tremolo", { &processorFactory<Tremolo>, { ProcessorType::Modulation, Tremolo::numInputs, Tremolo::numOutputs } } },
+    { "DC Blocker", { &processorFactory<DCBlocker>, { ProcessorType::Utility, 1, 1 } } },
+    { "Compressor", { &processorFactory<Compressor>, { ProcessorType::Other, Compressor::numInputs, Compressor::numOutputs } } },
+    { "Delay", { &processorFactory<DelayModule>, { ProcessorType::Other, 1, 1 } } },
+    { "Envelope Filter", { &processorFactory<EnvelopeFilter>, { ProcessorType::Other, EnvelopeFilter::numInputs, EnvelopeFilter::numOutputs } } },
+};
+
 class ForwardingParamStabilityTest : public UnitTest
 {
 public:
@@ -13,7 +77,7 @@ class ForwardingParamStabilityTest : public UnitTest
     bool addProcessor (ProcessorChain& chain, UndoManager* um)
     {
         // get random processor
-        auto& storeMap = ProcessorStore::getStoreMap();
+        auto& storeMap = minimalStore;
         auto storeIter = storeMap.begin();
         int storeIndex = rand.nextInt ((int) storeMap.size());
         std::advance (storeIter, storeIndex);
@@ -53,13 +117,19 @@ class ForwardingParamStabilityTest : public UnitTest
         };
 
         std::vector<Action> actions {
+            { "Add Processor", [&]
+              { return addProcessor (procChain, undoManager); } },
             { "Add Processor", [&]
               { return addProcessor (procChain, undoManager); } },
             { "Remove Processor", [&]
               { return removeProcessor (procChain); } },
         };
 
-        for (int count = 0; count < 100;)
+#if JUCE_DEBUG
+        for (int count = 0; count < 9;)
+#else
+        for (int count = 0; count < 19;)
+#endif
         {
             auto& action = actions[rand.nextInt ((int) actions.size())];
             if (action.action())
@@ -93,23 +163,151 @@ class ForwardingParamStabilityTest : public UnitTest
         for (auto [idx, param] : chowdsp::enumerate (forwardingParams))
         {
             if (auto* forwardedParam = param->getParam())
+            {
                 expectEquals (forwardedParam->getName (1024).toStdString(),
                               paramNames[idx],
                               "Parameter name mismatch");
+            }
             else
+            {
                 expectEquals (std::string {},
                               paramNames[idx],
                               "Parameter name mismatch");
+            }
+        }
+    }
+
+    void testLegacyState (int i)
+    {
+        /*
+        BYOD plugin;
+        auto* undoManager = plugin.getVTS().undoManager;
+        auto& procChain = plugin.getProcChain();
+
+        struct Action
+        {
+            String name;
+            std::function<bool()> action;
+        };
+
+        std::vector<Action> actions {
+            { "Add Processor", [&]
+              { return addProcessor (procChain, undoManager); } },
+        };
+
+        for (int count = 0; count < 9;)
+        {
+            auto& action = actions[rand.nextInt ((int) actions.size())];
+            if (action.action())
+            {
+                int timeUntilNextAction = rand.nextInt ({ 5, 500 });
+                juce::MessageManager::getInstance()->runDispatchLoopUntil (timeUntilNextAction);
+                count++;
+            }
+        }
+
+        auto& forwardingParams = plugin.getParamForwarder().getForwardedParameters();
+        juce::StringArray paramNames {};
+        for (auto [idx, param] : chowdsp::enumerate (forwardingParams))
+        {
+            if (auto* forwardedParam = param->getParam())
+                paramNames.add (forwardedParam->getName (1024).toStdString());
+            else
+                paramNames.add ("EMPTY");
+        }
+
+        juce::MemoryBlock state;
+        plugin.getStateInformation (state);
+
+        const auto file = juce::File { "test_" + juce::String { i } };
+        file.withFileExtension ("txt").replaceWithText (paramNames.joinIntoString (","));
+
+        plugin.getXmlFromBinary (state.getData(), state.getSize())->writeToFile (file.withFileExtension ("xml"), "");
+        */
+
+        const auto [stateFile, paramsFile] = [i]
+        {
+            const auto rootDir = juce::File { BYOD_ROOT_DIR };
+            const auto fileDir = rootDir.getChildFile ("src/headless/tests/OldParamForwardStates");
+            return std::make_pair (fileDir.getChildFile ("test_" + juce::String { i } + ".xml"),
+                                   fileDir.getChildFile ("test_" + juce::String { i } + ".txt"));
+        }();
+
+        juce::Logger::writeToLog ("Loading legacy state from file: " + stateFile.getFullPathName());
+        jassert (stateFile.existsAsFile());
+        const auto stateXml = XmlDocument::parse (stateFile);
+        jassert (stateXml != nullptr);
+        BYOD plugin;
+        juce::MemoryBlock state;
+        plugin.copyXmlToBinary (*stateXml, state);
+        plugin.setStateInformation (state.getData(), state.getSize());
+
+        juce::StringArray paramNames;
+        paramNames.addTokens (paramsFile.loadFileAsString(), ",", {});
+        jassert (paramNames.size() == 500);
+        for (auto& name : paramNames)
+            name = name.trimCharactersAtStart (" ");
+
+        auto& forwardingParams = plugin.getParamForwarder().getForwardedParameters();
+        for (auto [idx, param] : chowdsp::enumerate (forwardingParams))
+        {
+            if (auto* forwardedParam = param->getParam())
+            {
+                expectEquals (forwardedParam->getName (1024),
+                              paramNames[idx],
+                              "Parameter name mismatch");
+            }
+            else
+            {
+                expectEquals (juce::String { "EMPTY" },
+                              paramNames[idx],
+                              "Parameter name mismatch");
+            }
         }
+
+        for (int i = 0; i < 20; ++i)
+            removeProcessor (plugin.getProcChain());
+        jassert (plugin.getProcChain().getProcessors().size() == 0);
+
+        addProcessor (plugin.getProcChain(), &plugin.getUndoManager());
+        auto* ip = reinterpret_cast<juce::RangedAudioParameter*> (plugin.getProcChain().getProcessors()[0]->getParameters()[0]);
+
+        const auto& paramForwarder = plugin.getParamForwarder();
+        expect (paramForwarder.getForwardedParameterFromInternal (*ip) == paramForwarder.getForwardedParameters()[0],
+                "Newly added parameter is not in the first parameter slot!");
     }
 
     void runTest() override
     {
-        rand = Random { 1234 }; // getRandom();
+        beginTest ("Check Max Parameter Count");
+        runTestForAllProcessors (
+            this,
+            [this] (BaseProcessor* proc)
+            {
+                expectLessThan (proc->getParameters().size(),
+                                ParamForwardManager::maxParameterCount,
+                                "");
+            },
+            {},
+            false);
 
         beginTest ("Forwarding Parameter Stability Test");
-        const auto [paramNames, state] = runPlugin();
-        testPlugin (paramNames, state);
+        rand = Random { 1245 };
+#if JUCE_DEBUG
+        for (int i = 0; i < 1; ++i)
+#else
+        for (int i = 0; i < 10; ++i)
+#endif
+        {
+            const auto [paramNames, state] = runPlugin();
+            testPlugin (paramNames, state);
+        }
+
+        beginTest ("Legacy Forwarding Parameter Compatibility Test");
+        for (int i = 0; i < 5; ++i)
+        {
+            testLegacyState (i);
+        }
     }
 
     Random rand;

src/headless/tests/OldParamForwardStates/test_0.txt

@@ -0,0 +1 @@
+On/Off,Cutoff,On/Off,IR,Gain,Mix,On/Off,Drive,Presence,Low Cut,Level,Mode,On/Off,Range,Boost,High Quality,On/Off,Tone,On/Off,Resonance,Freq.,Speed,Sensitivity,Freq. Mod,Type,Direct Control,On/Off,Rate,Depth,Feedback,Mix,Delay Type,On/Off,Delay Time,Cutoff,Feedback,Mix,Delay Rhythm,Tempo Sync,Delay Type,Ping-Pong,On/Off,Rate,Depth,Feedback,Mix,Delay Type,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY,EMPTY