Fix auto-play next track during pause/unpause transitions

When rapidly pressing play/pause, MPV briefly reports idle-active as true during state transitions. Combined with our player_state being set to Playing after unpause, this incorrectly triggered the auto-play next track logic. Fix: Add is_paused() check to auto-play condition to ensure we only advance to next track when the current track actually ends, not during pause state transitions.
Document state management architecture principles
2025-12-11 16:20:14 +01:00 · 2025-12-11 16:11:45 +01:00 · 2025-12-11 15:37:28 +01:00 · 2025-12-11 15:32:37 +01:00 · 2025-12-11 15:16:57 +01:00
4 changed files with 147 additions and 179 deletions
--- a/CLAUDE.md
+++ b/CLAUDE.md
@ -14,6 +14,29 @@ A high-performance Rust-based TUI player for playing music and video files. Buil

 ## Architecture

+### State Management
+
+**CRITICAL:** Player state must be derived from MPV, not maintained separately.
+
+**Single Source of Truth:** MPV properties via IPC
+- `idle-active` (bool) - No file loaded or file ended
+- `pause` (bool) - Playback is paused
+
+**Derive PlayerState:**
+```rust
+if player.is_idle → PlayerState::Stopped
+if !player.is_idle && player.is_paused → PlayerState::Paused
+if !player.is_idle && !player.is_paused → PlayerState::Playing
+```
+
+**Rationale:**
+- Eliminates state synchronization bugs
+- MPV is always the authoritative source
+- No need to update state in multiple places
+- Simpler auto-play logic
+
+**Anti-pattern:** DO NOT maintain `state.player_state` that can desync from MPV
+
 ### Cache-Only Operation

 **CRITICAL:** Left panel shows ONLY cached data. Never browse filesystem directly during operation.
--- a/Cargo.toml
+++ b/Cargo.toml
@ -1,6 +1,6 @@
 [package]
 name = "cm-player"
-version = "0.1.18"
+version = "0.1.22"
 edition = "2021"

 [dependencies]
--- a/src/main.rs
+++ b/src/main.rs
@ -96,6 +96,8 @@ fn action_play_selection(state: &mut AppState, player: &mut player::Player) -> R
    state.play_selection();
    if let Some(ref path) = state.current_file {
        player.play(path)?;
+        // Explicitly resume playback in case MPV was paused
+        player.resume()?;
        state.player_state = PlayerState::Playing;
        player.update_metadata();
        tracing::info!("Playing: {:?} (playlist: {} tracks)", path, state.playlist.len());
@ -126,6 +128,7 @@ fn action_toggle_play_pause(state: &mut AppState, player: &mut player::Player) -
                state.player_state = PlayerState::Playing;
                if let Some(ref path) = state.current_file {
                    player.play(path)?;
+                    player.resume()?;
                    player.update_metadata();
                    tracing::info!("Restarting playback: {:?}", path);
                }
@ -144,6 +147,108 @@ fn action_stop(state: &mut AppState, player: &mut player::Player) -> Result<()>
    Ok(())
 }

