Cleanup Music DirectoryNodes, inconsistent "*All" item and node labeling

[DaveTBlake]

Clean up some things left over since when defining music library nodes via xml was introduced back in in 2015 by #6772

---

Make the two types of xml node - "folder" and "filter" type - behave more consistently

• "folder" type node take item list label from node xml file (like "filter" type nodes)
• allow "*All..." item on "artists" and "albums" nodes with options (but not smart playlists)

Fix navigation back from Default Library Folder.
The facility to use a "Default Library Folder" - the node that is displayed on ActivateWindow(Music) is almost forgotten functionality (removed from video lib in 2012, undocumented in Wiki and with many skins taking explicit control over navigation). Set using the "Mark Default"/"Clear Default" context menus this node is still the one that comes up on start-up if the user has set the start-up window to "music" ( see settings > interface > startup > startup window). However having reached a node that way, or from ActivateWindow(Music) on a keymap or script, navigation back just refreshes the node the first time and has to be clicked again. If the node is slow to load (e.g. has many items) this refresh is particularly irritating.

Remove CGUIWindowMusicNav::GetQuickpathName(), unnecessary as the default library folder setting is saved as node xml file path e.g. "library://music/recentlyplayedalbums.xml" so attempting to look-up and substitute an abbreviated name derived from DirectoryNode path e.g. "musicdb://recentlyplayedalbums/" is meaningless.

The intention is to move away from the historic approach of having a CDirectoryNode descendant for every node, but implement nodes in a user configurable way using "filter" type nodes with rules instead. Towards this end some of the CDirectoryNode descendants have now been superseded and can be removed.

• Remove CDirectoryNodeYearAlbum, CDirectoryNodeYearSong, CDirectoryNodeAlbumCompilations and CDirectoryNodeAlbumCompilationsSongs - the "compilations" and "years" family of directory nodes - CDirectoryNodeAlbum suffices.
  Remove "Top 100" family of directory nodes, deprecated in 2015 and broken before that.

---

Note it is not possible to simplify further and remove the "Top 100", "RecentlyAdded" and "RecentlyPlayed" families of CDirectoryNode classes at this time.
The implementation of "RecentlyAdded" and "RecentlyPlayed" CDirectoryNodes, despite the content being just filtered albums or songs, use specific database access that applies the sorting and limits at the database rather than fetch all the entire table into memory and do it in the file items list. This is fast and efficient compared to a comparable "filter" type node. It can be reviewd when (and if) we stop doing all the sorting and limiting in memory and use the db.

The "Top 100" family of CDirectoryNode classes when accessed via ActivateWindow(Music, Top100) etc. are also implemented separately from the equivalient default library nodes defined in xml

Note calls like ActivateWindow(Music, compiliations) or years etc. remain uneffected by this PR.

[ronie]

a few things i noticed while taking this for a test drive:

• when i'm either in the Top100Songs or Top100Albums listing and click the (..) up item, i end up in the root of the music section instead of in the parent Top100 node.

• this is a skin breaking change, as neither ActivateWindow(Music,Top100) nor ActivateWindow(Music,musicdb://top100/) will work anymore.

[DaveTBlake]

Thanks @ronie for taking a look. I had overlooked that some skins use ActivateWindow in that way. Is there a list of the path parameters e.g. "musicdb://top100/" that skinners expect to work? Is it part of the skins API?

Do you think it would it upset people if that top100 call was removed? I would split it into a clear skin breaking PR of course, make it obvious.

[ronie]

here's a list how skins can open certain sections of the library:
https://kodi.wiki/view/Opening_Windows_and_Dialogs

the removal of the top100 calls would introduce a big inconsistency.

[DaveTBlake]

@gade01 I'm sure you commented here, where did it go??

I will adjust this PR to restore the fixed paths that skins use to open certain windows, but as discussed on Slack having such a fixed list goes against the idea of user configurability. I will see if I can up with a separate PR to address that, or at least a place for more discussion.

[gade01]

@DaveTBlake
Sorry, I removed the questions here to have all discussions at Slack.

I'm completely with you on moving forward, and v19 seems like good place to start. But consistency is also very important in this process..
I'm happy we got the discussion on the topic started.

It's a great idea to restore fixed paths in this PR, and then we can continue discussion on how to completely rethink the approach with new PR's.

[zag2me]

Just making a quick comment that the Top 100 feature is well used and useful. I use it all the time to play my top music as a playlist. I totally understand if the idea is to replicate this with XML nodes but think it should remain as some kind of default node going forward in V19.

[DaveTBlake]

No worries @zag2me, I have no intention to deprecate any nodes/features.

The issue is that while on one hand the nodes in the default library menu are defined in xml and user configurable, on the other we have skins using ActivateWindow and a fixed list of paths that internally depend on CDirectoryNode descendants. These may use different queries to the xml nodes (so different results), are fixed (user changes to the default xml nodes don't apply, and the xml node could even be removed and the skin not know so still show that window action), and limited so can't cover all the possible nodes variations the user could create. A legacy it would be nice to resolve.

I just stumbled into that during this cleanup attempt because my code search didn't reveal the skin use.

EDIT
But I can see how my initial post saying "Remove "Top 100" family of directory nodes" could be misunderstood. I mean some of the unnecessary internal CDirectoryNode decendant classes, not the default library nodes that users use. But I need to insure that the implementation also continues to cover skin/addon use of paths in ActivateWindow.

[jjd-uk]

@zag2me you've got wrong end of stick.

Top100 will remain a default node, the discussions have been how to best access the node for example by skins. So the debate is whether to use musicdb:// paths or library:// paths

musicdb:// paths produce consistent results for skinners but won't reflect any changes done to the xml nodes by a user. So a skin menu item using a musicdb:// path may give a different result from going through the library structure to open a node.

library:// paths point to the xml for the node so will always produce the same result as opening the node through the library structure.

[DaveTBlake]

I have reduced the things I clean-up here to avoid the issues it caused with activation of the Top100 window @ronie spotted. Could you take it for another test drive please.

It still manages to remove some unneeded classes and types, but the issue of improved access to nodes for skins (allowing for user edits, flexibility etc.) is for another PR

[ronie]

tested and the issues with navigating one level up in the hierarchy still exist.
did you try to fix that as well?

[DaveTBlake]

Is that navigation issue just with Top100 Songs/albums @ronie or everything? Either way I'm not seeing it. Which skin are you testing with? Is it related to the skin using ActivateWindow?

[ronie]

i made a screenrecording of the issue i'm seeing:
top100.mkv.zip

using estuary, and it's not related to ActivateWindow

[DaveTBlake]

Thanks for the video @ronie , I was being blind!!
Correct Top100 (and other) back nagivation restored (as well as back from default library folder fixed). One more test drive if you can.

[ronie]

cheers! fixed now.