Sorting by role #1011

darrell-k · 2024-12-21T13:00:56Z

Add role sort options for Albums and Album-works.

LMS 9.1 from https://github.com/darrell-k/slimserver/tree/sorting-by-role (just updated) is required.

…ist roles

CDrummond · 2024-12-21T18:19:13Z

MaterialSkin/HTML/material/html/js/browse-functions.js

@@ -1882,6 +1882,39 @@ function browseHeaderAction(view, act, event, ignoreOpenMenus) {
        for (var i=0,len=sorts.length; i<len; ++i) {
            menuItems.push({key:sorts[i].key, label:sorts[i].label, selected:sort.by==sorts[i].key});
        }
+
+        if (LMS_VERSION>=90100 && ALBUM_SORTS_ACTION==act) {


Calling this every time the menu shown is inefficient. The result of this call should be stored with the current item, and reused if possible.

I was trying doing this, but as soon as I started pushing to an array other than menuItems, the application would no longer wait for the server command results. The problem is I don't understand how the async stuff works.

It does not 'wait', the menuItems is 'reactive' - so vue.js will update UI when anything changes. What you need to do is copy the (parsed) details of this response somewhere, and also update menuIitems. If this cached version exists, then use that for menuItems.

Ah, I understand, it's in the framework. No wonder I couldn't find it!

OK, I'm about to push some changes which hopefully cache the roles command results correctly.

These changes require the LMS from my updated branch https://github.com/darrell-k/slimserver/tree/sorting-by-role

I've also decided to clear any role sort when changing the view, writing the default to local storage for new views (except if we're viewing all albums, because by definition all roles sorts would be available).

This avoids having a stored sort (in local storage) which might not apply to a new view, because the set of albums do not contain the previously-selected sort role.

There is commented out code for a different approach, which "pretends" the sort is the default 'album' one if the actually used sort is on a role which isn't used in a new album list. This works because the LMS query will return the albums_loop in album name sequence for a role sort where the role doesn't exist for any albums. This means if the user subsequently displays an album list where the previously used sort role exists, it would still be used. But this is a bit messy and possibly fragile.

I suppose the ideal solution would be to have getAlbumSort always validate a role sort against the current albums list, but I think that would mean really having to wait for the LMS rolesQuery to return.

Don't know what you think on this?

Anyway, lots more testing to do, but I thought I'd push this so you can see where I am right now. (And I know I have a few debug messages to remove!)

CDrummond · 2024-12-21T18:19:28Z

MaterialSkin/HTML/material/html/js/browse-resp.js

+                let loopNotNulls = [];
+                let loopNulls = [];
+                const firstNullPos = data.result.albums_loop.map(e => e.rolesort_name).indexOf('');
+                console.log("DK albums_loop firstNullPos="+firstNullPos);


CDrummond · 2024-12-21T18:20:46Z

MaterialSkin/HTML/material/html/js/browse-resp.js

@@ -1010,8 +1010,17 @@ function parseBrowseResp(data, parent, options, cacheKey, parentCommand, parentG
                        }
                    }
                }
+                // sort field for role sorting might be null, if so make sure nulls stay below non-nulls even when reversed.
+                let loopNotNulls = [];


This is only of use if reversed (and not new music), no?

CDrummond · 2024-12-21T18:27:23Z

MaterialSkin/HTML/material/html/js/browse-resp.js

+                // sort field for role sorting might be null, if so make sure nulls stay below non-nulls even when reversed.
+                let loopNotNulls = [];
+                let loopNulls = [];
+                const firstNullPos = data.result.albums_loop.map(e => e.rolesort_name).indexOf('');


Out of curiosity, what impact does this lookup have on loading large lists?

It's bound to slow it down, I'll test with a 25,000 size albums list...

Well then it depends on how much - as this is not something I'd ever really use. However, I really cannot see if having a significant impact. As stated, I was just curious.

You're right, negligible: <1ms on a mid-power laptop. Rendering 25,000 items takes much longer. I suppose it's doing a binary chop on the array.

