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

THREE.MathUtils: added seededRandom() method #16655

Merged
merged 5 commits into from Jul 16, 2020

Conversation

WestLangley
Copy link
Collaborator

This is useful in generating repeatable results in an existing example.

THREE.Math.prng( 123 );

Math.random = THREE.Math.prng;

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 3, 2019

Similar code was removed from the examples here: #16576

And now you want to add it to the core? Um, does not seem logic to me...

@WestLangley
Copy link
Collaborator Author

@Mugen87 If you don't want users of the library to have access to this method, I'll close this.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 3, 2019

No offense, I'm just irritated by the PR. I think we made a decision in #16576 to not add such code to the lib.

Anyway, I don't feel strong about this. If you think it's a valuable addition, I won't block it.

@WestLangley
Copy link
Collaborator Author

WestLangley commented Jun 3, 2019

No offense, I'm just irritated by the PR

Please don't be. I was not intending to annoy you. I thought I was doing what you suggested when you said:

Okay, if @mrdoob wants to keep PRNG, I suggest to move it into the THREE namespace and the file to the math directory.

But I guess you were suggesting to move it to examples/js/math...

We can do that, merge this, or close this. I don't care.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 3, 2019

Yeah, in this sentence I was referring to examples/js/math^^. Anyway, since PRNG.js is now gone, I would only add it back if there is an actual user request.

@WestLangley WestLangley closed this Jun 3, 2019
@WestLangley WestLangley deleted the dev_math_prng branch June 3, 2019 17:37
@mrdoob
Copy link
Owner

mrdoob commented Jun 3, 2019

Seems like this was a misunderstanding. @Mugen87 suggested adding PRNG in the THREE namespace so we can generate the JSM version automatically.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 4, 2019

I actually wanted to remove PRNG.js in the first place since it was not used in the repo^^. So I think the status quo is okay now.

@WestLangley
Copy link
Collaborator Author

Trying again...

@WestLangley WestLangley reopened this May 30, 2020
@mrdoob
Copy link
Owner

mrdoob commented May 30, 2020

We just need a compelling use case.

@munrocket
Copy link
Contributor

munrocket commented Jun 6, 2020

var x = crypto.getRandomValues( new Uint32Array(1) )[0] / 0xFFFFFFFF;

Should work in any browser with webgl. Performance only twice worse, because it’s cryptography safe PRNG and supports an arrays.

@WestLangley
Copy link
Collaborator Author

Closing again...

@WestLangley WestLangley closed this Jun 8, 2020
@WestLangley WestLangley deleted the dev_math_prng branch June 8, 2020 02:38
@mrdoob
Copy link
Owner

mrdoob commented Jul 15, 2020

@WestLangley can we call this MahtUtils.deterministicRandom()?

@mrdoob mrdoob added this to the r119 milestone Jul 15, 2020
@mrdoob
Copy link
Owner

mrdoob commented Jul 15, 2020

Oh, can't reopen the PR because the branch is gone.

@WestLangley WestLangley restored the dev_math_prng branch July 15, 2020 13:25
@WestLangley WestLangley reopened this Jul 15, 2020
@WestLangley
Copy link
Collaborator Author

can we call this MathUtils.deterministicRandom()?

@mrdoob Maybe MathUtils.seededRandom() or .seedRandom()?

Similar nomenclature is used here: https://github.com/davidbau/seedrandom

@WestLangley WestLangley changed the title THREE.Math: added prng() method THREE.MathUtils: added prng() method Jul 15, 2020
@mrdoob
Copy link
Owner

mrdoob commented Jul 15, 2020

Hmm, seedRandom() is shorter... I have no preference, as long as is not an acronym.

@mrdoob
Copy link
Owner

mrdoob commented Jul 15, 2020

Actually, I think seededRandom() sounds better.

@WestLangley WestLangley changed the title THREE.MathUtils: added prng() method THREE.MathUtils: added seededRandom() method Jul 15, 2020
@mrdoob mrdoob merged commit 0aa4a84 into mrdoob:dev Jul 16, 2020
@mrdoob
Copy link
Owner

mrdoob commented Jul 16, 2020

Thanks!

@WestLangley WestLangley deleted the dev_math_prng branch July 16, 2020 08:45
@wrightwriter
Copy link

wrightwriter commented Feb 5, 2022

This function expects big numbers, otherwise it results in a very biased distribution.
I think this warrants a comment, explaining it, no?
image
image

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

Successfully merging this pull request may close these issues.

None yet

5 participants