fix: refactor player state machine to eliminate race conditions

Introduce a Connecting state so the UI reflects user intent immediately,
centralize navigation in MainActivity via state transitions, and replace
the pauseRequested volatile flag with controller state as single source
of truth.

Made-with: Cursor
This commit is contained in:
cottongin
2026-03-10 05:15:31 -04:00
parent 49bbb54bb9
commit 5dd7a411ed
10 changed files with 380 additions and 120 deletions

View File

@@ -0,0 +1,46 @@
# Player State Machine Refactor
## Task
Refactored the player state management to eliminate race conditions during rapid station tapping, stop/play transitions, and the NowPlayingScreen bounce-back bug.
## Changes Made
### PlaybackState.kt
- Added `Connecting` state variant to represent the period between user tap and service processing the intent.
### RadioController.kt
- `play()` now sets state to `Connecting` synchronously before sending the intent, eliminating the async gap.
- `stop()` now sets state to `Idle` synchronously before sending the intent.
- `pause()` now sets state to `Paused` synchronously before sending the intent.
- Added redundancy guard: double-tapping a station that's already `Connecting` is a no-op.
### RadioPlaybackService.kt
- Removed the `pauseRequested` volatile flag entirely.
- The `handlePlay()` finally block now checks `controller.state.value` (the single source of truth) instead of the volatile flag to determine cleanup behavior.
- Renamed `cleanup()` to `cleanupResources()` since it no longer sets `Idle` state (that's now the controller's responsibility).
- `handlePause()` and `handleStop()` no longer set `pauseRequested`.
### MainActivity.kt
- Centralized navigation logic via a `LaunchedEffect` that observes `RadioController.state`.
- Navigates to NowPlaying on `Idle -> Active` transitions.
- Navigates back to StationList on `Active -> Idle` transitions while on NowPlaying.
- Station switching (`Playing(A) -> Connecting(B)`) stays on NowPlaying.
### NowPlayingScreen.kt
- Removed the `LaunchedEffect(playbackState)` that called `onBack()` on `Idle` (was the source of the bounce-back bug).
- Added `Connecting` to the `when` branches, showing a loading spinner overlay.
- Disabled pause/skip-ahead buttons while connecting.
### NowPlayingViewModel.kt
- Added `Connecting` handling in session timer (uses `sessionStartedAt`), connection timer (shows 0), and artwork resolver (uses station default artwork).
### StationListScreen.kt
- Removed `onNavigateToNowPlaying()` from `onPlay` callbacks (navigation is now centralized in MainActivity).
- Added `Connecting` to mini player visibility and "now playing" station highlight.
### MiniPlayer.kt
- Added `Connecting` to the destructuring `when` block.
- Shows "Connecting..." subtitle for the `Connecting` state.
## Follow-up Items
- The `stayConnected` and `retryImmediatelyOnNetwork` volatile flags in RadioPlaybackService could be further consolidated into the state machine in a future pass.

View File

@@ -0,0 +1,38 @@
# UI polish: navigation, pause, artwork, scrollability
**Date:** 2026-03-09
## Task description
Five UI/UX improvements after manual testing.
## Changes made
### 1. Auto-navigate to Now Playing on station tap (StationListScreen.kt)
- `onPlay` callback now calls `onNavigateToNowPlaying()` immediately after `viewModel.playStation()`.
### 2. Auto-navigate back to station list on stop (NowPlayingScreen.kt)
- `LaunchedEffect(playbackState)` watches for `PlaybackState.Idle` and calls `onBack()`.
- Removed the "Nothing playing" dead-end screen.
### 3. Pause/resume support
- **PlaybackState.kt**: Added `Paused(station, metadata, sessionStartedAt)` state.
- **RadioController.kt**: Added `pause()` method sending `ACTION_PAUSE`.
- **RadioPlaybackService.kt**: `handlePause()` sets `pauseRequested = true` and stops engine. In `handlePlay`'s finally block, if paused: updates state to `Paused`, keeps listening session alive, doesn't cleanup service. On resume (ACTION_PLAY with same station while paused), reuses the session.
- **NowPlayingViewModel.kt**: Added `pause()` and `resume()`. Session timer keeps ticking during pause.
- **NowPlayingScreen.kt**: Transport controls adapt to state — paused shows large play/resume button + stop; playing shows skip-ahead + pause + stop.
### 4. Artwork in mini player and station list
- **MiniPlayer.kt**: Shows 40dp rounded artwork thumbnail from ICY StreamUrl or station's `defaultArtworkUrl`. Shows "Paused"/"Reconnecting..." subtitle as appropriate.
- **StationListScreen.kt**: Station rows show a 36dp circular artwork thumbnail from `defaultArtworkUrl` (EXTIMG) when available.
### 5. Scrollability / drag-n-drop readiness
- LazyColumn was already scrollable. Added `padding(vertical = 4.dp)` to station rows for better touch targets. All items have stable keys (`station.id`, `"playlist_header_${playlist.id}"`) ready for reorderable integration.
## Files changed
- `app/src/main/java/.../service/PlaybackState.kt`
- `app/src/main/java/.../service/RadioController.kt`
- `app/src/main/java/.../service/RadioPlaybackService.kt`
- `app/src/main/java/.../ui/screens/nowplaying/NowPlayingScreen.kt`
- `app/src/main/java/.../ui/screens/nowplaying/NowPlayingViewModel.kt`
- `app/src/main/java/.../ui/screens/stationlist/StationListScreen.kt`
- `app/src/main/java/.../ui/components/MiniPlayer.kt`