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

Wyrand seed size lower than recommendation? #18

Closed
dmyyy opened this issue Apr 30, 2024 · 7 comments · Fixed by #19
Closed

Wyrand seed size lower than recommendation? #18

dmyyy opened this issue Apr 30, 2024 · 7 comments · Fixed by #19
Assignees
Labels
enhancement New feature or request

Comments

@dmyyy
Copy link

dmyyy commented Apr 30, 2024

The Seed trait docs suggest seeding with an array of [u8; 12] to avoid picking RNGs with partially overlapping periods - Wyrand uses [u8; 8]: https://docs.rs/rand_core/latest/rand_core/trait.SeedableRng.html

We also need to seed the prng on plugin init via with_seed - was wondering if something like this has its place in the library.

const N: usize = 8;

#[derive(Resource)]
pub struct RngSeed(pub [u8; N]);

impl RngSeed {
    pub fn new() -> Self {
        let slice = [0; N];
        let mut rng = RngSeed(slice);
        rng.gen();
        rng
    }

    pub fn gen(&mut self) {
        let mut rng = rand::thread_rng();
        for i in 0..N {
            self.0[i] = rng.gen();
        }
    }
}

impl FromWorld for RngSeed {
    fn from_world(_world: &mut World) -> Self {
        RngSeed::new()
    }
}
@Bluefinger
Copy link
Owner

For WyRand, it's internal state is pretty much just a u64. It's not a cryptographically secure PRNG, and as such, the seeding requirements/recommendations by rand apply a bit less to it. [u8; 8] is the size of WyRand state, so anything more would just not get used/be wasted.

For the second part, seeding via a resource with a FromWorld implementation is something I do want to try out, but not for random seeds. The mechanism to source randomised seeds is already pretty solid and wouldn't need complicating (Default impl for random states). This would instead be for more seeding GlobalEntropy from a resource. EntropyComponent would be more complicated, but that could be for further exploration.

@Bluefinger Bluefinger self-assigned this May 1, 2024
@Bluefinger Bluefinger added the enhancement New feature or request label May 1, 2024
@dmyyy
Copy link
Author

dmyyy commented May 1, 2024

I didn't realize the plugin default impl was random - default calls new, which sets the Seed to None which is a little unintuitive. Found it after a bit more digging.

@dmyyy
Copy link
Author

dmyyy commented May 1, 2024

Looking at it again the one thing I want to do is print the current seed (think an egui widget where you can set the seed or generate one) - is it possible to get the current seed from GlobalEntropy today?

@Bluefinger
Copy link
Owner

Looking at it again the one thing I want to do is print the current seed (think an egui widget where you can set the seed or generate one) - is it possible to get the current seed from GlobalEntropy today?

No, because the internal state of a PRNG is considered sensitive, so none of the PRNG algorithms Debug implementations will yield the internal state. Also, depending on the algorithm, there's more involved than just the seed as well. Actually, you can get the current state of a PRNG... by serializing it. But setting one can be tricky, so I provided a reseed method that basically just initialises a new PRNG instance inside the resource/component.

@Bluefinger
Copy link
Owner

Exploring some of this, it seems because of the generics involved, RngSeed must also be generic, so I can associate correctly a FromWorld implementation to load the right RngSeed resource. So like:

#[derive(Debug, Default, Resource)]
pub struct RngSeed<R: Sized + Default + AsMut<[u8]> + Clone + Send + Sync> {
    seed: R,
}

This then allows me to invoke RngSeed::<R::Seed> within GlobalEntropy methods, so if I have initialised a RngSeed component, then obtaining it from the World can be done cleanly. Naturally, if one does not initialise the resource, GlobalEntropy will fall back to sourcing entropy from from_entropy().

impl<R: SeedableEntropySource + 'static> FromWorld for GlobalEntropy<R> where R::Seed: Send + Sync + Clone {
    fn from_world(world: &mut World) -> Self {
        if let Some(seed) = world.get_resource::<RngSeed<R::Seed>>() {
            Self::new(R::from_seed(seed.get_seed()))
        } else {
            Self::from_entropy()
        }
    }
}

Now, I want to figure out whether to always initialise a RngSeed resource by default, but this will only give you the "starting" seed for a given GlobalEntropy. It will not track the updated state once you begin using GlobalEntropy. But once in place, it wouldn't be difficult at all to hook up a system that checks whether RngSeed has changed or not so you can reseed GlobalEntropy with the new seed, thus having the functionality you want for debugging.

@dmyyy
Copy link
Author

dmyyy commented May 3, 2024

Naturally, if one does not initialise the resource, GlobalEntropy will fall back to sourcing entropy from from_entropy().

I think bevy_rand should always initialize it by default (moving the from_entropy call into there).

it wouldn't be difficult at all to hook up a system that checks whether RngSeed has changed or not so you can reseed GlobalEntropy with the new seed

I think this is a generally useful feature - if I'm going to all this effort to have things be deterministic, I want the ability to try out runs (in my case world generation runs) with specific seeds for debugging purposes.

@Bluefinger
Copy link
Owner

I think bevy_rand should always initialize it by default (moving the from_entropy call into there).

I need to support cases where folks might be doing the resource initialisation manually, so to not fail and fallback correctly to sourcing entropy from OS sources. Like, maybe they want the GlobalRngSeed in debug builds, but for production/release, they remove the code for that and have things initialise without the seed resource for extra security.

You can check out what I did with #19, where GlobalRngSeed will initialise by default with OS sourced entropy, or with a seed provided to it. GlobalEntropy then has a FromWorld impl that checks if the GlobalRngSeed is present to use that, else to default to from_entropy.

I think this is a generally useful feature - if I'm going to all this effort to have things be deterministic, I want the ability to try out runs (in my case world generation runs) with specific seeds for debugging purposes.

I think the same should apply to EntropyComponent as well, though I need to think carefully on this one. Or I could just be overthinking anyway and should just apply the same pattern, though it will likely need more manual "linking" of logic since I don't think it'll be as easy to do. I could also just... test it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants