Skip to content
This repository has been archived by the owner on Jan 6, 2022. It is now read-only.

Proof of Concept for sorting of playlists #225

Closed
wants to merge 15 commits into from

Conversation

SiverDX
Copy link
Contributor

@SiverDX SiverDX commented Sep 24, 2021

Sorting Playlists

Getting the data

At the start we get the playlist ID of the currently loaded page
This also makes sure that we actually do something - no playlist ID means nothing to do

Once we know we are in a playlist page we go through the document and get all nodes
For each node we create a unique ID by combining the following elements:

  • song name
  • artist name
  • album name

After that we can access the cache for MusicKit to get the data that was requested
(Which means we won't have to call the API as well)

We go through each cached song and assign a node to it
We also store the original index, in case we want to return to the original sorting

Duplicates

For both the node elements and the cached song data we retain a count on how many instances (duplicates) of that element exist
We do that by having a map with the either the song ID or the combined ID of the node element as the key

Once we notice that the maps for the nodes or the songs aleady have the ID as an entry we interact with this duplicate map

Sorting

Once we click on a header cell (like album, artists, ...) we save the corresponding attribute name of the song element, which we can use to sort

If we sort by artists that would be artistName, if we sort by time it would be durationInMillis
This allows us to have a single function which handles sorting by simply accessing these attributes like this:
songElement.data.attributes[sortConfig.attributeName]

Newly loaded items (infinite scroll)

Once we scroll to the end of the page new elements (nodes) get loaded
(And new request might get sent - if they're not already cached - that is why we process the cache again)
We handle these by appending an observer to the 'content node', to which these new nodes get appened to

In this observer we check for DIV nodes and add them to our songNodes map
After that we go through our song map to check if any of them are still missing their node elements
Once we have done all that we can sort again

Sorting the queue

Once the user double clicks a row, or clicks the play icon the queue will get loaded with all songs after the one you clicked on
In reality all songs of the playlist are in the queue but by using a position variable these elements get skipped
Since we cannot rely on this variable if we want to sort the queue, we will have to remove all elements we do not need

The safest way to do that is to simply create a new queue and set it

We start by sorting the current queue list with our sortConfig.attributeName and sortConfig.order
After that we get the currently playing song - with our sorted list and this current song element (index) we can simply use the splice() function - this way we are only left with the songs we need

We then set the queue with our sorted list
As of now there is a timeout required to reliably start the playback

Change to load.js

I had to add the ;0 part because Electron seems to clone something in my code which it does not like = causing an error

Since the return (cloned values) do not get used anyway, this doesn't seem to be a loss

+ added ;0 which stops electron from cloning weird stuff (which causes errors)
+ added a catch for the javascript execution promise (which happens if there is a problem loading the script)
+ sorting of newly loaded element via the observer function
+ split up functionalities in different functions
+ using a map now for the dom data - acces through an id (song name + artist name + album name)
+ data that has been already set (merged) does not get overwritten with musickit cache anymore (which meant the data had to be merged again)
+ updated regex for language code of url - can be 2 to 5 characters now (not sure if stuff like 'en_EN' is used or only 'en')
+ regex check ok now pls?
+ check the data that gets read from cache if it's usable
+ if data is missing disable sort feature
+ moving functions around
+ split up a function that got a bit too large
+ duplicates will no longer be removed in the document by the sorting
+ duplicates and queue don't work together
@SiverDX
Copy link
Contributor Author

SiverDX commented Sep 29, 2021

Sorting the library does not seem viable

Buggy behaviour

Apples API has problems fetching the songs
This seems to not be the case with playlists and only applies to the web version of Apple Music

Example:

callApple({limit: 100, offset: 0});

async function callApple({limit, offset}) {
    let developerToken = MusicKit.getInstance().api.library.developerToken;
    let mediaToken = MusicKit.getInstance().api.library.userToken;

    let url = 'https://amp-api.music.apple.com/v1/me/library/songs?limit=' + limit + '&offset=' + offset;

    fetch(url, {
        method: 'GET',
        headers: {
            'authorization': 'Bearer ' + developerToken,
            'media-user-token': mediaToken
        }
    }).then(respone => respone.json()).then(data => {
        console.log('offset:', offset, data);

        if (data.next) {
            callApple({limit: limit, offset: offset + 100});
        }
    }).catch(reason => console.error(reason));
}

image

Apple returned 104 out of 221 songs (and even has false duplicates)
This is consistent with their own API calls

Document

Apple creates skeleton elements that get filled with the actual song-data later on
The position of the elements gets decided by style.transform="translateY(44px)" (next element would be 88px and so on)

While scrolling elements get added and removed to the document (on top of managing their location by using the transform element) - not sure how you would add (and keep) some kind of sort with that

Other

The cached object for library songs is not reliable in terms of handling duplicates
In playlist we can add duplicates to the list since we can be sure that the cache is not present multiple times
But the API calls on the library-songs is kinda wonky, which causes it to store more cached objects than needed

image

If we were to implement sorting etc. it might be best to replace the current document with our own logic
(as in fetching data, handling play(), infinite scroll (copy behaviour from playlsit - load once you get to the end), etc.)

@coredev-uk coredev-uk marked this pull request as ready for review September 29, 2021 18:18
@coredev-uk coredev-uk marked this pull request as draft September 29, 2021 18:20
+ added support for sorting by different types + ascending / descending
+ removed the 'fixToAllowSort' stuff - might have been fixed by some other changes (it did not occur anymore)
+ generalised the sorting functions a bit
+ removed unused code for handling duplicates in queue
@coredev-uk coredev-uk added the enhancement New feature or request label Oct 3, 2021
+ added an indicator which type of sorting is applied (placeholder version)
+ shuffled around functions to group them together a bit better
+ fixed a bug where sorting the queue did not happen (blockQueueSorting had the initial value of 'true')
+ created a compare function which gets used for both instances of sorting (queue and songs)
@SiverDX
Copy link
Contributor Author

SiverDX commented Oct 5, 2021

Performance

Test with 100 elements

Using one of these makes appending take ~ 5 ms
But removing will take ~ 30 ms

while (contentNode.firstChild) {
    contentNode.firstChild.remove();
}
contentNode.replaceChildren();

Using this makes removing instant but appending will take ~ 35 ms

contentNode.innerHTLM = '';

Apparently, before being able to append a node somewhere else it needs to be removed from its current parent
Maybe setting the innerHTML does not do this (which is why the append has to handle it = it takes longer)

Using document.createDocumentFragment() does not save time
The time it takes depends on the way the nodes get removed (either almost instant or ~ 30 ms)
Appending the fragment to the contentNode also takes an additional ~ 5 ms

You're basically running into the same problem (removing nodes from one parent and appending them to another)

Using contentNode.replaceChildren(); and passing a list of nodes or the fragment takes ~ 35 ms

We cannot remove these elements entirely or simply change the text (of the artist name, etc.), since we cannot copy the event listeners of the nodes (and assign them to different nodes)

@coredev-uk coredev-uk linked an issue Oct 18, 2021 that may be closed by this pull request
@coredev-uk coredev-uk linked an issue Nov 1, 2021 that may be closed by this pull request
@quacksire
Copy link
Member

#430

@quacksire quacksire closed this Jan 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement] Sort Playlists by Recently Played or Recently Added [Enhancement] Sort Albums in Library
3 participants