diff --git a/handoff/rounds/commit-msg55.txt b/handoff/rounds/commit-msg55.txt new file mode 100644 index 0000000..aec5e26 --- /dev/null +++ b/handoff/rounds/commit-msg55.txt @@ -0,0 +1,40 @@ +android auto: Recent means albums and playlists, and is never empty over a full library + +"Recent" answered with TRACKS out of the `plays` table, and on a phone whose +history had just been wiped that was "No items" sitting over a full library — +the first thing a driver sees, saying the app has nothing. + +Three changes, each about what the word means in a car. + +**Albums, not tracks.** "Recent" in a car is "put that back on", and what +anyone means by that is the record. A list of single songs plays one and stops. +`plays` records what was played by NAME rather than by track id, so getting +back to an album means matching the pair again; anything played that has since +left the library drops out of the join, which is right for a row whose promise +is that tapping it works. + +**Playlists, which were absent entirely.** They are in the database, they are +local, and they play from cold exactly as albums do — they were missing only +because the four root slots were spent, and Recent is somewhere they can live +without taking one. Counted out of `playlist_items` rather than +`playlist_tracks`: it is the richer table and the one that knows whether an +entry is a local file or something that needs the resolver. A playlist that is +part catalog says so on the row — "Playlist · 4 of 9 here" — so the gap is +declared before the tap rather than discovered after it, and tapping plays the +local entries in order. Entries that need the phone are shown rather than +hidden, so the car's list matches what is on the phone, and marked unplayable +rather than pretending. + +**An unplayed library is not an empty shelf.** With no history at all, Recent +falls back to the newest albums, which is a defensible reading of the word and +a great deal better than an empty tab on a fresh install. + +`ss:playlist:` and `ss:pitem:` join the tree and `resolve_playable`, so +a playlist or one of its tracks plays with the rest behind it. + +Two fixes to the instrument that found this: `probe.sh` was passing +`--es play ""` when no query was given, which `am start` rejects outright +("Argument expected after play") — it is conditional now. And a splice left +`fn track_node(fn track_node(` in the source, which rustc reports as an +unclosed delimiter at the END of the file, pointing 400 lines away from the +mistake. diff --git a/src-tauri/src/auto_browse.rs b/src-tauri/src/auto_browse.rs index 88b8862..675da26 100644 --- a/src-tauri/src/auto_browse.rs +++ b/src-tauri/src/auto_browse.rs @@ -265,23 +265,133 @@ fn saved(conn: &Connection) -> rusqlite::Result> { } fn recent(conn: &Connection) -> rusqlite::Result> { - // `plays` records what was played by NAME, not by track id, so getting - // back to a playable row means matching the pair again. Anything played - // that is not in the library any more simply drops out of the join, which - // is the right answer for a tab whose whole promise is that tapping a row - // works. + // RECENTLY PLAYED ALBUMS, not tracks. A car's "Recent" is "put that back + // on", and what a driver means by that is the record — a list of single + // songs plays one and stops. + // + // `plays` records what was played by NAME rather than by track id, so + // getting back to an album means matching the pair again. Anything played + // that has since left the library drops out of the join, which is right + // for a row whose whole promise is that tapping it works. + let mut out: Vec = Vec::new(); let mut stmt = conn.prepare( - "SELECT t.id, t.title, t.artist, t.path, a.art_path, MAX(p.started_at) + "SELECT a.id, a.title, a.artist, a.art_path, COUNT(DISTINCT t.id), MAX(p.started_at) FROM plays p JOIN tracks t ON t.title = p.title COLLATE NOCASE AND t.artist = p.artist COLLATE NOCASE JOIN albums a ON a.id = t.album_id - GROUP BY t.id + GROUP BY a.id ORDER BY MAX(p.started_at) DESC + LIMIT 12", + )?; + if let Ok(rows) = stmt.query_map([], |r| { + let id: i64 = r.get(0)?; + let title: String = r.get(1)?; + let artist: String = r.get(2)?; + let art: Option = r.get(3)?; + let n: i64 = r.get(4)?; + Ok(Node { + id: format!("ss:album:{id}"), + title, + subtitle: format!("{artist} · {}", plural(n, "track", "tracks")), + browsable: true, + playable: false, + art, + path: None, + }) + }) { + out.extend(rows.flatten()); + } + + out.extend(playlists(conn).unwrap_or_default()); + + // A library that has never been played is not an empty shelf. On a fresh + // install — or a phone whose history was wiped — "Recent" would otherwise + // be the first thing a driver sees and it would say "No items" over a full + // library. Newest first is a defensible reading of recent either way. + if out.is_empty() { + out.extend(albums(conn)?.into_iter().take(12)); + } + Ok(out) +} + +/// Playlists, newest first, as browsable rows. +/// +/// Counted out of `playlist_items` rather than `playlist_tracks`: it is the +/// richer table, and the one that knows whether an entry is a local file +/// (`kind`/`path`) or something that needs the resolver. +fn playlists(conn: &Connection) -> rusqlite::Result> { + let mut stmt = conn.prepare( + "SELECT p.id, p.name, COUNT(i.id), + SUM(CASE WHEN i.kind = 'local' AND i.path IS NOT NULL THEN 1 ELSE 0 END), + MAX(i.art_path) + FROM playlists p LEFT JOIN playlist_items i ON i.playlist_id = p.id + GROUP BY p.id + ORDER BY p.created_at DESC LIMIT ?1", )?; - let rows = stmt.query_map([ROW_CAP as i64], |r| Ok(track_node(r)?))?; + let rows = stmt.query_map([ROW_CAP as i64], |r| { + let id: i64 = r.get(0)?; + let name: String = r.get(1)?; + let n: i64 = r.get(2)?; + let local: i64 = r.get(3).unwrap_or(0); + let art: Option = r.get(4)?; + // Say plainly when part of a playlist cannot play here. A row that + // silently plays four of its nine tracks is worse than one that says + // so before it is tapped. + let sub = if local < n { + format!("Playlist · {} of {} here", local, n) + } else { + format!("Playlist · {}", plural(n, "track", "tracks")) + }; + Ok(Node { + id: format!("ss:playlist:{id}"), + title: name, + subtitle: sub, + browsable: true, + playable: false, + art, + path: None, + }) + })?; + rows.collect() +} + +/// A playlist's entries, in order. Only the local ones are playable; the rest +/// need the resolver, which does not exist in a process with no WebView, and +/// are shown rather than hidden so the list matches the phone's. +fn playlist_items(conn: &Connection, playlist_id: i64) -> rusqlite::Result> { + let mut stmt = conn.prepare( + "SELECT id, title, artist, album, path, art_path, kind + FROM playlist_items + WHERE playlist_id = ?1 + ORDER BY position", + )?; + let rows = stmt.query_map([playlist_id], |r| { + let id: i64 = r.get(0)?; + let title: String = r.get(1)?; + let artist: String = r.get(2)?; + let album: Option = r.get(3)?; + let path: Option = r.get(4)?; + let art: Option = r.get(5)?; + let kind: String = r.get(6)?; + let local = kind == "local" && path.is_some(); + let sub = match (&album, local) { + (_, false) => format!("{artist} · on your phone"), + (Some(al), true) if !al.is_empty() => format!("{artist} · {al}"), + _ => artist.clone(), + }; + Ok(Node { + id: format!("ss:pitem:{id}"), + title, + subtitle: sub, + browsable: false, + playable: local, + art, + path, + }) + })?; rows.collect() } @@ -318,6 +428,11 @@ fn children(conn: &Connection, parent: &str) -> rusqlite::Result> { Ok(n) => album_tracks(conn, n), Err(_) => Ok(Vec::new()), } + } else if let Some(id) = parent.strip_prefix("ss:playlist:") { + match id.parse::() { + Ok(n) => playlist_items(conn, n), + Err(_) => Ok(Vec::new()), + } } else if let Some(name) = parent.strip_prefix("ss:artist:") { // The rest of the id is the artist's name verbatim, however // many colons it happens to contain. @@ -380,6 +495,49 @@ pub(crate) fn resolve_playable( let rows = album_tracks(id.parse::().ok()?).ok()?; return Some((rows.into_iter().map(|(_, t)| t).collect(), 0)); } + // A playlist plays its LOCAL entries, in order, skipping the ones that + // need the resolver. Silently dropping them is the right call here: the + // row that led here already said "4 of 9 here", so the gap was declared + // before the tap rather than discovered after it. + let playlist_local = |playlist_id: i64| -> rusqlite::Result> { + let mut stmt = conn.prepare( + "SELECT id, title, artist, album, duration_secs, path, art_path + FROM playlist_items + WHERE playlist_id = ?1 AND kind = 'local' AND path IS NOT NULL + ORDER BY position", + )?; + let rows = stmt.query_map([playlist_id], |r| { + Ok(( + r.get::<_, i64>(0)?, + PlayTrack { + title: r.get(1)?, + artist: r.get(2)?, + album: r.get::<_, Option>(3)?.unwrap_or_default(), + duration: r.get::<_, Option>(4)?.map(|s| s as f64), + path: PathBuf::from(r.get::<_, String>(5)?), + art: r.get(6)?, + }, + )) + })?; + rows.collect() + }; + if let Some(id) = media_id.strip_prefix("ss:playlist:") { + let rows = playlist_local(id.parse::().ok()?).ok()?; + return Some((rows.into_iter().map(|(_, t)| t).collect(), 0)); + } + if let Some(id) = media_id.strip_prefix("ss:pitem:") { + let item_id = id.parse::().ok()?; + let playlist_id: i64 = conn + .query_row( + "SELECT playlist_id FROM playlist_items WHERE id = ?1", + [item_id], + |r| r.get(0), + ) + .ok()?; + let rows = playlist_local(playlist_id).ok()?; + let idx = rows.iter().position(|(i, _)| *i == item_id).unwrap_or(0); + return Some((rows.into_iter().map(|(_, t)| t).collect(), idx)); + } if let Some(id) = media_id.strip_prefix("ss:track:") { let track_id = id.parse::().ok()?; let album_id: i64 = conn diff --git a/tools/auto-prober/probe.sh b/tools/auto-prober/probe.sh index b75fdad..7f9fbe6 100644 --- a/tools/auto-prober/probe.sh +++ b/tools/auto-prober/probe.sh @@ -46,7 +46,7 @@ echo "== cold: force-stopping SoulStream $(date +%T)" "$ADB" logcat -c "$ADB" shell am force-stop com.soulstream.app "$ADB" shell am start -W -n com.soulstream.autoprober/.ProbeActivity \ - --ei depth "$DEPTH" --ez recent "$RECENT" --es play "$PLAY" > /dev/null + --ei depth "$DEPTH" --ez recent "$RECENT" ${PLAY:+--es play "$PLAY"} > /dev/null if [ -n "$PLAY" ]; then sleep 25; else sleep 7; fi echo