queue/PlaylistControl: keep order list consistency in MoveOrderToCurrent()

Our previous use of Queue::SwapOrders() could cause surprising results: - sometimes, the old "current" song would be played again (if the newly selected song had not been played already) - sometimes, the old "current" song would not be played again (if the newly selected song had already been played) This is inconsistent, because it should not depend on whether the newly selected song had already been played. So instead of Queue::SwapOrders() we now use Queue::MoveOrderAfter() and Queue::MoveOrderBefore(), which is more expensive, but also more consistent. It attempts to retain as much from the previous order list as possible, and only moves the newly selected song around.

queue/PlaylistControl: keep order list consistency in MoveOrderToCurrent()
91254e92 · Max Kellermann · 0f79287b · 91254e92 · 91254e92
Commit 91254e92 authored Oct 17, 2017 by Max Kellermann
Hide whitespace changes
Inline Side-by-side

Showing with 16 additions and 9 deletions

NEWS NEWS +1 -0

PlaylistControl.cxx src/queue/PlaylistControl.cxx +15 -9

No files found.
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,7 @@ ver 0.20.11 (not yet released)
 * decoder
  - ffmpeg: more reliable song duration
  - gme: fix track numbering
+* improve random song order when switching songs manually
 * fix case insensitive search without libicu
 * fix endless loop when accessing malformed file names in ZIP files


--- a/src/queue/PlaylistControl.cxx
+++ b/src/queue/PlaylistControl.cxx
@@ -63,15 +63,21 @@ playlist::MoveOrderToCurrent(unsigned old_order)
 		/* no-op because there is no order list */
 		return old_order;

-	const unsigned destination_order = playing
-		? (unsigned)current
-		: 0;
-
-	/* swap the new song with the previous "current" one, so
-	   playback continues as planned */
-	queue.SwapOrders(old_order, destination_order);
-
-	return destination_order;
+	if (playing) {
+		/* already playing: move the specified song after the
+		   current one (because the current one has already
+		   been playing and shall not be played again) */
+		return queue.MoveOrderAfter(old_order, current);
+	} else if (current >= 0) {
+		/* not playing: move the specified song before the
+		   current one, so it will be played eventually */
+		return queue.MoveOrderBefore(old_order, current);
+	} else {
+		/* not playing anything: move the specified song to
+		   the front */
+		queue.SwapOrders(old_order, 0);
+		return 0;
+	}
 }

 void