Fixed divisional couplers with setter divisionals GrandOrgue#1787 (Gr…

…andOrgue#1897)
larspalo · Apr 29, 2024 · 470bb0f · 470bb0f
1 parent db03295
commit 470bb0f
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 41 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,4 @@
+- Fixed divisional couplers with setter divisionals https://github.com/GrandOrgue/grandorgue/issues/1787
 - Fixed behavior of "Detect complex MIDI setup" with Note events in different cases https://github.com/GrandOrgue/grandorgue/issues/1762
 - Fixed the order of sending midi events from an On indicator. Now they are sent after sending all events from other controls https://github.com/GrandOrgue/grandorgue/issues/1762
 # 3.14.1 (2024-04-17)

diff --git a/src/grandorgue/combinations/GODivisionalSetter.cpp b/src/grandorgue/combinations/GODivisionalSetter.cpp
@@ -131,10 +131,9 @@ GODivisionalSetter::~GODivisionalSetter() {
   m_OrganController->UnregisterSaveableObject(this);
 }
 
-void GODivisionalSetter::UpdateBankDisplay(unsigned manualN) {
+void GODivisionalSetter::UpdateBankDisplay(unsigned manualN, uint8_t bankN) {
   // 0 -> A, 19 -> T
-  m_BankLabels[manualN]->SetContent(
-    wxString::Format(wxT("%c"), 'A' + m_manualBanks[manualN]));
+  m_BankLabels[manualN]->SetContent(wxString::Format(wxT("%c"), 'A' + bankN));
 }
 
 void GODivisionalSetter::Save(GOConfigWriter &cfg) {
@@ -190,7 +189,8 @@ void GODivisionalSetter::Load(GOConfigReader &cfg) {
     divisionalNext->Init(cfg, buttonNextName, wxT("+"));
 
     // display the initial bank
-    UpdateBankDisplay(manualN);
+    m_manualBanks[manualN] = 0;
+    UpdateBankDisplay(manualN, 0);
   }
 }
 
@@ -339,12 +339,39 @@ void GODivisionalSetter::FromYaml(const YAML::Node &yamlNode) {
   }
 }
 
+template <typename F>
+void GODivisionalSetter::SwitchBank(unsigned manualN, const F &setNewBank) {
+  if (manualN < m_NManuals) {
+    uint8_t &currBank = m_manualBanks[manualN];
+    uint8_t oldBank = currBank;
+
+    setNewBank(currBank);
+
+    if (currBank != oldBank)
+      UpdateBankDisplay(manualN, currBank);
+  }
+}
+
+void GODivisionalSetter::SwitchBankToPrev(unsigned manualN) {
+  SwitchBank(manualN, [](uint8_t &currBank) {
+    if (currBank > 0)
+      currBank--;
+  });
+}
+
+void GODivisionalSetter::SwitchBankToNext(unsigned manualN) {
+  SwitchBank(manualN, [](uint8_t &currBank) {
+    if (currBank < DIVISIONAL_BANKS - 1)
+      currBank++;
+  });
+}
+
 void GODivisionalSetter::SwitchDivisionalTo(
   unsigned manualN, unsigned divisionalN) {
   if (manualN < m_NManuals && divisionalN < N_DIVISIONALS) {
+    uint8_t bankN = m_manualBanks[manualN];
     // absolute index of the divisional combination for the manual
-    unsigned divisionalIdx
-      = N_DIVISIONALS * m_manualBanks[manualN] + divisionalN;
+    unsigned divisionalIdx = N_DIVISIONALS * bankN + divisionalN;
     DivisionalMap &divMap = m_DivisionalMaps[manualN];
     // whether the combination is defined
     bool isExist = divMap.find(divisionalIdx) != divMap.end();
@@ -360,34 +387,26 @@ void GODivisionalSetter::SwitchDivisionalTo(
       divMap[divisionalIdx] = pCmb;
     }
 
-    if (pCmb)
+    if (pCmb) {
       // the combination was existing or has just been created
-      m_OrganController->PushDivisional(
-        *pCmb,
-        manualIndex,
-        manualIndex,
-        m_buttons[N_BUTTONS * manualN + divisionalN]);
-  }
-}
-
-void GODivisionalSetter::SwitchBankToPrev(unsigned manualN) {
-  if (manualN < m_NManuals) {
-    unsigned &currBank = m_manualBanks[manualN];
-
-    if (currBank > 0) {
-      currBank--;
-      UpdateBankDisplay(manualN);
-    }
-  }
-}
-
-void GODivisionalSetter::SwitchBankToNext(unsigned manualN) {
-  if (manualN < m_NManuals) {
-    unsigned &currBank = m_manualBanks[manualN];
-
-    if (currBank < DIVISIONAL_BANKS - 1) {
-      currBank++;
-      UpdateBankDisplay(manualN);
+      for (unsigned coupledManualIndex :
+           m_OrganController->GetCoupledManualsForDivisional(manualIndex)) {
+        unsigned coupledManualN = coupledManualIndex - m_FirstManualIndex;
+        DivisionalMap &coupledDivMap = m_DivisionalMaps[coupledManualN];
+
+        if (coupledDivMap.find(divisionalIdx) != coupledDivMap.end()) {
+          if (!r_SetterState.m_IsActive) {
+            // ensure that coupledManualN has the same current bank
+            SwitchBank(
+              coupledManualN, [=](uint8_t &currBank) { currBank = bankN; });
+          } // else PushDivisional does nothing for coupled manuals
+          m_OrganController->PushDivisional(
+            *coupledDivMap[divisionalIdx],
+            manualIndex,
+            coupledManualN,
+            m_buttons[N_BUTTONS * coupledManualN + divisionalN]);
+        }
+      }
     }
   }
 }

diff --git a/src/grandorgue/combinations/GODivisionalSetter.h b/src/grandorgue/combinations/GODivisionalSetter.h
@@ -1,13 +1,14 @@
 /*
  * Copyright 2006 Milan Digital Audio LLC
- * Copyright 2009-2023 GrandOrgue contributors (see AUTHORS)
+ * Copyright 2009-2024 GrandOrgue contributors (see AUTHORS)
  * License GPL-2.0 or later
  * (https://www.gnu.org/licenses/old-licenses/gpl-2.0.html).
  */
 
 #ifndef GODIVISIONALSETTER_H
 #define GODIVISIONALSETTER_H
 
+#include <cstdint>
 #include <map>
 #include <unordered_map>
 
@@ -56,12 +57,21 @@ class GODivisionalSetter : public GOElementCreator,
     unordered_map<const wxString, GOLabelControl *, wxStringHash, wxStringEqual>
       m_BankLabelsByName;
   // current bank numbers for each manual. 0 - A, 1 - B, etc
-  std::vector<unsigned> m_manualBanks;
+  std::vector<uint8_t> m_manualBanks;
   // All combinations for all nanuals
   std::vector<DivisionalMap> m_DivisionalMaps;
 
   // update the current bank on the m_BankLabels[manualN]
-  void UpdateBankDisplay(unsigned manualN);
+  void UpdateBankDisplay(unsigned manualN, uint8_t bankN);
+
+  /**
+   * Switches the current bank of the manual.
+   * @param manualN manual number in the setter
+   * @param setNewBank a function with (unsigned &currBank) parameter. It may
+   *   change currBank
+   */
+  template <typename F> void SwitchBank(unsigned manualN, const F &setNewBank);
+
   // delete all combinations from m_DivisionalMaps
   void ClearCombinations();
 
@@ -127,18 +137,18 @@ class GODivisionalSetter : public GOElementCreator,
     return m_BankLabelsByName[name];
   }
 
-  // Activates the combination for the manual as the button divisionalN is
-  // pressed. Current bank is taken into account.
-  // manualN, and divisionalN start with 0
-  void SwitchDivisionalTo(unsigned manualN, unsigned divisionalN);
-
   // Activate the previous bank for the manual if it exists.
   // manualN starts with 0
   void SwitchBankToPrev(unsigned manualN);
 
   // Activate the next bank for the manual if it exists
   // manualN starts with 0
   void SwitchBankToNext(unsigned manualN);
+
+  // Activates the combination for the manual as the button divisionalN is
+  // pressed. Current bank is taken into account.
+  // manualN, and divisionalN start with 0
+  void SwitchDivisionalTo(unsigned manualN, unsigned divisionalN);
 };
 
 #endif /* GODIVISIONALSETTER_H */