CDrummond · 2024-12-27T11:57:43Z

MaterialSkin/HTML/material/html/js/browse-functions.js

@@ -2540,7 +2598,8 @@ function browseReplaceCommandTerms(view, cmd, item) {
                }
            } else if (cmd.params[i].startsWith(SORT_KEY+ALBUM_SORT_PLACEHOLDER) ||
                       cmd.params[i].startsWith(SORT_KEY+ARTIST_ALBUM_SORT_PLACEHOLDER)) {
-                var sort=getAlbumSort(cmd, view.inGenre);
+console.log("DK item.id="+item.id);
+                var sort=getAlbumSort(cmd, view.inGenre, undefined, item.id=='mm:/myMusicAlbums');


Why is "myMusicAlbums' getting sorts for all roles? I'm not sure it makes sense to sort the LMS albums lit by (e.g.) 'Guitarist' - that info is never shown.

If you choose a role sort, LMS will return the sorted role person as the first artist in artists and artist_ids, causing the name to show first in the album list artist links.

Still, sounds like a change just because it can be done - not one, IMVHO, that make sense. I see a list of albums - ordering by _Album_Artist, Release date, or title make sense, anything else not so much. Why, with this particular list, would I want to sort by (e.g.) Guitarist? Makes no sense.

Yeah, I did mention earlier in this thread that I wasn't sure about role sorts at the top level albums list. But once I removed artist/albumartist/trackartist from the generated sorts, it looked OK to me, so I thought "why not?". It's not inconceivable that someone might want to see a list of their albums by guitarist, producer or whatever.

Sorry, not convinced. Just because you can do something does not mean you should.

Looking back at this, All that code is doing is ensuring that a previously-selected album sort is held when displaying all albums, on the basis that it has to be valid - see comment above:

I've also decided to clear any role sort when changing the view, writing the default to local storage for new views (except if we're viewing all albums, because by definition all roles sorts would be available)...

I suppose the wider point you are making is that you don't think the role sort options should be available in the all albums list at all. But why not there if we offer them (for example) when browsing albums for a particular genre? In the end, it's just some extra options added to the drop-down sort menu.

Because this very much sounds like adding something just because you can. In reality when would you ever want to order all your albums by the Guitarist? Why would you do that as opposed to entering the "Guitarist" browse mode?

...and the same goes for "Albums" under "Genres".

Do any other music servers add such a feature? As its not something I can personally see the point to. And I don't want to overcrowd a menu just because we can.

Why would you do that as opposed to entering the "Guitarist" browse mode? ...and the same goes for "Albums" under "Genres".

Because it would avoid having to set up Additional Browse Menus for each role by Album (in addition to Role by Artist), and allow the user to quickly find different roles in whatever set of albums they are currently viewing rather than having to go back to the main menu and select a new browse mode each time. And even after doing that, the album list would not be sorted by the browsed role, only restricted to it.

I'm probably being slow here, so sorry for being a pain, but... and? Why would you want to sort albums by Guitarist? Artist, yes, but Guitarist? Makes no sense. Why would you want role-by-album??? If you care about Guitarist then you'd want Guitarist -> ABC -> albums. But sorting ALL albums by Guitarist? No, don't see the use-case. this very much sounds like "we have all these new role options, lets put these in all the possible places they could (not should) go"

