From a3a3e028b0f8667e534e3110be51bb52e7cdaa94 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= <daschuer@mixxx.org>
Date: Sat, 30 Sep 2023 01:27:47 +0200
Subject: [PATCH] Prefer old explicit leader as soft leader after disabling it.

---
 src/engine/sync/enginesync.cpp  | 53 ++++++++++++++++++++-------------
 src/engine/sync/enginesync.h    |  4 +++
 src/engine/sync/synccontrol.cpp |  2 ++
 3 files changed, 38 insertions(+), 21 deletions(-)

diff --git a/src/engine/sync/enginesync.cpp b/src/engine/sync/enginesync.cpp
index 3ca1e0d8c2fa..b137a6a840ca 100644
--- a/src/engine/sync/enginesync.cpp
+++ b/src/engine/sync/enginesync.cpp
@@ -46,8 +46,7 @@ void EngineSync::requestSyncMode(Syncable* pSyncable, SyncMode mode) {
     // decks that need to change as a result.
     Syncable* oldLeader = m_pLeaderSyncable;
     switch (mode) {
-    case SyncMode::LeaderExplicit:
-    case SyncMode::LeaderSoft: {
+    case SyncMode::LeaderExplicit: {
         if (pSyncable->getBaseBpm().isValid()) {
             activateLeader(pSyncable, mode);
         } else {
@@ -58,21 +57,27 @@ void EngineSync::requestSyncMode(Syncable* pSyncable, SyncMode mode) {
         }
         break;
     }
-    case SyncMode::Follower: {
-        // A request for follower mode may be converted into an enabling of soft
-        // leader mode if this still be best choice
-        if (m_pLeaderSyncable == pSyncable) {
-            // Avoid that pickLeader() returns pSyncable with SyncMode::LeaderExplicit
-            m_pLeaderSyncable = nullptr;
-        }
-        Syncable* newLeader = pickLeader(pSyncable);
-        if (newLeader != pSyncable) {
-            activateFollower(pSyncable);
+    case SyncMode::LeaderSoft: {
+        if (pSyncable->getBaseBpm().isValid() &&
+                pSyncable->isPlaying() &&
+                pSyncable->isAudible()) {
+            activateLeader(pSyncable, mode);
+            break;
         }
-        if (newLeader && newLeader != m_pLeaderSyncable) {
+        [[fallthrough]];
+    }
+    case SyncMode::Follower: {
+        Syncable* pNewLeader = pickNewLeader(pSyncable);
+        if (pNewLeader == pSyncable) {
+            // A request for follower mode may be converted into an enabling of soft
+            // leader mode if this still be best choice
+            activateLeader(pNewLeader, SyncMode::LeaderSoft);
+        } else if (pNewLeader != m_pLeaderSyncable) {
             // if the leader has changed, activate it (this updates m_pLeaderSyncable)
             // But don't make an an explicit leader soft.
-            activateLeader(newLeader, SyncMode::LeaderSoft);
+            activateLeader(pNewLeader, SyncMode::LeaderSoft);
+        } else {
+            activateFollower(pSyncable);
         }
         break;
     }
@@ -201,23 +206,29 @@ void EngineSync::deactivateSync(Syncable* pSyncable) {
     }
 
     if (wasLeader) {
-        Syncable* newLeader = pickLeader(nullptr);
+        Syncable* newLeader = pickNewLeader(nullptr);
         if (newLeader != nullptr) {
             activateLeader(newLeader, SyncMode::LeaderSoft);
         }
     }
 }
 
-Syncable* EngineSync::pickLeader(Syncable* enabling_syncable) {
-    if (kLogger.traceEnabled()) {
-        kLogger.trace() << "EngineSync::pickLeader";
-    }
+Syncable* EngineSync::pickLeader(Syncable* pEnablingSyncable) {
     if (m_pLeaderSyncable &&
             m_pLeaderSyncable->getSyncMode() == SyncMode::LeaderExplicit &&
             m_pLeaderSyncable->getBaseBpm().isValid()) {
+        if (kLogger.traceEnabled()) {
+            kLogger.trace() << "EngineSync::pickLeader(): explicit leader found ";
+        }
         return m_pLeaderSyncable;
     }
+    return pickNewLeader(pEnablingSyncable);
+}
 
+Syncable* EngineSync::pickNewLeader(Syncable* pEnablingSyncable) {
+    if (kLogger.traceEnabled()) {
+        kLogger.trace() << "EngineSync::pickNewLeader";
+    }
     // First preference: some other sync deck that is playing.
     // Note, if we are using PREFER_LOCK_BPM we don't use this option.
     Syncable* first_other_playing_deck = nullptr;
@@ -235,7 +246,7 @@ Syncable* EngineSync::pickLeader(Syncable* enabling_syncable) {
             continue;
         }
 
-        if (pSyncable != enabling_syncable) {
+        if (pSyncable != pEnablingSyncable) {
             if (!pSyncable->getChannel()->isPrimaryDeck()) {
                 continue;
             }
@@ -248,7 +259,7 @@ Syncable* EngineSync::pickLeader(Syncable* enabling_syncable) {
             if (playing_deck_count == 0) {
                 first_playing_deck = pSyncable;
             }
-            if (!first_other_playing_deck && pSyncable != enabling_syncable) {
+            if (!first_other_playing_deck && pSyncable != pEnablingSyncable) {
                 first_other_playing_deck = pSyncable;
             }
             playing_deck_count++;
diff --git a/src/engine/sync/enginesync.h b/src/engine/sync/enginesync.h
index d072d80acb6d..b5345847c2a5 100644
--- a/src/engine/sync/enginesync.h
+++ b/src/engine/sync/enginesync.h
@@ -66,6 +66,10 @@ class EngineSync : public SyncableListener {
     /// Iterate over decks, and based on sync and play status, pick a new Leader.
     /// if enabling_syncable is not null, we treat it as if it were enabled because we may
     /// be in the process of enabling it.
+    Syncable* pickNewLeader(Syncable* enabling_syncable);
+
+    /// Return the explicit leader if the one has been selected or picks a new leader using
+    /// pickNewLeader();
     Syncable* pickLeader(Syncable* enabling_syncable);
 
     /// Find a deck to match against, used in the case where there is no sync Leader.
diff --git a/src/engine/sync/synccontrol.cpp b/src/engine/sync/synccontrol.cpp
index 17a1738eb89e..95dceb7e1a07 100644
--- a/src/engine/sync/synccontrol.cpp
+++ b/src/engine/sync/synccontrol.cpp
@@ -519,6 +519,8 @@ void SyncControl::slotSyncLeaderEnabledChangeRequest(double state) {
         // Turning off leader goes back to follower mode.
         switch (mode) {
         case SyncMode::LeaderExplicit:
+            m_pChannel->getEngineBuffer()->requestSyncMode(SyncMode::LeaderSoft);
+            break;
         case SyncMode::LeaderSoft:
             m_pChannel->getEngineBuffer()->requestSyncMode(SyncMode::Follower);
             break;