From a9d6e50ef42cb2a417747c0a2312208748a78ebc Mon Sep 17 00:00:00 2001
From: Adrian Ulrich <adrian@blinkenlights.ch>
Date: Fri, 16 Oct 2015 17:35:56 +0200
Subject: [PATCH] Simplify shuffle logic This will hopefully kill more bugs
 than creating new ones

---
 .../android/vanilla/SongTimeline.java         | 58 ++++++++++---------
 1 file changed, 30 insertions(+), 28 deletions(-)

diff --git a/src/ch/blinkenlights/android/vanilla/SongTimeline.java b/src/ch/blinkenlights/android/vanilla/SongTimeline.java
index 9b1430af..5b722bc3 100644
--- a/src/ch/blinkenlights/android/vanilla/SongTimeline.java
+++ b/src/ch/blinkenlights/android/vanilla/SongTimeline.java
@@ -232,9 +232,14 @@ public final class SongTimeline {
 	 * Must be one of SongTimeline.FINISH_*.
 	 */
 	private int mFinishAction;
-
-	// for shuffleAll()
-	private ArrayList<Song> mShuffledSongs;
+	/**
+	 * Prepared (shuffled) replacement playlist
+	 */
+	private ArrayList<Song> mShuffleCache;
+	/**
+	 * Hash code of mSongs while mShuffleCache was generated
+	 */
+	private int mShuffleTicket;
 
 	// for saveActiveSongs()
 	private Song mSavedPrevious;
@@ -464,12 +469,9 @@ public final class SongTimeline {
 
 		synchronized (this) {
 			saveActiveSongs();
-			mShuffledSongs = null;
 			mShuffleMode = mode;
 			if (mode != SHUFFLE_NONE && mFinishAction != FINISH_RANDOM && !mSongs.isEmpty()) {
-				shuffleAll();
-				ArrayList<Song> songs = mShuffledSongs;
-				mShuffledSongs = null;
+				ArrayList<Song> songs = getShuffledTimeline(false);
 				mCurrentPos = songs.indexOf(mSavedCurrent);
 				mSongs = songs;
 			}
@@ -492,20 +494,24 @@ public final class SongTimeline {
 	}
 
 	/**
-	 * Shuffle all the songs in the timeline, storing the result in
-	 * mShuffledSongs.
+	 * Returns a shuffled (according to mShuffleMode) version
+	 * of the timeline. The returned result will get cached
 	 *
-	 * @return The first song from the shuffled songs.
+	 * @param cached the function may return a cached version if true
+	 * @return a *copy* of a shuffled version of the timeline
 	 */
-	private Song shuffleAll()
+	private ArrayList<Song> getShuffledTimeline(boolean cached)
 	{
-		if (mShuffledSongs != null)
-			return mShuffledSongs.get(0);
+		if (cached == false)
+			mShuffleCache = null;
 
-		ArrayList<Song> songs = new ArrayList<Song>(mSongs);
-		MediaUtils.shuffle(songs, mShuffleMode == SHUFFLE_ALBUMS);
-		mShuffledSongs = songs;
-		return songs.get(0);
+		if (mShuffleCache == null) {
+			ArrayList<Song> songs = new ArrayList<Song>(mSongs);
+			MediaUtils.shuffle(songs, mShuffleMode == SHUFFLE_ALBUMS);
+			mShuffleCache = songs;
+			mShuffleTicket = mSongs.hashCode();
+		}
+		return new ArrayList<Song>(mShuffleCache);
 	}
 
 	/**
@@ -516,12 +522,7 @@ public final class SongTimeline {
 	{
 		synchronized (this) {
 			saveActiveSongs();
-
-			mShuffledSongs = null;
-			shuffleAll();
-			ArrayList<Song> songs = mShuffledSongs;
-			mShuffledSongs = null;
-
+			ArrayList<Song> songs = getShuffledTimeline(false);
 			int newPosition = songs.indexOf(mSavedCurrent);
 			Collections.swap(songs, newPosition, mCurrentPos);
 			mSongs = songs;
@@ -568,7 +569,7 @@ public final class SongTimeline {
 						// empty queue
 						return null;
 					else if (mShuffleMode != SHUFFLE_NONE)
-						song = shuffleAll();
+						song = getShuffledTimeline(true).get(0);
 					else
 						song = timeline.get(0);
 				}
@@ -597,9 +598,7 @@ public final class SongTimeline {
 
 		if (mFinishAction != FINISH_RANDOM && pos == mSongs.size()) {
 			if (mShuffleMode != SHUFFLE_NONE && !mSongs.isEmpty()) {
-				if (mShuffledSongs == null)
-					shuffleAll();
-				mSongs = mShuffledSongs;
+				mSongs = getShuffledTimeline(true);
 			}
 
 			pos = 0;
@@ -611,7 +610,6 @@ public final class SongTimeline {
 		}
 
 		mCurrentPos = pos;
-		mShuffledSongs = null;
 
 		if (mShuffleMode == SHUFFLE_CONTINUOUS) {
 			reshuffleTimeline();
@@ -979,6 +977,10 @@ public final class SongTimeline {
 	 */
 	private void changed()
 	{
+		// Invalidate shuffle cache if the timeline *contents* changed in the meantime
+		if (mShuffleCache != null && mShuffleTicket != mSongs.hashCode())
+			mShuffleCache = null;
+
 		if (mCallback != null)
 			mCallback.timelineChanged();
 	}