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

RNG.clone().setSeed() does not function as expected #201

Open
seiyria opened this issue Mar 3, 2022 · 8 comments
Open

RNG.clone().setSeed() does not function as expected #201

seiyria opened this issue Mar 3, 2022 · 8 comments

Comments

@seiyria
Copy link

seiyria commented Mar 3, 2022

Something I noticed today when porting my code from JS to TS, I wanted to use RNG.clone().setSeed() so I wasn't overriding the global RNG instance, and was using a unique one per map generation. I noticed that this causes inconsistent results in some weird ways. For example, my first few RNG picks match, but everything after that seem to go completely askew. A few notes:

  • my map generator is here - this is not the ported typescript code that I'm using, so it would require a little work to get working to pass an RNG instance around
  • my TS version is here so you can see differences if desired

pictures for comparison:

Orikurnis1-min
Orikurnis2-min

Might not mean a lot - but when I don't do a .clone() I always get the first image. When I clone the RNG before attempting to use it, I get some variation on the second one - it always picks the same 2 or 3 results the same, but then the rest is unpredictable.

Is there something with RNG.clone that's not being copied correctly?

@ondras
Copy link
Owner

ondras commented Mar 3, 2022

Hmmm. A brief look into the RNG implementation does not show anything suspicious: the clone() call correctly duplicates the state (represented by only 4 internal numbers).

It would be nice if you managed to reduce your test case to a minimal reproducible version, ideally without the map generator at all: making multiple clones of the RNG and generating a list of random numbers.

Few immediate ideas:

  • is this related to seeding, or not? What happens if you create many clones but do not seed them?
  • can you try to reproduce your problem in multiple different browsers (i.e. Chrome + Firefox) and with DevTools both open and closed?

@seiyria
Copy link
Author

seiyria commented Mar 4, 2022

In this case, this is node-based, not browser, so browser wouldn't affect anything. I'm not sure if it's related to seeding, but I don't think I would be able to tell if it wasn't, as I'm only using seeds.

A simple code repro also does not seem to cause it:

const ROT = require('rot-js');

const cloned = ROT.RNG.clone().setSeed(1123123);
const seeded = ROT.RNG.setSeed(1123123);

const arr = [1,2,3,4,5,6,7,8,9,10];

for(let i = 0; i < 10; i++) {
  const seedVal = seeded.getItem(arr);
  const cloneVal = cloned.getItem(arr);
  console.log('base', seedVal, 'clone', cloneVal, 'equal', seedVal === cloneVal);
}

All the more confusing, IMO, when all I did was change RNG.clone().setSeed() to RNG.setSeed(). The only other fundamental difference is using JS vs TS, but that really wouldn't matter, I think? I did have some trouble importing it in TS though, as I had to import from rot-js/dist/rot instead of just rot-js.

Sorry I can't be more helpful. I figured I'd report it in case you had an idea, but if not feel free to close this; it may be some strange quirk that's not worth tracking down.

@ondras
Copy link
Owner

ondras commented Mar 4, 2022

A simple code repro also does not seem to cause it:

To clarify: this small code example shows the issue, or works correctly? I am browser-only at the moment, so cannot verify via node.js.

In this case, this is node-based, not browser, so browser wouldn't affect anything.

In the rare case of a JS/JIT bug, a browser-based alternative would show different results when compared to your serverside run. So it would make sense - if possible - to run the code client-side as well for comparison.

Generally speaking, to find the bug, we will need to reduce the code down as much as possible. If the snippet above does not exihibit the behavior, it means the reduction was too large and we need to reduce less.

@ondras
Copy link
Owner

ondras commented Mar 4, 2022

Note (not sure if relevant): cloning a RNG does not seem to persist the seed. In other words:

RNG.setSeed(123)
RNG.clone().getSeed() != 123

But you are not reading the seed, right?

@seiyria
Copy link
Author

seiyria commented Mar 4, 2022

To clarify: this small code example shows the issue, or works correctly? I am browser-only at the moment, so cannot verify via node.js.

This code shows that in my specific example, there is not an issue (as the randomness matches for at most 3 getItem calls, then deviates).

But you are not reading the seed, right?

I did do a getSeed() as well, and it read my seed back to me. So it was definitely setting the seed correctly.

So it would make sense - if possible - to run the code client-side as well for comparison.

I believe the repo I linked (solokar-map-generator) would run in the browser if the json it required was inlined (the only other things it does are output some json, which can be skipped). Unfortunately I won't be able to help much more until I finish shipping this feature. If I end up thinking of something else that might help I'll post back though.

@ondras
Copy link
Owner

ondras commented Mar 4, 2022

The complete RNG state can be seen by calling rng.getState(). So you can debug by comparing states of your multiple (same-seeded) RNGs during map creation...

@seiyria
Copy link
Author

seiyria commented Mar 4, 2022

I see!

2022-03-04T13:26:05.410Z [RNGDungeonGenerator] Today's seed for Orikurnis: "1"
[
  2.3283064365386963e-10,
  0.000016081612557172775,
  0.11074089794419706,
  1
]
2022-03-04T13:26:14.407Z [GMCommand] Healer running @resetdungeon w/ "Orikurnis 1".
2022-03-04T13:26:15.450Z [RNGDungeonGenerator] Today's seed for Orikurnis: "1"
[
  2.3283064365386963e-10,
  0.000016081612557172775,
  0.11074089794419706,
  1
]

Yeah, unfortunately the states are exactly the same, even though the results are different. And I honestly have no idea why at this point, as I can see no compelling reason for this to be broken. Also, I am making sure the seed is a number (despite logs above).

Oddly, I've also tried discarding more results to see how many results would be the same, and I think there's just some deep-seated mess I've made. No matter what I do, the first "real" thing the RNG has to do (pick a visual combo) is always the same for the same seed, but everything after differs. So I think there's just some really weird issue on my side.

Ultimately, I think it's not a big deal and I'll just not clone the RNG, although my confusion as to why cloning the RNG does this is still there.

@vegeta897
Copy link

vegeta897 commented Mar 10, 2022

I just encountered this issue myself. I know what the problem was in my case, but I'm not 100% sure if it applies to yours.

My problem was that even though I was cloning the global RNG instance and setting a seed, none of the map methods such as .randomize() use that cloned instance. So if you don't set the seed of the global RNG instance, it will be random every time.

I mitigated this by copying state from my cloned instance into the global instance before running any of those methods, then copying the state from the global instance back to my clone right after.

RNG.setState(worldRNG.getState())
caves.randomize(0.5)
worldRNG.setState(RNG.getState())

If I wasn't in the middle of 7DRL lazy I'd make a PR to allow Map classes to accept an RNG instance in the constructor which they would use instead of the global instance.

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

No branches or pull requests

3 participants