Skip to content

Commit

Permalink
Fix for missing album metadata when song played from album (#581)
Browse files Browse the repository at this point in the history
Fixes #425 and #368
  • Loading branch information
jacksongoode authored Jan 30, 2025
1 parent 8b9f719 commit 5934e94
Show file tree
Hide file tree
Showing 9 changed files with 143 additions and 34 deletions.
12 changes: 12 additions & 0 deletions psst-gui/src/data/album.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,18 @@ impl Album {
pub fn has_explicit(&self) -> bool {
self.tracks.iter().any(|t| t.explicit)
}

pub fn into_tracks_with_context(self: Arc<Self>) -> Vector<Arc<Track>> {
let album_link = self.link();
self.tracks
.iter()
.map(|track| {
let mut track = track.as_ref().clone();
track.album = Some(album_link.clone());
Arc::new(track)
})
.collect()
}
}

#[derive(Clone, Debug, Data, Lens, Eq, PartialEq, Hash, Deserialize, Serialize)]
Expand Down
25 changes: 25 additions & 0 deletions psst-gui/src/data/playback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,31 @@ impl NowPlaying {
Playable::Episode(episode) => Some(&episode.image(width, height)?.url),
}
}

pub fn cover_image_metadata(&self) -> Option<(&str, (u32, u32))> {
match &self.item {
Playable::Track(track) => track.album.as_ref().and_then(|album| {
album.images.get(0).map(|img| {
(
&*img.url,
(
img.width.unwrap_or(0) as u32,
img.height.unwrap_or(0) as u32,
),
)
})
}),
Playable::Episode(episode) => episode.images.get(0).map(|img| {
(
&*img.url,
(
img.width.unwrap_or(0) as u32,
img.height.unwrap_or(0) as u32,
),
)
}),
}
}
}

#[derive(Clone, Debug, Data)]
Expand Down
29 changes: 29 additions & 0 deletions psst-gui/src/delegate.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
use directories::UserDirs;
use druid::{
commands, AppDelegate, Application, Command, DelegateCtx, Env, Event, Handled, Target,
WindowDesc, WindowId,
};
use std::fs;
use std::io::Read;
use threadpool::ThreadPool;

