Skip to content

Improve lmms::fastRand() and use it instead of std::rand()#7741

Merged
rubiefawn merged 36 commits intoLMMS:masterfrom
rubiefawn:fix/rand
Jan 16, 2026
Merged

Improve lmms::fastRand() and use it instead of std::rand()#7741
rubiefawn merged 36 commits intoLMMS:masterfrom
rubiefawn:fix/rand

Conversation

@rubiefawn
Copy link
Contributor

@rubiefawn rubiefawn commented Mar 2, 2025

  • Combine overloads of lmms::fastRand() into one template
    • Remove RAND_MAX constant since it is not used outside lmms::fastRand() (and all places that did previously use it should have been calling lmms::fastRand() anyway)
    • This is also done to lmms::fastLog10f() for consistency, though it is not related to the goal of the PR
  • Add lmms::oneIn() convenience function to replace occurences of (fastRand() % chance == 0)
  • Replace uses of std::rand() with lmms::fastRand() for consistency
  • Replace uses of scale * lmms::fastRand() / RAND_MAX - offset with lmms::fastRand(min, max)
// Old
int x = static_cast<int>(10.0f * rand() / RAND_MAX - 3.0f);

// New
int x = fastRand(-3, 7);

Testing

  • Triple Oscillator noise sounds the same
  • Organic randomize preset button works
  • Multiple Peak Controllers can coexist
  • Sfxr randomize preset button works
  • SID plugin sounds the same
  • Vibed plugin sounds the same
  • .ds samples with noise sound the same
  • Undo/redo doesn't instantly explode
  • Mixer channel color randomization works
  • Clip color randomization works
  • Track color randomization works

@rubiefawn rubiefawn marked this pull request as ready for review March 3, 2025 02:10
@rubiefawn rubiefawn added the needs code review A functional code review is currently required for this PR label Mar 19, 2025
@rubiefawn rubiefawn added the needs testing This pull request needs more testing label May 5, 2025
@bratpeki bratpeki self-assigned this Jun 11, 2025
Copy link
Member

@messmerd messmerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't reviewed everything yet, but here's what I have so far

@rubiefawn rubiefawn removed the needs testing This pull request needs more testing label Oct 21, 2025
@rubiefawn rubiefawn changed the title Replace uses of std::rand() with lmms:fastRand() Improve lmms::fastRand() and use it instead of std::rand() Oct 21, 2025
Also Github's diff viewer is cursed and I am now filled with distrust
towards it
@bratpeki
Copy link
Member

bratpeki commented Nov 1, 2025

Briefly tested Mallets, Spectrum Analyzer, ReverbSC, Flanger and Ogranic. I think we can (finally) merge! 🎉

@szeli1 szeli1 self-requested a review November 3, 2025 20:31
Copy link
Contributor

@szeli1 szeli1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm changing my approval in light of std::nearbyint.

@Veratil Veratil requested a review from szeli1 November 3, 2025 20:50
Out of scope, these belong on a different branch
@Veratil Veratil dismissed szeli1’s stale review November 3, 2025 20:57

Dismissing by request on Discord

Copy link
Member

@regulus79 regulus79 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was asked to review this PR; I have looked through all the code, and the changes appear fine to me. I am not an expert in pseudorandom number generation, but assuming people have tested it, I think it's fine.

@rubiefawn
Copy link
Contributor Author

Now that this has an approval from a dev with write access, I plan to merge this PR on 2026-01-08, provided no issues are found in the meantime. If anyone else would like to review this PR, please do so before that date.

Copy link
Member

@messmerd messmerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick review - haven't looked at things too closely yet

rubiefawn and others added 4 commits December 10, 2025 17:10
oopsie daisy

Co-authored-by: Dalton Messmer <messmer.dalton@gmail.com>
Also use "upper" instead of "range" for fastRand(x) functions, since the
lone parameter is not itself a range, but rather the upper limit of one.

Today I learned that I do not know how to spell "pseudorandom" lmao
Didn't know you could just put em in the angle brackets lol, neat
@rubiefawn rubiefawn merged commit 5e3a67c into LMMS:master Jan 16, 2026
11 checks passed
@rubiefawn rubiefawn deleted the fix/rand branch January 16, 2026 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs code review A functional code review is currently required for this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants