queue logic fixes

2 files changed, 117 insertions, 29 deletions

diff --git a/src/queue.rs b/src/queue.rs
index 1f2f0fc..c3f8dbe 100644
--- a/src/queue.rs
+++ b/src/queue.rs
@@ -202,7 +202,6 @@ pub struct Queue {
     position: usize,
     shuffle: ShuffleMode,
     shuffled_indices: Vec<usize>,
-    original_tracks: Vec<Track>,
 }
 
 impl Default for Queue {
@@ -218,7 +217,6 @@ impl Queue {
             position: 0,
             shuffle: ShuffleMode::None,
             shuffled_indices: Vec::new(),
-            original_tracks: Vec::new(),
         }
     }
 
@@ -248,62 +246,59 @@ impl Queue {
     }
 
     pub fn set_shuffle(&mut self, mode: ShuffleMode) {
-        match mode {
+        self.shuffle = mode;
+        match self.shuffle {
             ShuffleMode::None => {
-                self.shuffle = ShuffleMode::None;
                 self.restore_from_original();
             }
             ShuffleMode::All => {
-                self.original_tracks = self.tracks.clone();
-                self.shuffle = ShuffleMode::All;
                 self.update_shuffle_indices();
             }
             ShuffleMode::Album => {
-                self.original_tracks = self.tracks.clone();
-                self.shuffle = ShuffleMode::Album;
                 self.update_shuffle_indices_album();
             }
             ShuffleMode::Albums => {
-                self.original_tracks = self.tracks.clone();
-                self.shuffle = ShuffleMode::Albums;
                 self.update_shuffle_indices_albums();
             }
         }
     }
 
     fn restore_from_original(&mut self) {
-        if self.original_tracks.is_empty() {
+        // Restore linear (unshuffled) order. `tracks` is never reordered by
+        // shuffle — only `shuffled_indices` is a permutation — so the live
+        // list already carries the original insertion order. Keep the played
+        // segment (positions 0..=current in shuffled order) in the order it
+        // was played, then append the unplayed entries in original order.
+        // Entries are tracked by identity, so duplicate paths are preserved
+        // and entries added during shuffle survive.
+        if self.tracks.is_empty() {
+            self.position = 0;
             self.shuffled_indices.clear();
             return;
         }
 
-        let current_pos = self.position;
+        let current_pos = self.position.min(self.tracks.len().saturating_sub(1));
 
-        // Find which tracks have been played (positions 0..=current_pos in shuffled order)
-        let mut played_track_paths: Vec<std::path::PathBuf> = Vec::new();
+        // Played entries, in played order (positions 0..=current_pos in shuffled order).
+        let mut played: Vec<Track> = Vec::new();
+        let mut played_idx: Vec<usize> = Vec::new();
         for i in 0..=current_pos {
-            if let Some(&idx) = self.shuffled_indices.get(i)
-                && let Some(track) = self.tracks.get(idx)
-            {
-                played_track_paths.push(track.path.clone());
+            if let Some(&idx) = self.shuffled_indices.get(i) {
+                if let Some(track) = self.tracks.get(idx) {
+                    played.push(track.clone());
+                    played_idx.push(idx);
+                }
             }
         }
 
-        // Keep the played tracks in their current order
-        let mut new_tracks: Vec<Track> = Vec::new();
-        for path in &played_track_paths {
-            if let Some(track) = self.tracks.iter().find(|t| &t.path == path) {
+        // Unplayed entries keep their original (insertion) order.
+        let mut new_tracks = played;
+        for (idx, track) in self.tracks.iter().enumerate() {
+            if !played_idx.contains(&idx) {
                 new_tracks.push(track.clone());
             }
         }
 
-        // Add remaining tracks from original in original order
-        for orig_track in &self.original_tracks {
-            let is_played = played_track_paths.iter().any(|p| p == &orig_track.path);
-            if !is_played {
-                new_tracks.push(orig_track.clone());
-            }
-        }
         self.tracks = new_tracks;
         self.position = current_pos;
         self.shuffled_indices.clear();
diff --git a/tests/queue.rs b/tests/queue.rs
index 2eaa6f3..d338f2a 100644
--- a/tests/queue.rs
+++ b/tests/queue.rs
@@ -474,6 +474,99 @@ fn test_clear_resets_shuffle_indices() {
     assert!(q.shuffled_indices().is_empty());
 }
 
+// ---------- Regression tests ----------
+// These encode invariants the queue must uphold across shuffle transitions.
+// They previously failed and now pin the fixed behavior.
+
+#[test]
+fn test_add_during_shuffle_survives_shuffle_off() {
+    // A track added while Shuffle::All is active must survive turning shuffle
+    // off: `restore_from_original` rebuilds the queue from the live track
+    // list (entries, not paths), so late additions are kept.
+    // (src/queue.rs: add_track 223, restore_from_original 266)
+    let mut q = Queue::new();
+    add_track(&mut q, "/music/a.mp3");
+    add_track(&mut q, "/music/b.mp3");
+    add_track(&mut q, "/music/c.mp3");
+    q.set_shuffle(ShuffleMode::All);
+
+    // User adds a track while shuffle is active (e.g. `rsp play --next`).
+    add_track(&mut q, "/music/d.mp3");
+    assert_eq!(q.len(), 4);
+
+    q.set_shuffle(ShuffleMode::None);
+
+    // d.mp3 must still be in the queue.
+    assert_eq!(
+        q.len(),
+        4,
+        "queue lost a track after shuffle-off; len = {}",
+        q.len()
+    );
+    assert!(
+        q.tracks().iter().any(|t| t.path == PathBuf::from("/music/d.mp3")),
+        "track added during shuffle was dropped when shuffle was turned off"
+    );
+}
+
+#[test]
+fn test_clear_then_shuffle_off_does_not_resurrect_queue() {
+    // A cleared queue must stay empty across a later shuffle-off: `clear()`
+    // wipes tracks/position/shuffled_indices, and `restore_from_original`
+    // must not resurrect entries from any lingering state.
+    // (src/queue.rs: clear 546 / restore_from_original 266;
+    //  src/daemon.rs: stop 815, queue-end 353-354)
+    let mut q = Queue::new();
+    add_track(&mut q, "/music/a.mp3");
+    add_track(&mut q, "/music/b.mp3");
+    add_track(&mut q, "/music/c.mp3");
+    q.set_shuffle(ShuffleMode::All);
+
+    q.clear();
+    assert!(q.is_empty());
+    assert_eq!(q.len(), 0);
+
+    // Later the user toggles shuffle off: the queue must stay empty.
+    q.set_shuffle(ShuffleMode::None);
+    assert!(
+        q.is_empty(),
+        "cleared queue was resurrected from the stale shuffle snapshot (len = {})",
+        q.len()
+    );
+    assert_eq!(q.len(), 0);
+}
+
+#[test]
+fn test_shuffle_off_keeps_unplayed_duplicate_entries() {
+    // Duplicates are distinct queue entries: `restore_from_original` rebuilds
+    // by entry identity (played indices vs. the rest), never by path, so an
+    // unplayed duplicate of an already-played path must survive shuffle-off.
+    // (src/queue.rs: restore_from_original 266)
+    let mut q = Queue::new();
+    add_track(&mut q, "/music/a.mp3");
+    add_track(&mut q, "/music/b.mp3");
+    add_track(&mut q, "/music/a.mp3"); // duplicate entry (same path)
+
+    // Play the first two entries (position 1). The third entry is unplayed.
+    q.next();
+    assert_eq!(q.position(), 1);
+
+    // Position is 1 with len 3: the shuffled tail is a single element, so the
+    // shuffled indices stay identity and the restore is deterministic.
+    q.set_shuffle(ShuffleMode::All);
+    q.set_shuffle(ShuffleMode::None);
+
+    // All three entries must survive shuffle-off (duplicates are distinct
+    // queue entries, not a single path).
+    assert_eq!(
+        q.len(),
+        3,
+        "unplayed duplicate entry was dropped on shuffle-off (len = {})",
+        q.len()
+    );
+    assert_eq!(q.position(), 1);
+}
+
 // ---------- Track struct ----------
 
 #[test]