Skip to content

Perfect privacy for mutable component access #5799

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

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions benches/benches/bevy_ecs/world/world_get.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use bevy_ecs::{
bundle::Bundle,
component::Component,
component::WriteComponent,
entity::Entity,
prelude::*,
system::{Query, SystemState},
Expand Down Expand Up @@ -29,7 +29,7 @@ fn deterministic_rand() -> ChaCha8Rng {
ChaCha8Rng::seed_from_u64(42)
}

fn setup<T: Component + Default>(entity_count: u32) -> World {
fn setup<T: WriteComponent + Default>(entity_count: u32) -> World {
let mut world = World::default();
world.spawn_batch((0..entity_count).map(|_| (T::default(),)));
black_box(world)
Expand Down
15 changes: 14 additions & 1 deletion crates/bevy_ecs/macros/src/component.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use std::str::FromStr;

use bevy_macro_utils::{get_lit_str, Symbol};
use proc_macro::TokenStream;
use proc_macro2::{Span, TokenStream as TokenStream2};
use quote::{quote, ToTokens};
use quote::{quote, quote_spanned, ToTokens};
use syn::{parse_macro_input, parse_quote, DeriveInput, Error, Ident, Path, Result};

pub fn derive_resource(input: TokenStream) -> TokenStream {
Expand Down Expand Up @@ -41,17 +43,22 @@ pub fn derive_component(input: TokenStream) -> TokenStream {
let struct_name = &ast.ident;
let (impl_generics, type_generics, where_clause) = &ast.generics.split_for_impl();

let marker = attrs.marker.unwrap_or_else(|| quote! { () });

TokenStream::from(quote! {
impl #impl_generics #bevy_ecs_path::component::Component for #struct_name #type_generics #where_clause {
type Storage = #storage;
}
impl #impl_generics #bevy_ecs_path::component::WriteAccess::<#marker> for #struct_name #type_generics #where_clause {}
})
}

pub const COMPONENT: Symbol = Symbol("component");
pub const STORAGE: Symbol = Symbol("storage");
pub const VISIBILITY: Symbol = Symbol("write_marker");

struct Attrs {
marker: Option<TokenStream2>,
storage: StorageTy,
}

Expand All @@ -69,6 +76,7 @@ fn parse_component_attr(ast: &DeriveInput) -> Result<Attrs> {
let meta_items = bevy_macro_utils::parse_attrs(ast, COMPONENT)?;

let mut attrs = Attrs {
marker: None,
storage: StorageTy::Table,
};

Expand All @@ -93,6 +101,11 @@ fn parse_component_attr(ast: &DeriveInput) -> Result<Attrs> {
}
};
}
Meta(NameValue(m)) if m.path == VISIBILITY => {
let lit = get_lit_str(STORAGE, &m.lit)?.value();
let marker_expr = TokenStream2::from_str(&lit)?;
attrs.marker = Some(quote_spanned! { m.lit.span() => #marker_expr });
}
Meta(meta_item) => {
return Err(Error::new_spanned(
meta_item.path(),
Expand Down
6 changes: 6 additions & 0 deletions crates/bevy_ecs/macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream {
let mut field_component_ids = Vec::new();
let mut field_get_components = Vec::new();
let mut field_from_components = Vec::new();
let mut field_assert = Vec::new();
for ((field_type, is_bundle), field) in
field_type.iter().zip(is_bundle.iter()).zip(field.iter())
{
Expand All @@ -136,6 +137,9 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream {
field_from_components.push(quote! {
#field: func(ctx).read::<#field_type>(),
});
field_assert.push(quote! {
#ecs_path::component::assert_has_write_access::<#field_type>();
});
}
}
let field_len = field.len();
Expand All @@ -160,6 +164,8 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream {
where
__F: FnMut(&mut __T) -> #ecs_path::ptr::OwningPtr<'_>
{
// Make sure each component has unrestricted write access.
#(#field_assert)*
Self {
#(#field_from_components)*
}
Expand Down
8 changes: 6 additions & 2 deletions crates/bevy_ecs/src/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,18 @@ pub use bevy_ecs_macros::Bundle;

use crate::{
archetype::{AddBundle, Archetype, ArchetypeId, Archetypes, ComponentStatus},
component::{Component, ComponentId, ComponentTicks, Components, StorageType},
component::{ComponentId, ComponentTicks, Components, StorageType},
entity::{Entities, Entity, EntityLocation},
storage::{SparseSetIndex, SparseSets, Storages, Table},
};
use bevy_ecs_macros::all_tuples;
use bevy_ptr::OwningPtr;
use std::{any::TypeId, collections::HashMap};

// We don't use this in code, but we need it for docs.
#[allow(unused_imports)]
use crate::component::Component;

/// An ordered collection of [`Component`]s.
///
/// Commonly used for spawning entities and adding and removing components in bulk. This
Expand Down Expand Up @@ -103,7 +107,7 @@ macro_rules! tuple_impl {
// - `Bundle::component_ids` returns the `ComponentId`s for each component type in the
// bundle, in the exact order that `Bundle::get_components` is called.
// - `Bundle::from_components` calls `func` exactly once for each `ComponentId` returned by `Bundle::component_ids`.
unsafe impl<$($name: Component),*> Bundle for ($($name,)*) {
unsafe impl<$($name: $crate::component::WriteComponent),*> Bundle for ($($name,)*) {
#[allow(unused_variables)]
fn component_ids(components: &mut Components, storages: &mut Storages) -> Vec<ComponentId> {
vec![$(components.init_component::<$name>(storages)),*]
Expand Down
158 changes: 158 additions & 0 deletions crates/bevy_ecs/src/component.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Types for declaring and storing [`Component`]s.

use crate::{
bundle::Bundle,
change_detection::MAX_CHANGE_AGE,
storage::{SparseSetIndex, Storages},
system::Resource,
Expand All @@ -11,6 +12,7 @@ use std::{
alloc::Layout,
any::{Any, TypeId},
borrow::Cow,
marker::PhantomData,
mem::needs_drop,
};

Expand Down Expand Up @@ -113,6 +115,162 @@ pub trait Component: Send + Sync + 'static {
type Storage: ComponentStorage;
}

/// Marker trait that allows a [`Component`] to be mutated.
///
/// The type parameter `Marker` is used to control the privacy of the access.
/// You can only mutate a component if you can name the marker type.
/// By default the marker is the unit type, so anyone can mutate most components.
pub trait WriteAccess<Marker = ()> {}

/// Asserts at compile time that the specified component has public write access.
pub fn assert_has_write_access<T: Component + WriteAccess>() {}

/// [`Component`]s that have [write access](WriteAccess) - shorthand for `Component + WriteAccess`.
pub trait WriteComponent<Marker = ()>: Component + WriteAccess<Marker> {}
impl<T: ?Sized, Marker> WriteComponent<Marker> for T where T: Component + WriteAccess<Marker> {}

/// A bundle that allows inserting/removing protected components. This is not unsafe, just easily abused.
///
/// Note that even being able to *name* this type can be abused, as it would
/// allow users to call `remove_bundle::<Unlocked<T>>()` to remove protected values.
pub(crate) struct Unlocked<T: Component>(pub T);

// SAFETY: There is only one ComponentId, so the order does not matter.
unsafe impl<T> Bundle for Unlocked<T>
where
T: Component,
{
fn component_ids(
components: &mut Components,
storages: &mut crate::storage::Storages,
) -> Vec<ComponentId> {
vec![components.init_component::<T>(storages)]
}
unsafe fn from_components<U, F>(ctx: &mut U, mut func: F) -> Self
where
F: FnMut(&mut U) -> crate::ptr::OwningPtr,
{
Self(func(ctx).read::<T>())
}
fn get_components(self, func: impl FnMut(crate::ptr::OwningPtr)) {
crate::ptr::OwningPtr::make(self.0, func);
}
}

/// A [`Bundle`] type that can contain a [`Component`] with protected write access.
///
/// The resulting `Unlocked<T, Marker>` type is itself private, so it must be encapsulated
/// somehow, which prevents outside modification of the protected component.
///
/// # Examples
///
/// ```
/// use bevy_ecs::{prelude::*, component::WriteAccess};
///
/// // A set of components that we want to restrict mutable access to.
/// // By marking the components with this type, they can only be mutated
/// // in places that `Marker` can be named.
/// struct Marker;
///
/// #[derive(Component)]
/// #[component(write_marker = "Marker")]
/// pub struct Chassis;
///
/// #[derive(Component)]
/// #[component(write_marker = "Marker")]
/// pub struct Axle {
/// // The fields can be public, since no one outside this module can get mutable access
/// // once this component has been inserted into the world.
/// pub torque: f64,
/// }
///
/// #[derive(Component)]
/// #[component(write_marker = "Marker")]
/// pub struct Tires {
/// pub width: f64,
/// }
/// #
/// # use bevy_ecs::component::ProtectedBundle;
///
/// // In order to include the components in a bundle, we must encapsulate the `Marker` type.
///
/// #[derive(Bundle)]
/// pub struct Automobile {
/// // Flatten the three smaller bundles to make one big bundle.
/// #[bundle]
/// chassis: ProtectedBundle<Chassis, Marker>,
/// #[bundle]
/// axle: ProtectedBundle<Axle, Marker>,
/// #[bundle]
/// tires: ProtectedBundle<Tires, Marker>,
/// }
/// #
/// // `new` method omitted.
/// # impl Automobile {
/// # pub fn new(axle: Axle, tires: Tires) -> Self {
/// # Self {
/// # chassis: ProtectedBundle::new(Chassis),
/// # axle: ProtectedBundle::new(axle),
/// # tires: ProtectedBundle::new(tires),
/// # }
/// # }
/// # }
///
/// fn setup(mut commands: Commands) {
/// commands.spawn_bundle(
/// Automobile::new(Axle { torque: 100. }, Tires { width: 20. })
/// );
/// }
/// #
/// # bevy_ecs::system::assert_is_system(setup);
/// ```
pub struct ProtectedBundle<T, Marker: 'static>
where
T: WriteComponent<Marker>,
{
// FIXME: Derive this when we can do #[bundle(ignore)].
// https://github.com/bevyengine/bevy/pull/5628
val: Unlocked<T>,
_marker: PhantomData<fn() -> Marker>,
}

impl<T, Marker: 'static> ProtectedBundle<T, Marker>
where
T: WriteComponent<Marker>,
{
pub const fn new(val: T) -> Self {
Self {
val: Unlocked(val),
_marker: PhantomData,
}
}
}

// SAFETY: Defer to the safety of `Unlocked`.
unsafe impl<T, Marker: 'static> Bundle for ProtectedBundle<T, Marker>
where
T: WriteComponent<Marker>,
{
fn component_ids(
components: &mut Components,
storages: &mut crate::storage::Storages,
) -> Vec<ComponentId> {
<Unlocked<T> as Bundle>::component_ids(components, storages)
}
unsafe fn from_components<U, F>(ctx: &mut U, func: F) -> Self
where
F: FnMut(&mut U) -> crate::ptr::OwningPtr,
{
Self {
val: Unlocked::from_components(ctx, func),
_marker: PhantomData,
}
}
fn get_components(self, func: impl FnMut(crate::ptr::OwningPtr)) {
self.val.get_components(func);
}
}

pub struct TableStorage;
pub struct SparseStorage;

Expand Down
Loading