From 3137e971393a6992c3ffeef535785b857ebc8ebf Mon Sep 17 00:00:00 2001 From: larspalo Date: Tue, 7 May 2024 23:54:47 +0200 Subject: [PATCH] Fixed ignoring and warning about duplicate read Switch entries. Changed Switch referencing to take function changes into account. Allow multiple switch selections for Divisional Coupler the same as other drawstop types. --- CHANGELOG.md | 8 ++++ src/CouplerPanel.cpp | 23 +++++++++-- src/DivisionalCouplerPanel.cpp | 73 ++++++++++++++++++++++++++-------- src/Drawstop.cpp | 12 +++++- src/Drawstop.h | 1 + src/StopPanel.cpp | 23 +++++++++-- src/SwitchPanel.cpp | 21 +++++++++- src/TremulantPanel.cpp | 23 +++++++++-- 8 files changed, 157 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1887352..ae598ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,10 +10,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Option to create new panel from selection on existing panel display. +- Option to duplicate an existing panel. ### Changed - Gui Label elements update their display name in tree to be easier to identify. +- Changing function to Input when it was something else now also removes any existing Switch references. +- Changing function to Not when having more than one Switch reference removes all but the first reference. +- Divisional coupler switch listboxes now allow multiple selection like other drawstop types. + +### Fixed + +- Warn about and ignore read duplicate Switch entries. ## [0.12.0] - 2024-05-05 diff --git a/src/CouplerPanel.cpp b/src/CouplerPanel.cpp index 2e00344..f173976 100644 --- a/src/CouplerPanel.cpp +++ b/src/CouplerPanel.cpp @@ -758,8 +758,17 @@ void CouplerPanel::OnFunctionChange(wxCommandEvent& WXUNUSED(event)) { wxString selectedText = m_functionChoice->GetString(m_functionChoice->GetSelection()); m_coupler->setFunction(selectedText); if (m_coupler->getFunction().IsSameAs(wxT("Input"))) { - m_availableSwitches->Deselect(m_availableSwitches->GetSelection()); - m_referencedSwitches->Deselect(m_referencedSwitches->GetSelection()); + if (m_coupler->getNumberOfSwitches()) { + m_coupler->removeAllSwitchReferences(); + m_referencedSwitches->Clear(); + } + wxArrayInt existingSelections; + m_availableSwitches->GetSelections(existingSelections); + if (!existingSelections.IsEmpty()) { + for (unsigned i = 0; i < existingSelections.GetCount(); i++) { + m_availableSwitches->Deselect(existingSelections[i]); + } + } m_availableSwitches->Enable(false); m_referencedSwitches->Enable(false); m_addReferencedSwitch->Enable(false); @@ -767,6 +776,12 @@ void CouplerPanel::OnFunctionChange(wxCommandEvent& WXUNUSED(event)) { } else { m_availableSwitches->Enable(true); m_referencedSwitches->Enable(true); + if (m_coupler->getFunction().IsSameAs(wxT("NOT"), false) && m_coupler->getNumberOfSwitches() > 1) { + while (m_coupler->getNumberOfSwitches() > 1) { + m_coupler->removeSwitchReferenceAt(m_coupler->getNumberOfSwitches() - 1); + UpdateReferencedSwitches(); + } + } } ::wxGetApp().m_frame->m_organ->setModified(true); } @@ -920,7 +935,9 @@ void CouplerPanel::OnAddSwitchReferenceBtn(wxCommandEvent& WXUNUSED(event)) { m_availableSwitches->GetSelections(selectedSwitches); if (!selectedSwitches.IsEmpty()) { for (unsigned i = 0; i < selectedSwitches.GetCount(); i++) { - if (!m_coupler->hasSwitchReference(::wxGetApp().m_frame->m_organ->getOrganSwitchAt(selectedSwitches[i]))) { + if (!m_coupler->hasSwitchReference(::wxGetApp().m_frame->m_organ->getOrganSwitchAt(selectedSwitches[i])) && + (!(m_coupler->getFunction().IsSameAs(wxT("NOT"), false) && m_coupler->getNumberOfSwitches())) + ) { m_coupler->addSwitchReference(::wxGetApp().m_frame->m_organ->getOrganSwitchAt(selectedSwitches[i])); } } diff --git a/src/DivisionalCouplerPanel.cpp b/src/DivisionalCouplerPanel.cpp index bb1a199..f550ca2 100644 --- a/src/DivisionalCouplerPanel.cpp +++ b/src/DivisionalCouplerPanel.cpp @@ -133,7 +133,12 @@ DivisionalCouplerPanel::DivisionalCouplerPanel(wxWindow *parent) : wxPanel(paren thirdRow1stCol->Add(availableReferencesText, 0, wxALIGN_CENTER_HORIZONTAL|wxALL, 5); m_availableSwitches = new wxListBox( this, - ID_DIV_CPLR_AVAILABLE_SWITCHES + ID_DIV_CPLR_AVAILABLE_SWITCHES, + wxDefaultPosition, + wxDefaultSize, + 0, + NULL, + wxLB_EXTENDED ); thirdRow1stCol->Add(m_availableSwitches, 1, wxEXPAND|wxALL, 5); thirdRow->Add(thirdRow1stCol, 1, wxEXPAND); @@ -162,7 +167,12 @@ DivisionalCouplerPanel::DivisionalCouplerPanel(wxWindow *parent) : wxPanel(paren thirdRow3rdCol->Add(chosenReferencesText, 0, wxALIGN_CENTER_HORIZONTAL|wxALL, 5); m_referencedSwitches = new wxListBox( this, - ID_DIV_CPLR_REFERENCED_SWITCHES + ID_DIV_CPLR_REFERENCED_SWITCHES, + wxDefaultPosition, + wxDefaultSize, + 0, + NULL, + wxLB_EXTENDED ); thirdRow3rdCol->Add(m_referencedSwitches, 1, wxEXPAND|wxALL, 5); thirdRow->Add(thirdRow3rdCol, 1, wxEXPAND); @@ -462,8 +472,17 @@ void DivisionalCouplerPanel::OnFunctionChange(wxCommandEvent& WXUNUSED(event)) { wxString selectedText = m_functionChoice->GetString(m_functionChoice->GetSelection()); m_divCplr->setFunction(selectedText); if (m_divCplr->getFunction().IsSameAs(wxT("Input"))) { - m_availableSwitches->Deselect(m_availableSwitches->GetSelection()); - m_referencedSwitches->Deselect(m_referencedSwitches->GetSelection()); + if (m_divCplr->getNumberOfSwitches()) { + m_divCplr->removeAllSwitchReferences(); + m_referencedSwitches->Clear(); + } + wxArrayInt existingSelections; + m_availableSwitches->GetSelections(existingSelections); + if (!existingSelections.IsEmpty()) { + for (unsigned i = 0; i < existingSelections.GetCount(); i++) { + m_availableSwitches->Deselect(existingSelections[i]); + } + } m_availableSwitches->Enable(false); m_referencedSwitches->Enable(false); m_addReferencedSwitch->Enable(false); @@ -471,6 +490,12 @@ void DivisionalCouplerPanel::OnFunctionChange(wxCommandEvent& WXUNUSED(event)) { } else { m_availableSwitches->Enable(true); m_referencedSwitches->Enable(true); + if (m_divCplr->getFunction().IsSameAs(wxT("NOT"), false) && m_divCplr->getNumberOfSwitches() > 1) { + while (m_divCplr->getNumberOfSwitches() > 1) { + m_divCplr->removeSwitchReferenceAt(m_divCplr->getNumberOfSwitches() - 1); + UpdateReferencedSwitches(); + } + } } ::wxGetApp().m_frame->m_organ->setModified(true); } @@ -576,33 +601,49 @@ void DivisionalCouplerPanel::OnReferencedManualSelection(wxCommandEvent& WXUNUSE } void DivisionalCouplerPanel::OnAddSwitchReferenceBtn(wxCommandEvent& WXUNUSED(event)) { - if (m_availableSwitches->GetSelection() != wxNOT_FOUND) { - unsigned selected = (unsigned) m_availableSwitches->GetSelection(); - if (!m_divCplr->hasSwitchReference(::wxGetApp().m_frame->m_organ->getOrganSwitchAt(selected))) { - m_divCplr->addSwitchReference(::wxGetApp().m_frame->m_organ->getOrganSwitchAt(selected)); - UpdateReferencedSwitches(); + wxArrayInt selectedSwitches; + m_availableSwitches->GetSelections(selectedSwitches); + if (!selectedSwitches.IsEmpty()) { + for (unsigned i = 0; i < selectedSwitches.GetCount(); i++) { + if (!m_divCplr->hasSwitchReference(::wxGetApp().m_frame->m_organ->getOrganSwitchAt(selectedSwitches[i])) && + (!(m_divCplr->getFunction().IsSameAs(wxT("NOT"), false) && m_divCplr->getNumberOfSwitches())) + ) { + m_divCplr->addSwitchReference(::wxGetApp().m_frame->m_organ->getOrganSwitchAt(selectedSwitches[i])); + } } + UpdateReferencedSwitches(); + ::wxGetApp().m_frame->m_organ->setModified(true); } - ::wxGetApp().m_frame->m_organ->setModified(true); } void DivisionalCouplerPanel::OnRemoveSwitchReferenceBtn(wxCommandEvent& WXUNUSED(event)) { - if (m_referencedSwitches->GetSelection() != wxNOT_FOUND) { - unsigned selected = (unsigned) m_referencedSwitches->GetSelection(); - m_divCplr->removeSwitchReference(m_divCplr->getSwitchAtIndex(selected)); + wxArrayInt selectedSwitches; + m_referencedSwitches->GetSelections(selectedSwitches); + if (!selectedSwitches.IsEmpty()) { + std::list switchesToRemove; + for (unsigned i = 0; i < selectedSwitches.GetCount(); i++) { + switchesToRemove.push_back(m_divCplr->getSwitchAtIndex(selectedSwitches[i])); + } + for (GoSwitch *sw : switchesToRemove) { + m_divCplr->removeSwitchReference(sw); + } UpdateReferencedSwitches(); + ::wxGetApp().m_frame->m_organ->setModified(true); } - ::wxGetApp().m_frame->m_organ->setModified(true); } void DivisionalCouplerPanel::OnSwitchListboxSelection(wxCommandEvent& WXUNUSED(event)) { - if (m_availableSwitches->GetSelection() != wxNOT_FOUND) { + wxArrayInt selectedSwitches; + m_availableSwitches->GetSelections(selectedSwitches); + if (!selectedSwitches.IsEmpty()) { m_addReferencedSwitch->Enable(true); } } void DivisionalCouplerPanel::OnReferencedSwitchSelection(wxCommandEvent& WXUNUSED(event)) { - if (m_referencedSwitches->GetSelection() != wxNOT_FOUND) { + wxArrayInt selectedSwitches; + m_referencedSwitches->GetSelections(selectedSwitches); + if (!selectedSwitches.IsEmpty()) { m_removeReferencedSwitch->Enable(true); } } diff --git a/src/Drawstop.cpp b/src/Drawstop.cpp index c248621..63943c7 100644 --- a/src/Drawstop.cpp +++ b/src/Drawstop.cpp @@ -89,7 +89,13 @@ void Drawstop::read(wxFileConfig *cfg, bool usingOldPanelFormat, Organ *readOrga // if the number of the referenced switch is lower than the number of switches in the organ it's ok if (swRefNbr > 0 && swRefNbr <= (int) readOrgan->getNumberOfSwitches()) { // but the index of the switch is actually one lower than in the organ file - addSwitchReference(readOrgan->getOrganSwitchAt(swRefNbr - 1)); + // also check that it's not already added + if (!hasSwitchReference(readOrgan->getOrganSwitchAt(swRefNbr - 1))) + addSwitchReference(readOrgan->getOrganSwitchAt(swRefNbr - 1)); + else { + wxLogWarning("Switch%0.3d (%s) is already added to %s! This additional switch entry will be ignored!", swRefNbr, readOrgan->getOrganSwitchAt(swRefNbr - 1)->getName(), getName()); + ::wxGetApp().m_frame->GetLogWindow()->Show(true); + } } } } @@ -167,6 +173,10 @@ void Drawstop::removeSwitchReferenceAt(unsigned index) { m_switches.erase(it); } +void Drawstop::removeAllSwitchReferences() { + m_switches.clear(); +} + bool Drawstop::hasSwitchReference(GoSwitch *sw) { bool found = false; for (auto& a_switch : m_switches) { diff --git a/src/Drawstop.h b/src/Drawstop.h index 134f893..2bdfdf7 100644 --- a/src/Drawstop.h +++ b/src/Drawstop.h @@ -49,6 +49,7 @@ class Drawstop : public Button { unsigned getIndexOfSwitch(GoSwitch *switchToFind); void removeSwitchReference(GoSwitch *sw); void removeSwitchReferenceAt(unsigned index); + void removeAllSwitchReferences(); bool hasSwitchReference(GoSwitch *sw); bool isStoreInDivisional(); void setStoreInDivisional(bool storeInDivisional); diff --git a/src/StopPanel.cpp b/src/StopPanel.cpp index 43e2048..7b3e1b0 100644 --- a/src/StopPanel.cpp +++ b/src/StopPanel.cpp @@ -727,8 +727,17 @@ void StopPanel::OnFunctionChange(wxCommandEvent& WXUNUSED(event)) { wxString selectedText = m_functionChoice->GetString(m_functionChoice->GetSelection()); m_stop->setFunction(selectedText); if (m_stop->getFunction().IsSameAs(wxT("Input"))) { - m_availableSwitches->Deselect(m_availableSwitches->GetSelection()); - m_referencedSwitches->Deselect(m_referencedSwitches->GetSelection()); + if (m_stop->getNumberOfSwitches()) { + m_stop->removeAllSwitchReferences(); + m_referencedSwitches->Clear(); + } + wxArrayInt existingSelections; + m_availableSwitches->GetSelections(existingSelections); + if (!existingSelections.IsEmpty()) { + for (unsigned i = 0; i < existingSelections.GetCount(); i++) { + m_availableSwitches->Deselect(existingSelections[i]); + } + } m_availableSwitches->Enable(false); m_referencedSwitches->Enable(false); m_addReferencedSwitch->Enable(false); @@ -736,6 +745,12 @@ void StopPanel::OnFunctionChange(wxCommandEvent& WXUNUSED(event)) { } else { m_availableSwitches->Enable(true); m_referencedSwitches->Enable(true); + if (m_stop->getFunction().IsSameAs(wxT("NOT"), false) && m_stop->getNumberOfSwitches() > 1) { + while (m_stop->getNumberOfSwitches() > 1) { + m_stop->removeSwitchReferenceAt(m_stop->getNumberOfSwitches() - 1); + UpdateReferencedSwitches(); + } + } } ::wxGetApp().m_frame->m_organ->setModified(true); } @@ -822,7 +837,9 @@ void StopPanel::OnAddSwitchReferenceBtn(wxCommandEvent& WXUNUSED(event)) { m_availableSwitches->GetSelections(selectedSwitches); if (!selectedSwitches.IsEmpty()) { for (unsigned i = 0; i < selectedSwitches.GetCount(); i++) { - if (!m_stop->hasSwitchReference(::wxGetApp().m_frame->m_organ->getOrganSwitchAt(selectedSwitches[i]))) { + if (!m_stop->hasSwitchReference(::wxGetApp().m_frame->m_organ->getOrganSwitchAt(selectedSwitches[i])) && + (!(m_stop->getFunction().IsSameAs(wxT("NOT"), false) && m_stop->getNumberOfSwitches())) + ) { m_stop->addSwitchReference(::wxGetApp().m_frame->m_organ->getOrganSwitchAt(selectedSwitches[i])); } } diff --git a/src/SwitchPanel.cpp b/src/SwitchPanel.cpp index ad208a6..76f8fae 100644 --- a/src/SwitchPanel.cpp +++ b/src/SwitchPanel.cpp @@ -412,6 +412,17 @@ void SwitchPanel::OnFunctionChange(wxCommandEvent& WXUNUSED(event)) { wxString selectedText = m_functionChoice->GetString(m_functionChoice->GetSelection()); m_switch->setFunction(selectedText); if (m_switch->getFunction().IsSameAs(wxT("Input"))) { + if (m_switch->getNumberOfSwitches()) { + m_switch->removeAllSwitchReferences(); + m_referencedSwitches->Clear(); + } + wxArrayInt existingSelections; + m_availableSwitches->GetSelections(existingSelections); + if (!existingSelections.IsEmpty()) { + for (unsigned i = 0; i < existingSelections.GetCount(); i++) { + m_availableSwitches->Deselect(existingSelections[i]); + } + } m_availableSwitches->Enable(false); m_referencedSwitches->Enable(false); m_addReferencedSwitch->Enable(false); @@ -421,6 +432,12 @@ void SwitchPanel::OnFunctionChange(wxCommandEvent& WXUNUSED(event)) { m_referencedSwitches->Enable(true); m_addReferencedSwitch->Enable(true); m_removeReferencedSwitch->Enable(true); + if (m_switch->getFunction().IsSameAs(wxT("NOT"), false) && m_switch->getNumberOfSwitches() > 1) { + while (m_switch->getNumberOfSwitches() > 1) { + m_switch->removeSwitchReferenceAt(m_switch->getNumberOfSwitches() - 1); + UpdateReferencedSwitches(); + } + } } ::wxGetApp().m_frame->m_organ->setModified(true); } @@ -507,7 +524,9 @@ void SwitchPanel::OnAddSwitchReferenceBtn(wxCommandEvent& WXUNUSED(event)) { m_availableSwitches->GetSelections(selectedSwitches); if (!selectedSwitches.IsEmpty()) { for (unsigned i = 0; i < selectedSwitches.GetCount(); i++) { - if (!m_switch->hasSwitchReference(::wxGetApp().m_frame->m_organ->getOrganSwitchAt(selectedSwitches[i]))) { + if (!m_switch->hasSwitchReference(::wxGetApp().m_frame->m_organ->getOrganSwitchAt(selectedSwitches[i])) && + (!(m_switch->getFunction().IsSameAs(wxT("NOT"), false) && m_switch->getNumberOfSwitches())) + ) { m_switch->addSwitchReference(::wxGetApp().m_frame->m_organ->getOrganSwitchAt(selectedSwitches[i])); } } diff --git a/src/TremulantPanel.cpp b/src/TremulantPanel.cpp index 90bcebb..ec202fd 100644 --- a/src/TremulantPanel.cpp +++ b/src/TremulantPanel.cpp @@ -541,8 +541,17 @@ void TremulantPanel::OnFunctionChange(wxCommandEvent& WXUNUSED(event)) { wxString selectedText = m_functionChoice->GetString(m_functionChoice->GetSelection()); m_tremulant->setFunction(selectedText); if (m_tremulant->getFunction().IsSameAs(wxT("Input"))) { - m_availableSwitches->Deselect(m_availableSwitches->GetSelection()); - m_referencedSwitches->Deselect(m_referencedSwitches->GetSelection()); + if (m_tremulant->getNumberOfSwitches()) { + m_tremulant->removeAllSwitchReferences(); + m_referencedSwitches->Clear(); + } + wxArrayInt existingSelections; + m_availableSwitches->GetSelections(existingSelections); + if (!existingSelections.IsEmpty()) { + for (unsigned i = 0; i < existingSelections.GetCount(); i++) { + m_availableSwitches->Deselect(existingSelections[i]); + } + } m_availableSwitches->Enable(false); m_referencedSwitches->Enable(false); m_addReferencedSwitch->Enable(false); @@ -550,6 +559,12 @@ void TremulantPanel::OnFunctionChange(wxCommandEvent& WXUNUSED(event)) { } else { m_availableSwitches->Enable(true); m_referencedSwitches->Enable(true); + if (m_tremulant->getFunction().IsSameAs(wxT("NOT"), false) && m_tremulant->getNumberOfSwitches() > 1) { + while (m_tremulant->getNumberOfSwitches() > 1) { + m_tremulant->removeSwitchReferenceAt(m_tremulant->getNumberOfSwitches() - 1); + UpdateReferencedSwitches(); + } + } } ::wxGetApp().m_frame->m_organ->setModified(true); } @@ -680,7 +695,9 @@ void TremulantPanel::OnAddSwitchReferenceBtn(wxCommandEvent& WXUNUSED(event)) { m_availableSwitches->GetSelections(selectedSwitches); if (!selectedSwitches.IsEmpty()) { for (unsigned i = 0; i < selectedSwitches.GetCount(); i++) { - if (!m_tremulant->hasSwitchReference(::wxGetApp().m_frame->m_organ->getOrganSwitchAt(selectedSwitches[i]))) { + if (!m_tremulant->hasSwitchReference(::wxGetApp().m_frame->m_organ->getOrganSwitchAt(selectedSwitches[i])) && + (!(m_tremulant->getFunction().IsSameAs(wxT("NOT"), false) && m_tremulant->getNumberOfSwitches())) + ) { m_tremulant->addSwitchReference(::wxGetApp().m_frame->m_organ->getOrganSwitchAt(selectedSwitches[i])); } }