(I'm using Guitarist as an example, I realise there are many more roles)

Again, does any other media server do such a thing (by default)?

On other media servers I don't know, I don't use anything but LMS. A user did mention one which was more flexible than LMS, in that it allowed browsing by Album then Artist Role (or any arbitrary hierarchy - I think it might have been Minim), as well as Artist Role then Album, which is sort of in the same area.

Now, LMS does not offer this directly, but through the sort mechanism, we can offer the route.

For example, if I wanted to see all my jazz clarinet stuff, I could go to Genres -> Jazz -> Releases and then sort by Clarinet Player. Of course there are other routes, but I think this is a valid and useful one.

Bear in mind that only roles used on the albums being displayed will be added to the sort options. So if the user hasn't tagged composer/conductor/band or created user-defined roles for anything in the list, they won't see anything other than the existing sort options. And if they have tagged those roles it likely they are important to them. So it's not going to bombard the user with sort options irrelevant to them.

The origin of this PR was a request by users with a lot of instances of a work, and after displaying the works-album list they want to be able to sort the performances by orchestra, conductor, soloist, whatever. We could restrict the additional sort options to work-albums only, but as you will have worked out by now, I think that would be a shame, and once it's there for work-albums someone is bound to ask why it's not there for albums in general.

Another way of looking at this:

LMS long ago went beyond just Album/Artist/Year, with the introduction of Composer, Conductor and Band, and now we have user-defined roles. For many users, the days when Artist is the only (or even most important) role in a library are long gone.

But LMS sort options didn't keep up: the list of available sorts was hardcoded in the albums query, and it would throw an error if it got a sort parameter that it didn't recognise. On that basis, Material has the same hardcoded list. But now, if LMS is to allow sorting by these additional roles, why would Material not?

CDrummond · 2025-01-04T18:14:58Z

How close to completion is this?

Not sure this is possible, and not worth recreating the pull request if not, but can the target branch be changed to dev? I've just created a new 'dev' branch for 5.8.0

darrell-k · 2025-01-04T20:05:12Z

I'm currently making the changes on the Default Skin side to allow role sorting options. How close do I need to be?

CDrummond · 2025-01-04T23:13:33Z

Not sure I get what you mean by "How close do I need to be?" ?? I was just wondering if this pull request is complete or not.

I make releases from the master branch, so don't want to merge this feature change into there just yet. I usually make feature changes in a dev branch, and only when ready to I merge that into master. Hence my asking if it'd be possible to change the target branch, but I'm not sure if this is possible with github...

darrell-k · 2025-01-05T09:29:19Z

OK, I can try to change the target, I'm pretty sure I've done it before.

darrell-k added 6 commits December 18, 2024 17:37

sorting by role: WIP

52d1891

Include roles in VALID_ALBUM_SORTS, allow sort for works, exclude art…

45cf021

…ist roles

retrieve roles for all displayed albums when viewing by artist

d415061

Merge branch 'master' into sorting-by-role

879a2af

Merge branch 'fix-descending-sort-toggle' into sorting-by-role

1d57ab3

various refinements

8605973

CDrummond reviewed Dec 21, 2024

View reviewed changes

Cacheing,control rolesort when viewed albums change

6570844

CDrummond reviewed Dec 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sorting by role #1011

Sorting by role #1011

darrell-k commented Dec 21, 2024

CDrummond Dec 21, 2024

darrell-k Dec 21, 2024

CDrummond Dec 21, 2024

darrell-k Dec 21, 2024

darrell-k Dec 23, 2024

CDrummond Dec 21, 2024

CDrummond Dec 21, 2024

CDrummond Dec 21, 2024

darrell-k Dec 21, 2024

CDrummond Dec 21, 2024

darrell-k Dec 21, 2024

CDrummond Dec 27, 2024

darrell-k Dec 27, 2024

CDrummond Dec 27, 2024

darrell-k Dec 27, 2024

CDrummond Dec 27, 2024

darrell-k Dec 29, 2024

CDrummond Dec 29, 2024

darrell-k Dec 29, 2024 •

edited

Loading

CDrummond Dec 29, 2024

darrell-k Dec 29, 2024

CDrummond commented Jan 4, 2025

darrell-k commented Jan 4, 2025

CDrummond commented Jan 4, 2025

darrell-k commented Jan 5, 2025

Sorting by role #1011

Are you sure you want to change the base?

Sorting by role #1011

Conversation

darrell-k commented Dec 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

darrell-k Dec 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CDrummond commented Jan 4, 2025

darrell-k commented Jan 4, 2025

CDrummond commented Jan 4, 2025

darrell-k commented Jan 5, 2025

darrell-k Dec 29, 2024 •

edited

Loading