diff --git a/BookPlayer/Player/PlayerManager.swift b/BookPlayer/Player/PlayerManager.swift index 4c4c0d59b..d0fc7722d 100755 --- a/BookPlayer/Player/PlayerManager.swift +++ b/BookPlayer/Player/PlayerManager.swift @@ -791,14 +791,24 @@ extension PlayerManager { } func skipToPreviousChapter() { - if let currentChapter = currentItem?.currentChapter, - let previousChapter = currentItem?.previousChapter(before: currentChapter) - { + defer { NotificationCenter.default.post(name: .listeningProgressChanged, object: nil) } + + guard let currentItem, + let currentChapter = currentItem.currentChapter + else { + playPreviousItem() + return + } + + if !currentItem.isNearChapterStart { + /// First press while into the chapter goes back to its start + jumpToChapter(currentChapter) + } else if let previousChapter = currentItem.previousChapter(before: currentChapter) { + /// Already near the chapter start, so step back to the previous chapter jumpToChapter(previousChapter) } else { playPreviousItem() } - NotificationCenter.default.post(name: .listeningProgressChanged, object: nil) } func skip(_ interval: TimeInterval) { diff --git a/BookPlayerTests/PlayableItemTests.swift b/BookPlayerTests/PlayableItemTests.swift index ed7090c0a..419b113f1 100644 --- a/BookPlayerTests/PlayableItemTests.swift +++ b/BookPlayerTests/PlayableItemTests.swift @@ -84,4 +84,35 @@ class PlayableItemTests: XCTestCase { XCTAssert(remainingTimeInBook == -50) } + + // MARK: - isNearChapterStart + + func testIsNearChapterStartWithinThreshold() { + sut.currentChapter = sut.chapters[0] // starts at 0 + sut.currentTime = 2 + + XCTAssertTrue(sut.isNearChapterStart) + } + + func testIsNearChapterStartAtChapterStart() { + sut.currentChapter = sut.chapters[1] // starts at 51 + sut.currentTime = 51 + + XCTAssertTrue(sut.isNearChapterStart) + } + + func testIsNotNearChapterStartAtThresholdBoundary() { + /// Exactly `chapterStartThreshold` (3s) past the start is NOT near the start + sut.currentChapter = sut.chapters[1] // starts at 51 + sut.currentTime = 54 + + XCTAssertFalse(sut.isNearChapterStart) + } + + func testIsNotNearChapterStartMidChapter() { + sut.currentChapter = sut.chapters[1] // starts at 51 + sut.currentTime = 100 + + XCTAssertFalse(sut.isNearChapterStart) + } } diff --git a/BookPlayerTests/PlayerManagerTests.swift b/BookPlayerTests/PlayerManagerTests.swift index d2bd5ff21..e9191b628 100644 --- a/BookPlayerTests/PlayerManagerTests.swift +++ b/BookPlayerTests/PlayerManagerTests.swift @@ -24,6 +24,11 @@ class PlayerManagerTests: XCTestCase { UserDefaults.sharedDefaults.removeObject(forKey: Constants.UserDefaults.remainingTimeEnabled) self.playbackServiceMock = PlaybackServiceProtocolMock() + /// Mirror production `PlaybackService.updatePlaybackTime(item:time:)`, which advances the + /// playhead, so seek targets are observable in tests instead of staying frozen. + self.playbackServiceMock.updatePlaybackTimeItemTimeClosure = { item, time in + item.currentTime = time + } self.sut = PlayerManager( libraryService: LibraryServiceProtocolMock(), playbackService: playbackServiceMock, @@ -69,6 +74,43 @@ class PlayerManagerTests: XCTestCase { ) } + /// Two-chapter item using production's 1-based chapter indexing, so + /// `nextChapter`/`previousChapter` navigation resolves correctly. + private func generateChapteredItem() -> PlayableItem { + let chapter1 = PlayableChapter( + title: "chapter 1", + author: "author", + start: 0, + duration: 50, + relativePath: "", + remoteURL: nil, + index: 1 + ) + let chapter2 = PlayableChapter( + title: "chapter 2", + author: "author", + start: 51, + duration: 100, + relativePath: "", + remoteURL: nil, + index: 2 + ) + return PlayableItem( + title: "test book", + author: "test author", + chapters: [chapter1, chapter2], + currentTime: 0, + duration: 151, + relativePath: "", + uuid: "LEGACY_UUID", + parentFolder: nil, + percentCompleted: 10, + lastPlayDate: nil, + isFinished: false, + isBoundBook: false + ) + } + func testUpdatingEmptyNowPlayingBookTime() { self.sut.setNowPlayingBookTime() @@ -198,4 +240,61 @@ class PlayerManagerTests: XCTestCase { XCTAssertNil(nextItem) XCTAssertTrue(playbackServiceMock.getPlayableItemAfterParentFolderAutoplayedRestartFinishedCallsCount == 2) } + + // MARK: - skipToPreviousChapter + + func testSkipToPreviousChapterMidChapterRestartsCurrentChapter() { + let item = generateChapteredItem() + item.currentChapter = item.chapters[1] // chapter 2, starts at 51 + item.currentTime = 100 // well past the start threshold + sut.currentItem = item + + sut.skipToPreviousChapter() + + // Restarts the current chapter (seeks to its start) rather than stepping back + XCTAssertEqual(sut.currentItem?.currentChapter?.index, 2) + XCTAssertEqual(sut.currentItem?.currentTime ?? 0, 51.1, accuracy: 0.0001) + XCTAssertEqual(playbackServiceMock.getPlayableItemBeforeParentFolderCallsCount, 0) + } + + func testSkipToPreviousChapterNearStartStepsToPreviousChapter() { + let item = generateChapteredItem() + item.currentChapter = item.chapters[1] // chapter 2, starts at 51 + item.currentTime = 52 // within the start threshold + sut.currentItem = item + + sut.skipToPreviousChapter() + + // Steps back to the previous chapter's start + XCTAssertEqual(sut.currentItem?.currentChapter?.index, 1) + XCTAssertEqual(sut.currentItem?.currentTime ?? -1, 0.1, accuracy: 0.0001) + XCTAssertEqual(playbackServiceMock.getPlayableItemBeforeParentFolderCallsCount, 0) + } + + func testSkipToPreviousChapterFirstChapterMidChapterRestartsInstead() { + let item = generateChapteredItem() + item.currentChapter = item.chapters[0] // chapter 1, starts at 0 + item.currentTime = 30 // mid-chapter, past the threshold + sut.currentItem = item + + sut.skipToPreviousChapter() + + // Restarts chapter 1 (seeks to its start) instead of jumping to the previous item + XCTAssertEqual(sut.currentItem?.currentChapter?.index, 1) + XCTAssertEqual(sut.currentItem?.currentTime ?? -1, 0.1, accuracy: 0.0001) + XCTAssertEqual(playbackServiceMock.getPlayableItemBeforeParentFolderCallsCount, 0) + } + + func testSkipToPreviousChapterFirstChapterNearStartPlaysPreviousItem() { + let item = generateChapteredItem() + item.currentChapter = item.chapters[0] // chapter 1, starts at 0 + item.currentTime = 1 // within the start threshold, no previous chapter + sut.currentItem = item + + sut.skipToPreviousChapter() + + // Falls back to the previous item without seeking within the current one + XCTAssertEqual(playbackServiceMock.getPlayableItemBeforeParentFolderCallsCount, 1) + XCTAssertEqual(sut.currentItem?.currentTime ?? -1, 1, accuracy: 0.0001) + } } diff --git a/BookPlayerWatch/LocalPlayback/Player/PlayerManager.swift b/BookPlayerWatch/LocalPlayback/Player/PlayerManager.swift index b26ba4944..3d522d618 100644 --- a/BookPlayerWatch/LocalPlayback/Player/PlayerManager.swift +++ b/BookPlayerWatch/LocalPlayback/Player/PlayerManager.swift @@ -745,14 +745,24 @@ extension PlayerManager { } func skipToPreviousChapter() { - if let currentChapter = currentItem?.currentChapter, - let previousChapter = currentItem?.previousChapter(before: currentChapter) - { + defer { NotificationCenter.default.post(name: .listeningProgressChanged, object: nil) } + + guard let currentItem, + let currentChapter = currentItem.currentChapter + else { + playPreviousItem() + return + } + + if !currentItem.isNearChapterStart { + /// First press while into the chapter goes back to its start + jumpToChapter(currentChapter) + } else if let previousChapter = currentItem.previousChapter(before: currentChapter) { + /// Already near the chapter start, so step back to the previous chapter jumpToChapter(previousChapter) } else { playPreviousItem() } - NotificationCenter.default.post(name: .listeningProgressChanged, object: nil) } func skip(_ interval: TimeInterval) { diff --git a/Shared/CoreData/Lightweight-Models/PlayableItem.swift b/Shared/CoreData/Lightweight-Models/PlayableItem.swift index 4b0397177..717cb641e 100644 --- a/Shared/CoreData/Lightweight-Models/PlayableItem.swift +++ b/Shared/CoreData/Lightweight-Models/PlayableItem.swift @@ -128,6 +128,18 @@ public final class PlayableItem: NSObject, Identifiable { : self.duration } + /// Seconds from a chapter's start within which the playhead is still considered to be "at the start". + /// Shared by rewind clamping and the previous-chapter button so they stay in lockstep. + private static let chapterStartThreshold: TimeInterval = 3 + + /// Whether the playhead is close enough to the current chapter's start that a backward action + /// should step into the previous chapter rather than restart the current one. + public var isNearChapterStart: Bool { + guard let chapter = self.currentChapter else { return false } + + return self.currentTime < chapter.start + Self.chapterStartThreshold + } + public func getInterval(from proposedInterval: TimeInterval) -> TimeInterval { let interval = proposedInterval > 0 @@ -144,9 +156,7 @@ public final class PlayableItem: NSObject, Identifiable { return proposedInterval } - let chapterThreshold: TimeInterval = 3 - - if chapter.start + chapterThreshold > currentTime { + if self.isNearChapterStart { return proposedInterval }