fix(qconnect): badge clear, multi-track enqueue, shuffle WS-authoritative, cursor/history/pause
- badge: gate is-remote on transport_connected so it clears on every session-end path (transport drop / watchdog / reconnect-exhausted), not only toggle-off - multi-track: route a whole album/playlist/favorites enqueue to the peer in one QueueAddTracks/QueueInsertTracks (all-or-nothing admission), not single-track only - shuffle inbound: SetShuffleMode renderer arm is flag-only (set_shuffle_flag) so the renderer never invents a local order; the cloud's echoed order is authoritative - shuffle outbound: send shuffle_seed + shuffle_pivot_queue_item_id (the cloud needs the seed to build the order); hoist resolve_qconnect_shuffle_pivot to qconnect-app - start index: resolve_remote_start_index validates the track_id before trusting a qid, so a freshly-pushed album no longer inherits a stale cursor (wrong NowPlayingBar track) - history: play_index records history only when actually moving, so the current track is no longer duplicated in the History tab - pause-to-peer: resolve current_queue_item from the core-aligned cursor instead of the stale renderer snapshot, so pausing an iOS renderer (reports null qid) is accepted cc
This commit is contained in:
10 files changed
+499
-49
No files matched your search
@@ -722,11 +722,19 @@ impl QueueManager {
|
||||
return None;
|
||||
}
|
||||
|
||||
// Save current to history
|
||||
// Save current to history — ONLY when actually moving to a DIFFERENT
|
||||
// track. Jumping to the index already current (e.g. the QConnect
|
||||
// controller's `materialize_remote_queue` re-aligning the cursor to the
|
||||
// same index via `play_index`, since the stopped local player makes the
|
||||
// alignment fire unconditionally) must NOT record a spurious "previous"
|
||||
// entry, or the current track shows up duplicated in the History tab.
|
||||
// Matches `sync_current_to_id`'s `moved` guard.
|
||||
if let Some(curr_idx) = state.current_index {
|
||||
state.history.push_back(curr_idx);
|
||||
while state.history.len() > 50 {
|
||||
state.history.pop_front();
|
||||
if curr_idx != index {
|
||||
state.history.push_back(curr_idx);
|
||||
while state.history.len() > 50 {
|
||||
state.history.pop_front();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1213,6 +1221,29 @@ mod tests {
|
||||
assert_eq!(after.history[0].id, 123);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn play_index_to_same_track_does_not_push_history() {
|
||||
let queue = QueueManager::new();
|
||||
queue.add_track(create_test_track(1));
|
||||
queue.add_track(create_test_track(2));
|
||||
queue.play_index(0); // current None -> 0, no push
|
||||
|
||||
// Re-align to the SAME index — the QConnect controller materialize path
|
||||
// calls play_index with the index already current. It must NOT record a
|
||||
// spurious "previous" entry (the track-shows-twice-in-History bug).
|
||||
queue.play_index(0);
|
||||
assert!(
|
||||
queue.get_state().history.is_empty(),
|
||||
"re-aligning to the current track must not add a history entry"
|
||||
);
|
||||
|
||||
// A genuine move still records the outgoing track.
|
||||
queue.play_index(1);
|
||||
let history = queue.get_state().history;
|
||||
assert_eq!(history.len(), 1);
|
||||
assert_eq!(history[0].id, 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_move_track_down_without_current_track() {
|
||||
let queue = QueueManager::new();
|
||||
|
||||
@@ -1484,6 +1484,16 @@ pub fn enqueue_album(runtime: Runtime, weak: slint::Weak<AppWindow>, handle: tok
|
||||
if tracks.is_empty() {
|
||||
return;
|
||||
}
|
||||
// QConnect CONTROLLER mode: route the whole album to the peer's queue when
|
||||
// a PEER renderer owns playback. All-or-nothing admission inside the
|
||||
// router; returns false when no peer is active, so the local append runs.
|
||||
if let Some(svc) = crate::qconnect_service::service() {
|
||||
let routed: Vec<(u64, Option<String>)> =
|
||||
tracks.iter().map(|qt| (qt.id, qt.source.clone())).collect();
|
||||
if svc.add_to_queue_batch_on_peer_if_active(&routed).await {
|
||||
return;
|
||||
}
|
||||
}
|
||||
runtime.core().add_tracks(tracks).await;
|
||||
refresh_sidebar(false);
|
||||
crate::toast::success_weak(&weak, "Added to queue");
|
||||
@@ -1566,6 +1576,16 @@ pub fn enqueue_album_next(
|
||||
if tracks.is_empty() {
|
||||
return;
|
||||
}
|
||||
// QConnect CONTROLLER mode: route the whole album to the peer (single
|
||||
// QueueInsertTracks in NATURAL order — the server preserves block order).
|
||||
// All-or-nothing admission inside the router; false when no peer is active.
|
||||
if let Some(svc) = crate::qconnect_service::service() {
|
||||
let routed: Vec<(u64, Option<String>)> =
|
||||
tracks.iter().map(|qt| (qt.id, qt.source.clone())).collect();
|
||||
if svc.play_next_batch_on_peer_if_active(&routed).await {
|
||||
return;
|
||||
}
|
||||
}
|
||||
// Insert in reverse so the tracks end up in the correct order.
|
||||
for track in tracks.into_iter().rev() {
|
||||
runtime.core().add_track_next(track).await;
|
||||
@@ -1721,6 +1741,21 @@ pub fn enqueue_playlist(
|
||||
if tracks.is_empty() {
|
||||
return;
|
||||
}
|
||||
// QConnect CONTROLLER mode: route the whole playlist to the peer's queue
|
||||
// (insert-next or append). All-or-nothing admission inside the router;
|
||||
// returns false when no peer is active, so the local path runs unchanged.
|
||||
if let Some(svc) = crate::qconnect_service::service() {
|
||||
let routed: Vec<(u64, Option<String>)> =
|
||||
tracks.iter().map(|qt| (qt.id, qt.source.clone())).collect();
|
||||
let handled = if next {
|
||||
svc.play_next_batch_on_peer_if_active(&routed).await
|
||||
} else {
|
||||
svc.add_to_queue_batch_on_peer_if_active(&routed).await
|
||||
};
|
||||
if handled {
|
||||
return;
|
||||
}
|
||||
}
|
||||
if next {
|
||||
// Reverse so the inserted block keeps the playlist's order.
|
||||
for track in tracks.into_iter().rev() {
|
||||
@@ -1746,6 +1781,23 @@ pub fn enqueue_tracks(
|
||||
return;
|
||||
}
|
||||
handle.spawn(async move {
|
||||
// QConnect CONTROLLER mode: route the batch to the peer's queue when a
|
||||
// PEER renderer owns playback. The favorites bulk bar holds Qobuz catalog
|
||||
// tracks (source defaults to "qobuz" => castable); all-or-nothing admission
|
||||
// inside the router refuses the whole batch if any item is local/plex.
|
||||
// Returns false when no peer is active, so the local loop runs unchanged.
|
||||
if let Some(svc) = crate::qconnect_service::service() {
|
||||
let routed: Vec<(u64, Option<String>)> =
|
||||
tracks.iter().map(|track| (track.id, None)).collect();
|
||||
let handled = if next {
|
||||
svc.play_next_batch_on_peer_if_active(&routed).await
|
||||
} else {
|
||||
svc.add_to_queue_batch_on_peer_if_active(&routed).await
|
||||
};
|
||||
if handled {
|
||||
return;
|
||||
}
|
||||
}
|
||||
// For "play next" each insert lands right after the current track,
|
||||
// so reverse the batch to preserve the selection's order.
|
||||
let ordered: Vec<qbz_models::Track> = if next {
|
||||
|
||||
@@ -83,6 +83,9 @@ impl QconnectRendererEngine for SlintRendererEngine {
|
||||
async fn set_shuffle(&self, enabled: bool) {
|
||||
self.core().set_shuffle(enabled).await
|
||||
}
|
||||
async fn set_shuffle_flag(&self, enabled: bool) {
|
||||
self.core().set_shuffle_with_order(enabled, None).await
|
||||
}
|
||||
async fn get_all_queue_tracks(&self) -> (Vec<QueueTrack>, Option<usize>) {
|
||||
self.core().get_all_queue_tracks().await
|
||||
}
|
||||
|
||||
@@ -188,10 +188,23 @@ impl SlintQconnectEventSink {
|
||||
/// PEER renderer owns playback; `cast-target` is its friendly name;
|
||||
/// `volume-locked` is true when that renderer disallows remote volume.
|
||||
async fn refresh_now_playing_remote_state(&self) {
|
||||
// Badge gate: if the transport is down (terminal teardown OR a transient
|
||||
// reconnect blip), the renderer/controller badge must read NOT remote. The
|
||||
// in-memory session can still name a peer as active_renderer_id long after
|
||||
// the session ended (freeze sets playing_state=UNKNOWN but leaves
|
||||
// active_renderer_id, and disconnect() only runs on the user toggle, not on
|
||||
// transport-drop / reconnect-exhausted). Mirrors the Tauri
|
||||
// fetchQconnectRuntimeState early-return on !transport_connected. On
|
||||
// reconnect, TransportConnected re-runs this refresh and the repopulated
|
||||
// session restores the badge.
|
||||
let transport_connected = match self.app.get().and_then(Weak::upgrade) {
|
||||
Some(app) => app.state_handle().lock().await.transport_connected,
|
||||
None => false,
|
||||
};
|
||||
let (is_remote, cast_target, volume_locked) = {
|
||||
let st = self.sync_state.lock().await;
|
||||
let session = &st.session;
|
||||
let is_remote = is_peer_renderer_active(session);
|
||||
let is_remote = transport_connected && is_peer_renderer_active(session);
|
||||
let active = session.active_renderer_id;
|
||||
let active_info = active.and_then(|active_id| {
|
||||
session.renderers.iter().find(|r| r.renderer_id == active_id)
|
||||
|
||||
@@ -818,6 +818,153 @@ impl SlintQconnectService {
|
||||
true
|
||||
}
|
||||
|
||||
/// Controller add-to-queue routing for a MULTI-track batch (album / playlist /
|
||||
/// favorites bulk). Same contract as the single-track
|
||||
/// `add_to_queue_on_peer_if_active`, but admission is ALL-OR-NOTHING: if ANY
|
||||
/// track in the batch is non-castable (`local` / `plex`), the WHOLE batch is
|
||||
/// refused (toast + `return true`, nothing routed and nothing added locally) —
|
||||
/// mirroring the queue-sync's all-or-nothing rule. A single
|
||||
/// `CtrlSrvrQueueAddTracks` carries every id (the protocol `track_ids` is a
|
||||
/// full `Vec`), and the cloud echoes a `QueueUpdated` that
|
||||
/// `materialize_remote_queue` applies locally, so we never mutate the local
|
||||
/// queue here. Returns `false` only when no peer is active (caller appends
|
||||
/// locally) or the batch is empty (caller no-ops).
|
||||
pub async fn add_to_queue_batch_on_peer_if_active(
|
||||
&self,
|
||||
tracks: &[(u64, Option<String>)],
|
||||
) -> bool {
|
||||
if !self.is_peer_renderer_active().await {
|
||||
return false;
|
||||
}
|
||||
if tracks.is_empty() {
|
||||
return false;
|
||||
}
|
||||
|
||||
if let Some((bad_id, _)) = tracks
|
||||
.iter()
|
||||
.find(|(id, source)| !self.is_track_castable(*id, source.as_deref()))
|
||||
{
|
||||
log::info!(
|
||||
"[QConnect] add_to_queue_batch_on_peer: track {bad_id} not Qobuz-castable; refusing whole batch"
|
||||
);
|
||||
crate::toast::error_weak(&self.window, "Some tracks can't be cast to Qobuz Connect");
|
||||
dev_push_event(
|
||||
&self.window,
|
||||
format!(
|
||||
"-> add_to_queue REFUSED (non-Qobuz track {bad_id} in batch of {})",
|
||||
tracks.len()
|
||||
),
|
||||
);
|
||||
return true;
|
||||
}
|
||||
|
||||
let ids: Vec<i64> = tracks.iter().map(|(id, _)| *id as i64).collect();
|
||||
let count = ids.len();
|
||||
let payload = json!({
|
||||
"track_ids": ids,
|
||||
"context_uuid": Uuid::new_v4().to_string(),
|
||||
"autoplay_reset": false,
|
||||
"autoplay_loading": false,
|
||||
});
|
||||
|
||||
match self
|
||||
.send_command(QueueCommandType::CtrlSrvrQueueAddTracks, payload)
|
||||
.await
|
||||
{
|
||||
Ok(_) => {
|
||||
log::info!("[QConnect] add_to_queue_batch_on_peer: appended {count} tracks");
|
||||
dev_push_event(
|
||||
&self.window,
|
||||
format!("-> add_to_queue QueueAddTracks {count} tracks"),
|
||||
);
|
||||
}
|
||||
Err(err) => {
|
||||
log::warn!("[QConnect] add_to_queue_batch_on_peer: append failed: {err}");
|
||||
}
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
/// Controller play-next routing for a MULTI-track batch. Same all-or-nothing
|
||||
/// admission as `add_to_queue_batch_on_peer_if_active`. The server
|
||||
/// `CtrlSrvrQueueInsertTracks` inserts the whole `track_ids` block right after
|
||||
/// `insert_after` and PRESERVES the list order, so the ids are passed in
|
||||
/// NATURAL order here (unlike the LOCAL fall-through, which reverses per-track
|
||||
/// `add_track_next` inserts to achieve the same effect).
|
||||
pub async fn play_next_batch_on_peer_if_active(
|
||||
&self,
|
||||
tracks: &[(u64, Option<String>)],
|
||||
) -> bool {
|
||||
if !self.is_peer_renderer_active().await {
|
||||
return false;
|
||||
}
|
||||
if tracks.is_empty() {
|
||||
return false;
|
||||
}
|
||||
|
||||
if let Some((bad_id, _)) = tracks
|
||||
.iter()
|
||||
.find(|(id, source)| !self.is_track_castable(*id, source.as_deref()))
|
||||
{
|
||||
log::info!(
|
||||
"[QConnect] play_next_batch_on_peer: track {bad_id} not Qobuz-castable; refusing whole batch"
|
||||
);
|
||||
crate::toast::error_weak(&self.window, "Some tracks can't be cast to Qobuz Connect");
|
||||
dev_push_event(
|
||||
&self.window,
|
||||
format!(
|
||||
"-> play_next REFUSED (non-Qobuz track {bad_id} in batch of {})",
|
||||
tracks.len()
|
||||
),
|
||||
);
|
||||
return true;
|
||||
}
|
||||
|
||||
// Resolve insert_after from the peer's current track (omit when unknown).
|
||||
let insert_after = self
|
||||
.effective_remote_renderer_snapshot()
|
||||
.await
|
||||
.ok()
|
||||
.flatten()
|
||||
.and_then(|(renderer, _queue, _session)| {
|
||||
renderer
|
||||
.current_track
|
||||
.as_ref()
|
||||
.and_then(|item| i64::try_from(item.queue_item_id).ok())
|
||||
});
|
||||
|
||||
let ids: Vec<i64> = tracks.iter().map(|(id, _)| *id as i64).collect();
|
||||
let count = ids.len();
|
||||
let mut payload = json!({
|
||||
"track_ids": ids,
|
||||
"context_uuid": Uuid::new_v4().to_string(),
|
||||
"autoplay_reset": false,
|
||||
"autoplay_loading": false,
|
||||
});
|
||||
if let Some(insert_after) = insert_after {
|
||||
payload["insert_after"] = json!(insert_after);
|
||||
}
|
||||
|
||||
match self
|
||||
.send_command(QueueCommandType::CtrlSrvrQueueInsertTracks, payload)
|
||||
.await
|
||||
{
|
||||
Ok(_) => {
|
||||
log::info!(
|
||||
"[QConnect] play_next_batch_on_peer: inserted {count} tracks (after={insert_after:?})"
|
||||
);
|
||||
dev_push_event(
|
||||
&self.window,
|
||||
format!("-> play_next QueueInsertTracks {count} tracks after={insert_after:?}"),
|
||||
);
|
||||
}
|
||||
Err(err) => {
|
||||
log::warn!("[QConnect] play_next_batch_on_peer: insert failed: {err}");
|
||||
}
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
/// True when a PEER renderer currently owns playback (controller mode). Reads
|
||||
/// the session under the sync-state lock. Shared by the play-next /
|
||||
/// add-to-queue routing entry points.
|
||||
@@ -1224,17 +1371,38 @@ impl SlintQconnectService {
|
||||
let current_position = renderer
|
||||
.current_position_ms
|
||||
.and_then(|value| i32::try_from(value).ok());
|
||||
let current_queue_item = renderer.current_track.as_ref().and_then(|item| {
|
||||
i32::try_from(item.queue_item_id).ok().map(|queue_item_id| {
|
||||
QconnectSetPlayerStateQueueItemPayload {
|
||||
queue_version: Some(QconnectQueueVersionPayload {
|
||||
major: queue.version.major,
|
||||
minor: queue.version.minor,
|
||||
}),
|
||||
id: Some(queue_item_id),
|
||||
}
|
||||
// Resolve the current queue item from the CORE-aligned cursor, NOT the
|
||||
// renderer snapshot. `align_queue_cursor` keeps the core on the peer's
|
||||
// ACTUAL track even when the peer reports a null `current_queue_item_id`
|
||||
// (iOS does this continuously while playing). The renderer snapshot's
|
||||
// `current_track`, by contrast, goes stale on a null-qid report
|
||||
// (`build_effective_renderer_snapshot` only refreshes it when the qid is
|
||||
// Some), and sending that stale qid made iOS REJECT the pause (the
|
||||
// official Qobuz client CAN pause iOS, so this was our bug, not an iOS
|
||||
// limitation — only remote VOLUME is genuinely refused by iOS). WebPlayer
|
||||
// reports its qid, so the core and the snapshot agree there: no change.
|
||||
// Falls back to omitting the item (a bare play/pause the renderer applies
|
||||
// to whatever it is currently on) rather than ever sending a stale qid.
|
||||
let current_queue_item = self
|
||||
.runtime
|
||||
.core()
|
||||
.current_track()
|
||||
.await
|
||||
.and_then(|core_track| {
|
||||
queue
|
||||
.queue_items
|
||||
.iter()
|
||||
.find(|item| item.track_id == core_track.id)
|
||||
.map(|item| item.queue_item_id)
|
||||
})
|
||||
});
|
||||
.and_then(|queue_item_id| i32::try_from(queue_item_id).ok())
|
||||
.map(|queue_item_id| QconnectSetPlayerStateQueueItemPayload {
|
||||
queue_version: Some(QconnectQueueVersionPayload {
|
||||
major: queue.version.major,
|
||||
minor: queue.version.minor,
|
||||
}),
|
||||
id: Some(queue_item_id),
|
||||
});
|
||||
|
||||
let payload = serde_json::to_value(QconnectSetPlayerStateRequest {
|
||||
playing_state: Some(next_playing_state),
|
||||
@@ -1377,14 +1545,40 @@ impl SlintQconnectService {
|
||||
/// `toggle_shuffle_if_remote`.
|
||||
pub async fn toggle_shuffle_if_remote(&self) -> Result<bool, String> {
|
||||
let remote_context = self.effective_remote_renderer_snapshot().await?;
|
||||
let Some((renderer, _queue, session)) = remote_context else {
|
||||
let Some((renderer, queue, session)) = remote_context else {
|
||||
return Ok(false);
|
||||
};
|
||||
|
||||
let current_shuffle = renderer.shuffle_mode.unwrap_or(false);
|
||||
let next_shuffle = !current_shuffle;
|
||||
|
||||
let payload = json!({ "shuffle_mode": next_shuffle });
|
||||
// The cloud REQUIRES a `shuffle_seed` when enabling shuffle ("shuffleSeed
|
||||
// is undefined" error otherwise) and uses it to generate the authoritative
|
||||
// order — WS-authoritative is preserved: QBZ supplies only a seed + pivot,
|
||||
// never a local order, and applies only the cloud's echoed order. No `rand`
|
||||
// crate here (unlike Tauri), so seed from the wall clock; mask to i32::MAX
|
||||
// so it fits the wire `fixed32` as a positive value. The pivot keeps the
|
||||
// currently-playing track at the front of the shuffled order. Mirrors the
|
||||
// Tauri `apply_qconnect_shuffle_mode` payload.
|
||||
let shuffle_seed: Option<u32> = next_shuffle.then(|| {
|
||||
let nanos = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.map(|d| d.as_nanos() as u32)
|
||||
.unwrap_or(1);
|
||||
nanos & (i32::MAX as u32)
|
||||
});
|
||||
let pivot_queue_item_id =
|
||||
qconnect_app::queue_resolution::resolve_qconnect_shuffle_pivot(&queue, &renderer);
|
||||
|
||||
let payload = json!({
|
||||
"shuffle_mode": next_shuffle,
|
||||
"shuffle_seed": shuffle_seed.map(i64::from),
|
||||
"shuffle_pivot_queue_item_id": pivot_queue_item_id
|
||||
.and_then(|value| i32::try_from(value).ok())
|
||||
.map(i64::from),
|
||||
"autoplay_reset": false,
|
||||
"autoplay_loading": false,
|
||||
});
|
||||
self.send_command(QueueCommandType::CtrlSrvrSetShuffleMode, payload)
|
||||
.await?;
|
||||
|
||||
|
||||
@@ -374,7 +374,22 @@ pub fn resolve_remote_start_index(
|
||||
.iter()
|
||||
.position(|item| item.queue_item_id == queue_item_id)
|
||||
{
|
||||
return Some(index);
|
||||
// Only trust the queue_item_id when the track at that position matches
|
||||
// the renderer's reported track (or no track was reported). A qid that
|
||||
// resolves to a DIFFERENT track means the cached renderer projection is
|
||||
// STALE relative to THIS queue — e.g. a fresh album was just pushed (new
|
||||
// track_context_uuid, autoplay_reset) while the projection still names
|
||||
// the PREVIOUS queue's item. Trusting the stale qid lands the cursor on
|
||||
// the wrong track (the "NowPlayingBar shows track 4 on a freshly-pushed
|
||||
// album" bug, controlling a peer that was already rendering). Fall
|
||||
// through to the track_id lookup; it won't find the old track in the new
|
||||
// queue, so the caller defaults to the queue head.
|
||||
let track_matches = renderer_track_id
|
||||
.map(|track_id| queue_state.queue_items[index].track_id == track_id)
|
||||
.unwrap_or(true);
|
||||
if track_matches {
|
||||
return Some(index);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -391,6 +406,38 @@ pub fn resolve_remote_start_index(
|
||||
None
|
||||
}
|
||||
|
||||
/// Resolve the `shuffle_pivot_queue_item_id` for an outbound
|
||||
/// `CtrlSrvrSetShuffleMode` command: the queue item the cloud keeps fixed while
|
||||
/// it generates the shuffled order (so the currently-playing track stays at the
|
||||
/// front). Prefers the renderer's reported `queue_item_id`; falls back to the
|
||||
/// item carrying the renderer's `track_id` when the qid is a placeholder; `None`
|
||||
/// when the renderer has no current track. Frontend-agnostic (ADR-006): used by
|
||||
/// both the Tauri and Slint controller shuffle paths.
|
||||
pub fn resolve_qconnect_shuffle_pivot(
|
||||
queue: &QConnectQueueState,
|
||||
renderer: &QConnectRendererState,
|
||||
) -> Option<u64> {
|
||||
let current_track = renderer.current_track.as_ref()?;
|
||||
|
||||
if queue
|
||||
.queue_items
|
||||
.iter()
|
||||
.any(|item| item.queue_item_id == current_track.queue_item_id)
|
||||
{
|
||||
return Some(current_track.queue_item_id);
|
||||
}
|
||||
|
||||
if let Some(item) = queue
|
||||
.queue_items
|
||||
.iter()
|
||||
.find(|item| item.track_id == current_track.track_id)
|
||||
{
|
||||
return Some(item.queue_item_id);
|
||||
}
|
||||
|
||||
None
|
||||
}
|
||||
|
||||
pub fn resolve_core_shuffle_order(
|
||||
queue_state: &QConnectQueueState,
|
||||
renderer_queue_item_id: Option<u64>,
|
||||
@@ -479,3 +526,90 @@ pub fn normalize_current_queue_item_id_from_queue_state(
|
||||
queue.queue_items[current_index].queue_item_id
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use qconnect_core::QueueItem;
|
||||
|
||||
fn item(queue_item_id: u64, track_id: u64) -> QueueItem {
|
||||
QueueItem {
|
||||
track_context_uuid: String::new(),
|
||||
track_id,
|
||||
queue_item_id,
|
||||
}
|
||||
}
|
||||
|
||||
fn queue(items: Vec<QueueItem>) -> QConnectQueueState {
|
||||
QConnectQueueState {
|
||||
queue_items: items,
|
||||
..Default::default()
|
||||
}
|
||||
}
|
||||
|
||||
/// Regression (controller of a peer that was already rendering): a fresh album
|
||||
/// is pushed, but the cached renderer projection still names the PREVIOUS
|
||||
/// queue's item (qid=3 + the old track_id). The old track is absent from the
|
||||
/// new queue, so the stale qid must NOT resolve the cursor to the new queue's
|
||||
/// item 3 ("NowPlayingBar shows track 4 on a freshly-pushed album"). It falls
|
||||
/// through to the track_id lookup (miss) → None → caller defaults to the head.
|
||||
#[test]
|
||||
fn stale_qid_with_mismatched_track_does_not_land_on_wrong_track() {
|
||||
let q = queue(vec![
|
||||
item(0, 52848233),
|
||||
item(1, 52848234),
|
||||
item(2, 52848235),
|
||||
item(3, 52848236),
|
||||
]);
|
||||
assert_eq!(
|
||||
resolve_remote_start_index(&q, Some(3), Some(126886856)),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
/// A consistent projection (qid + the matching track at that position) still
|
||||
/// resolves to its index — the normal track-change / takeback path is unchanged.
|
||||
#[test]
|
||||
fn consistent_qid_and_track_resolves_to_index() {
|
||||
let q = queue(vec![item(0, 100), item(1, 200), item(2, 300)]);
|
||||
assert_eq!(resolve_remote_start_index(&q, Some(2), Some(300)), Some(2));
|
||||
}
|
||||
|
||||
/// When no track is reported, the qid is trusted as before (some events carry
|
||||
/// only a queue_item_id).
|
||||
#[test]
|
||||
fn qid_without_track_id_is_trusted() {
|
||||
let q = queue(vec![item(0, 100), item(1, 200)]);
|
||||
assert_eq!(resolve_remote_start_index(&q, Some(1), None), Some(1));
|
||||
}
|
||||
|
||||
/// An absent qid falls through to the track_id lookup.
|
||||
#[test]
|
||||
fn track_id_lookup_when_qid_absent_from_queue() {
|
||||
let q = queue(vec![item(0, 100), item(7, 200)]);
|
||||
assert_eq!(resolve_remote_start_index(&q, Some(99), Some(200)), Some(1));
|
||||
}
|
||||
|
||||
/// Shuffle pivot prefers the renderer's reported queue_item_id.
|
||||
#[test]
|
||||
fn shuffle_pivot_from_renderer_queue_item_id() {
|
||||
let q = queue(vec![item(10, 100), item(11, 101), item(12, 102)]);
|
||||
let renderer = QConnectRendererState {
|
||||
current_track: Some(item(11, 101)),
|
||||
..Default::default()
|
||||
};
|
||||
assert_eq!(resolve_qconnect_shuffle_pivot(&q, &renderer), Some(11));
|
||||
}
|
||||
|
||||
/// When the renderer's qid is a placeholder (0), fall back to the item that
|
||||
/// carries the renderer's track_id.
|
||||
#[test]
|
||||
fn shuffle_pivot_by_track_id_when_qid_is_placeholder() {
|
||||
let q = queue(vec![item(20, 200), item(21, 201), item(22, 202)]);
|
||||
let renderer = QConnectRendererState {
|
||||
current_track: Some(item(0, 202)),
|
||||
..Default::default()
|
||||
};
|
||||
assert_eq!(resolve_qconnect_shuffle_pivot(&q, &renderer), Some(22));
|
||||
}
|
||||
}
|
||||
@@ -480,7 +480,12 @@ pub async fn apply_renderer_command(
|
||||
}
|
||||
RendererCommand::SetShuffleMode { shuffle_mode } => {
|
||||
let enabled = renderer_state.shuffle_mode.unwrap_or(*shuffle_mode);
|
||||
engine.set_shuffle(enabled).await;
|
||||
// WS-authoritative: flip ONLY the flag here. Never generate a local
|
||||
// shuffle order — the cloud owns queue order, which arrives separately
|
||||
// via `sync_remote_shuffle_projection` / `materialize_remote_queue`.
|
||||
// Calling the order-generating `set_shuffle` would produce a divergent
|
||||
// local random order ("es un infierno" — the documented failure mode).
|
||||
engine.set_shuffle_flag(enabled).await;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -839,6 +844,7 @@ mod tests {
|
||||
set_volumes: Vec<f32>,
|
||||
set_repeat_modes: u32,
|
||||
set_shuffles: Vec<bool>,
|
||||
set_shuffle_flags: Vec<bool>,
|
||||
set_queue_with_order: Vec<(bool, Option<Vec<usize>>)>,
|
||||
set_queues: u32,
|
||||
clear_queues: Vec<bool>,
|
||||
@@ -907,6 +913,9 @@ mod tests {
|
||||
async fn set_shuffle(&self, enabled: bool) {
|
||||
self.calls().set_shuffles.push(enabled);
|
||||
}
|
||||
async fn set_shuffle_flag(&self, enabled: bool) {
|
||||
self.calls().set_shuffle_flags.push(enabled);
|
||||
}
|
||||
async fn get_all_queue_tracks(&self) -> (Vec<QueueTrack>, Option<usize>) {
|
||||
(self.queue_tracks.clone(), self.queue_index)
|
||||
}
|
||||
@@ -1079,6 +1088,36 @@ mod tests {
|
||||
assert_eq!(engine.calls().seeks, vec![40]);
|
||||
}
|
||||
|
||||
/// #4 — WS-authoritative shuffle: a standalone SetShuffleMode must flip the
|
||||
/// flag ONLY (set_shuffle_flag), NEVER generate a local order. It must not
|
||||
/// call the order-generating set_shuffle, and must not apply any queue order
|
||||
/// (set_queue_with_order) — the cloud's order arrives separately.
|
||||
#[tokio::test]
|
||||
async fn apply_renderer_command_setshufflemode_is_flag_only() {
|
||||
let mut engine = MockEngine::new();
|
||||
engine.queue_tracks = vec![mock_queue_track(1), mock_queue_track(2), mock_queue_track(3)];
|
||||
engine.queue_index = Some(0);
|
||||
let sync = sync();
|
||||
let cmd = RendererCommand::SetShuffleMode { shuffle_mode: true };
|
||||
apply_renderer_command(&engine, &sync, &cmd, &QConnectRendererState::default())
|
||||
.await
|
||||
.unwrap();
|
||||
let calls = engine.calls();
|
||||
assert_eq!(
|
||||
calls.set_shuffle_flags,
|
||||
vec![true],
|
||||
"SetShuffleMode must take the flag-only path"
|
||||
);
|
||||
assert!(
|
||||
calls.set_shuffles.is_empty(),
|
||||
"SetShuffleMode must NEVER call the order-generating set_shuffle (WS-authoritative rule)"
|
||||
);
|
||||
assert!(
|
||||
calls.set_queue_with_order.is_empty(),
|
||||
"SetShuffleMode must not apply any local order; the cloud's order arrives separately"
|
||||
);
|
||||
}
|
||||
|
||||
/// #3 — a state-only SetState (command.current_track = None) must NOT align the
|
||||
/// cursor or load a track from the renderer_state's stale current_track (the
|
||||
/// iOS-pause-jumped-back fix); the playing_state is still applied.
|
||||
|
||||
@@ -52,6 +52,14 @@ pub trait QconnectRendererEngine: Send + Sync {
|
||||
// ---- queue / mode (async) ----
|
||||
async fn set_repeat_mode(&self, mode: RepeatMode);
|
||||
async fn set_shuffle(&self, enabled: bool);
|
||||
/// Set the shuffle FLAG only, WITHOUT generating any local order. QConnect is
|
||||
/// WS-authoritative for shuffle: the renderer must never invent a random
|
||||
/// order; the cloud's authoritative order is applied separately via
|
||||
/// `set_queue_with_order` (`sync_remote_shuffle_projection` /
|
||||
/// `materialize_remote_queue`). Forwards to
|
||||
/// `QbzCore::set_shuffle_with_order(enabled, None)` -> identity order on
|
||||
/// enable (current order preserved, NO RNG), clear on disable.
|
||||
async fn set_shuffle_flag(&self, enabled: bool);
|
||||
async fn get_all_queue_tracks(&self) -> (Vec<QueueTrack>, Option<usize>);
|
||||
async fn set_queue(&self, tracks: Vec<QueueTrack>, start_index: Option<usize>);
|
||||
/// The deferred-shuffle core: a real shuffle order, never an invented
|
||||
|
||||
Reference in new issue
Block a user