Godot: randi() returns 0 for FIRST number after seed()

[krayon]

CODE:

seed(0)
print(randi())
print(randi())
seed(1)
print(randi())
print(randi())

OUTPUT (Godot 2.1.4):

719435623
1221583151
16807
282475249

OUTPUT (Godot 3.0.2):

Godot 3.0.2
0
1613493245
0
3114030964

If randomize() is used, the same issue doesn't seem to occur.

Is this expected behaviour?

[willnationsdev]

Oh geez, that's pretty terrible. XD We'll have to fix that...

[tagcup]

Just because you get 0 when you need it with 0 and 1 doesn't mean it "always" returns 0. Current PRNG essentially expects back huge seed. randomize specifically does that. Yes, it's expected, look for a PR by me about PCG64 for details.

[akien-mga]

Thanks for the explanation, this just needs a mention in the documentation of seed() for users which might be confused by this implementation detail then.

[tagcup]

Err I meant PCG32

I didn't think this was worth mentioning in the docs at the time because it's just an implementation quirk, but yeah, maybe I makes sense

[puppetmaster-]

If this is not a bug, how is the correct procedure to always get the same numbers after setting the seed without getting an 0 in the first place?

[krayon]

@tagcup Thanks for the info and correction. My use of "always" may have been SLIGHTLY overblown :P

You're not wrong on the big number expected. An 8 digit seed still returns 0 first.

[clayjohn]

I've been looking into this in more detail and it seems that the first value returned after a call to seed is almost always garbage essentially. It is consistently lower than 10 for seeds less than 2^30. And above that it slowly increases based on the number passed into seed. Although this may be expected behavior to the people who implemented the RNG system it will come as a surprise to any user that the size of the first returned value is heavily based on the size of the seed.

Documenting this behavior is a good first step, but realistically most users don't care about the implementation, they just want good values returned every time, currently you have to call randi() once after your call to seed so you can get acceptable values. What we should look into long term is either calling it once internally for the user or coming up with a better RNG.

What makes this even worse is that randf() calls rand() internally, so for most seeds you are getting a value that is essentially zero.

[Omicron666]

From http://www.pcg-random.org/using-pcg-c-basic.html
This was expected...

Failing to correctly initialize the state can result in a generator with improper behavior (for example, all-zeros initialization will produce a generator that always returns zero). If for some reason you cannot call pcg32_srandom_r, you can initialize a variable using the special constant PCG32_INITIALIZER which expands a standard C brace-initialization constant with some suitably chosen fixed constants. The global generator is suitably initialized in this way so that it will work even without a call to pcg32_srandom (although it will always produce the same pseudorandom sequence).

[akien-mga]

@tagcup I know I asked you to use the most minimal implementation back then, but I'm slowly changing my mind :P I still think the 2k LOC PCG C++ implementation seems overkill, but maybe we can use the complete C basic implementation: https://github.com/imneme/pcg-c-basic/blob/master/pcg_basic.c

This one includes pcg32_srandom_r mentioned in @Omicron666's comment, and is initialized to PCG32_INITIALIZER, so I guess it might behave better than our current implementation.

[tagcup]

Yeah, sure sounds good to me. Should I submit a PR for that?

[tagcup]

The C++ version shouldn't be much of an overhead either actually when compiled

[mwerezak]

I've experienced the same problem as well, except with randf().

I discovered this when I started noticing wonky results with randomly generated map decoration in a game I'm working on. When I tracked down the commit causing the problem, I saw that the only change was that I replaced a call to rand_seed() with seed(). When I changed it back to rand_seed() everything started working fine again.

[mwerezak]

In my case to make sure the map was consistent between plays I set the seed to static_map_seed ^ hash(position) before selecting the type of decoration to place there. This had the result of (1) making this bug very obvious, and (2) since the seeds come from a hash they are always large numbers.

Therefore the point that @tagcup brought up earlier doesn't seem correct (as @krayon and @clayjohn noted later). I think there is evidence of an actual bug here.

[tomkun]

I was toying with seed based, deterministic procedural generation and run into the same issue.

I've tested seed() within the 0-200,000,000 range, from seed(0)to seed(134217727)the first call to randi() returns 0 every time. From 134217728 on randi() generously switches to 1.

After the first one with new seed, all subsequent calls to randi() seem to be working as expected.

If this is indeed the desired behaviour, I would consider at the very least mentioning it in the docs.

Tested on 3.0.6 x64

[mwerezak]

If this is somehow desired behaviour, it's a very poor choice of behaviour.

If not, someone please update the labels.

[Sslaxx]

Is this still an issue?

[volzhs]

did a quick test with 3.1.beta3 35bb52011

0
1613493245
0
3114030964

[Sslaxx]

@Chaosus - is this intended behaviour then?

[Chaosus]

I think its incorrect behavior, but it will break compatibility if we change it right now. Its too late to push it to 3.1, so maybe make a patch for 3.2/4.0.