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

Introduce xoshiro RNG to generate dungeon seeds #7030

Merged
merged 5 commits into from
Sep 17, 2024

Conversation

StephenCWills
Copy link
Member

Implements the technique for mitigating overlapping random number sequences described in #6438 (comment).

Summary of changes:

  • Introduce the xoshiro128++ RNG
  • Rename GenerateSeed() to GenerateRandomNumber()
  • Use xoshiro128++ to reimplement GenerateSeed()
  • Rename GameData::dwSeed to GameData::gameSeed
  • Use 64 bits for the game seed and use std::chrono to generate it
  • Update logic for generating dungeon seeds using xoshiro128++ instead of LCG

The 32-bit overload for xoshiro128plusplus::seed() is intended to provide a way to reseed xoshiro128++ using a dungeon seed. This will become necessary to create a deterministic sequence of seeds for monsters, objects, and items during dungeon generation. It feels a bit like falling back on old habits to use this random number generator to seed itself, but the SplitMix32 implementation makes it work.

This replaces #6438
This resolves #6261

Source/engine/random.hpp Outdated Show resolved Hide resolved
Source/engine/random.hpp Outdated Show resolved Hide resolved
Source/engine/random.hpp Outdated Show resolved Hide resolved
Source/engine/random.hpp Outdated Show resolved Hide resolved
Source/engine/random.hpp Outdated Show resolved Hide resolved
Source/engine/random.hpp Outdated Show resolved Hide resolved
@AJenbo AJenbo requested a review from ephphatha March 18, 2024 00:41
ephphatha
ephphatha previously approved these changes Mar 18, 2024
Copy link
Contributor

@ephphatha ephphatha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this implementation (I was looking at using xoshiro128** myself but couldn't think of a good initial use case :D), a few nits and the only real concern is whether we want to give the option of using the old dungeon table logic for people who want to stay close to the original game.

Regarding "It feels a bit like falling back on old habits to use this random number generator to seed itself, but the SplitMix32 implementation makes it work." the flaw with the old logic is that it's using a single set of parameters for the LCG to seed itself, so all sequences are overlapping. This implementation solves that since you're using three different families of function (SplitMix > xoshiro > LCG) to seed each other.

You could even use the global xoshiro128plusplus generator to spawn instanced xoshiro128plusplus generators by making use of the jump() function, the purpose of that is to advance the state far enough that the sequences will not overlap in their usage lifetime.

Real excited about potential future improvements too. It would be amazing to see instances of xoshiro128++ used for monster AI, that'd let multiplayer games get away from constantly reseeding the global RNG in an attempt to sync monster behaviour.

Source/engine/random.hpp Outdated Show resolved Hide resolved
Source/multi.cpp Outdated Show resolved Hide resolved
Source/msg.cpp Show resolved Hide resolved
Source/multi.cpp Outdated Show resolved Hide resolved
@StephenCWills StephenCWills force-pushed the xoshiro-dungeon-seeds branch 2 times, most recently from 2754e84 to ebdad6e Compare March 19, 2024 03:30
Source/multi.h Outdated Show resolved Hide resolved
Source/engine/random.hpp Outdated Show resolved Hide resolved
Source/engine/random.cpp Outdated Show resolved Hide resolved
Source/engine/random.hpp Outdated Show resolved Hide resolved
Source/engine/random.hpp Outdated Show resolved Hide resolved
ephphatha
ephphatha previously approved these changes Mar 20, 2024
Copy link
Contributor

@ephphatha ephphatha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RNG implementation looks good to me, if we find a platform that implements std::random_device as a SplitMix64 generator seeded with the current unix timestamp then I guess we gotta revisit how the global rng gets seeded :D

@StephenCWills
Copy link
Member Author

Assuming this PR makes it into 1.6, don't forget to merge diasurgical/devilutionx-gamelist#27 and update the Discord bot at some point before the release.

Copy link
Member

@AJenbo AJenbo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scary stuff, lets hope this works out as planned :)

@AJenbo AJenbo merged commit cfe9a8c into diasurgical:master Sep 17, 2024
22 checks passed
@StephenCWills StephenCWills deleted the xoshiro-dungeon-seeds branch September 17, 2024 21:35
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.

Finding two identical items in the same session
3 participants