Perfect privacy for mutable component access

[joseph-gio]

Objective

Preventing mutation is very difficult in bevy. Even if you encapsulate an entire type, you can still acquire a &mut T and std::mem::swap it out.
The most obvious example of this is the hierarchy. We would like to keep it consistent at all times, but that is not currently possible since anyone can swap out components at any time.

Related to / evolution of #5356

Solution

• Use marker types to limit where you can acquire mutable access or insert/remove components.
• Components that do not opt in to this feature are unaffected.
• Perfect privacy: since we're leveraging the type sytem, it should be impossible to bypass in safe code.
• Zero-cost: every thing is done statically; no runtime checks; no carrying values around.

Example

Defining protected components.

/// Marker type which is only visible within this crate.
pub(crate) type VisMarker;

// Component that stores the ID of the parent entity.
// The field can be public, since no one else can mutate it once it's in the `World`.
#[derive(Component)]
#[component(write_marker = "VisMarker")]
pub struct Parent(pub Entity);

Trying to modify protected components incorrectly.

fn modify_parent(query: Query<&mut Parent>) {
    // This fails to compile since `&mut T` (in a query) requires unrestricted mutable access:
    // error[E0277]: the trait bound `Parent: WriteAccess` is not satisfied
}

Modifying the components properly.

use bevy_ecs::component::Write;
fn swap_parents(
    // `Write<>` is a WorldQuery that gives mutable access to a component.
    // Naming the visibility marker "unlocks" access to the component.
    mut children: Query<Write<Parent, VisMarker>>,
) {
    // Swap everyone's parents around.
    let mut combinations = query.iter_combinations_mut();
    while let Some([parent_a, parent_b]) = combinations.fetch_next() {
        std::mem::swap(&mut *parent_a, &mut *parent_b);
    }

    // What we just did is very bad, which is why no one outside of this crate is allowed to do it anymore.
}

Modifying through direct world access.

fn exclusive(world: &mut World) {
    let my_entity = ...;
    // Type inference fills in `VisMarker` for us.
    let parent = world.get_mut_protected::<Parent, _>(my_entity).unwrap();
}

---

Changelog

• Added the ability to protect components from being mutated outside of their defining crates.
• Made the types bevy_hierarchy::{Parent, Children} externally immutable.

Migration Guide

When writing systems with generic components, you now must specify the trait bound bevy_ecs::component::WriteComponent in order to mutate/insert/remove them.

Before:

fn deref_all<T: Component + DerefMut>(mut query: Query<&mut T>) {
    for mut val in &mut query {
        let val = val.deref_mut();
        // ... do something with the value
    }
}

After:

fn deref_all<T: WriteComponent + DerefMut>(mut query: Query<&mut T>) {
    for mut val in &mut query {
        // ...
    }
}

[joseph-gio]

I removed this test because it should not be possible to trigger a hierarchy cycle outside of bevy_hierarchy anymore.

[alice-i-cecile]

For future searchability: this is an implementation of "permissions" as discussed by @BoxyUwU and @TheRawMeatball :)

[TheRawMeatball]

While i don't have a full opinion on this yet, we should also consider the other two options we have for implementing perfect privacy:

• use this form of gating on Mut instead, where the deref impl is gated instead of the WorldQuery impl of &mut T.
• allow for deriving WorldQuery with component types that are private - this'd allow for true type privacy by not requiring the component type be pub at all, instead gating all accesses through the interface for the type that derives WorldQuery.

Personally, given that I'd consider the second a bug that needs to be fixed eventually, I'm partial to that solution.

[joseph-gio]

• allow for deriving WorldQuery with component types that are private - this'd allow for true type privacy by not requiring the component type be pub at all, instead gating all accesses through the interface for the type that derives WorldQuery.

I don't think this is as valuable, as it would make it really awkward to get read access to protected components. I think that anyone who doesn't need write access shouldn't even have to worry about this feature at all.

[TheRawMeatball]

This is much worse in practice, I've found. It was the first sort of thing I tried and it didn't work very well. I can try to elaborate more later, if you're interested.

It'd probably be nice to have that noted down somewhere, for the sake of future reference.

[joseph-gio]

• use this form of gating on Mut instead, where the deref impl is gated instead of the WorldQuery impl of &mut T.

There is a few different problems with this, that I am aware of.

Cannot implement traits

In order to return Mut<T, Marker> from a fn, the function needs to be generic over Marker. Ignoring some other points of complexity, Query::iter_mut would have to look something like:

// Entirely strawman syntax, only pay attention to the generics
pub fn iter_mut<Marker>(&mut self) -> Self::Item<Marker> where Self::Item: WriteAccesss<Marker>
{ ... }

This is fine: it's how EntityMut::insert works, for example. Type inference works perfectly and the marker doesn't get in the way at all. The problem is that you can only parameterize fns in this way, not impls.

// This fails to compile, since `Marker` is not bounded by either the trait or the type.
impl<T: WorldQuery, Marker> IntoIterator for Query<T> where T: WriteAccess<Marker>
{ ... }

In order to implement traits for Query<>, we'd have to make it generic over Marker.

pub struct Query<Q: WorldQuery, F: WorldQuery = (), Marker = ()> where Q: WriteAccess<Marker>
{ ... }

// `Protected1` and `Protected2` are components, protected by the types `Marker1` and `Marker2`.
fn my_system(
    query: Query<(Entity, &mut Protected1, &mut Protected2), (), ((), Marker1, Marker2)>,
) { ... }

But, this is the same solution as with Write<T, Marker>, only it's far more difficult to implement, unergonomic to use, and has no advantages that I am aware of.

Unnecessary complexity

Even if we figure out a way of implementing this approach, it would add a significant amount of complexity for little to no benefit. We'd have to expand the concept of privacy to WorldQuery, which would require an all_tuples!() invocation, or more likely a custom proc macro.

With the solution described in this PR, privacy is only a property of components -- if you don't have write access, then you can't even declare a mutable WorldQuery (or Bundle) in the first place. I settled on the current design since it keeps the privacy "perfect" while retaining a fairly simple implementation, does not make the visibility markers infectious, and has little impact on end users (usually no impact at all).

[TheRawMeatball]

Hm, this isn't exactly what I meant by gating it on Mut though. My idea would've been to change the DerefMut impl of Mut to something like this:

impl<T: Component> DerefMut for Mut<'_, T>
where
    T: Component<MutPerm = ()>,
{}

and then, add an explicit method to Mut like this:

impl<'a, T: Component> Mut<'a, T> {
    fn get_with_perm(&mut self, _perm: T::MutPerm -> &mut T {}
}

[joseph-gio]

Well then yeah, that should work as an alternative to Write<T, _>

[BoxyUwU]

nile's idea was to not have a generic on Mut for the mut token it would just be struct Mut<'a, T> not struct Mut<'a, T, Token>

[joseph-gio]

Ah, right. My bad