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

COMMON: avoid bad rng seeds #3340

Closed
wants to merge 1 commit into from
Closed

Conversation

@Die4Ever
Copy link
Member

@Die4Ever Die4Ever commented Sep 4, 2021

RandomSource has some seeds that produce short loops

I wrote a test for this

#include <iostream>
#include <thread>
#include <vector>
#include <mutex>

std::mutex mtx;
typedef unsigned int uint32;

uint32 rng(uint32 _randSeed) {
	_randSeed = 0xDEADBF03 * (_randSeed + 1);
	_randSeed = (_randSeed >> 13) | (_randSeed << 19);
	return _randSeed;
}

int test_seed(uint32 orig_seed) {
	uint32 s = orig_seed;
	uint32 prev = s;
	for (int i = 0; i < 50; i++) {
		s = rng(s);
		if (s == orig_seed) return i+1;
		if (s == prev) return i + 1;
		prev = s;
	}
	return -1;
}

void test_seeds(uint32 first, uint32 last) {
	for (uint32 s = first; ; s++) {
		int loops = test_seed(s);
		if (loops != -1) {
			std::lock_guard<std::mutex> lck(mtx);
			std::cout << s << " failed with " << loops << " iterations\n";
		}
		if (s == last) break;
	}
}

void test_rng() {
	std::vector<std::thread> threads;
	uint32 start = 0;
	int num_threads = 12;
	uint32 max = UINT_MAX;
	//uint32 max = 100000000;
	uint32 each = max / num_threads;
	for (int i = 0; i < num_threads-1; i++) {
		threads.push_back(std::thread(test_seeds, start, start +each));
		start += each + 1;
	}
	threads.push_back(std::thread(test_seeds, start, max));

	for (uint32 i = 0; i < threads.size(); i++) {
		threads[i].join();
	}
}

int main()
{
	std::cout << "start!\n\n";
	test_rng();
}

which produced this output

1800586416 failed with 48 iterations
1087576404 failed with 48 iterations
3593762925 failed with 11 iterations
730316237 failed with 48 iterations
2527622093 failed with 48 iterations
3969000752 failed with 48 iterations
3997635582 failed with 48 iterations
782266141 failed with 48 iterations
2923531432 failed with 48 iterations
3285614266 failed with 48 iterations
790957358 failed with 48 iterations
2228155406 failed with 11 iterations
2934411674 failed with 48 iterations
3295602753 failed with 48 iterations
3662902891 failed with 11 iterations
450572919 failed with 48 iterations
2598359503 failed with 48 iterations
3675745779 failed with 48 iterations
3314127686 failed with 48 iterations
1183117626 failed with 48 iterations
1184201285 failed with 1 iterations
1532332879 failed with 48 iterations
2985431791 failed with 48 iterations
3728428744 failed with 48 iterations
3732357426 failed with 11 iterations
4103830679 failed with 48 iterations
159978368 failed with 48 iterations
1960730702 failed with 48 iterations
888568342 failed with 48 iterations
535309383 failed with 11 iterations
895276097 failed with 11 iterations
1612982501 failed with 48 iterations
3056724351 failed with 48 iterations
3424350891 failed with 48 iterations
3086530181 failed with 48 iterations
3825293921 failed with 48 iterations
3463439511 failed with 48 iterations
1326826220 failed with 48 iterations
3465351362 failed with 48 iterations
2406416072 failed with 48 iterations
2407747594 failed with 48 iterations
2051590013 failed with 48 iterations
3122928263 failed with 11 iterations
3123690540 failed with 48 iterations
2777780976 failed with 48 iterations
2060636277 failed with 48 iterations
4215288456 failed with 48 iterations
3871845201 failed with 11 iterations
1010819822 failed with 48 iterations
1019732222 failed with 11 iterations
3889985373 failed with 48 iterations
3890987163 failed with 48 iterations
3893166069 failed with 48 iterations
3902497450 failed with 48 iterations
688539017 failed with 48 iterations
1050591644 failed with 48 iterations
1052334923 failed with 48 iterations
1765143913 failed with 48 iterations
1768895800 failed with 11 iterations
2142534809 failed with 11 iterations

We might also want to switch to a more robust rng function. @TehGelly made pull request #3341 which replaces the rng function, we don't need both of these so I'll let you guys pick.

RandomSource has some seeds that produce short loops
@Die4Ever Die4Ever changed the title COMMON: avoid bad seeds COMMON: avoid bad rng seeds Sep 4, 2021
TehGelly added a commit to TehGelly/scummvm that referenced this issue Sep 4, 2021
The old RNG method had non-standard periods, ranging from some seeds looping on themselves (seed = 1184201285) to some seeds having periods as low as 11 or 48, as listed in scummvm#3340. This is a problem even for games that run the RNG once a frame, as the possibilities for random events is greatly reduced should the initial seed be in one of these sets of small periods.

Xorshift is a standard, fast, non-cryptographic PRNG with academic backing that has period 2^32-1 (all seeds lead to another seed except 0, which is excluded from the initial seeds).  Many different flavors are possible, as listed in the paper, but the choice implemented in this pull request uses only a single 32-bit integer as a state, like the old PRNG.
@criezy
Copy link
Member

@criezy criezy commented Sep 6, 2021

Pull request #3341 would have my preference.
Blacklisting all the seeds above 100000000 because some of them have short loops seems a bit hackish to me.

@Die4Ever
Copy link
Member Author

@Die4Ever Die4Ever commented Sep 6, 2021

Pull request #3341 would have my preference.
Blacklisting all the seeds above 100000000 because some of them have short loops seems a bit hackish to me.

still a way larger pool of seeds than we had 10 days ago lol (#3310 ), but yea I also prefer pull request #3341 as a more robust alternative to this

@bluegr
Copy link
Member

@bluegr bluegr commented Sep 6, 2021

This is more or less a hack that blacklists specific seeds. Closing in favor of #3341

@bluegr bluegr closed this Sep 6, 2021
bluegr added a commit that referenced this issue Sep 9, 2021
The old RNG method had non-standard periods, ranging from some seeds looping on themselves (seed = 1184201285) to some seeds having periods as low as 11 or 48, as listed in #3340. This is a problem even for games that run the RNG once a frame, as the possibilities for random events is greatly reduced should the initial seed be in one of these sets of small periods.

Xorshift* is a standard, fast, non-cryptographic PRNG with academic backing that has period 2^32-1 (all seeds lead to another seed except 0, which is excluded from the initial seeds).  Many different flavors are possible, as listed in the paper, but the choice implemented in this pull request uses only a single 32-bit integer as a state, like the old PRNG.

Co-authored-by: Thierry Crozat <criezy@scummvm.org>
Co-authored-by: Filippos Karapetis <bluegr@gmail.com>
@Die4Ever Die4Ever deleted the rng-avoid-bad-seeds branch Sep 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants