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

replace rng seed generation #141

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

phylter-qw
Copy link
Contributor

@phylter-qw phylter-qw commented Jun 5, 2024

Motivation

Duelers were alarmed by an apparent lack of randomness in starting spawns.

Some kind of seeding problem was suspected.

Investigation

phylter and Zorak discovered the following:

Sys_DoubleTime returns approximate seconds since MVDSV launch.

RNG is seeded with randomSeed = (int)(Sys_DoubleTime() * 100000).

Maximum value randomSeed can hold is 2147483647.

(int)(Sys_DoubleTime() * 100000) exceeds 2147483647 in just under 6 hours, after which the value stops increasing (or potentially behaves in an undefined way).

The code in question was introduced in 2021 (commit 837bdfc).

On affected servers, after 6 hours of uptime, it is possible to lock the starting spawns entirely by minimizing player activity (which can trigger RNG calls) during pre-war.

Solution

Seed the RNG directly with elapsed seconds since epoch.

Replace

(int)(Sys_DoubleTime() * 100000)

with

(int)time(NULL)

This provides a source of unique seeds for a very long time.

Notes

There is some concern that epoch functions might behave badly in the year 2038 on some old 32-bit machines... the so-called "2038 problem".

We figure such systems, already in the extreme minority, will face much bigger problems than running quakeworld.

Most MVDSV instances are already 64-bit, and are therefore unaffected.

There is a non-epoch alternative worthy of mention (suggested by foobar):

Replace

(int)(Sys_DoubleTime() * 100000)

with something like

double t = Sys_DoubleTime();
randomSeed = *(int*)&t;

This is elegant but cannot make any guarantees about seeds being unique. Presumably the repeats would be far apart.

@ciscon ciscon requested review from qqshka and ciscon June 5, 2024 21:32
@ciscon
Copy link
Collaborator

ciscon commented Jun 5, 2024

it should probably be mentioned that we can also introduce more entropy into the seed in ktx's g_seed_random function, as of now we're just using this data and turning it into something large enough for what is expected, but nothing is stopping us from using other sources as well when we actually do the seeding.

@VVD
Copy link

VVD commented Jun 5, 2024

This is related to issue #141

#140

@ciscon
Copy link
Collaborator

ciscon commented Jun 5, 2024

This is related to issue #141

#140

thanks, it autocompleted while i was typing it and i thought it knew better than i, heh.

@ciscon ciscon requested review from vikpe, meag and tcsabina June 6, 2024 02:34
@krizej
Copy link

krizej commented Jun 6, 2024

double t = Sys_DoubleTime();
randomSeed = *(int*)&t;

This is elegant but cannot make any guarantees about seeds being unique. Presumably the repeats would be far apart.

it's also undefined behavior afaik

@ciscon ciscon merged commit 5e3a784 into QW-Group:master Jun 11, 2024
6 checks passed
@phylter-qw phylter-qw deleted the seed-overflow-bug-fix branch June 11, 2024 19:51
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.

4 participants