use crate::ui::playlist::{
RENAME_PLAYLIST, RENAME_PLAYLIST_CONFIRM, UNFOLLOW_PLAYLIST, UNFOLLOW_PLAYLIST_CONFIRM,
};
use crate::ui::theme;
use crate::ui::DOWNLOAD_ARTWORK;
use crate::{
cmd,
data::{AppState, Config},
Expand Down Expand Up @@ -210,6 +214,31 @@ impl AppDelegate<AppState> for Delegate {
} else if cmd.is(crate::cmd::SHOW_ARTWORK) {
self.show_artwork(ctx);
Handled::Yes
} else if let Some((url, title)) = cmd.get(DOWNLOAD_ARTWORK) {
let safe_title = title.replace(['/', '\\', ':', '*', '?', '"', '<', '>', '|'], "_");
let file_name = format!("{} cover.jpg", safe_title);

if let Some(user_dirs) = UserDirs::new() {
if let Some(download_dir) = user_dirs.download_dir() {
let path = download_dir.join(file_name);

match ureq::get(url)
.call()
.and_then(|response| {
response
.into_reader()
.bytes()
.collect::<Result<Vec<_>, _>>()
.map_err(|e| e.into())
})
.and_then(|bytes| fs::write(&path, bytes).map_err(|e| e.into()))
{
Ok(_) => data.info_alert("Cover saved to Downloads folder."),
Err(_) => data.error_alert("Failed to download and save artwork"),
}
}
}
Handled::Yes
} else {
Handled::No
}
Expand Down
22 changes: 20 additions & 2 deletions psst-gui/src/ui/album.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ use druid::{
use crate::{
cmd,
data::{
Album, AlbumDetail, AlbumLink, AppState, ArtistLink, Cached, Ctx, Library, Nav, WithCtx,
Album, AlbumDetail, AlbumLink, AppState, ArtistLink, Cached, Ctx, Library, Nav, Playable,
PlaybackOrigin, WithCtx,
},
ui::playable::PlayableIter,
webapi::WebApi,
widget::{icons, Async, MyWidgetExt, RemoteImage},
};
Expand Down Expand Up @@ -194,7 +196,7 @@ fn album_menu(album: &Arc<Album>, library: &Arc<Library>) -> Menu<AppState> {
let more_than_one_artist = album.artists.len() > 1;
let title = if more_than_one_artist {
LocalizedString::new("menu-item-show-artist-name")
.with_placeholder(format!("Go to Artist “{}”", artist_link.name))
.with_placeholder(format!("Go to Artist \"{}\"", artist_link.name))
} else {
LocalizedString::new("menu-item-show-artist").with_placeholder("Go to Artist")
};
Expand Down Expand Up @@ -233,3 +235,19 @@ fn album_menu(album: &Arc<Album>, library: &Arc<Library>) -> Menu<AppState> {

menu
}

impl PlayableIter for Arc<Album> {
fn origin(&self) -> PlaybackOrigin {
PlaybackOrigin::Album(self.link())
}

fn for_each(&self, mut cb: impl FnMut(Playable, usize)) {
for (position, track) in self.clone().into_tracks_with_context().iter().enumerate() {
cb(Playable::Track(track.clone()), position);
}
}

fn count(&self) -> usize {
self.tracks.len()
}
}
61 changes: 51 additions & 10 deletions psst-gui/src/ui/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ use crate::{
},
};
use credits::TrackCredits;
use druid::widget::Controller;
use druid::KbKey;
use druid::{
im::Vector,
widget::{CrossAxisAlignment, Either, Flex, Label, List, Scroll, Slider, Split, ViewSwitcher},
Expand Down Expand Up @@ -47,6 +49,8 @@ pub mod track;
pub mod user;
pub mod utils;

pub const DOWNLOAD_ARTWORK: Selector<(String, String)> = Selector::new("app.artwork.download");

pub fn main_window(config: &Config) -> WindowDesc<AppState> {
let win = WindowDesc::new(root_widget())
.title(compute_main_window_title)
Expand Down Expand Up @@ -151,21 +155,58 @@ fn account_setup_widget() -> impl Widget<AppState> {
)
}

fn artwork_widget() -> impl Widget<AppState> {
struct ArtworkController;

impl<W: Widget<AppState>> Controller<AppState, W> for ArtworkController {
fn event(
&mut self,
child: &mut W,
ctx: &mut druid::EventCtx,
event: &druid::Event,
data: &mut AppState,
env: &druid::Env,
) {
if let druid::Event::WindowConnected = event {
ctx.request_focus();
ctx.set_handled();
}

if let druid::Event::KeyDown(key_event) = event {
// Handle D key for download
if key_event.key == KbKey::Character('d'.into()) {
if let Some(np) = &data.playback.now_playing {
if let Some((url, _)) = np.cover_image_metadata() {
let title = match &np.item {
Playable::Track(track) => track
.album
.as_ref()
.map(|a| a.name.as_ref())
.unwrap_or("Unknown Album"),
Playable::Episode(episode) => episode.show.name.as_ref(),
};
ctx.submit_command(
DOWNLOAD_ARTWORK.with((url.to_string(), title.to_string())),
);
ctx.set_handled();
}
}
}
}
child.event(ctx, event, data, env);
}
}

pub fn artwork_widget() -> impl Widget<AppState> {
RemoteImage::new(utils::placeholder_widget(), move |data: &AppState, _| {
let url = data
.playback
data.playback
.now_playing
.as_ref()
.and_then(|np| {
let url = np.cover_image_url(500.0, 500.0);
url
})
.map(|url| url.into());
url
.and_then(|np| np.cover_image_url(512.0, 512.0))
.map(|url| url.into())
})
.expand() // Fill the entire window
.expand()
.background(theme::BACKGROUND_DARK)
.controller(ArtworkController)
}

fn root_widget() -> impl Widget<AppState> {
Expand Down
18 changes: 1 addition & 17 deletions psst-gui/src/ui/playable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use druid::{
use crate::{
cmd,
data::{
Album, ArtistTracks, CommonCtx, FindQuery, MatchFindQuery, Playable, PlaybackOrigin,
ArtistTracks, CommonCtx, FindQuery, MatchFindQuery, Playable, PlaybackOrigin,
PlaybackPayload, PlaylistTracks, Recommendations, SavedTracks, SearchResults, ShowEpisodes,
Track, WithCtx,
},
Expand Down Expand Up @@ -136,22 +136,6 @@ pub trait PlayableIter {
fn for_each(&self, cb: impl FnMut(Playable, usize));
}

impl PlayableIter for Arc<Album> {
fn origin(&self) -> PlaybackOrigin {
PlaybackOrigin::Album(self.link())
}

fn for_each(&self, mut cb: impl FnMut(Playable, usize)) {
for (position, track) in self.tracks.iter().enumerate() {
cb(Playable::Track(track.to_owned()), position);
}
}

fn count(&self) -> usize {
self.tracks.len()
}
}

// This should change to a more specific name as it could be confusing for others
// As at the moment this is only used for the home page!
impl PlayableIter for Vector<Arc<Track>> {
Expand Down
4 changes: 2 additions & 2 deletions psst-gui/src/ui/track.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ pub fn playable_widget(track: &Track, display: Display) -> impl Widget<PlayRow<A
return *target_id == row.item.id;
}
// Otherwise check if it's playing or is the current track
row.is_playing || row.ctx.now_playing.as_ref().map_or(false, |playable| {
row.is_playing || row.ctx.now_playing.as_ref().is_some_and(|playable| {
matches!(playable, Playable::Track(track) if track.id == row.item.id)
})
})
Expand Down Expand Up @@ -239,7 +239,7 @@ pub fn track_menu(
let more_than_one_artist = track.artists.len() > 1;
let title = if more_than_one_artist {
LocalizedString::new("menu-item-show-artist-name")
.with_placeholder(format!("Go to Artist “{}”", artist_link.name))
.with_placeholder(format!("Go to Artist \"{}\"", artist_link.name))
} else {
LocalizedString::new("menu-item-show-artist").with_placeholder("Go to Artist")
};
Expand Down
4 changes: 2 additions & 2 deletions psst-gui/src/webapi/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,11 @@ impl LocalTrackManager {
.artists
.iter()
.next()
.map_or(false, |t2_artist| t2_artist.name != t1.artist);
.is_some_and(|t2_artist| t2_artist.name != t1.artist);
let album_mismatch = t2
.album
.as_ref()
.map_or(false, |t2_album| t2_album.name != t1.album);
.is_some_and(|t2_album| t2_album.name != t1.album);
!(artist_mismatch || album_mismatch)
}
}
Expand Down
2 changes: 1 addition & 1 deletion psst-gui/src/widget/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl<T: Data> Widget<T> for Link<T> {
let is_active = self
.is_active
.as_ref()
.map_or(false, |predicate| predicate(data, env));
.is_some_and(|predicate| predicate(data, env));
if is_active {
env.get(theme::LINK_ACTIVE_COLOR)
} else {
Expand Down

0 comments on commit 5934e94

Please sign in to comment.