Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve shuffle #157

Closed
thecodrr opened this issue Jul 26, 2017 · 5 comments
Closed

Improve shuffle #157

thecodrr opened this issue Jul 26, 2017 · 5 comments

Comments

@thecodrr
Copy link
Owner

@charliefoxtwo on #17 :

@theweavrs ... However, for a 12 track library, I noticed severe problems with the track selection and playback - it will play a few songs, before getting stuck on one. Trying to skip forward or backward only pauses the current song. In other cases when on shuffle, it will skip between the same few songs. I also observed this behavior when sorting by artist or genre....

On a 12 song library, the repeat skipping is to be expected because the library is so small. Basically, what BreadPlayer does when you enable Shuffle is create a random Array of Mediafiles and then when a user presses the next button, it takes the *Index of the currently playing song from the OriginalCollection and adds 1 to it. The next song is the song that sits at the resulting integer in the shuffled collection.

A little easier explanation:
Suppose the currently playing index is 15. Add 1 into 15, the result is 16. The next song will be the song that sits at number 16 in the shuffled collection.

Not a bad implementation, but there are a few bad things.

  1. A IndexOutOfRange expection will be thrown if we get a index that is outside the maximum count. (This will happen if we are playing the last song in the collection).
  2. We will be looping through the same collection again and again. So basically, it won't be random in the random sense.

What can we do to make both of these better:

  1. Reshuffle when we reach the end of list and set the index to a random number within collection bounds.

All input to make this better will be very much appreciated.

@charliefoxtwo
Copy link

It probably isn't even necessary to set the index to a random number after reshuffling, just setting it to 0 should be fine.

Possible issue: it could end up playing the same song twice in a row. While this might actually be "truly" random, I assume most users don't want this.

@thecodrr
Copy link
Owner Author

@charliefoxtwo good point.

@charliefoxtwo
Copy link

I think the best way to do this would be to create an array of all of the songs in the playlist, and shuffle said array. When we get to the end, just start from the beginning again. I think this is what other players do, and personally I've never had an issue with it.

@thecodrr
Copy link
Owner Author

@charliefoxtwo I have already fixed and committed it, just wanted your input. Will make changes accordingly. Most of the bugs should now be fixed. I am working on Playlists and fixing bugs there.

@thecodrr
Copy link
Owner Author

Closing this as the task is done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants