Respect playlist sort order in watch-video-playlist

[jasonhenriquez]

Respect playlist sort order in watch-video-playlist

Pull Request Type

• Feature Implementation

Related issue

#5008 (comment)

Description

Implements sort order for a given user playlist when opening it with watch-video-playlist. This ensures sort order is respected in the order it was presented on the Playlists page.

Screenshots

Testing

• Test sort order is reflected in watch-video-playlist for arbitrary sort
• Insert inserted elements are shown in the proper place
• (REG) Test reverse/shuffle work as intended

Desktop

• OS: OpenSUSE
• OS Version: TW

Additional context

Playlist sort is computed when the user playlist in watch-video-playlist is first loaded and when it is updated with additions / removals during that session. While this is not necessarily having an apparent performance effect in my testing, I would imagine it's at least a potential concern w.r.t. sum power draw across our user base.

As it stands, the O(nlogn) sorting operation is executed for each playlist load, or upon each addition/removal to a playlist while it is currently being watched. In the future, 1) we may want to consider adding an ordering property to playlistItems for the custom sort order instead of relying on its default order, 2) thus allowing the "sort" action to be one-and-done per sort preference change by storing that playlist in its sort preference, and 3) using binary search for each subsequent insertion into a playlist. This would remove the O(nlogn) operation upon each playlist load, and it would reduce the addition cost to O(klogn), where k is the number of elements being added.

• Alternatively, instead of implementing 1, we can 1b) just remove "Custom". although we'll still have to implement 2.
• Note that 2 means we'll have to update all playlists upon each sort, unless we implement it in a way that delays the sort change on a given playlist until that playlist is actually accessed (e.g., having a property on the DB object called "sortOrder" that we check if it lines up with the userPlaylistSortOrder on DB retrieval).
• We can make "Added First" the new default logical stored ordering with 1 or 1b, thus improving performance and minimizing processing power even more.
• By modifying the base ordering, the playlist thumbnail will update as per the user's preferences instead of the currently disparate logical order, which is a more preferable state of affairs.
• Having a separate user playlist update function on the Watch route specifically for incrementing/decrementing the number (and thus making an individual insert/delete) would be a noticeable improvement for large playlists, where I notice quick bookmarking or removing the quick bookmark takes a noticeable amount of time.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

Conflicts have been resolved. A maintainer will review the pull request shortly.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

Conflicts have been resolved. A maintainer will review the pull request shortly.

[jasonhenriquez]

I'm still feeling shaky about the energy expenditure aspect of it, so I'm leaning towards shelving it this release and working on the performance suggestions.

[jasonhenriquez]

Update 5/23:

• Nedb doesn't let you insert at a specific place. So then I thought that maybe you can do the sorting in grabAllPlaylists, but you'd also have to do it every time the locale changed as well if it's set to one of the alphabetical sorting options. I suppose that both changing your language frequently and using an alphabetical sorting option are pretty rare, so I think this is still a pretty clear net benefit.
  • Delaying applying the sort until the playlist is accessed is weird too, as the thumbnails on the UserPlaylists page would be misrepresentative, unless we want to also just load all of the sorts there as well.
• Has anyone ever looked at how YT handles moving playlist videos? It's very weird, I'm surprised it is the way it is. I'm still trying to wrap my head around it. It seems like whenever you change the sort order of the playlist, all of your changes are wiped. When you change the sort order and click "Play All," it opens & plays it in your suggested sort order. But when you move the videos around while the playlist is being watched in a different sort order, the next time you open the playlist page, it maintains the position of those manual moved video entries, but with the default Date added (newest) as the sort!

[absidue]

I think it's best to keep the current solution of sorting on demand, rather than storing the sorted playlist. It will also make adding a video to a playlist more expensive, as we'll have to sort the playlist every time you add a video to the playlist, regardless of what page you are on, instead of just if you are on the playlist or watch page.

We can skip the first sort on the watch page by using the already sorted playlist from the playlists page, by using the cached playlist trick that we use for remote playlists (currently not done for the user playlists as we fetch them from the playlists store every time), where we pass through the videos from the playlist page to the watch page, by saving them to the utils store. You will still need to fetch it from the store and sort it if the user accesses the watch page directly (e.g. middle clicking on a video on the playlists page, which would open a new window right to the watch page).

[jasonhenriquez]

I'm seeing merit to what you're saying, although:

It will also make adding a video to a playlist more expensive, as we'll have to sort the playlist every time you add a video to the playlist, regardless of what page you are on, instead of just if you are on the playlist or watch page.

This isn't exactly true, to be pedantic. If you have the sorting applied in grabAllPlaylists, on sort change, and (for specifically the alphabetical sort options) on locale change, then you can do a binary insertion or deletion in the mutation logic at O(logn) cost (Well, O(n) counting splicing and resizing respectively), which is different from sorting the playlist. The main struggle I'm having is that when watching a playlist and adding or removing videos to it, it will require the playlist to be re-sorted in full each time, which is the full O(nlogn).

We can skip the first sort on the watch page by using the already sorted playlist from the playlists page, by using the cached playlist trick that we use for remote playlists (currently not done for the user playlists as we fetch them from the playlists store every time)

This is definitely the right choice if we're to go this path. Thanks for this suggestion.