Refactor AudioPlayerManager boolean flags into an explicit state machine #17

New issue

Open

opened 2026-07-08 12:48:06 +00:00 by llmad · 0 comments

llmad commented

2026-07-08 12:48:06 +00:00

Owner

AudioPlayerManager currently tracks playback state through a cluster of loosely-coupled boolean flags and optionals:

isDragging
isSeeking
shouldResumeAfterSeek
shouldPlayAfterSeek
pendingSeekTime
scrobbled / isScrobbling
playGeneration

Examples of current bugs

Player item fails to load after reloadAndSeek(): observeValue(forKeyPath:) hits .failed, but isSeeking is never reset.
handleSeekFailure() retries, but if shouldPlayAfterSeek flips to false during the wait, the guard returns early without resetting isSeeking.
Stream URL is valid but .readyToPlay never fires (e.g., hung network): isSeeking stays true indefinitely until the user force-quits or changes song.

Proposed solution

Replace the boolean flags with a single explicit state enum, e.g.:

private enum PlaybackState {
    case idle
    case loading
    case playing
    case paused
    case seeking(targetTime: TimeInterval, resumeAfter: Bool)
    case failed(Error)
}

All state transitions should go through a single method:

private func transition(to newState: PlaybackState)

This method would enforce valid transitions, reset transient values automatically, and cancel/schedule tasks as needed.

Acceptance criteria

Introduce PlaybackState enum covering all current states.
Remove isSeeking, shouldResumeAfterSeek, shouldPlayAfterSeek, and pendingSeekTime.
Ensure isSeeking cannot get stuck true in any failure path.
Ensure seeking/playback cancellation remains correct.
No regression in play/pause/seek/next/previous behavior.

AudioPlayerManager currently tracks playback state through a cluster of loosely-coupled boolean flags and optionals: - isDragging - isSeeking - shouldResumeAfterSeek - shouldPlayAfterSeek - pendingSeekTime - scrobbled / isScrobbling - playGeneration ### Examples of current bugs 1. Player item fails to load after reloadAndSeek(): observeValue(forKeyPath:) hits .failed, but isSeeking is never reset. 2. handleSeekFailure() retries, but if shouldPlayAfterSeek flips to false during the wait, the guard returns early without resetting isSeeking. 3. Stream URL is valid but .readyToPlay never fires (e.g., hung network): isSeeking stays true indefinitely until the user force-quits or changes song. ### Proposed solution Replace the boolean flags with a single explicit state enum, e.g.: ```swift private enum PlaybackState { case idle case loading case playing case paused case seeking(targetTime: TimeInterval, resumeAfter: Bool) case failed(Error) } ``` All state transitions should go through a single method: ```swift private func transition(to newState: PlaybackState) ``` This method would enforce valid transitions, reset transient values automatically, and cancel/schedule tasks as needed. ### Acceptance criteria - [ ] Introduce PlaybackState enum covering all current states. - [ ] Remove isSeeking, shouldResumeAfterSeek, shouldPlayAfterSeek, and pendingSeekTime. - [ ] Ensure isSeeking cannot get stuck true in any failure path. - [ ] Ensure seeking/playback cancellation remains correct. - [ ] No regression in play/pause/seek/next/previous behavior.