Playlists: allow to adopt current order (sorted) as playlist order #15319

ronso0 · 2025-09-06T15:44:38Z

select a playlist with 2++ tracks, ensure # colummn is visible (just for feedback)
sort playlist by BPM (or key or whatever, just not by # obviously)
open track table header menu, click "Adopt sort order"
tracks' # is now ascending, sort by # and BPM are ascending

I decided for a separate function instead of doing multiple moveTrack.

Swiftb0y · 2025-09-19T22:46:24Z

Haven't looked at this or tested, thank you for picking this up. I also tried to implement this a while ago but failed back then.

Swiftb0y · 2025-09-19T22:48:01Z

src/library/dao/playlistdao.cpp


-        //qDebug() << "Swapping tracks " << trackAPosition << " and " << trackBPosition;
-        trackPositionIds.insert(trackAPosition, trackBId);
-        trackPositionIds.insert(trackBPosition, trackAId);
+        // qDebug() << "Swapping tracks " << trackAPosition << " and " << trackBPosition;


hmm.. whoopsy?

Swiftb0y · 2025-09-19T22:49:44Z

src/library/dao/playlistdao.cpp

+    for (auto oldIdAndPos : newOrder) {
+        int oldPos = oldIdAndPos.second;
+        const TrackId trackId = oldIdAndPos.first;


NIT: you should be able to use destructuring on a pair

Suggested change

for (auto oldIdAndPos : newOrder) {

int oldPos = oldIdAndPos.second;

const TrackId trackId = oldIdAndPos.first;

for (auto [trackId, oldPos] : newOrder) {

Swiftb0y · 2025-09-19T22:51:46Z

src/library/dao/playlistdao.cpp

+    // Print out any SQL error, if there was one.
+    if (query.lastError().isValid()) {
+        qDebug() << query.lastError();
+    }


dumb question: will this run if there was an error? Don't we just return then?

Idk 🤷‍♂️
I adopted it from the other methods.

right, but those don't return early for failed queries, right?

Swiftb0y · 2025-09-19T22:54:38Z

src/library/dao/playlistdao.cpp

+    if (newOrder.isEmpty() || playlistId == kInvalidPlaylistId) {
+        return;
+    }


should we also check that newOrder.size() matches the playlist size?

Hmmmaybe.. why not.
FWIWI also had in mind that we might use this for AutoDJ and then we'd need to exclude the first track which is already loaded as "next" (like in AutoDJ's own Shuffle function)

Swiftb0y · 2025-09-19T22:56:33Z

src/library/playlisttablemodel.cpp

+    QList<std::pair<TrackId, int>> idPosList;
+    int numOfTracks = rowCount();


NIT:

Suggested change

QList<std::pair<TrackId, int>> idPosList;

int numOfTracks = rowCount();

QList<std::pair<TrackId, int>> idPosList;

int numOfTracks = rowCount();

idPosList.reserve(numOfTracks);

cuz why not

Swiftb0y · 2025-09-19T22:59:37Z

src/widget/wtracktableview.cpp

+    pTrackModel->orderTracksByCurrPos();
+    // TODO should we resort by # ??


can you elaborate?

I wonder whether we should now sort by #, like is that what the user wants to see/use after adopting the order?

Probably yes, but my coffee break was over ; )

Also, if yes, PlaylistDAO::orderTracksByCurrPos should return true on succes, and only then we re-sort (for good UX)

yeah, I agree.

I'll try to implement it.

Swiftb0y · 2025-09-20T00:07:04Z

I just tried this locally and got tripped up UX wise. I expected the "Adopt sort order" to be an action on the playlist context menu (next to "shuffle playlist") instead of the table header. How feasible is that to implement?

ronso0 · 2025-09-20T00:14:36Z

next to "shuffle playlist") instead of the table header. How feasible is that to implement?

(I think I recently looked at the Shuffle action for Tracks, Crates which is also in the header)
Makes sense.
And that's how I just did it for "Remove duplicate tracks" (will push this soon).
It would also make it easier to disable the action for locked playlists. Not sure how to get the current sorting though.
Will check.

github-actions bot added library ui labels Sep 6, 2025

ronso0 added the playlist label Sep 6, 2025

Playlists: allow to adopt current order (sorted) as playlist order

8686932

ronso0 force-pushed the playlist-adopt-order-for-position branch from 169868d to 8686932 Compare September 15, 2025 08:51

ronso0 mentioned this pull request Sep 19, 2025

Reset track order in playlist #15378

Closed

Swiftb0y reviewed Sep 19, 2025

View reviewed changes

		QList<std::pair<TrackId, int>> idPosList;
		int numOfTracks = rowCount();

		pTrackModel->orderTracksByCurrPos();
		// TODO should we resort by # ??

Uh oh!

Playlists: allow to adopt current order (sorted) as playlist order #15319

Are you sure you want to change the base?

Playlists: allow to adopt current order (sorted) as playlist order #15319

Uh oh!

Conversation

ronso0 commented Sep 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Swiftb0y commented Sep 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Swiftb0y commented Sep 20, 2025

Uh oh!

ronso0 commented Sep 20, 2025

Uh oh!

Uh oh!

ronso0 commented Sep 6, 2025 •

edited

Loading