We use inconsistent approaches for random number generation across the codebase, and should pick one convention and apply it everywhere.
Two styles in use today
Verbose <random> style (std::random_device + std::mt19937 + std::uniform_int_distribution), e.g.:
client_generic/ContentDownloader/PlaylistManager.cpp:249-251 (and reused at lines 265, 293, 340, 377, 433-435, 451-453) — the keyframe-selection paths in getNextUncachedDream().
Simple C rand() style (rand() % n), e.g.:
client_generic/ContentDownloader/PlaylistManager.cpp:546, 578, 606, 799client_generic/ContentDownloader/SheepDownloader.cpp:551client_generic/Common/MathBase.h:46-47 (#define Random ...rand()...)
The same file (PlaylistManager.cpp) mixes both styles, sometimes within adjacent functions.
Ask
- Decide on a single convention for random selection/number generation and apply it consistently.
- Review the choice carefully rather than just standardizing on whichever is more common:
rand() is simple but low-quality and not thread-safe (it shares global state, and this code generates randomness from multiple threads — playback and the background downloader). The <random> approach is higher quality but verbose; a small shared helper (e.g. a thread-safe RandomInt(n)/RandomChoice(vec) in Common/) could give us the best of both.
- Once decided, sweep the call sites above (this is not necessarily exhaustive — grep for
rand(), mt19937, random_device, and the Random macro).
Context: came up while fixing #521, where the new shuffle-download path intentionally matched the surrounding rand() % style for local consistency.
We use inconsistent approaches for random number generation across the codebase, and should pick one convention and apply it everywhere.
Two styles in use today
Verbose
<random>style (std::random_device+std::mt19937+std::uniform_int_distribution), e.g.:client_generic/ContentDownloader/PlaylistManager.cpp:249-251(and reused at lines 265, 293, 340, 377, 433-435, 451-453) — the keyframe-selection paths ingetNextUncachedDream().Simple C
rand()style (rand() % n), e.g.:client_generic/ContentDownloader/PlaylistManager.cpp:546, 578, 606, 799client_generic/ContentDownloader/SheepDownloader.cpp:551client_generic/Common/MathBase.h:46-47(#define Random ...rand()...)The same file (
PlaylistManager.cpp) mixes both styles, sometimes within adjacent functions.Ask
rand()is simple but low-quality and not thread-safe (it shares global state, and this code generates randomness from multiple threads — playback and the background downloader). The<random>approach is higher quality but verbose; a small shared helper (e.g. a thread-safeRandomInt(n)/RandomChoice(vec)inCommon/) could give us the best of both.rand(),mt19937,random_device, and theRandommacro).Context: came up while fixing #521, where the new shuffle-download path intentionally matched the surrounding
rand() %style for local consistency.