michaelherger / lms-plugin-tidal

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

Default action is not "play" anymore #68

Closed kaste closed 3 months ago

kaste commented 3 months ago

I'm comparing 7a5ac77 with current main at 64acec0. (T.i. after the big changes introduced by @philippe44)

On current main, clicking on an item (song) does NOT start the playback at that song, loading the complete album into the queue.

https://github.com/michaelherger/lms-plugin-tidal/assets/8558/aacef12b-f488-455f-86de-5e3574dd8d11

The video shows SqueezePlay but the Squeezebox Touch behaves the same. And also the Material Skin browser on my phone behaves similar in that it only opens the context/long press menu.

On 7a5ac77 (t.i. my last PR checked in), it looks like this:

https://github.com/michaelherger/lms-plugin-tidal/assets/8558/9239b4cf-6a6e-4a1b-af32-934fc4de2337

philippe44 commented 3 months ago

Yo have to long press it

kaste commented 3 months ago

The long press opens the menu as always, but the short tap does nothing. Before the change a short tap enqueued the whole album and started playback at the song I tapped.

philippe44 commented 3 months ago

Yes I know but that's the only option to get an info menu with links to navigate through other related items as well as allowing adding the track to favs and other playlist. Same for Deezer plugin

kaste commented 3 months ago

Oh, that's a bummer. My (retarded) child can't use the menu.

Can we at least re-arrange the items in the menu so that "play" is the first item?

philippe44 commented 3 months ago

I might have found a solution with Jive-specific code

philippe44 commented 3 months ago

If you edit Plugin.pm::renderTrack and add, after the "itemAction"

jive => {
            nextWindow => 'nowPlaying',
            actions => {
                go => {
                    player => 0,
                    cmd    => [ 'tidal_browse', 'playlist', 'play' ],
                    params => { id => $item->{id} },
                }
            },
        },

Does it do what you want?

kaste commented 3 months ago

I have this diff now

diff --git a/Plugin.pm b/Plugin.pm
index b2cf7f3..f640df9 100644
--- a/Plugin.pm
+++ b/Plugin.pm
@@ -915,6 +915,16 @@ sub _renderTrack {
                },
            },
        },
+       jive => {
+           nextWindow => 'nowPlaying',
+           actions => {
+               go => {
+                   player => 0,
+                   cmd    => [ 'tidal_browse', 'playlist', 'play' ],
+                   params => { id => $item->{id} },
+               }
+           },
+       },
    };
 }

but that doesn't change anything on tap, i.e. it does nothing. It breaks the long tap with the following LOG:

[24-04-01 11:41:18.6905] Slim::Control::Request::execute (1880) Error: While trying to run function coderef [Plugins::TIDAL::InfoMenu::menuInfoWeb]: [Can't use an undefined value as a subroutine reference at /var/lib/squeezeboxserver/cache/InstalledPlugins/Plugins/TIDAL/InfoMenu.pm line 147.
]
philippe44 commented 3 months ago

Oh, I did not check that, but did you also use my most recent PR. It requires the full extended browse option.

kaste commented 3 months ago

I tried with 04bbc94 just now with no success. The strange UX is that when you browse "my music -> albums" it works and starts playback just by tapping on a track, but when you browse "tidal -> albums" it switches to the "now playing" view but nothing happens actually.

philippe44 commented 3 months ago

This is weird as this is just what I tried on my squeezeplay. What is the setting of defeat touch to play ?

philippe44 commented 3 months ago

No, you're right this is still no the correct fix. I'll look at that

philippe44 commented 3 months ago

I hope this is the right one https://github.com/michaelherger/lms-plugin-tidal/pull/71

michaelherger commented 3 months ago

Fixed via https://github.com/michaelherger/lms-plugin-tidal/commit/7dd217e09327959323b243c85ced459ec32abd82.

kaste commented 3 months ago

You're gold here. You both are.