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

randomize() often producing same seed when called in _ready() #48087

Closed
mrimvo opened this issue Apr 22, 2021 · 3 comments · Fixed by #48098
Closed

randomize() often producing same seed when called in _ready() #48087

mrimvo opened this issue Apr 22, 2021 · 3 comments · Fixed by #48098

Comments

@mrimvo
Copy link

mrimvo commented Apr 22, 2021

Godot version:
3.3

OS/device including version:
Ubuntu 18.04

Issue description:
Calling randomize() in _ready() often produces the very same seed, and therefore the identical pseudo random number sequence.

I think I found the reason for this in the Godot sources in random_pcg.cpp:

void RandomPCG::randomize() {
    seed(OS::get_singleton()->get_ticks_usec() * pcg.state + PCG_DEFAULT_INC_64);
}

The problem is get_ticks_usec(). Because randomize() gets called during the startup process in _ready(), chances are we get the same startup time and thus the very same seed.

Minimal reproduction project:

func _ready():
  randomize()
  print(randi())

This would print the same random number often. Like about every 1000th run. This becomes a serious problem when trying to generate UIDs that way.

Proposal
How about adding OS::get_singleton()->OS.get_system_time_msecs() to the mix? Adding up system time and engine startup time would solve the problem I think. Or maybe something like time_msecs<<16 + ticks_usecs would work even better, because simply adding them up could result in similar problems, just time shifted.

Workaround
Maybe something like this works in GDScript. But not sure really:

seed(OS.get_system_time_msecs()<<16 + OS.get_ticks_usec())
@Xrayez
Copy link
Contributor

Xrayez commented Apr 22, 2021

This becomes a serious problem when trying to generate UIDs that way.

For this use case you're probably looking for godotengine/godot-proposals#1654.


In my experience, calling randomize() works well for prototyping and other similar purposes. You're likely going to implement your own seeding function via script if you'd like more control over it.

But I'm not excluding the possibility of improving the existing function, sure. Using system time seems to be more standard for randomization methods in other environments. So, this one could as well be considered a bug on some level...

@akien-mga
Copy link
Member

Yeah I would actually have expected this to use system time, this might have been a typo (OS.get_ticks_usec() may sound like OS unix time). I'm not sure engine ticks are worth taking into account at all.

@Xrayez
Copy link
Contributor

Xrayez commented Apr 22, 2021

Documentation says it's time-based in fact:

image

So yeah most likely it's a bug, or documentation is wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants