diff --git a/crabidy-server/src/lib.rs b/crabidy-server/src/lib.rs index 1284855..71f6e35 100644 --- a/crabidy-server/src/lib.rs +++ b/crabidy-server/src/lib.rs @@ -255,10 +255,12 @@ pub enum ResolveKind { Replace, /// Every chunk appends at the end. Append, - /// Chunks insert after the given position, each advancing the cursor so - /// the resolved collection stays contiguous and in order. `Queue` - /// (play-after-current) is an `InsertAfter` at the current position. - InsertAfter(u32), + /// Chunks insert **at** the given index, each advancing it so the resolved + /// collection stays contiguous and in order. The index is the one + /// `QueueManager::insert_tracks` takes: whatever sits there shifts down, + /// `0` is the front. `Queue` (play-after-current) is an `InsertAt` of the + /// current position **+ 1**. + InsertAt(u32), } /// The playback loop's bookkeeping for one in-flight resolve operation. @@ -340,13 +342,12 @@ impl PendingResolve { queue.replace_with_tracks(tracks) } ResolveKind::Append => queue.append_tracks(tracks), - ResolveKind::InsertAfter(position) => { - // Advance the cursor so this op's next chunk lands right - // behind this one, keeping the collection contiguous. - // `insert_tracks` inserts *at* an index and clamps past the - // end, so "after N" is "at N + 1". - self.kind = ResolveKind::InsertAfter(position + tracks.len() as u32); - queue.insert_tracks(position + 1, tracks) + ResolveKind::InsertAt(index) => { + // Advance so this op's next chunk lands right behind this one, + // keeping the collection contiguous. `insert_tracks` clamps + // past the end. + self.kind = ResolveKind::InsertAt(index + tracks.len() as u32); + queue.insert_tracks(index, tracks) } }; if started.is_some() { @@ -599,8 +600,7 @@ impl QueueManager { /// what a client needs to paste *before* the first row. An empty queue is /// a replace, which starts playback. /// - /// Callers wanting "after track N" (play-next, `ResolveKind::InsertAfter`) - /// pass `N + 1`. + /// Callers wanting "after track N" (play-next) pass `N + 1`. pub fn insert_tracks(&mut self, position: u32, tracks: &[Track]) -> Option { let len = self.tracks.len(); if len == 0 { @@ -815,9 +815,10 @@ mod tests { } #[test] - fn insert_after_op_keeps_chunks_contiguous_and_in_order() { + fn insert_op_keeps_chunks_contiguous_and_in_order() { let mut q = queue_with(3); // playing track 0 - let mut op = PendingResolve::new(ResolveKind::InsertAfter(0)); + // "after track 0" is "at index 1". + let mut op = PendingResolve::new(ResolveKind::InsertAt(1)); assert!(op.apply_chunk(&mut q, &[track(10), track(11)]).is_none()); assert!(op.apply_chunk(&mut q, &[track(12)]).is_none()); // Both chunks sit as one contiguous run right after the current @@ -829,9 +830,9 @@ mod tests { } #[test] - fn insert_after_op_clamps_past_the_end() { + fn insert_op_clamps_past_the_end() { let mut q = queue_with(1); - let mut op = PendingResolve::new(ResolveKind::InsertAfter(99)); + let mut op = PendingResolve::new(ResolveKind::InsertAt(99)); assert!(op.apply_chunk(&mut q, &[track(10)]).is_none()); assert!(op.apply_chunk(&mut q, &[track(11)]).is_none()); assert_eq!(titles(&q), vec!["track 0", "track 10", "track 11"]); diff --git a/crabidy-server/src/playback.rs b/crabidy-server/src/playback.rs index 9b2e636..49d7d67 100644 --- a/crabidy-server/src/playback.rs +++ b/crabidy-server/src/playback.rs @@ -192,7 +192,8 @@ impl Playback { }; queue.current_position() as u32 }; - self.start_resolve(ResolveKind::InsertAfter(position), paths); + // Play-next means "right after the current track". + self.start_resolve(ResolveKind::InsertAt(position + 1), paths); } PlaybackCommand::Append { paths } => { @@ -238,7 +239,9 @@ impl Playback { } PlaybackCommand::Insert { position, paths } => { - self.start_resolve(ResolveKind::InsertAfter(position), paths); + // The RPC's `position` is the index to insert *at*: whatever + // sits there shifts down, 0 is the front. No offset here. + self.start_resolve(ResolveKind::InsertAt(position), paths); } PlaybackCommand::Clear { exclude_current } => { @@ -904,7 +907,12 @@ mod tests { ) } - #[cfg(feature = "fs")] + /// Queue titles in queue order, for order assertions. + fn queue_titles(playback: &Playback) -> Vec { + let proto: ProtoQueue = playback.queue.lock().expect("queue lock").clone().into(); + proto.tracks.iter().map(|t| t.title.clone()).collect() + } + fn fill_queue(playback: &Playback, n: usize) { let tracks: Vec = (0..n).map(track).collect(); let mut queue = playback.queue.lock().expect("queue lock"); @@ -1055,4 +1063,91 @@ mod tests { let snapshot = rx.borrow().clone().expect("snapshot sent"); assert_eq!(snapshot.tracks.len(), 1); } + + /// The gap that let the paste bug through: every previous test drove + /// either `insert_tracks` or `PendingResolve` directly, so none of them + /// pinned what the **`Insert` command** does with its position. It is an + /// index — the row there shifts down — and 0 is the front. + #[tokio::test] + async fn insert_command_places_tracks_at_the_given_index() { + let playback = playback_with( + #[cfg(feature = "fs")] + None, + ); + fill_queue(&playback, 3); + + // The first op registered gets id 0; the resolve itself goes nowhere + // in tests (no provider), so the chunk is applied by hand. + playback + .handle_command(PlaybackCommand::Insert { + position: 1, + paths: vec!["/x".to_string()], + }) + .await; + playback + .handle_command(PlaybackCommand::ApplyResolvedChunk { + op_id: 0, + tracks: vec![track(9)], + }) + .await; + + assert_eq!( + queue_titles(&playback), + vec!["track 0", "track 9", "track 1", "track 2"], + "position 1 must push the row that was there down" + ); + } + + /// Play-next (`L`) must still land right *after* the current track, not + /// on top of it — the same command-level check for the other caller of + /// the insert op. + #[tokio::test] + async fn queue_command_places_tracks_after_the_current_track() { + let playback = playback_with( + #[cfg(feature = "fs")] + None, + ); + fill_queue(&playback, 3); // playing track 0 + playback + .handle_command(PlaybackCommand::Queue { + paths: vec!["/x".to_string()], + }) + .await; + playback + .handle_command(PlaybackCommand::ApplyResolvedChunk { + op_id: 0, + tracks: vec![track(9)], + }) + .await; + assert_eq!( + queue_titles(&playback), + vec!["track 0", "track 9", "track 1", "track 2"] + ); + } + + /// And position 0 reaches the very front — what `P` on the first row needs. + #[tokio::test] + async fn insert_command_at_zero_reaches_the_front() { + let playback = playback_with( + #[cfg(feature = "fs")] + None, + ); + fill_queue(&playback, 2); + playback + .handle_command(PlaybackCommand::Insert { + position: 0, + paths: vec!["/x".to_string()], + }) + .await; + playback + .handle_command(PlaybackCommand::ApplyResolvedChunk { + op_id: 0, + tracks: vec![track(9)], + }) + .await; + assert_eq!( + queue_titles(&playback), + vec!["track 9", "track 0", "track 1"] + ); + } }