sync icon indicating copy to clipboard operation
sync copied to clipboard

Shuffle playlist isn't very random

Open mikedesu opened this issue 5 years ago • 5 comments

Shuffling the playlist does not seem to use a very good random implementation. Reproducible sequences of playlist shuffling on subsequent website reloads and shuffling of new playlists is possible. The RNG seed seems to be some bad default. I'd fix it myself but that takes time. cytu.be needs to be reloaded once y'all patch it.

mikedesu avatar Nov 04 '19 02:11 mikedesu

Did this start happening recently? If so, I wonder if there is a bug in https://github.com/calzoneman/sync/commit/1ec3eab0dc312e505dc725119cbbd8c784ed6d9d (introduced by #812).

Something doesn't add up here, shuffle uses Math.random(), certainly not a CSPRNG, but shouldn't be trivially predicable either. It's shared across each instance of the application, so I'm not sure why reloading a channel (which doesn't restart the process) would cause reproducible sequences.

calzoneman avatar Nov 04 '19 02:11 calzoneman

Can you provide more information about the "reproducible sequences"? I tested a few times and didn't notice any obvious patterns.

calzoneman avatar Nov 04 '19 03:11 calzoneman

Sure thing.

I just loaded up a playlist of mine with 13 episodes, then shuffled, wrote down the sequence, then cleared the list. I did this 3 times. The sequences I got were:

1,6,13,10,9,5,12,2,4,11,3,7,8 1,11,13,4,12,8,10,2,9,3,5,6,7 1,4,11,13,8,12,9,10,5,2,6,7,3

Notice how close together 13, 7, 2, 12 appear relative to their positions in the list. Also, notice that 1 is locked at front because this is new functionality. At some point, the shuffle no longer fully shuffles the playlist, but instead continues to play whatever is currently playing (in essence, shuffling everything in the playlist except for whatever is currently playing). I don't know but it just feels like the rng sucks now and I didn't use to feel this way. Also, I'd like the old shuffle back. When I hit shuffle, I want a completely different item on the playlist to start playing immediately. It's annoying and it wasn't like that before.

Comment edited: please refrain from attaching advertisement signatures in your GitHub comments --calzoneman

On Sun, Nov 3, 2019 at 10:13 PM Calvin Montgomery [email protected] wrote:

Can you provide more information about the "reproducible sequences"? I tested a few times and didn't notice any obvious patterns.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/calzoneman/sync/issues/836?email_source=notifications&email_token=AABILA6SX6ASULU6L53YKITQR6ANRA5CNFSM4JIOMCQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC6FJIY#issuecomment-549213347, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABILA2T7VDHUVI6RNVXKKDQR6ANRANCNFSM4JIOMCQQ .

mikedesu avatar Nov 04 '19 04:11 mikedesu

The patterns in shuffle results could be influenced by bias introduced by multiplying Math.random() (which returns a floating point number in [0, 1)) by the array length and flooring it, which does not guarantee uniform distribution. This behavior has been the same since shuffle was implemented, but it's possibly more noticeable when shorter playlists are shuffled.

As for the currently playing item retaining its position, that was introduced by #812, although I have now received feedback from a few people who aren't happy with it so perhaps that feature was accepted too hastily. At the least it should be possible to opt out of it.

calzoneman avatar Nov 07 '19 03:11 calzoneman

Absolutely agree that 812 was hasty. Should be a boolean channel option defaulted to on, but I personally would have it off.

Xaekai avatar Nov 08 '19 03:11 Xaekai