Fixed disengaging manually enabled stops when a crescendo was in the Override=Off mode https://github.com/GrandOrgue/grandorgue/issues/1935

CHANGELOG.md

@@ -1,3 +1,4 @@
+- Fixed disengaging manually enabled stops when a crescendo was in the Override=Off mode https://github.com/GrandOrgue/grandorgue/issues/1935
 - Fixed non bringing a dialog windows on top when it had been already open https://github.com/GrandOrgue/grandorgue/issues/1961
 - Fixed combination button lighting when a crescendo was in the Override=Off mode https://github.com/GrandOrgue/grandorgue/issues/1935
 # 3.15.1 (2024-09-03)

src/grandorgue/combinations/GOSetter.cpp

@@ -351,6 +351,18 @@ GOSetter::GOSetter(GOOrganController *organController)
 GOSetter::~GOSetter() {}
 
 static const wxString WX_OVERRIDE_MODE = wxT("OverrideMode");
+static const wxString WX_EMPTY_STRING = wxEmptyString;
+
+static wxString crescendo_cmb_state_name(
+  bool isOverride, uint8_t crescendoIdx) {
+  return isOverride ? WX_EMPTY_STRING
+                    : wxString::Format("crescendo-%c", 'A' + crescendoIdx);
+}
+
+wxString GOSetter::GetCrescendoCmbStateName(uint8_t crescendoIdx) const {
+  return crescendo_cmb_state_name(
+    m_CrescendoOverrideMode[crescendoIdx], crescendoIdx);
+}
 
 void GOSetter::Load(GOConfigReader &cfg) {
   m_OrganController->RegisterSaveableObject(this);
@@ -382,14 +394,20 @@ void GOSetter::Load(GOConfigReader &cfg) {
     m_CrescendoOverrideMode[i] = cfg.ReadBoolean(
       CMBSetting, buffer, WX_OVERRIDE_MODE, false, defaultAddMode);
   }
-  for (unsigned i = 0; i < N_CRESCENDOS * CRESCENDO_STEPS; i++) {
-    m_crescendo.push_back(new GOGeneralCombination(*m_OrganController, true));
-    m_CrescendoExtraSets.emplace_back();
-    buffer.Printf(
-      wxT("SetterCrescendo%d_%03d"),
-      (i / CRESCENDO_STEPS) + 1,
-      (i % CRESCENDO_STEPS) + 1);
-    m_crescendo[i]->Load(cfg, buffer);
+  for (uint8_t crescendoIdx = 0; crescendoIdx < N_CRESCENDOS; crescendoIdx++) {
+    wxString cmbStateName = GetCrescendoCmbStateName(crescendoIdx);
+
+    for (unsigned i = 0; i < CRESCENDO_STEPS; i++) {
+      GOGeneralCombination *pCmb
+        = new GOGeneralCombination(*m_OrganController, true);
+
+      m_crescendo.push_back(pCmb);
+      pCmb->Load(
+        cfg,
+        wxString::Format(
+          wxT("SetterCrescendo%d_%03d"), crescendoIdx + 1, i + 1));
+      pCmb->SetCombinationStateName(cmbStateName);
+    }
   }
 
   m_buttons[ID_SETTER_PREV]->Init(cfg, wxT("SetterPrev"), _("Previous"));
@@ -678,8 +696,6 @@ void GOSetter::FromYaml(const YAML::Node &yamlNode) {
 }
 
 void GOSetter::ButtonStateChanged(int id, bool newState) {
-  GOCombination::ExtraElementsSet elementSet;
-
   switch (id) {
 
   case ID_SETTER_REFRESH_FILES:
@@ -868,10 +884,16 @@ void GOSetter::ButtonStateChanged(int id, bool newState) {
       nullptr);
     break;
 
-  case ID_SETTER_CRESCENDO_OVERRIDE:
+  case ID_SETTER_CRESCENDO_OVERRIDE: {
+    wxString cmbStateName = crescendo_cmb_state_name(newState, m_crescendobank);
+
     m_CrescendoOverrideMode[m_crescendobank] = newState;
+    for (unsigned i = 0; i < CRESCENDO_STEPS; i++)
+      m_crescendo[N_CRESCENDOS * m_crescendobank + i]->SetCombinationStateName(
+        cmbStateName);
     m_buttons[ID_SETTER_CRESCENDO_OVERRIDE]->Display(newState);
-
+    break;
+  }
   case ID_SETTER_PITCH_M1:
     m_OrganController->GetRootPipeConfigNode().ModifyManualTuning(-1);
     m_OrganController->GetRootPipeConfigNode().ModifyAutoTuningCorrection(-1);
@@ -1022,13 +1044,10 @@ void GOSetter::UpdateAllSetsButtonsLight(
 
 void GOSetter::PushGeneral(
   GOGeneralCombination &cmb, GOButtonControl *pButtonToLight) {
-  GOCombination::ExtraElementsSet elementSet;
-  const GOCombination::ExtraElementsSet *pExtraSet
-    = GetCrescendoAddSet(elementSet);
-
-  NotifyCmbPushed(cmb.Push(m_state, pExtraSet));
-  if (pButtonToLight || !pExtraSet) { // Otherwise the crescendo in add mode:
-                                      // not to switch off combination buttons
+  NotifyCmbPushed(cmb.Push(m_state));
+  if (pButtonToLight || IsCurrentCrescendoOverride()) {
+    // Otherwise the crescendo in add mode: not to switch off combination
+    // buttons
     UpdateAllSetsButtonsLight(pButtonToLight, -1);
   }
 }
@@ -1039,12 +1058,8 @@ void GOSetter::PushDivisional(
   unsigned cmbManual,
   GOButtonControl *pButtonToLight) {
   if (cmbManual == startManual || !m_state.m_IsActive) {
-    GOCombination::ExtraElementsSet elementSet;
-    const GOCombination::ExtraElementsSet *pExtraSet
-      = GetCrescendoAddSet(elementSet);
-
-    NotifyCmbPushed(cmb.Push(m_state, pExtraSet));
-    if (pButtonToLight || !pExtraSet)
+    NotifyCmbPushed(cmb.Push(m_state));
+    if (pButtonToLight || IsCurrentCrescendoOverride())
       UpdateAllSetsButtonsLight(pButtonToLight, cmbManual);
   }
 }
@@ -1074,18 +1089,6 @@ void GOSetter::SetCrescendoType(unsigned no) {
     m_CrescendoOverrideMode[m_crescendobank]);
 }
 
-const GOCombination::ExtraElementsSet *GOSetter::GetCrescendoAddSet(
-  GOCombination::ExtraElementsSet &elementSet) {
-  const GOCombination::ExtraElementsSet *pResElementSet = nullptr;
-
-  if (!m_CrescendoOverrideMode[m_crescendobank]) {
-    m_crescendo[m_crescendopos + m_crescendobank * CRESCENDO_STEPS]
-      ->GetEnabledElements(elementSet);
-    pResElementSet = &elementSet;
-  }
-  return pResElementSet;
-}
-
 void GOSetter::UpdatePosition(int pos) {
   if (pos != (int)m_pos)
     SetPosition(pos);
@@ -1131,24 +1134,16 @@ void GOSetter::Crescendo(int newpos, bool force) {
     const unsigned oldIdx = m_crescendopos + m_crescendobank * CRESCENDO_STEPS;
     const unsigned newIdx = oldIdx + 1;
 
-    if (crescendoAddMode)
-      m_crescendo[oldIdx]->GetExtraSetState(m_CrescendoExtraSets[oldIdx]);
-    else
-      m_CrescendoExtraSets[oldIdx].clear();
     ++m_crescendopos;
-    changed = changed
-      || m_crescendo[newIdx]->Push(
-        m_state, crescendoAddMode ? &m_CrescendoExtraSets[oldIdx] : nullptr);
+    changed = changed || m_crescendo[newIdx]->Push(m_state);
   }
 
   while (pos < m_crescendopos) {
     --m_crescendopos;
 
     const unsigned newIdx = m_crescendopos + m_crescendobank * CRESCENDO_STEPS;
 
-    changed = changed
-      || m_crescendo[newIdx]->Push(
-        m_state, crescendoAddMode ? &m_CrescendoExtraSets[newIdx] : nullptr);
+    changed = changed || m_crescendo[newIdx]->Push(m_state);
   }
   // switch combination buttons off in the crescendo override mode
   if (changed && !crescendoAddMode)

src/grandorgue/combinations/GOSetter.h

@@ -60,7 +60,6 @@ class GOSetter : private GOSoundStateHandler,
   ptr_vector<GOGeneralCombination> m_framegeneral;
   ptr_vector<GOGeneralCombination> m_general;
   ptr_vector<GOGeneralCombination> m_crescendo;
-  std::vector<GOCombination::ExtraElementsSet> m_CrescendoExtraSets;
   bool m_CrescendoOverrideMode[N_CRESCENDOS];
   GOLabelControl m_CurrFileDisplay;
   GOLabelControl m_PosDisplay;
@@ -81,6 +80,10 @@ class GOSetter : private GOSoundStateHandler,
 
   void SetSetterType(GOSetterState::SetterType type);
   void SetCrescendoType(unsigned no);
+  bool IsCurrentCrescendoOverride() const {
+    return m_CrescendoOverrideMode[m_crescendobank];
+  }
+  wxString GetCrescendoCmbStateName(uint8_t crescendoIdx) const;
   void Crescendo(int pos, bool force = false);
 
   static const struct ButtonDefinitionEntry m_element_types[];
@@ -205,14 +208,6 @@ class GOSetter : private GOSoundStateHandler,
     unsigned cmbManual,
     GOButtonControl *pButtonToLight) override;
 
-  /*
-   * If current crescendo is in override mode then returns nullptr
-   * If current crescendo is in add mode then fills elementSet and returns a
-   * pointer to it
-   */
-  const GOCombination::ExtraElementsSet *GetCrescendoAddSet(
-    GOCombination::ExtraElementsSet &elementSet);
-
   void Next();
   void Prev();
   void Push();

src/grandorgue/combinations/model/GOCombination.cpp

@@ -213,28 +213,6 @@ void GOCombination::SetStatesFromYaml(
   }
 }
 
-void GOCombination::GetExtraSetState(
-  GOCombination::ExtraElementsSet &extraSet) {
-  extraSet.clear();
-  // May be called from init a crescendo before init combinations
-  EnsureElementStatesAllocated();
-  for (unsigned i = 0; i < r_ElementDefinitions.size(); i++) {
-    if (
-      m_ElementStates[i] == BOOL3_FALSE
-      && r_ElementDefinitions[i].control->GetCombinationState())
-      extraSet.insert(i);
-  }
-}
-
-void GOCombination::GetEnabledElements(
-  GOCombination::ExtraElementsSet &enabledElements) {
-  enabledElements.clear();
-  for (unsigned i = 0; i < r_ElementDefinitions.size(); i++) {
-    if (to_bool(m_ElementStates[i]))
-      enabledElements.insert(i);
-  }
-}
-
 void GOCombination::EnsureElementStatesAllocated() {
   unsigned defSize = r_ElementDefinitions.size();
 
@@ -438,9 +416,7 @@ bool GOCombination::FillWithCurrent(
   return used;
 }
 
-bool GOCombination::Push(
-  const GOSetterState &setterState,
-  const GOCombination::ExtraElementsSet *extraSet) {
+bool GOCombination::Push(const GOSetterState &setterState) {
   bool used = false;
 
   if (setterState.m_IsActive) {
@@ -451,12 +427,12 @@ bool GOCombination::Push(
   } else {
     EnsureElementStatesAllocated();
     for (unsigned i = 0; i < r_ElementDefinitions.size(); i++) {
-      if (
-        m_ElementStates[i] != BOOL3_DEFAULT
-        && (!extraSet || extraSet->find(i) == extraSet->end())) {
+      if (m_ElementStates[i] != BOOL3_DEFAULT) {
+        bool elementState = to_bool(m_ElementStates[i]);
+
         r_ElementDefinitions[i].control->SetCombinationState(
-          to_bool(m_ElementStates[i]), m_CombinationStateName);
-        used |= to_bool(m_ElementStates[i]);
+          elementState, m_CombinationStateName);
+        used = used || elementState;
       }
     }
   }

src/grandorgue/combinations/model/GOCombination.h

@@ -23,9 +23,6 @@
 class GOOrganModel;
 
 class GOCombination : public GOSaveableObject, public GOSaveableToYaml {
-public:
-  using ExtraElementsSet = std::unordered_set<unsigned>;
-
 protected:
   GOOrganModel &r_OrganModel;
 
@@ -145,8 +142,6 @@ class GOCombination : public GOSaveableObject, public GOSaveableToYaml {
 
   bool IsEmpty() const;
   GOBool3 GetElementState(unsigned no) const { return m_ElementStates[no]; }
-  void GetExtraSetState(ExtraElementsSet &extraSet);
-  void GetEnabledElements(GOCombination::ExtraElementsSet &enabledElements);
 
   void Copy(const GOCombination *combination);
   void Clear();
@@ -182,9 +177,7 @@ class GOCombination : public GOSaveableObject, public GOSaveableToYaml {
 
   void FromYaml(const YAML::Node &yamlNode) override;
 
-  bool Push(
-    const GOSetterState &setterState,
-    const ExtraElementsSet *extraSet = nullptr);
+  bool Push(const GOSetterState &setterState);
 };
 
 #endif

src/grandorgue/model/GODrawStop.cpp

@@ -163,12 +163,26 @@ void GODrawstop::Save(GOConfigWriter &cfg) {
   GOButtonControl::Save(cfg);
 }
 
+void GODrawstop::SetResultState(bool resState) {
+  if (IsEngaged() != resState) {
+    Display(resState);
+    // must be before calling m_ControlledDrawstops[i]->Update();
+    OnDrawstopStateChanged(resState);
+    for (auto *pDrawstop : m_ControlledDrawstops)
+      pDrawstop->Update(); // reads IsEngaged()
+  }
+}
+
 void GODrawstop::Reset() {
-  if (IsReadOnly())
-    return;
-  if (m_GCState < 0)
-    return;
-  SetButtonState(m_GCState > 0 ? true : false);
+  if (!IsReadOnly() && m_GCState >= 0) {
+    if (m_GCState == 0) {
+      // Clear all internal states
+      for (auto &intState : m_InternalStates)
+        intState.second = false;
+      SetResultState(false);
+    } else
+      SetButtonState(true);
+  }
 }
 
 void GODrawstop::SetInternalState(bool on, const wxString &stateName) {
@@ -181,21 +195,10 @@ void GODrawstop::SetInternalState(bool on, const wxString &stateName) {
 
     for (const auto &intState : m_InternalStates)
       resState = resState || intState.second;
-    if (IsEngaged() != resState) {
-      Display(resState);
-      // must be before calling m_ControlledDrawstops[i]->Update();
-      OnDrawstopStateChanged(resState);
-      for (auto *pDrawstop : m_ControlledDrawstops)
-        pDrawstop->Update(); // reads m_ActiveState
-    }
+    SetResultState(resState);
   }
 }
 
-void GODrawstop::SetButtonState(bool on) {
-  if (IsEngaged() != on)
-    SetDrawStopState(on);
-}
-
 void GODrawstop::SetCombinationState(bool on, const wxString &stateName) {
   if (!IsReadOnly())
     SetInternalState(on, stateName);

src/grandorgue/model/GODrawStop.h

@@ -39,7 +39,11 @@ class GODrawstop : public GOButtonControl, virtual public GOCombinationElement {
 
   bool IsControlledByUser() const override { return !IsReadOnly(); }
 
+  // set the result state of the internal OR switch
+  void SetResultState(bool on);
+  // set one entry of the internal OR switch
   void SetInternalState(bool on, const wxString &stateName);
+  // set the default entry of the internal OR switch
   void SetDrawStopState(bool on) { SetInternalState(on, wxEmptyString); }
 
 protected:
@@ -82,7 +86,7 @@ class GODrawstop : public GOButtonControl, virtual public GOCombinationElement {
   void Load(GOConfigReader &cfg, const wxString &group);
   void RegisterControlled(GODrawstop *sw);
   void UnRegisterControlled(GODrawstop *sw);
-  virtual void SetButtonState(bool on) override;
+  virtual void SetButtonState(bool on) override { SetDrawStopState(on); }
   virtual void Update();
   void Reset();
   void SetCombinationState(bool on, const wxString &stateName) override;