m-i-n-a-r / randomix

🎲 An open source app to choose randomly between numbers, answers, options and so on
MIT License
171 stars 20 forks source link

Range Roulette always generates same sequence after app restart #57

Closed gandro closed 9 months ago

gandro commented 1 year ago

Thanks a lot for this app! We've observed that the Range Roulette always generates the same sequence of events. The sequence seems to be different on different phones, but restarting the app and e.g. creating a roulette with range 1-20 always generates the same sequence of rolls.

Looking at the code, it seems that Range Roulette is relying on ThreadLocalRandom: https://github.com/m-i-n-a-r/randomix/blob/9e0f4172313f6095816b307af2abcefcace38d19/app/src/main/java/com/minar/randomix/fragments/RouletteFragment.java#L284-L286

Apparently, this is a bug in ThreadLocalRandom in Android 8 and above, see e.g. those bug reports Kotlin Issue KT-52618, Android Issue 234631055 and Android Issue 181797098

While this will be fixed in upcoming versions of Android, it might be still worthwhile working around this Android bug in randomix to prevent cheating.

m-i-n-a-r commented 1 year ago

Sorry for the late reply! I didn't know about this issue and i agree, I should try to fix it. I'll update the issue as soon as i work on it 😄

m-i-n-a-r commented 9 months ago

Hi again. After a (big) pause, I'm working on this bug and more features. At the moment, I can't recreate it on my side (Android 13). Is that normal? What are the step to recreate it?

gandro commented 9 months ago

Hi, so I am still able to reproduce on my Samsung S20 FE with Android 13 (edit: originally we discovered this on a Fairphone 4 with Android 12). Here is a screen recording:

https://github.com/m-i-n-a-r/randomix/assets/50564/459208a6-381f-4b2c-a99c-599294657545

Note that after restarting the app, the sequence generated for the roulette is identical, i.e. "9, 3, 18, 17, 9". This is different from other random functions in the app (e.g. dice rolls) which always generate new (i.e. unpredictable) sequences after an app restart.

m-i-n-a-r commented 9 months ago

That's a real mistery! I did the same procedure on my Oneplus 7 Pro running an Android 13 rom, and for 4 times, I had different sequences. This means that the bug only affects certain devices? This makes no sense since we're running the same android version. I'll try to recreate it on other test devices (Android 9, 11 and 12), let's see what happens 😕 Of course, this is a bug and I have to fix it nevertheless.

gandro commented 9 months ago

That is odd indeed! I don't know much about modern Android development, might it be an issue that is fixed by recompilation using a newer SDK? I'm running the v2.5 build from FDroid - perhaps recompiling it with a newer SDK might include a version of java.util.concurrent.ThreadLocalRandom which has the bugfix already applied?

Edit: At least the Kotlin commit seems to imply that: https://github.com/jetbrains/kotlin/commit/a2e8ed00329421a9a2997c87daa6b28c55ef9edc

m-i-n-a-r commented 9 months ago

Yep, it makes sense. Anyway, just to make sure this problem is fixed, I'm looking for an alternative approach 😄

m-i-n-a-r commented 9 months ago

I recreated it successfully on a Galaxy A32 running android 13, while I can't recreate it on my Oneplus 7 pro, even after reinstalling your exact same version from f-droid. The only possible explanation is that I have some fix in my rom that other devices haven't. Anyway, as I said, I'm going to use the "standard" Random and nextInt() function, just to make sure this is fixed :)

m-i-n-a-r commented 9 months ago

Done in aa642a15a2a2047facb54daac3456c4509c2d385 it should work properly now. I still have a translation to wait and a couple fixes left, but I should release the update quite soon (finally 😆)

gandro commented 9 months ago

Awesome, thank you!