michaelherger / lms-plugin-tidal

A TIDAL plugin to integrate with the Lyrion Music Server (fka. Logitech Media Server)
43 stars 8 forks source link

Add direct links to the current album and artist to the more menu #60

Closed kaste closed 6 months ago

kaste commented 6 months ago

That's the start of the more menu:

image

I still think searching for an artist/album/song name still relevant; but the direct links are very common.

kaste commented 6 months ago

Help wanted: I have a hard time with the strings.txt

These throw all the time now.

[24-03-19 21:53:22.6453] Slim::Display::Display::string (887) Error: missing string PLUGIN_TIDAL_TRACK_MIX
[24-03-19 21:53:22.6460] Slim::Display::Display::string (887) Backtrace:

   frame 0: Slim::Utils::Log::logBacktrace (/usr/share/perl5/Slim/Display/Display.pm line 887)
   frame 1: Slim::Display::Display::string (/usr/share/perl5/Slim/Player/Player.pm line 192)
   frame 2: Slim::Player::Player::string (/usr/share/perl5/Slim/Utils/Strings.pm line 548)
   frame 3: Slim::Utils::Strings::clientString (/var/lib/squeezeboxserver/cache/InstalledPlugins/Plugins/TIDAL/Plugin.pm line 313)
   frame 4: Plugins::TIDAL::Plugin::trackInfoMenu (/usr/share/perl5/Slim/Menu/TrackInfo.pm line 279)
   frame 5: (eval) (/usr/share/perl5/Slim/Menu/TrackInfo.pm line 279)
   frame 6: Slim::Menu::TrackInfo::__ANON__ (/usr/share/perl5/Slim/Menu/TrackInfo.pm line 313)
   frame 7: Slim::Menu::TrackInfo::menu (/usr/share/perl5/Slim/Menu/TrackInfo.pm line 1374)
   frame 8: Slim::Menu::TrackInfo::cliQuery (/usr/share/perl5/Slim/Control/Request.pm line 1875)
   frame 9: (eval) (/usr/share/perl5/Slim/Control/Request.pm line 1875)
   frame 10: Slim::Control::Request::execute (/usr/share/perl5/Slim/Web/JSONRPC.pm line 475)
   frame 11: Slim::Web::JSONRPC::requestMethod (/usr/share/perl5/Slim/Web/JSONRPC.pm line 274)
   frame 12: (eval) (/usr/share/perl5/Slim/Web/JSONRPC.pm line 274)
   frame 13: Slim::Web::JSONRPC::handleURI (/usr/share/perl5/Slim/Web/HTTP.pm line 484)
   frame 14: Slim::Web::HTTP::processHTTP (/usr/share/perl5/Slim/Networking/IO/Select.pm line 122)
   frame 15: (eval) (/usr/share/perl5/Slim/Networking/IO/Select.pm line 118)
   frame 16: Slim::Networking::IO::Select::__ANON__ (/usr/share/perl5/Slim/Networking/IO/Select.pm line 167)
   frame 17: (eval) (/usr/share/perl5/Slim/Networking/IO/Select.pm line 167)
   frame 18: Slim::Networking::IO::Select::loop (/usr/sbin/squeezeboxserver line 726)
   frame 19: main::idle (/usr/sbin/squeezeboxserver line 675)
   frame 20: main::main (/usr/sbin/squeezeboxserver line 1214)

as you can see in the OP pic I don't get the translations but the raw name (e.g. PLUGIN_TIDAL_TRACK_MIX). Is that a whitespace issue? I write the code on Windows but ssh into my raspberry DietPi box.

michaelherger commented 6 months ago

