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

PseudoRandom always generating same sequence with big seed #3237

Closed
eduardomezencio opened this issue Oct 8, 2015 · 6 comments
Closed

PseudoRandom always generating same sequence with big seed #3237

eduardomezencio opened this issue Oct 8, 2015 · 6 comments
Assignees
Labels
Bug Issues that were confirmed to be a bug @ Mapgen @ Script API

Comments

@eduardomezencio
Copy link

When seed is >= 2^31, PseudoRandom is always generating the same sequence. This could be a bug to be fixed or a feature, I don't know (or maybe a bug that will not be fixed).

If it's not a thing to be fixed, I think there are 2 problems:

  • The main problem: the blockseed given by minetest.register_on_generated is often larger than 2^31, making it not usable with PseudoRandom
  • Also, this is not in the documentation, so some people will take a long time to notice that their random stuff is actually repeating.

(I have tested both in 0.4.13 and compiling the latest version from git)

@est31 est31 added Bug Issues that were confirmed to be a bug @ Script API @ Mapgen labels Oct 8, 2015
@kwolekr
Copy link
Contributor

kwolekr commented Oct 9, 2015

Yeah, internally all of the old random/noise functions internally use a 32 bit number as a seed. Unfortunately, "fixing" this would break compatibility with all old maps. That's the primary reason why it hasn't been done yet.

I suppose this could be "solved" by making a note of it in some documentation.

@kwolekr
Copy link
Contributor

kwolekr commented Oct 9, 2015

Not really sure why a larger integer makes it "not usable" with PseudoRandom.
The higher bits should be ignored, no?

@eduardomezencio
Copy link
Author

If the higher bit were really ignored, then there would be no problem.

Look at the sequences I pasted at the end of this text. As you can see, all numbers that are >= 2147483648 are generating the same sequence, meaning that the higher bit is not being ignored. If that was the case, then 2147483649 would have to give the same sequence as 1, 2147483650 the same as 2 and etc.

I said it's not usable because lots and lots of block seeds will end up giving the same numbers an generating the same things.

I agree with you that one solution would be to simply add this to the documentation, so the modders will know they can use something like: random = PseudoRandom(blockseed % 2147483648) and then it'll work alright.

But another solution would be just to make it ignore the higher bits.

Seed = 2147483644
30950 18769 9485 7756 24358 29358 20952 3494 28531 25705 17542 18657 24730 550 9302 26433 
Seed = 2147483645
15022 3060 9611 3154 19143 18059 27918 23941 32621 21382 8029 4372 28154 2851 20441 18102 
Seed = 2147483646
31860 20119 9736 31320 13929 6759 2115 11619 3942 17058 31285 22854 31579 5155 31579 9770 
Seed = 2147483647
15929 4410 9863 26718 8714 28226 9080 32063 8032 12735 21774 8569 2236 7456 9950 1439 
Seed = 2147483648
1 21468 9988 22117 3499 16927 16046 19741 12122 8411 12261 27053 5659 9759 21088 25876 
Seed = 2147483649
1 21468 9988 22117 3499 16927 16046 19741 12122 8411 12261 27053 5659 9759 21088 25876 
Seed = 2147483650
1 21468 9988 22117 3499 16927 16046 19741 12122 8411 12261 27053 5659 9759 21088 25876 
Seed = 2147483651
1 21468 9988 22117 3499 16927 16046 19741 12122 8411 12261 27053 5659 9759 21088 25876 
Seed = 2147483652
1 21468 9988 22117 3499 16927 16046 19741 12122 8411 12261 27053 5659 9759 21088 25876 
Seed = 2147483653
1 21468 9988 22117 3499 16927 16046 19741 12122 8411 12261 27053 5659 9759 21088 25876 
Seed = 2147483654
1 21468 9988 22117 3499 16927 16046 19741 12122 8411 12261 27053 5659 9759 21088 25876 

@kahrl
Copy link
Contributor

kahrl commented Oct 13, 2015

The problem lies here. The return value of luaL_checknumber, a lua_Number (= double), is converted to int there. As the C++ standard says: "The behavior is undefined if the truncated value cannot be
represented in the destination type." (C++03 4.9).

For example, the x86 instructions FISTP and CVTTSD2SI return the so-called "indefinite integer value", 0x80000000 (= -2147483648), if the value cannot be represented. With this as the seed, PseudoRandom returns the sequence 1 21468 9988 22117 ....

Behaviour on other architectures or with different compilers may be different.

@ShadowNinja
Copy link
Member

Considering that this behavior is compiler and architecture dependant, maps can currently be considered broken -- and a change that simply drops the top bits could be accepted as a fix for map compatability I think. It may be technically incompatible, but this only affects Lua mapgens, so it shouldn't be too much of an issue.

@kwolekr kwolekr self-assigned this Oct 18, 2015
@kwolekr
Copy link
Contributor

kwolekr commented Oct 26, 2015

bc0318d

@kwolekr kwolekr closed this as completed Oct 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues that were confirmed to be a bug @ Mapgen @ Script API
Projects
None yet
Development

No branches or pull requests

5 participants