(fix) Controller preferences: don't reload mapping after MIDI learn

ronso0 · Jan 30, 2025 · 7ff97ee · 7ff97ee
1 parent 7b6001c
commit 7ff97ee
Showing 1 changed file with 20 additions and 12 deletions.
diff --git a/src/controllers/dlgprefcontroller.cpp b/src/controllers/dlgprefcontroller.cpp
@@ -219,10 +219,6 @@ void DlgPrefController::showLearningWizard() {
             &DlgControllerLearning::stopListeningForClicks,
             pControllerLearning,
             &ControllerLearningEventFilter::stopListening);
-    connect(m_pDlgControllerLearning,
-            &DlgControllerLearning::stopLearning,
-            this,
-            &DlgPrefController::show);
     connect(m_pDlgControllerLearning,
             &DlgControllerLearning::inputMappingsLearned,
             this,
@@ -242,6 +238,7 @@ void DlgPrefController::slotStopLearning() {
     }
 
     applyMappingChanges();
+
     if (m_pMapping->filePath().isEmpty()) {
         // This mapping was created when the learning wizard was started
         if (m_pMapping->isDirty()) {
@@ -268,6 +265,7 @@ void DlgPrefController::slotStopLearning() {
         }
     }
 
+    // This will show() -> slotUpdate() -> enumerateMappings() etc.
     emit mappingEnded();
 }
 
@@ -507,9 +505,10 @@ void DlgPrefController::slotUpdate() {
     // Force updating the controller settings
     slotMappingSelected(m_ui.comboBoxMapping->currentIndex());
 
-    // enumeratePresets calls slotPresetSelected which will check the m_ui.chkEnabledDevice
-    // checkbox if there is a valid mapping saved in the mixxx.cfg file. However, the
-    // checkbox should only be checked if the device is currently enabled.
+    // enumerateMappings() calls slotMappingSelected() which will tick the 'Enabled'
+    // checkbox if there is a valid mapping saved in the mixxx.cfg file.
+    // However, the checkbox should only be checked if the device is currently enabled.
+    // TODO fix in slotMappingSelected()?
     m_ui.chkEnabledDevice->setChecked(m_pController->isOpen());
 
     // If the controller is not mappable, disable the input and output mapping
@@ -620,6 +619,10 @@ QString DlgPrefController::mappingFilePathFromIndex(int index) const {
 }
 
 void DlgPrefController::slotMappingSelected(int chosenIndex) {
+    // Note that this is also called by slotUpdate() after MIDI learning finished
+    // and we may have pending changes. Force-reloading the mapping from file
+    // would wipe those so we need to make sure to return before
+    // LegacyControllerMappingFileHandler::loadMapping()
     QString mappingFilePath = mappingFilePathFromIndex(chosenIndex);
     if (mappingFilePath.isEmpty()) { // User picked "No Mapping" item
         m_ui.chkEnabledDevice->setEnabled(false);
@@ -645,10 +648,16 @@ void DlgPrefController::slotMappingSelected(int chosenIndex) {
     }
 
     // Check if the mapping is different from the configured mapping
-    if (m_GuiInitialized &&
-            m_pControllerManager->getConfiguredMappingFileForDevice(
+    if (m_GuiInitialized) {
+        if (m_pControllerManager->getConfiguredMappingFileForDevice(
                     m_pController->getName()) != mappingFilePath) {
-        setDirty(true);
+            setDirty(true);
+        } else if (m_pMapping && m_pMapping->isDirty()) {
+            // We have pending changes, don't reload the mapping from file!
+            // This is called by show()/slotUpdate() after MIDI learning ended
+            // and there is no need to update the GUI.
+            return;
+        }
     }
 
     applyMappingChanges();
@@ -675,9 +684,8 @@ void DlgPrefController::slotMappingSelected(int chosenIndex) {
         // We might have saved the previous preset with a new name, so update
         // the preset combobox.
         enumerateMappings(mappingFilePath);
-    } else {
-        slotShowMapping(pMapping);
     }
+    slotShowMapping(pMapping);
 }
 
 bool DlgPrefController::saveMapping() {