From 941012b7d298d0ea002a89a9c26a300f60316375 Mon Sep 17 00:00:00 2001 From: Drew Weymouth Date: Thu, 16 Feb 2023 17:55:33 -0800 Subject: [PATCH 1/2] make sure calls to server are async in UI callbacks; fix genre page not playing albums regression --- ui/browsing/albumpage.go | 30 +++++++++++------------ ui/browsing/albumspage.go | 2 +- ui/browsing/artistpage.go | 41 ++++++++++++++++---------------- ui/browsing/artistsgenrespage.go | 7 +++--- ui/browsing/favoritespage.go | 2 +- ui/browsing/genrepage.go | 11 +++++---- ui/browsing/nowplayingpage.go | 17 +++++++------ ui/browsing/playlistpage.go | 30 +++++++++++------------ ui/browsing/router.go | 2 +- 9 files changed, 72 insertions(+), 70 deletions(-) diff --git a/ui/browsing/albumpage.go b/ui/browsing/albumpage.go index 639a7e8..12b7dd9 100644 --- a/ui/browsing/albumpage.go +++ b/ui/browsing/albumpage.go @@ -77,7 +77,7 @@ func NewAlbumPage( container.New(&layouts.MaxPadLayout{PadLeft: 15, PadRight: 15, PadTop: 15, PadBottom: 10}, a.header), nil, nil, nil, container.New(&layouts.MaxPadLayout{PadLeft: 15, PadRight: 15, PadBottom: 15}, a.tracklist)) - a.loadAsync() + go a.load() return a } @@ -105,7 +105,7 @@ func (a *AlbumPage) OnSongChange(song *subsonic.Child, lastScrobbledIfAny *subso } func (a *AlbumPage) Reload() { - a.loadAsync() + go a.load() } func (a *AlbumPage) Tapped(*fyne.PointEvent) { @@ -117,20 +117,20 @@ func (a *AlbumPage) SelectAll() { } func (a *AlbumPage) onPlayTrackAt(tracknum int) { - a.pm.PlayAlbum(a.albumID, tracknum) + a.pm.LoadTracks(a.tracklist.Tracks, false, false) + a.pm.PlayTrackAt(tracknum) } -func (a *AlbumPage) loadAsync() { - go func() { - album, err := a.lm.GetAlbum(a.albumID) - if err != nil { - log.Printf("Failed to get album: %s", err.Error()) - return - } - a.header.Update(album, a.im) - a.tracklist.Tracks = album.Song - a.tracklist.SetNowPlaying(a.nowPlayingID) - }() +// should be called asynchronously +func (a *AlbumPage) load() { + album, err := a.lm.GetAlbum(a.albumID) + if err != nil { + log.Printf("Failed to get album: %s", err.Error()) + return + } + a.header.Update(album, a.im) + a.tracklist.Tracks = album.Song + a.tracklist.SetNowPlaying(a.nowPlayingID) } type AlbumPageHeader struct { @@ -156,7 +156,7 @@ type AlbumPageHeader struct { func NewAlbumPageHeader(page *AlbumPage) *AlbumPageHeader { a := &AlbumPageHeader{page: page} a.ExtendBaseWidget(a) - a.cover = widgets.NewTappableImage(a.showPopUpCover) + a.cover = widgets.NewTappableImage(func() { go a.showPopUpCover() }) a.cover.FillMode = canvas.ImageFillContain a.cover.SetMinSize(fyne.NewSize(225, 225)) // due to cache warming we can probably immediately set the cover diff --git a/ui/browsing/albumspage.go b/ui/browsing/albumspage.go index c5b83c8..5d0b2a1 100644 --- a/ui/browsing/albumspage.go +++ b/ui/browsing/albumspage.go @@ -188,7 +188,7 @@ func (a *AlbumsPage) doSearch(query string) { } func (a *AlbumsPage) onPlayAlbum(albumID string) { - a.pm.PlayAlbum(albumID, 0) + go a.pm.PlayAlbum(albumID, 0) } func (a *AlbumsPage) onShowArtistPage(artistID string) { diff --git a/ui/browsing/artistpage.go b/ui/browsing/artistpage.go index b4f66a5..3f2d119 100644 --- a/ui/browsing/artistpage.go +++ b/ui/browsing/artistpage.go @@ -54,7 +54,7 @@ func NewArtistPage(artistID string, sm *backend.ServerManager, im *backend.Image a.container = container.NewBorder( container.New(&layouts.MaxPadLayout{PadLeft: 15, PadRight: 15, PadTop: 15, PadBottom: 10}, a.header), nil, nil, nil, layout.NewSpacer()) - a.loadAsync() + go a.load() return a } @@ -67,7 +67,7 @@ func (a *ArtistPage) SetPlayAlbumCallback(cb func(string, int)) { } func (a *ArtistPage) Reload() { - a.loadAsync() + go a.load() } func (a *ArtistPage) Save() SavedPage { @@ -85,25 +85,24 @@ func (a *ArtistPage) onShowAlbumPage(albumID string) { a.nav(AlbumRoute(albumID)) } -func (a *ArtistPage) loadAsync() { - go func() { - artist, err := a.sm.Server.GetArtist(a.artistID) - if err != nil { - log.Printf("Failed to get artist: %s", err.Error()) - return - } - a.header.Update(artist) - ag := widgets.NewFixedAlbumGrid(artist.Album, a.im, true /*showYear*/) - ag.OnPlayAlbum = a.onPlayAlbum - ag.OnShowAlbumPage = a.onShowAlbumPage - a.container.Objects[0] = ag - a.container.Refresh() - info, err := a.sm.Server.GetArtistInfo2(a.artistID, nil) - if err != nil { - log.Printf("Failed to get artist info: %s", err.Error()) - } - a.header.UpdateInfo(info) - }() +// should be called asynchronously +func (a *ArtistPage) load() { + artist, err := a.sm.Server.GetArtist(a.artistID) + if err != nil { + log.Printf("Failed to get artist: %s", err.Error()) + return + } + a.header.Update(artist) + ag := widgets.NewFixedAlbumGrid(artist.Album, a.im, true /*showYear*/) + ag.OnPlayAlbum = a.onPlayAlbum + ag.OnShowAlbumPage = a.onShowAlbumPage + a.container.Objects[0] = ag + a.container.Refresh() + info, err := a.sm.Server.GetArtistInfo2(a.artistID, nil) + if err != nil { + log.Printf("Failed to get artist info: %s", err.Error()) + } + a.header.UpdateInfo(info) } func (a *ArtistPage) CreateRenderer() fyne.WidgetRenderer { diff --git a/ui/browsing/artistsgenrespage.go b/ui/browsing/artistsgenrespage.go index 4ce0b56..b10cdb1 100644 --- a/ui/browsing/artistsgenrespage.go +++ b/ui/browsing/artistsgenrespage.go @@ -51,11 +51,12 @@ func NewArtistsGenresPage(isGenresPage bool, sm *backend.ServerManager, nav func } } a.buildContainer() - go a.loadAsync() + go a.load() return a } -func (a *ArtistsGenresPage) loadAsync() { +// should be called asynchronously +func (a *ArtistsGenresPage) load() { if a.isGenresPage { genres, err := a.sm.Server.GetGenres() if err != nil { @@ -80,7 +81,7 @@ func (a *ArtistsGenresPage) Route() Route { } func (a *ArtistsGenresPage) Reload() { - go a.loadAsync() + go a.load() } func (a *ArtistsGenresPage) Save() SavedPage { diff --git a/ui/browsing/favoritespage.go b/ui/browsing/favoritespage.go index 16f6832..9e3e416 100644 --- a/ui/browsing/favoritespage.go +++ b/ui/browsing/favoritespage.go @@ -140,7 +140,7 @@ func (a *FavoritesPage) doSearch(query string) { } func (a *FavoritesPage) onPlayAlbum(albumID string) { - a.pm.PlayAlbum(albumID, 0) + go a.pm.PlayAlbum(albumID, 0) } func (a *FavoritesPage) onShowAlbumPage(albumID string) { diff --git a/ui/browsing/genrepage.go b/ui/browsing/genrepage.go index 5d68e78..b75b5ba 100644 --- a/ui/browsing/genrepage.go +++ b/ui/browsing/genrepage.go @@ -18,6 +18,7 @@ type GenrePage struct { genre string im *backend.ImageManager + pm *backend.PlaybackManager lm *backend.LibraryManager nav func(Route) grid *widgets.AlbumGrid @@ -31,9 +32,10 @@ type GenrePage struct { container *fyne.Container } -func NewGenrePage(genre string, lm *backend.LibraryManager, im *backend.ImageManager, nav func(Route)) *GenrePage { +func NewGenrePage(genre string, pm *backend.PlaybackManager, lm *backend.LibraryManager, im *backend.ImageManager, nav func(Route)) *GenrePage { g := &GenrePage{ genre: genre, + pm: pm, lm: lm, im: im, nav: nav, @@ -74,6 +76,7 @@ func (g *GenrePage) createContainer(searchGrid bool) { func restoreGenrePage(saved *savedGenrePage) *GenrePage { g := &GenrePage{ genre: saved.genre, + pm: saved.pm, lm: saved.lm, im: saved.im, nav: saved.nav, @@ -121,6 +124,7 @@ func (g *GenrePage) Save() SavedPage { sg := &savedGenrePage{ genre: g.genre, searchText: g.searchText, + pm: g.pm, lm: g.lm, im: g.im, nav: g.nav, @@ -139,9 +143,7 @@ func (g *GenrePage) SearchWidget() fyne.Focusable { } func (a *GenrePage) onPlayAlbum(albumID string) { - if a.OnPlayAlbum != nil { - a.OnPlayAlbum(albumID, 0) - } + go a.pm.PlayAlbum(albumID, 0) } func (a *GenrePage) onShowArtistPage(artistID string) { @@ -184,6 +186,7 @@ func (g *GenrePage) doSearch(query string) { type savedGenrePage struct { genre string searchText string + pm *backend.PlaybackManager lm *backend.LibraryManager im *backend.ImageManager nav func(Route) diff --git a/ui/browsing/nowplayingpage.go b/ui/browsing/nowplayingpage.go index 377cff4..6fc93e7 100644 --- a/ui/browsing/nowplayingpage.go +++ b/ui/browsing/nowplayingpage.go @@ -52,7 +52,7 @@ func NewNowPlayingPage( a.title.Segments[0].(*widget.TextSegment).Style.SizeName = widget.RichTextStyleHeading.SizeName a.container = container.New(&layouts.MaxPadLayout{PadLeft: 15, PadRight: 15, PadTop: 5, PadBottom: 15}, container.NewBorder(a.title, nil, nil, nil, a.tracklist)) - a.loadAsync() + a.load() return a } @@ -88,7 +88,7 @@ func (a *NowPlayingPage) OnSongChange(song *subsonic.Child, lastScrobbledIfAny * } func (a *NowPlayingPage) Reload() { - a.loadAsync() + a.load() } func (a *NowPlayingPage) onPlayTrackAt(tracknum int) { @@ -98,15 +98,14 @@ func (a *NowPlayingPage) onPlayTrackAt(tracknum int) { func (a *NowPlayingPage) onRemoveSelectedFromQueue() { a.pm.RemoveTracksFromQueue(a.tracklist.SelectedTrackIndexes()) a.tracklist.UnselectAll() - go a.Reload() + a.Reload() } -func (a *NowPlayingPage) loadAsync() { - go func() { - queue := a.pm.GetPlayQueue() - a.tracklist.Tracks = queue - a.tracklist.SetNowPlaying(a.nowPlayingID) - }() +// does not make calls to server - can safely be run in UI callbacks +func (a *NowPlayingPage) load() { + queue := a.pm.GetPlayQueue() + a.tracklist.Tracks = queue + a.tracklist.SetNowPlaying(a.nowPlayingID) } func (s *nowPlayingPageState) Restore() Page { diff --git a/ui/browsing/playlistpage.go b/ui/browsing/playlistpage.go index 0db1cb8..9b42426 100644 --- a/ui/browsing/playlistpage.go +++ b/ui/browsing/playlistpage.go @@ -67,7 +67,7 @@ func NewPlaylistPage( a.container = container.NewBorder( container.New(&layouts.MaxPadLayout{PadLeft: 15, PadRight: 15, PadTop: 15, PadBottom: 10}, a.header), nil, nil, nil, container.New(&layouts.MaxPadLayout{PadLeft: 15, PadRight: 15, PadBottom: 15}, a.tracklist)) - a.loadAsync() + go a.load() return a } @@ -95,7 +95,7 @@ func (a *PlaylistPage) OnSongChange(song *subsonic.Child, lastScrobbledIfAny *su } func (a *PlaylistPage) Reload() { - a.loadAsync() + go a.load() } func (a *PlaylistPage) Tapped(*fyne.PointEvent) { @@ -107,21 +107,21 @@ func (a *PlaylistPage) SelectAll() { } func (a *PlaylistPage) onPlayTrackAt(tracknum int) { - a.pm.PlayPlaylist(a.playlistID, tracknum) + a.pm.LoadTracks(a.tracklist.Tracks, false, false) + a.pm.PlayTrackAt(tracknum) } -func (a *PlaylistPage) loadAsync() { - go func() { - playlist, err := a.sm.Server.GetPlaylist(a.playlistID) - if err != nil { - log.Printf("Failed to get playlist: %s", err.Error()) - return - } - a.tracklist.Tracks = playlist.Entry - a.tracklist.SetNowPlaying(a.nowPlayingID) - a.tracklist.Refresh() - a.header.Update(playlist) - }() +// should be called asynchronously +func (a *PlaylistPage) load() { + playlist, err := a.sm.Server.GetPlaylist(a.playlistID) + if err != nil { + log.Printf("Failed to get playlist: %s", err.Error()) + return + } + a.tracklist.Tracks = playlist.Entry + a.tracklist.SetNowPlaying(a.nowPlayingID) + a.tracklist.Refresh() + a.header.Update(playlist) } func (a *PlaylistPage) onRemoveSelectedFromPlaylist() { diff --git a/ui/browsing/router.go b/ui/browsing/router.go index ae006b2..0146a99 100644 --- a/ui/browsing/router.go +++ b/ui/browsing/router.go @@ -97,7 +97,7 @@ func (r Router) CreatePage(rte Route) Page { case Favorites: return NewFavoritesPage(r.App.ServerManager, r.App.PlaybackManager, r.App.LibraryManager, r.App.ImageManager, r.OpenRoute) case Genre: - return NewGenrePage(rte.Arg, r.App.LibraryManager, r.App.ImageManager, r.OpenRoute) + return NewGenrePage(rte.Arg, r.App.PlaybackManager, r.App.LibraryManager, r.App.ImageManager, r.OpenRoute) case Genres: return NewArtistsGenresPage(true, r.App.ServerManager, r.OpenRoute) case NowPlaying: From 28236ee08c52b3e78e7ab99b9c9d31fc44a007c4 Mon Sep 17 00:00:00 2001 From: Drew Weymouth Date: Thu, 16 Feb 2023 18:02:34 -0800 Subject: [PATCH 2/2] make shuffle and favorite button callbacks non-blocking --- ui/browsing/albumpage.go | 4 ++-- ui/browsing/playlistpage.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ui/browsing/albumpage.go b/ui/browsing/albumpage.go index 12b7dd9..e37e979 100644 --- a/ui/browsing/albumpage.go +++ b/ui/browsing/albumpage.go @@ -182,10 +182,10 @@ func NewAlbumPageHeader(page *AlbumPage) *AlbumPageHeader { page.onPlayTrackAt(0) }) shuffleBtn := widget.NewButtonWithIcon(" Shuffle", res.ResShuffleInvertSvg, func() { - page.pm.LoadAlbum(page.albumID, false, true) + page.pm.LoadTracks(page.tracklist.Tracks, false, true) page.pm.PlayFromBeginning() }) - a.toggleFavButton = widgets.NewFavoriteButton(a.toggleFavorited) + a.toggleFavButton = widgets.NewFavoriteButton(func() { go a.toggleFavorited() }) // Todo: there's got to be a way to make this less convoluted. Custom layout? a.container = container.NewBorder(nil, nil, a.cover, nil, diff --git a/ui/browsing/playlistpage.go b/ui/browsing/playlistpage.go index 9b42426..1a70ea6 100644 --- a/ui/browsing/playlistpage.go +++ b/ui/browsing/playlistpage.go @@ -165,7 +165,7 @@ func NewPlaylistPageHeader(page *PlaylistPage) *PlaylistPageHeader { }) // TODO: find way to pad shuffle svg rather than using a space in the label string shuffleBtn := widget.NewButtonWithIcon(" Shuffle", res.ResShuffleInvertSvg, func() { - page.pm.LoadPlaylist(page.playlistID, false /*append*/, true /*shuffle*/) + page.pm.LoadTracks(page.tracklist.Tracks, false /*append*/, true /*shuffle*/) page.pm.PlayFromBeginning() })