-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Provide bevy_entropy
crate for RNG sources within ecs
#7871
Conversation
Welcome, new contributor! Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨ |
Is there a reason why this shouldn't be just a third party crate? |
There was interest in a first-party solution, and the defacto standard for RNG in the rust ecosystem is |
Your PR increases Bevy Minimum Supported Rust Version. Please update the |
3 similar comments
Your PR increases Bevy Minimum Supported Rust Version. Please update the |
Your PR increases Bevy Minimum Supported Rust Version. Please update the |
Your PR increases Bevy Minimum Supported Rust Version. Please update the |
The previously mentioned RFC goes into reasoning: https://github.com/LegNeato/bevy-rfcs/blob/entropy/rfcs/55-deterministic_rng.md#motivation |
What about using the hasher as a source of entropy? |
Also, feel free to grab any of the user level guide stuff from the RFC |
|
As a further follow-up, |
Ah yeah, great point |
7754dd9
to
a9ab1a0
Compare
5114269
to
d2b8a70
Compare
0ffd290
to
700ef36
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some reflection-related comments
crates/bevy_entropy/src/component.rs
Outdated
)] | ||
#[cfg_attr( | ||
all(feature = "serialize", feature = "bevy_reflect"), | ||
reflect_value(Debug, PartialEq, Serialize, Deserialize) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should likely also register the following type data: ReflectFromReflect
and (if we want this to be used in scenes) ReflectComponent
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this basically forces the use of only seedable RNG sources, so SeedableRng
only types. That at least removes the possibility of using something like OsRng
from rand_core
, so ensuring deterministic algorithms only and no hardware/OS RNG sources.
I'm okay with that though, as this then ensures people have to provide a user-land algorithm.
crates/bevy_entropy/src/component.rs
Outdated
#[cfg_attr( | ||
all(feature = "serialize", feature = "bevy_reflect"), | ||
reflect_value(Debug, PartialEq, Serialize, Deserialize) | ||
)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably want to split this into two attributes, right? That way we can keep the reflection registrations without needing the serde ones (I know the bevy_reflect
feature relies on serialize
, but just for clarity and ease of separation in the future).
crates/bevy_entropy/src/resource.rs
Outdated
#[cfg_attr( | ||
all(feature = "serialize", feature = "bevy_reflect"), | ||
reflect_value(Debug, PartialEq, Serialize, Deserialize) | ||
)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here. I think we should split these.
Also, we can probably register the ReflectFromReflect
and ReflectResource
type data.
I don't see where, especially given the RFC seem to justify itself (given its name) on the basis we need a deterministic RNG, then goes:
So, what is the rationale for this not being a third-party crate? Everything I saw in the RFC seem to be easily implemented in a 3P crate. The only reason I'd see would be if several crates want to interop with each other via RNG, and so a built-in crate would enforce a common interface, but I'm not sure why they would want to do this (can't think of any example). |
The rationale being that pretty much the RNG side of the rust ecosystem has converged on It can live as a third-party crate, sure, but
The common interface already exists with |
Your questions are answered in what you link to and it acknowledges it could be a third-party crate with different tradeoffs. TL;DR: it needs to be in bevy to coordinate between ecosystem crates, enable advanced game features like lockstep, and make unit testing games better in general. Randomness is clearly used a ton in real world use cases...most of bevys examples use it, which points to being in bevy proper. Every other engine has randomness built in as a primitive, so there is prior art that points to including it. |
TBF, one of the biggest cases for Deterministic RNG is being able to test systems that deal with random output. Being able to enforce the correct patterns for this is a win for the bevy ecosystem. It might not make sense for quick and dirty examples, but for anything approaching a serious project, being able to test your random parts is a huge boon. |
This is a rationale for converging on
Same, probably useful, but also probably doable as a third-party crate.
But does Bevy internally uses some PRNG? If not, examples can consume a 3P crate, and there's no argument to make this a first-party one.
And perhaps also we can do better as a third-party crate? I fail to see any correlation between the problems raised in the RFC and here, and the requirements to be a first-party crate.
Completely agreed, that sounds very reasonable and useful. And Bevy's policy is to provide such features as third-party crates to iterate over, until proven useful enough and stable enough to merge into core, unless there's a clear argument making it impossible to do so in the first place. The I think it would be easier to convince reviewer if the following questions where answered:
My personal understanding so far is that:
The determinism raised in the RFC is a very valid concern. I just don't think the RFC answers the 1P/3P question that we ask all crates to answer. |
In the mean time then, I've published https://github.com/Bluefinger/bevy_rand which is basically this crate, but with some updates. So now it exists as a third-party solution to battle-test what this crate is aiming to provide. I'm leaving |
I'm not doing a great job explaining first party vs third party. It feels a bit like trying to prove a negative but I'll give it a other shot: What is better for the bevy ecosystem...having every single third party crate using randomness standardize on rand and dependency injection patterns (via components or resources) with potentially differing API, feature names, etc, or putting it in bevy? Randomness/entropy for games is as core as time IMHO. Punting implementation to a third party crate with adhoc coordination fragments the ecosystem--all it takes is one crate in your dependency chain that doesn't know about the 3rd party crate and your test and game determinism is blown out. You can of course go to that crate and fix it, but why play whack-a-mole across the whole ecosystem by default? There is a reason why every other engine has randomness built in...almost every usage of the engine will need randomness, and almost every third party lib will want to use it in the same way. I'd love to learn about a full-featured engine that doesn't have randomness in core...Unity, Unreal, Godot, Source, etc all have it. When building distributed systems (which is what bevy is structured as, it just happens to run on one machine), there are certain things you make sure to control to enable scenario testing as well as advanced runtime patterns--time, I/O (mem, disk, net, etc), and entropy are the big ones. Bevy has time (bevy_time) and I/O already (ecs takes care of mem, loaders take care of disk, punts on networking for now) but lacks an abstraction for randomness/entropy. |
I think that's the part that's not as obvious as you are saying. There's plenty of projects that don't need randomness. Almost all of bevy's examples don't need it and neither does bevy itself except in, I believe, one limited situation. |
I'm not sure you can say the examples do not need it when they almost all currently have it 😂. Sure, one can not use randomness in toy examples or narrow use-cases...but I don't think that is representative. When I get time, I'll look at the bevy jam entries...I bet well over 50% use randomness! |
There's 165 files with .rs extension in the examples folder and only 15 of those use rand and half of those are for stress tests. In most cases rand is only used as a workaround to spawn a lot of stuff which would generally not be done like that in a lot of games. I'm not saying randomness is uncommon, just that it's not as common as you are saying. |
Fair, the number of examples changed since the original RFC was written. |
I think the problem currently is that patterns/abstractions for coordinating the ecosystem are not yet established in the context of Bevy. Thus it is hard to argue for first-party basis on that. While this is explored, I think folks are okay with things being fragmented until the dust settles on a good way forward, and exploring that will be easier as a third party crate (which is what is being argued here and is the accepted modus operandi of the bevy crate ecosystem). So for anyone reading this, please do go over to bevy_rand to help with establishing said patterns and providing use-cases, ideas, abstractions, etc, if you are interested in contributing in this area. |
There is a chicken and egg here. I'm not sure why entropy / randomness has a higher bar than previous plugins with desirable functionality that were included in core and then iterated on (for example, UI). The abstractions can't be flexed if everyone just uses While 18 /209 of examples use rand, 2/4 of the game examples do. I don't think the examples are representative of use, but as mentioned in the RFC every other major engine includes this functionality! Shall we port the examples to the third-party crate in hopes that more people pick it up and try the API? If so, how is that better/worse than just including it in bevy as unstable? If not, how do we intend to get people to give feedback on the API? What are next steps @alice-i-cecile ? Is Bevy even using the RFC process anymore? |
@LegNeato I think the argument about other engines applies less to Bevy because Bevy is designed to be extremely modular and flexible, with the ability to plug in all sorts of functionality, first-party and third-party and have it just work. And what Bevy exposes through its APIs allows for third-party crates to do a lot in the first place. So there's less need to have things first-party unless absolutely necessary. So, I think for the moment, it should remain a community based effort until the need arises to make it first party. There's already a case that because of the need for something like Again, I'm more than happy to have folks help me maintain and expand |
Sounds good <3 The RFC process is still in use, but is likely due for an overhaul. I would like to get you a proper final decision on this, but I can say the same for about 100 other open work items, and can't + shouldn't act unilaterally here. Given that a third-party crate is relatively viable here, I'm in favor of developing and promoting that for now. |
Seconding @alice-i-cecile, reading the RFC I don't really see a clear advantage to a built-in solution over a 3P crate. Maybe we should move the discussion there to clarify the value first before proposing an actual PR? |
I recommend we close this. The third party crate has been successful, and the changes are stale. |
Objective
rand_core
andrand
are the defacto standard of handling PRNGs in the rust ecosystem. There's a wide ecosystem behind it andrand
continues to be maintained and expanded upon by a whole community behind it.RngCore
(like withfastrand
).Solution
RngCore
andSeedableRng
, as well as additional traits for supporting reflection (serdeSerialize
andDeserialize
). The crate exposes a plugin for easy integration, or the components/resource for applying/accessing PRNGs in a way that will work well in an ECS environment. Ideas here are all from lessons learned frombevy_turborand
.RngCore
PRNG, and the component and resource itself also implementsRngCore
to allow for easy use withrand
traits and features.RngCore
API, so accessingGlobalEntropy
resource will greatly limit parallelism of systems querying it. Instead, RNG sources should be moved to entities viaEntropyComponent
, to allow for increased parallelism for groupings of systems/entities that do not overlap each other with mut accesses of the RNG sources.rand
on its own brings in a lot of code, as well as additional PRNGs. For focusing on determinism, that also means determinism in algorithm used for seeding entropy, asrand
makes no guarantee that the output ofthread_rng
is deterministic in terms of underlying algorithm. Same withStdRng
andSmallRng
.rand
recommends using PRNG crates directly if determinism of output is important. Therefore, this crate implements everything needed with justrand_core
as the base, leaving it to developers to decide on when to importrand
and what features they use fromrand
.Example setup of
bevy_entropy
:Todos
Open Questions
rand_chacha
by default, or can we fallback to OS entropy sources for components? There's overheads here potentially, but someone might want less deps or other use-cases.Also, all naming in this crate is subject to further bikeshedding, and feedback on the initial API is welcome.
Changelog
Added
bevy_entropy
crate added for integrating RNGs into bevy.