Do you see that stack trace right after the server start? That string should be ok (unless you've modified since the latest merge)?

kaste commented 6 months ago

Not after restart. I just need to open the TIDAL menu where these translation strings are used. Then it throws and the constants are displayed. I have a strings.txt file that works but current main does not. Probably only on my side to be clear.

michaelherger commented 6 months ago

It could be a whitespace issue. Make sure you're always using tabs to separate language code and content. It's working fine here:

Screenshot 2024-03-20 at 06 48 26
michaelherger commented 6 months ago

Seeing that screenshot I believe I'll have to change the strings anyway 😀. You're German speaking, aren't you? This doesn't sound right to me...

kaste commented 6 months ago

Oh, das ist übel.

Aber wir haben dann ein gender Problem. 'Such nach dem Künstler "Taylor Swift"' ist inakzeptabel heutzutage. Langform ist wohl 'Suche nach "Taylor Swift" in der Kategorie "Künstler".' ... 'Suche nach "Taylor Swift" unter Interpreten'.

Ideally we have " and not ' too for the quotes.

I have problems with the strings.txt from the beginning. I have the repo cloned in Windows, obviously using a Windows git. And then I've added a worktree using a mapped drive (fftp over ssh). I kept my normal workflow, so I edit the files directly on the mapped drive using that worktree.

kaste commented 6 months ago

Why ist that looking so nice (accessed via My Music):

image

while we only have this for the album view:

image

And which handler is called when I click "More" on this album track listing as currently we only get the url on more. But none of our three handlers is called here:

image

Can I make post requests somehow? That would be needed for liking/unliking.

philippe44 commented 6 months ago

You might want to look at what I did for Deezer exactly for the same purpose. Maybe I missed something, but I had to do a fair bit of code to have a nice-looking 'More' menu (for any track, playing or not)

kaste commented 6 months ago

None of these https://github.com/michaelherger/lms-plugin-tidal/blob/73eace98cd90cf913b99f6d786bc8f257ffca707/Plugin.pm#L43-L57 trigger. So is there a fourth handler? Or a custom one. When we show the tracks they have probably a menu/goto handler already attached to their data.

kaste commented 6 months ago

The Deezer plugin is already complex, and you don't have atomic commits (neither here nor there) that explain how the changes work out (tbf).

philippe44 commented 6 months ago

Most of the related code is in info.pm. And yes, no atomic commit at this point because I've spent a lot of time and effort in a big rush just to provide something to this community. As the plugin share the same core made by michael, I'm just offering help, but you don't have to take it.

michaelherger commented 6 months ago

Those menu items are triggered when you hit the "More" menu on a track from your "My Music" menu.

kaste commented 6 months ago

@michaelherger Ah yeah, I come to that later. Just focus on what's in this PR. @philippe44 I appreciate the help. I maybe know how this should work out. The indirection that we call Slim::Control::XMLBrowser::cliQuery in our handlers, and the form $items then takes, is surprising. Would like to see the documentation/more code for the "type -> opml" thing.

michaelherger commented 6 months ago

Has your string token issue been resolved? Is this ready to be merged?

Thanks @kaste!

kaste commented 6 months ago

Yes. And yes.

michaelherger commented 6 months ago

Thanks a lot!

kaste commented 6 months ago

I don't think the UI is pretty enough tbh. After your suggested changes I have the artists pictures but just a general icon on the album, like so on my phone:

image

and that just doesn't look right to me. WDYT? Maybe ideally we had Search icons for the three search variants; and the album artwork again for the direct link to the album. (?)

michaelherger commented 6 months ago

Agreed, that looks odd. I'd support your suggestions. Is this the committed code, or some change you have on your end? I probably commented on the icons as I wasn't aware of the others' existence?

kaste commented 6 months ago

That's the code in main ("committed and pulled").

With the following diff

diff --git a/Plugin.pm b/Plugin.pm
index 9ed4abd..bde33a5 100644
--- a/Plugin.pm
+++ b/Plugin.pm
@@ -296,12 +296,10 @@ sub trackInfoMenu {

    push @$items, {
        name => $album,
-       line1 => $album,
-       line2 => $artist,
        favorites_url => 'tidal://album:' . $albumId,
        type => 'playlist',
        url => \&getAlbum,
-       image => 'html/images/albums.png',
+       image => Plugins::TIDAL::API->getImageUrl($remoteMeta, 'usePlaceholder'),
        passthrough => [{ id => $albumId }],
    } if $albumId;

it becomes

image

It also would remove the artist name on line2 as that's should be almost or always just a duplication of the text to the next link, the one bringing you to the artists page.

WDYT?