+fn action_remove_from_playlist(state: &mut AppState, player: &mut player::Player) -> Result<()> {
+    let was_playing_removed = state.playlist_index == state.selected_playlist_index;
+    state.remove_selected_playlist_item();
+
+    if state.playlist.is_empty() {
+        state.player_state = PlayerState::Stopped;
+        state.current_file = None;
+        player.stop()?;
+    } else if was_playing_removed && state.player_state == PlayerState::Playing {
+        state.current_file = Some(state.playlist[state.playlist_index].clone());
+        if let Some(ref path) = state.current_file {
+            player.play(path)?;
+            // Explicitly resume playback in case MPV was paused
+            player.resume()?;
+            player.update_metadata();
+        }
+    }
+    Ok(())
+}
+
+fn action_play_from_playlist(state: &mut AppState, player: &mut player::Player, preserve_pause: bool) -> Result<()> {
+    state.playlist_index = state.selected_playlist_index;
+    state.current_file = Some(state.playlist[state.playlist_index].clone());
+
+    if preserve_pause {
+        match state.player_state {
+            PlayerState::Playing => {
+                if let Some(ref path) = state.current_file {
+                    player.play(path)?;
+                    player.resume()?;
+                    player.update_metadata();
+                    tracing::info!("Jumped to track: {:?}", path);
+                }
+            }
+            PlayerState::Paused => {
+                if let Some(ref path) = state.current_file {
+                    player.play(path)?;
+                    player.update_metadata();
+                    player.pause()?;
+                    tracing::info!("Jumped to track (paused): {:?}", path);
+                }
+            }
+            PlayerState::Stopped => {
+                state.player_state = PlayerState::Playing;
+                if let Some(ref path) = state.current_file {
+                    player.play(path)?;
+                    player.resume()?;
+                    player.update_metadata();
+                    tracing::info!("Started playing track: {:?}", path);
+                }
+            }
+        }
+    } else {
+        state.player_state = PlayerState::Playing;
+        if let Some(ref path) = state.current_file {
+            player.play(path)?;
+            // Explicitly resume playback in case MPV was paused
+            player.resume()?;
+            player.update_metadata();
+            tracing::info!("Playing from playlist: {:?}", path);
+        }
+    }
+    Ok(())
+}
+
+fn handle_context_menu_action(menu_type: state::ContextMenuType, selected: usize, state: &mut AppState, player: &mut player::Player) -> Result<()> {
+    match menu_type {
+        state::ContextMenuType::FilePanel => {
+            match selected {
+                0 => action_play_selection(state, player)?,
+                1 => state.add_to_playlist(),
+                _ => {}
+            }
+        }
+        state::ContextMenuType::Playlist => {
+            match selected {
+                0 => action_remove_from_playlist(state, player)?,
+                1 => {
+                    state.shuffle_playlist();
+                    tracing::info!("Playlist randomised from context menu");
+                }
+                _ => {}
+            }
+        }
+        state::ContextMenuType::TitleBar => {
+            match selected {
+                0 => action_stop(state, player)?,
+                1 => {
+                    state.cycle_play_mode();
+                    tracing::info!("Play mode: {:?}", state.play_mode);
+                }
+                2 => {
+                    state.show_refresh_confirm = true;
+                    tracing::info!("Refresh requested from context menu");
+                }
+                _ => {}
+            }
+        }
+    }
+    Ok(())
+}
+
 async fn run_app<B: ratatui::backend::Backend>(
    terminal: &mut Terminal<B>,
    state: &mut AppState,
@ -199,7 +304,8 @@ async fn run_app<B: ratatui::backend::Backend>(

            // Check if track ended and play next (but only if track was actually loaded AND played)
            // Require position > 0.5 to ensure track actually started playing (not just loaded)
-            if player.is_idle() && state.player_state == PlayerState::Playing && state.current_duration > 0.0 && state.current_position > 0.5 {
+            // Also check !is_paused to avoid triggering during pause/unpause transitions
+            if player.is_idle() && !player.is_paused() && state.player_state == PlayerState::Playing && state.current_duration > 0.0 && state.current_position > 0.5 {
                state.play_next();
                // play_next() handles the play mode and may stop if in Normal mode at end
                if state.player_state == PlayerState::Playing {
@ -210,6 +316,7 @@ async fn run_app<B: ratatui::backend::Backend>(
                        last_position = 0.0;

                        player.play(path)?;
+                        player.resume()?;
                    }
                    // Update metadata immediately when track changes
                    player.update_metadata();
@ -361,64 +468,7 @@ async fn handle_key_event<B: ratatui::backend::Backend>(terminal: &mut Terminal<
                let menu_type = menu.menu_type;
                let selected = menu.selected_index;
                state.context_menu = None;
-
-                match menu_type {
-                    ContextMenuType::FilePanel => {
-                        match selected {
-                            0 => action_play_selection(state, player)?,
-                            1 => state.add_to_playlist(),
-                            _ => {}
-                        }
-                    }
-                    ContextMenuType::Playlist => {
-                        match selected {
-                            0 => {
-                                // Remove
-                                let was_playing_removed = state.playlist_index == state.selected_playlist_index;
-                                state.remove_selected_playlist_item();
-
-                                // Handle edge cases after removal
-                                if state.playlist.is_empty() {
-                                    state.player_state = PlayerState::Stopped;
-                                    state.current_file = None;
-                                    player.stop()?;
-                                } else if was_playing_removed && state.player_state == PlayerState::Playing {
-                                    // Removed currently playing track, start new one at same index
-                                    state.current_file = Some(state.playlist[state.playlist_index].clone());
-                                    if let Some(ref path) = state.current_file {
-                                        player.play(path)?;
-                                        player.update_metadata();
-                                    }
-                                }
-                            }
-                            1 => {
-                                // Randomise
-                                state.shuffle_playlist();
-                                tracing::info!("Playlist randomised from context menu");
-                            }
-                            _ => {}
-                        }
-                    }
-                    ContextMenuType::TitleBar => {
-                        match selected {
-                            0 => {
-                                // Stop
-                                action_stop(state, player)?;
-                            }
-                            1 => {
-                                // Toggle Loop
-                                state.cycle_play_mode();
-                                tracing::info!("Play mode: {:?}", state.play_mode);
-                            }
-                            2 => {
-                                // Refresh
-                                state.show_refresh_confirm = true;
-                                tracing::info!("Refresh requested from context menu");
-                            }
-                            _ => {}
-                        }
-                    }
-                }
+                handle_context_menu_action(menu_type, selected, state, player)?;
            }
            KeyCode::Esc => {
                state.context_menu = None;
@ -480,6 +530,7 @@ async fn handle_key_event<B: ratatui::backend::Backend>(terminal: &mut Terminal<
                            // Keep playing
                            if let Some(ref path) = state.current_file {
                                player.play(path)?;
+                                player.resume()?;
                                player.update_metadata(); // Update metadata immediately
                                tracing::info!("Next track: {:?}", path);
                            }
@ -518,6 +569,7 @@ async fn handle_key_event<B: ratatui::backend::Backend>(terminal: &mut Terminal<
                            // Keep playing
                            if let Some(ref path) = state.current_file {
                                player.play(path)?;
+                                player.resume()?;
                                player.update_metadata(); // Update metadata immediately
                                tracing::info!("Previous track: {:?}", path);
                            }
@ -575,12 +627,12 @@ async fn handle_key_event<B: ratatui::backend::Backend>(terminal: &mut Terminal<
                state.move_selection_down();
            }
        }
-        (KeyCode::Char('h'), _) => {
+        (KeyCode::Char('h'), _) | (KeyCode::Left, _) => {
            if !state.focus_playlist {
                state.collapse_selected();
            }
        }
-        (KeyCode::Char('l'), _) => {
+        (KeyCode::Char('l'), _) | (KeyCode::Right, _) => {
            if !state.focus_playlist {
                state.expand_selected();
            }
@ -604,39 +656,13 @@ async fn handle_key_event<B: ratatui::backend::Backend>(terminal: &mut Terminal<
        }
        (KeyCode::Char('d'), _) => {
            if state.focus_playlist {
-                // Remove selected track from playlist
-                state.remove_selected_playlist_item();
-                // If removed currently playing track, handle it
-                if state.playlist.is_empty() {
-                    state.player_state = PlayerState::Stopped;
-                    state.current_file = None;
-                    player.stop()?;
-                } else if state.playlist_index == state.selected_playlist_index {
-                    // Removed currently playing track, play next one
-                    if state.playlist_index < state.playlist.len() {
-                        state.current_file = Some(state.playlist[state.playlist_index].clone());
-                        if state.player_state == PlayerState::Playing {
-                            if let Some(ref path) = state.current_file {
-                                player.play(path)?;
-                                player.update_metadata();
-                            }
-                        }
-                    }
-                }
+                action_remove_from_playlist(state, player)?;
            }
        }
        (KeyCode::Enter, _) => {
            if state.focus_playlist {
-                // Play selected track from playlist
                if state.selected_playlist_index < state.playlist.len() {
-                    state.playlist_index = state.selected_playlist_index;
-                    state.current_file = Some(state.playlist[state.playlist_index].clone());
-                    state.player_state = PlayerState::Playing;
-                    if let Some(ref path) = state.current_file {
-                        player.play(path)?;
-                        player.update_metadata();
-                        tracing::info!("Playing from playlist: {:?}", path);
-                    }
+                    action_play_from_playlist(state, player, false)?;
                }
            } else {
                action_play_selection(state, player)?;
@ -758,63 +784,7 @@ fn handle_mouse_event(state: &mut AppState, mouse: MouseEvent, title_bar_area: r
                    let menu_type = menu.menu_type;
                    let selected = relative_y;
                    state.context_menu = None;
-
-                    match menu_type {
-                        ContextMenuType::FilePanel => {
-                            match selected {
-                                0 => action_play_selection(state, player)?,
-                                1 => state.add_to_playlist(),
-                                _ => {}
-                            }
-                        }
-                        ContextMenuType::Playlist => {
-                            match selected {
-                                0 => {
-                                    // Remove
-                                    let was_playing_removed = state.playlist_index == state.selected_playlist_index;
-                                    state.remove_selected_playlist_item();
-
-                                    // Handle edge cases after removal
-                                    if state.playlist.is_empty() {
-                                        state.player_state = PlayerState::Stopped;
-                                        state.current_file = None;
-                                        player.stop()?;
-                                    } else if was_playing_removed && state.player_state == PlayerState::Playing {
-                                        state.current_file = Some(state.playlist[state.playlist_index].clone());
-                                        if let Some(ref path) = state.current_file {
-                                            player.play(path)?;
-                                            player.update_metadata();
-                                        }
-                                    }
-                                }
-                                1 => {
-                                    // Randomise
-                                    state.shuffle_playlist();
-                                    tracing::info!("Playlist randomised from context menu (mouse)");
-                                }
-                                _ => {}
-                            }
-                        }
-                        ContextMenuType::TitleBar => {
-                            match selected {
-                                0 => {
-                                    // Stop
-                                    action_stop(state, player)?;
-                                }
-                                1 => {
-                                    // Toggle Loop
-                                    state.cycle_play_mode();
-                                    tracing::info!("Play mode: {:?} (mouse)", state.play_mode);
-                                }
-                                2 => {
-                                    // Refresh
-                                    state.show_refresh_confirm = true;
-                                    tracing::info!("Refresh requested from context menu (mouse)");
-                                }
-                                _ => {}
-                            }
-                        }
-                    }
+                    handle_context_menu_action(menu_type, selected, state, player)?;
                }
                return Ok(());
            } else {
@ -994,38 +964,9 @@ fn handle_mouse_event(state: &mut AppState, mouse: MouseEvent, title_bar_area: r
                            };

                            if is_double_click {
-                                // Double click = play the track
-                                state.playlist_index = actual_track;
-                                state.current_file = Some(state.playlist[state.playlist_index].clone());
-
-                                match state.player_state {
-                                    PlayerState::Playing => {
-                                        // Keep playing
-                                        if let Some(ref path) = state.current_file {
-                                            player.play(path)?;
-                                            player.update_metadata();
-                                            tracing::info!("Jumped to track: {:?}", path);
-                                        }
-                                    }
-                                    PlayerState::Paused => {
-                                        // Load but stay paused
-                                        if let Some(ref path) = state.current_file {
-                                            player.play(path)?;
-                                            player.update_metadata();
-                                            player.pause()?;
-                                            tracing::info!("Jumped to track (paused): {:?}", path);
-                                        }
-                                    }
-                                    PlayerState::Stopped => {
-                                        // Start playing from clicked track
-                                        state.player_state = PlayerState::Playing;
-                                        if let Some(ref path) = state.current_file {
-                                            player.play(path)?;
-                                            player.update_metadata();
-                                            tracing::info!("Started playing track: {:?}", path);
-                                        }
-                                    }
-                                }
+                                // Double click = play the track (preserve pause state)
+                                state.selected_playlist_index = actual_track;
+                                action_play_from_playlist(state, player, true)?;
                                // Reset click tracking after action
                                state.last_click_time = None;
                                state.last_click_index = None;
--- a/src/player/mod.rs
+++ b/src/player/mod.rs
@ -327,6 +327,10 @@ impl Player {
        self.is_idle
    }

+    pub fn is_paused(&self) -> bool {
+        self.is_paused
+    }
+
    pub fn is_process_alive(&mut self) -> bool {
        // Check if mpv process is still running
        match self.process.try_wait() {
Author	SHA1	Message	Date
Christoffer Martinsson	55e3f04e2c	Fix auto-play next track during pause/unpause transitions All checks were successful Build and Release / build-and-release (push) Successful in 53s Details When rapidly pressing play/pause, MPV briefly reports idle-active as true during state transitions. Combined with our player_state being set to Playing after unpause, this incorrectly triggered the auto-play next track logic. Fix: Add is_paused() check to auto-play condition to ensure we only advance to next track when the current track actually ends, not during pause state transitions.	2025-12-11 16:20:14 +01:00
Christoffer Martinsson	1c2c942e4b	Document state management architecture principles Add critical guidelines for deriving player state from MPV rather than maintaining duplicate state. Documents the single source of truth pattern to prevent state synchronization bugs.	2025-12-11 16:11:45 +01:00
Christoffer Martinsson	3e7707e883	Add arrow key support for folder navigation All checks were successful Build and Release / build-and-release (push) Successful in 54s Details Add Left/Right arrow keys as alternatives to h/l for collapsing and expanding folders in the file panel. Provides more intuitive navigation for users not familiar with vim keybindings. - Left arrow: Collapse selected folder (same as 'h') - Right arrow: Expand selected folder (same as 'l')	2025-12-11 15:37:28 +01:00
Christoffer Martinsson	b59d1aed65	Fix MPV pause state bug when loading new files All checks were successful Build and Release / build-and-release (push) Successful in 52s Details When MPV is paused and a new file is loaded via loadfile command, MPV loads the file but remains in a paused state. This caused the UI to show "Playing" while no audio was actually playing. Fix: Explicitly call resume() after every play() call to ensure MPV unpauses when loading new files. This applies to: - Playing new folder/file selections - Playing from playlist - Auto-play next/previous track - Removing currently playing track from playlist Fixes bug where after pause, playing another folder would show "Playing" status but remain silent at 00:00.	2025-12-11 15:32:37 +01:00
Christoffer Martinsson	f1412b4f8c	Refactor to eliminate keyboard/mouse handler disconnects All checks were successful Build and Release / build-and-release (push) Successful in 53s Details Extract duplicate logic into shared action functions to ensure consistent behavior between keyboard and mouse interactions: - action_remove_from_playlist: Unified playlist removal logic - action_play_from_playlist: Unified playlist playback with optional pause state preservation - handle_context_menu_action: Unified context menu execution Fixes: - Remove from playlist now checks index before removal (was broken in keyboard 'd' handler) - Mouse double-click on playlist now preserves pause state - Context menu handling no longer duplicated across 400+ lines All keyboard and mouse actions now use identical code paths, eliminating state bugs from inconsistent implementations.	2025-12-11 15:16:57 +01:00