feat: add fixedSeed param to GameSetup and use it to initialize RNG #875
Conversation
By specifying a fixed seed, this fixes variance between benchmark runs of, say, fightertest. See thread in: https://discord.com/channels/549281623154229250/1122484736913318021
Bruno-DaSilva
force-pushed
the
bruno/consistent-benchmarks
branch
from
ed4fde1 to
3822bb9
Compare
|
I dunno how to push to this specific branch, my desktop version seems a tad broken when i tried to switch, could you please add this to https://github.com/beyond-all-reason/spring/blob/BAR105/doc/StartScriptFormat.txt FixedSeed = 1; //Initialize the synced RNG with a specific seed for reproducible benchmarking runs only |
|
@Beherith I bet it's because it's in my fork instead of the main repo - you'd have to setup your local repo with a different remote url to https://github.com/Bruno-DaSilva/rts-spring branch Went ahead and added that line - can you confirm it's in the right place? |
|
Gets the LGTM from me, ill wait for the others to sign off too |
doc/StartScriptFormat.txt
Outdated
| @@ -50,6 +50,8 @@ | |||
| NumTeams=y; // same here, set this to check if the script is right | |||
| NumAllyTeams=z; // see above | |||
|
|
|||
| FixedSeed = 1; //Initialize the synced RNG with a specific seed for reproducible benchmarking runs only | |||
There was a problem hiding this comment.
Why 'benchmarking runs only'? Surely it's useful for things like say reproducible cutscenes in missions.
There was a problem hiding this comment.
Didnt think of that, thats a good use case!
There was a problem hiding this comment.
fixed - check out the new description
rts/Net/GameServer.cpp
Outdated
| rng.Seed(gameID.intArray[0] ^ gameID.intArray[1] ^ gameID.intArray[2] ^ gameID.intArray[3]); | ||
| Broadcast(CBaseNetProtocol::Get().SendRandSeed(rng())); | ||
| if (myGameSetup->fixedSeed == 0) { | ||
| GenerateAndSendGameID(); |
There was a problem hiding this comment.
Shouldn't gameID be sent regardless?
There was a problem hiding this comment.
good find, you're absolutely right this should live outside the condition
| @@ -582,6 +584,7 @@ bool CGameSetup::Init(const std::string& buf) | |||
|
|
|||
| file.GetTDef(mapSeed, unsigned(0), "GAME\\MapSeed"); | |||
There was a problem hiding this comment.
Looks like "map seed" exists which makes "fixed seed" not obviously RNG related. Perhaps call it "FixedRNGSeed" or something? Otherwise looks good though.
There was a problem hiding this comment.
FixedRNGSeed is much more clear. Changing that 🙌
| dsGameData->SetRandomSeed(rng.NextInt()); | ||
| } else { | ||
| dsGameData->SetRandomSeed(dsGameSetup->fixedSeed); | ||
| } |
|
Hey @Beherith , @sprunk , please watch this PR related to this one. |
|
@FIr3baL I was waiting for when engine was pushed out (was tested and pushed pushed fully out successfully?), else this would cause errors when running the scenario, I think. |
|
nah it should've been safe |
By specifying a fixed seed, this fixes variance between benchmark runs of, say, fightertest. See thread in: https://discord.com/channels/549281623154229250/1122484736913318021
Credit to @Beherith for figuring out this was a seeding issue
Feel free to nitpick, so I can get a better idea of what's important to y'all.