bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
36.03k stars 3.56k forks source link

Allow certain components to be marked immutable #16208

Open nakedible opened 3 hours ago

nakedible commented 3 hours ago

What problem does this solve or what need does it fill?

There are needs to enforce invariants, to build indexes or otherwise keep things consistent in the ECS between multiple entities or between multiple components in a single entity. The new "component hooks" system goes a long way towards making it easy to keep things in sync, as they can be triggered on component addition, removal and replace.

However, the hooks system does not catch mutation currently, and catching mutation has a lot of difficult problems to solve if it needs to be synchronously and with every way of mutating a component. Even if all the fields inside a component are marked private, that doesn't prevent the user from assigning a new instance of that component to it, overwriting the data for the old component - and no hooks will be fired, only asynchronous change events. This means that it is currently impossible to use hooks to maintain an invariant if the user mutates components directly.

What solution would you like?

There could be a flag on components that marks the component immutable. When set, it would mean that no API will return mutable references to the component, making it impossible to mutate the values in the component. This doesn't mean that there would be any change in how insert, remove, take etc. would work, so it would still be fully permitted to remove the component, or add a new one, or replace an existing with a new one. The current hooks system will fire on_replace and on_insert hooks when a component is replaced this way instead of mutated in-place.

How the mutation APIs should behave when called on a component that is marked immutable is still unspecified. Perhaps they should panic when trying to mutate a component that can't be mutated. Or maybe they act as if the component doesn't exist on the object if attempting to get a mutable reference. Or perhaps this can somehow be extended all the way to the type system so the calls which attempt to take a mutable reference to a component simply won't compile because those are not implemented for components which are immutable.

What alternative(s) have you considered?

The big alternative is that absolutely no change is needed. Preventing mutation is just a safeguard for certain kinds of components, and if the fields of the component are private, doing a mutable assignment on a component is not easy to accidentally do – so for most usages it's probably enough to just say in documentation "please don't mutate this component".

The second alternative is also that absolutely no change is needed. Such components may be made private, and exposed through QueryData and Bundle in interfaces such that the real component is never accessible, hence never mutated. It might still be accessible via reflection, save/load, etc. but all bets are off there anyway. This is not as nice as it requires a bunch of boilerplate, and the whole point of component hooks was to allow such invariants to be implemented without creating a shadow API to do all the ECS operations while keeping invariants.

The third alternative is to somehow make synchronous change detection hooks work. There is already change detection, so it's not so impossible to think that it could run a hook real time, possibly both before change and after a change. Then we could just have on_mutate and that would probably cover the vast majority of cases that would benefit from immutable components.

Additional context

I'm writing this issue mainly as a placeholder, and to see comments. It's one problem related to hooks that may or may not be common enough to warrant doing something about it. If there comes a strong use case for this that isn't easily solved by other means, and this problem is the primary motivation to make components private, then I will likely want to revisit this.

bushrat011899 commented 2 hours ago

The status-quo for this is to create a private Component and a public QueryData, but that doesn't allow replacing the value (instead you'd need to also add some commands). Supporting an immutable Component would eliminate this boilerplate.

Ideally, it would be a compiler error to ask for a mutable reference to an immutable component. Perhaps Component as a trait could be modified to include a parameter indicating mutability, and the mutable QueryData implementation would not be available for said parameter? Or Component could be split into a Component and ComponentMut pair of traits?

iiYese commented 1 hour ago

It could be as simple as the Component derive also adding an impl for a Mutable trait. You can have a #[immutable] attribute macro that opts out of the Mutable impl. Then just:

impl<T: Component + Mutable> QueryData for &'static mut T {
    // ..
}
SpecificProtagonist commented 59 minutes ago

Then just impl<T: Component + Mutable> QueryData for &'static mut T

I like this. Additionally we still need to store a mutability flag in the ComponentInfo and then check it when accessing components by id.