Fixed ignoring and warning about duplicate read Switch entries. Chang…

…ed Switch referencing to take function changes into account. Allow multiple switch selections for Divisional Coupler the same as other drawstop types.
GrandOrgue · May 7, 2024 · 3137e97 · 3137e97
1 parent f66ccf8
commit 3137e97
Show file tree

Hide file tree

Showing 8 changed files with 157 additions and 27 deletions.
diff --git 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
@@ -758,15 +758,30 @@ 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);
 		m_removeReferencedSwitch->Enable(false);
 	} 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
@@ -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,15 +472,30 @@ 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);
 		m_removeReferencedSwitch->Enable(false);
 	} 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<GoSwitch*> 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
@@ -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
@@ -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
@@ -727,15 +727,30 @@ 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);
 		m_removeReferencedSwitch->Enable(false);
 	} 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
@@ -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
@@ -541,15 +541,30 @@ 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);
 		m_removeReferencedSwitch->Enable(false);
 	} 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]));
 			}
 		}