Album shuffle isn't random about vanilla HOT 6 CLOSED

It seems as though albums with more songs tend to rise to the top. From 12 "PlayAll" presses from the song list, I saw only 3 different albums first and they tended to be close to the top consistently. I have 100+ albums and the 3 are the ones I have the most songs for.

from vanilla.

Glancing at the code, it would take some more time to go through and see if there are any logic errors. It might me fairer (though slower) to get a list of albums, shuffle it and then expand each album into a list of songs.

from vanilla.

I've been bothered by this too and I've finally got round to writing a solution which is now in my fork. I'll put in a pull request for it to be pulled into a future version.

The old album shuffle was an attempt at a Fisher-Yates shuffle and made the mistake that it picked each album to shuffle by picking a random song in the sorted song list and then finding the start of that album, which means that it was more likely to pick bigger albums first.

Fisher-Yates doesn't really work in this case as the albums are not all the same length, so I just pulled out a list of albums and used Collections.shuffle(). This is basically what you've said mathstuf and it isn't slower, it's faster as the old method did a lot of excess arrayCopys. I've tested it both in code and by repeatedly calling play all and looking at the queue and it seems to me to fix this bug.

from vanilla.

On Mon, Feb 17, 2014 at 15:48:58 -0800, piemonkey wrote:

I've been bothered by this too and I've finally got round to writing a
solution which is now in my fork. I'll put in a pull request for it to
be pulled into a future version.

Woo! Thanks!

The old album shuffle was an attempt at a Fisher-Yates shuffle and
made the mistake that it picked each album to shuffle by picking a
random song in the sorted song list and then finding the start of that
album, which means that it was more likely to pick bigger albums
first.

Hmm. I thought it picked randomly until it found a first-song-in-album.
Maybe I'm remembering incorrectly (it has been a year...).

Fisher-Yates doesn't really work in this case as the albums are not
all the same length, so I just pulled out a list of albums and used
Collections.shuffle(). This is basically what you've said mathstuf and
it isn't slower, it's faster as the old method did a lot of excess
arrayCopys. I've tested it both in code and by repeatedly calling play
all and looking at the queue and it seems to me to fix this bug.

Yeah, that's what I thought when I looked at it, but my Java-fu is weak
and I got frustrated fighting against Eclipse.

from vanilla.

"I thought it picked randomly until it found a first-song-in-album."

In the first while loop within the album shuffle for loop it takes the random j value, checks if the corresponding song is the start of an album and if not does j -= 1. What it should maybe have done is pick another random j but that would have slowed it down even further.

from vanilla.

Closing this old bug as the patch from issue 71 was already merged

from vanilla.