Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#4127 closed defect (fixed)

[PATCH] Actually initialize random number generator seed in the simulation

Reported by: elexis Owned by: elexis
Priority: Must Have Milestone: Alpha 21
Component: Core engine Keywords: patch
Cc: Patch:

Description

Reproduce non-randomness:

  • With Regicide mode, the same hero is spawned everytime on the map new_rms_test.json
  • Math.random() delivers the same number everytime on that map, different ones on other maps
  • Reducing elements from anatolian plateau leads to repeated random numbers instead of different ones every generation

Code: Currently the simulation doesn't initialize the seed for the (custom) Math.random() function being set in the ComponentManager.cpp, see this TODO:

// TODO: ought to seed the RNG (in a network-synchronised way) before we use it

There are three different RNGs: in the ComponentManager, AIManager and MapGenerator. As they are defined in individual scriptinterfaces / contexts, they ought to be independent from each other.

However the observation doesn't match this. Apparently the mapgeneration randomness seeds the simulation randomness too.

Either way the seed should be set and could just reuse the seed determined in gamesetup.js:

g_GameAttributes.settings.Seed = Math.floor(Math.random() * 65536);

Also the code should at least use 32bit, not 16bit random numbers while at it.

One could implement a different seed for both map generation and simulation. That however would complicate the implementation (atlas) and debugging without any real benefit.

Not sure (yet?) on how to test that the random numbers returned are actually random.

Attachments (2)

init_simulation_seed_v1.patch (6.9 KB ) - added by elexis 4 years ago.
The tests added only test that boosts returns random numbers, not that the seed is actually initialized in the simulation. Not sure how to do that, there should be Simulation2 tests that include mapreader tests.
init_simulation_seed_v1.1.patch (6.6 KB ) - added by elexis 4 years ago.
Taking into account sanderd17 comments on August 1st and 2nd. (Removing one test that attempted to ensure randomness. Can't use shift operator as that only supports i32 and not u32.)

Download all attachments as: .zip

Change History (5)

by elexis, 4 years ago

The tests added only test that boosts returns random numbers, not that the seed is actually initialized in the simulation. Not sure how to do that, there should be Simulation2 tests that include mapreader tests.

by elexis, 4 years ago

Taking into account sanderd17 comments on August 1st and 2nd. (Removing one test that attempted to ensure randomness. Can't use shift operator as that only supports i32 and not u32.)

comment:1 by elexis, 4 years ago

Owner: set to elexis
Resolution: fixed
Status: newclosed

In 18604:

Actually seed the random number generator used in the simulation. Reviewed by sanderd17, fixes #4127.

comment:2 by elexis, 4 years ago

Keywords: rfc removed

comment:3 by elexis, 3 years ago

In 19749:

Always set the Seed property from autostarted games, because it is required and used to initialize the RNG of the simulation following rP18604.

Differential Revision: https://code.wildfiregames.com/D592
Refs #4127, #3965

Note: See TracTickets for help on using tickets.