fix(queue): clicking a track respects shuffle order (#327)
The frontend was resolving queue-panel clicks via `actualIndex = current_index + trackIndex`, mixing the unshuffled canonical position with the shuffle-aware display position. On shuffle mode, the resulting index pointed at a different track than the one the user clicked, which is exactly what issue #327 reports. Solution: - New `QueueManager::play_upcoming_at(upcoming_index)` resolves the click position through `shuffle_order` + `shuffle_position` when shuffle is on, and falls back to `current_index + upcoming_index + 1` otherwise. - Exposed through `QbzCore::play_upcoming_at` (emits QueueUpdated) and `CoreBridge::play_upcoming_at`. - New `v2_play_queue_upcoming_at` tauri command. - `QueuePanel.onPlayTrack` now carries the `originalIndex` so the caller knows the exact upcoming-list position the user clicked. - `handleQueueTrackPlay` calls the new command directly — no more client-side index arithmetic. Also deletes the two dead `From<crate::queue::QueueTrack>` impls in commands_v2/queue.rs (canonical queue lives in qbz-player now), per the zero-legacy V2 policy. Tests: two new unit tests cover the linear and shuffle-aware paths. Session persistence is untouched — the new method only consults existing state. cc
This commit is contained in:
8 files changed
+135
-77
No files matched your search
@@ -580,6 +580,30 @@ impl QueueManager {
|
||||
prev_idx.and_then(|idx| state.tracks.get(idx).cloned())
|
||||
}
|
||||
|
||||
/// Jump to a track by its position in the `upcoming` list as returned by
|
||||
/// `get_state`. This is the position the user sees in the Queue sidebar;
|
||||
/// the method resolves it to the correct canonical index even when
|
||||
/// shuffle is active (where the display order differs from the canonical
|
||||
/// `tracks` order).
|
||||
///
|
||||
/// Used by the "click a track in the queue panel" path — fixes issue
|
||||
/// #327 where shuffle mode caused a different track than the one
|
||||
/// clicked to be played.
|
||||
pub fn play_upcoming_at(&self, upcoming_index: usize) -> Option<QueueTrack> {
|
||||
let canonical_index = {
|
||||
let state = self.state.lock().unwrap();
|
||||
match state.current_index {
|
||||
Some(_) if state.shuffle => state
|
||||
.shuffle_order
|
||||
.get(state.shuffle_position + 1 + upcoming_index)
|
||||
.copied(),
|
||||
Some(curr_idx) => Some(curr_idx + 1 + upcoming_index),
|
||||
None => Some(upcoming_index),
|
||||
}
|
||||
};
|
||||
canonical_index.and_then(|idx| self.play_index(idx))
|
||||
}
|
||||
|
||||
/// Jump to a specific track by index
|
||||
pub fn play_index(&self, index: usize) -> Option<QueueTrack> {
|
||||
let mut state = self.state.lock().unwrap();
|
||||
@@ -1103,6 +1127,51 @@ mod tests {
|
||||
assert_eq!(state.upcoming.len(), 10);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_play_upcoming_at_without_shuffle_uses_linear_offset() {
|
||||
let queue = QueueManager::new();
|
||||
for i in 1..=5 {
|
||||
queue.add_track(create_test_track(i));
|
||||
}
|
||||
queue.play_index(1); // current = track id 2
|
||||
|
||||
// upcoming list is [3, 4, 5]; clicking position 1 must play id 4
|
||||
let track = queue.play_upcoming_at(1).expect("track");
|
||||
assert_eq!(track.id, 4);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_play_upcoming_at_with_shuffle_follows_shuffle_order() {
|
||||
let queue = QueueManager::new();
|
||||
for i in 1..=5 {
|
||||
queue.add_track(create_test_track(i));
|
||||
}
|
||||
|
||||
// Authoritative shuffle: playing head is shuffle[0]=2 (id 3),
|
||||
// upcoming order becomes [5, 2, 4, 1] (track ids).
|
||||
queue.set_queue_with_order(
|
||||
(1..=5).map(create_test_track).collect(),
|
||||
Some(2),
|
||||
true,
|
||||
Some(vec![2, 4, 1, 3, 0]),
|
||||
);
|
||||
|
||||
let state = queue.get_state();
|
||||
assert_eq!(
|
||||
state
|
||||
.upcoming
|
||||
.iter()
|
||||
.map(|t| t.id)
|
||||
.collect::<Vec<_>>(),
|
||||
vec![5, 2, 4, 1]
|
||||
);
|
||||
|
||||
// Clicking upcoming position 2 must play track id 4, not id 5
|
||||
// (which would be the "current_index + 2 + 1" = 5 broken path).
|
||||
let track = queue.play_upcoming_at(2).expect("track");
|
||||
assert_eq!(track.id, 4);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_set_shuffle_with_order_uses_authoritative_order() {
|
||||
let queue = QueueManager::new();
|
||||
|
||||
Reference in new issue
Block a user