Merge pull request #1058 from TortugaPower/fix-lastplayed-progress

Fix local playback overriding progress while fetching remote progress

BookPlayer/Coordinators/FolderListCoordinator.swift

@@ -72,6 +72,18 @@ class FolderListCoordinator: ItemListCoordinator {
   }
 
   override func syncList() {
+    let userDefaultsKey = "\(Constants.UserDefaults.lastSyncTimestamp)_\(folderRelativePath)"
+    let now = Date().timeIntervalSince1970
+    let lastSync = UserDefaults.standard.double(forKey: userDefaultsKey)
+
+    /// Do not sync if one minute hasn't passed since last sync
+    guard now - lastSync > 60 else {
+      Self.logger.trace("Throttled sync operation")
+      return
+    }
+
+    UserDefaults.standard.set(now, forKey: userDefaultsKey)
+
     Task { @MainActor in
       do {
         _ = try await syncService.syncListContents(at: folderRelativePath)

BookPlayer/Coordinators/LibraryListCoordinator.swift

@@ -18,6 +18,8 @@ class LibraryListCoordinator: ItemListCoordinator, UINavigationControllerDelegat
   var importOperationSubscription: AnyCancellable?
   /// Reference to know if the import screen is already being shown (or in the process of showing)
   weak var importCoordinator: ImportCoordinator?
+  /// Reference to ongoing library fetch task
+  var contentsFetchTask: Task<(), Error>?
 
   private var disposeBag = Set<AnyCancellable>()
 
@@ -212,7 +214,21 @@ class LibraryListCoordinator: ItemListCoordinator, UINavigationControllerDelegat
   }
 
   override func syncList() {
-    Task { @MainActor in
+    let userDefaultsKey = "\(Constants.UserDefaults.lastSyncTimestamp)_library"
+    let now = Date().timeIntervalSince1970
+    let lastSync = UserDefaults.standard.double(forKey: userDefaultsKey)
+
+    /// Do not sync if one minute hasn't passed since last sync
+    guard now - lastSync > 60 else {
+      Self.logger.trace("Throttled sync operation")
+      return
+    }
+
+    UserDefaults.standard.set(now, forKey: userDefaultsKey)
+
+    /// Create new task to sync the library and the last played
+    contentsFetchTask?.cancel()
+    contentsFetchTask = Task { @MainActor in
       do {
         if UserDefaults.standard.bool(forKey: Constants.UserDefaults.hasScheduledLibraryContents) == true {
           try await syncService.syncListContents(at: nil)
@@ -267,3 +283,9 @@ class LibraryListCoordinator: ItemListCoordinator, UINavigationControllerDelegat
     )
   }
 }
+
+extension LibraryListCoordinator: PlaybackSyncProgressDelegate {
+  func waitForSyncInProgress() async {
+    _ = await contentsFetchTask?.result
+  }
+}

BookPlayer/Coordinators/MainCoordinator.swift

@@ -89,6 +89,7 @@ class MainCoordinator: NSObject {
       playbackService: self.playbackService,
       syncService: syncService
     )
+    playerManager.syncProgressDelegate = libraryCoordinator
     self.libraryCoordinator = libraryCoordinator
     libraryCoordinator.tabBarController = tabBarController
     libraryCoordinator.start()

BookPlayer/Generated/AutoMockable.generated.swift

@@ -1085,6 +1085,7 @@ class PlayerManagerProtocolMock: PlayerManagerProtocol {
         set(value) { underlyingIsPlaying = value }
     }
     var underlyingIsPlaying: Bool!
+    var syncProgressDelegate: PlaybackSyncProgressDelegate?
     //MARK: - load
 
     var loadAutoplayCallsCount = 0

BookPlayer/Player/PlayerManager.swift

@@ -14,10 +14,11 @@ import MediaPlayer
 
 // swiftlint:disable:next file_length
 /// sourcery: AutoMockable
-public protocol PlayerManagerProtocol {
+public protocol PlayerManagerProtocol: AnyObject {
   var currentItem: PlayableItem? { get set }
   var currentSpeed: Float { get set }
   var isPlaying: Bool { get }
+  var syncProgressDelegate: PlaybackSyncProgressDelegate? { get set }
 
   func load(_ item: PlayableItem, autoplay: Bool)
   func hasLoadedBook() -> Bool
@@ -41,6 +42,11 @@ public protocol PlayerManagerProtocol {
   func currentItemPublisher() -> AnyPublisher<PlayableItem?, Never>
 }
 
+/// Delegate that hooks into the playback sequence
+public protocol PlaybackSyncProgressDelegate: AnyObject {
+  func waitForSyncInProgress() async
+}
+
 final class PlayerManager: NSObject, PlayerManagerProtocol {
   private let libraryService: LibraryServiceProtocol
   private let playbackService: PlaybackServiceProtocol
@@ -81,6 +87,9 @@ final class PlayerManager: NSObject, PlayerManagerProtocol {
     }
   }
 
+  weak var syncProgressDelegate: PlaybackSyncProgressDelegate?
+  /// Reference to the ongoing play task
+  private var playTask: Task<(), Error>?
   private var playerItem: AVPlayerItem?
   private var loadChapterTask: Task<(), Never>?
   @Published var currentItem: PlayableItem?
@@ -274,6 +283,7 @@ final class PlayerManager: NSObject, PlayerManagerProtocol {
 
   private func load(_ item: PlayableItem, autoplay: Bool, forceRefreshURL: Bool) {
     /// Cancel in case there's an ongoing load task
+    playTask?.cancel()
     loadChapterTask?.cancel()
 
     // Recover in case of failure
@@ -653,10 +663,7 @@ extension PlayerManager {
 // MARK: - Playback
 
 extension PlayerManager {
-  func play() {
-    /// Ignore play commands if there's no item loaded
-    guard let currentItem else { return }
-
+  func prepareForPlayback(_ currentItem: PlayableItem) async -> Bool {
     /// Allow refetching remote URL if the action was initiating by the user
     canFetchRemoteURL = true
 
@@ -669,7 +676,7 @@ extension PlayerManager {
           load(currentItem, autoplay: true)
         }
       }
-      return
+      return false
     }
 
     guard playerItem.status == .readyToPlay && playerItem.error == nil else {
@@ -682,44 +689,63 @@ extension PlayerManager {
         self.observeStatus = true
       }
 
-      return
+      return false
     }
 
-    self.userActivityManager.resumePlaybackActivity()
+    /// Update nowPlaying state so the UI displays correctly
+    playbackQueued = true
+    await syncProgressDelegate?.waitForSyncInProgress()
 
-    do {
-      let audioSession = AVAudioSession.sharedInstance()
-      try audioSession.setCategory(
-        AVAudioSession.Category.playback,
-        mode: .spokenAudio,
-        options: []
-      )
-      try audioSession.setActive(true)
-    } catch {
-      fatalError("Failed to activate the audio session, \(error), description: \(error.localizedDescription)")
-    }
+    return true
+  }
 
-    self.createOrUpdateAutomaticBookmark(
-      at: currentItem.currentTime,
-      relativePath: currentItem.relativePath,
-      type: .play
-    )
+  func play() {
+    playTask?.cancel()
+    playTask = Task { @MainActor in
+      /// Ignore play commands if there's no item loaded,
+      /// and only continue if the item is loaded and ready
+      guard
+        let currentItem,
+        await prepareForPlayback(currentItem),
+        !Task.isCancelled
+      else { return }
+
+      userActivityManager.resumePlaybackActivity()
+
+      do {
+        let audioSession = AVAudioSession.sharedInstance()
+        try audioSession.setCategory(
+          AVAudioSession.Category.playback,
+          mode: .spokenAudio,
+          options: []
+        )
+        try audioSession.setActive(true)
+      } catch {
+        fatalError("Failed to activate the audio session, \(error), description: \(error.localizedDescription)")
+      }
+
+      createOrUpdateAutomaticBookmark(
+        at: currentItem.currentTime,
+        relativePath: currentItem.relativePath,
+        type: .play
+      )
 
-    // If book is completed, stop
-    if Int(currentItem.duration) == Int(CMTimeGetSeconds(self.audioPlayer.currentTime())) { return }
+      // If book is completed, stop
+      if Int(currentItem.duration) == Int(CMTimeGetSeconds(audioPlayer.currentTime())) { return }
 
-    self.handleSmartRewind(currentItem)
+      handleSmartRewind(currentItem)
 
-    self.fadeTimer?.invalidate()
-    self.shakeMotionService.stopMotionUpdates()
-    self.boostVolume = UserDefaults.standard.bool(forKey: Constants.UserDefaults.boostVolumeEnabled)
-    bindInterruptObserver()
-    // Set play state on player and control center
-    self.audioPlayer.playImmediately(atRate: self.currentSpeed)
+      fadeTimer?.invalidate()
+      shakeMotionService.stopMotionUpdates()
+      boostVolume = UserDefaults.standard.bool(forKey: Constants.UserDefaults.boostVolumeEnabled)
+      bindInterruptObserver()
+      // Set play state on player and control center
+      audioPlayer.playImmediately(atRate: currentSpeed)
+      /// Clean up flag after player starts playing
+      playbackQueued = nil
 
-    self.setNowPlayingBookTitle(chapter: currentItem.currentChapter)
+      setNowPlayingBookTitle(chapter: currentItem.currentChapter)
 
-    DispatchQueue.main.async {
       NotificationCenter.default.post(name: .bookPlayed, object: nil, userInfo: ["book": currentItem])
     }
   }
@@ -819,6 +845,7 @@ extension PlayerManager {
     // Set pause state on player and control center
     audioPlayer.pause()
     playbackQueued = nil
+    playTask?.cancel()
     loadChapterTask?.cancel()
     nowPlayingInfo[MPNowPlayingInfoPropertyPlaybackRate] = 0.0
     MPNowPlayingInfoCenter.default().playbackState = .paused
@@ -858,6 +885,7 @@ extension PlayerManager {
     playbackQueued = nil
 
     audioPlayer.pause()
+    playTask?.cancel()
     loadChapterTask?.cancel()
 
     userActivityManager.stopPlaybackActivity()

Shared/Services/Sync/SyncService.swift

@@ -8,7 +8,6 @@
 
 import Combine
 import Foundation
-import RevenueCat
 
 /// Sync errors that must be handled (not shown as alerts)
 public enum BPSyncError: Error {
@@ -164,20 +163,6 @@ public final class SyncService: SyncServiceProtocol, BPLogger {
       throw BookPlayerError.runtimeError("Can't fetch items while there are sync operations in progress")
     }
 
-    let userDefaultsKey = "\(Constants.UserDefaults.lastSyncTimestamp)_\(relativePath ?? "library")"
-    let now = Date().timeIntervalSince1970
-    let lastSync = UserDefaults.standard.double(forKey: userDefaultsKey)
-
-    /// Do not sync if one minute hasn't passed since last sync
-    guard now - lastSync > 60 else {
-      throw BookPlayerError.networkError("Throttled sync operation")
-    }
-
-    UserDefaults.standard.set(
-      Date().timeIntervalSince1970,
-      forKey: userDefaultsKey
-    )
-
     Self.logger.trace("Fetching list of contents")
 
     let response = try await fetchContents(at: relativePath)