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

Add Immutable Component Support #16372

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
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
22 changes: 22 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -1943,6 +1943,28 @@ description = "Creates a hierarchy of parents and children entities"
category = "ECS (Entity Component System)"
wasm = false

[[example]]
name = "immutable_components_dynamic"
path = "examples/ecs/immutable_components_dynamic.rs"
doc-scrape-examples = true

[package.metadata.example.immutable_components_dynamic]
name = "Immutable Dynamic Components"
description = "Demonstrates the creation and utility of dynamic immutable components"
category = "ECS (Entity Component System)"
wasm = false

[[example]]
name = "immutable_components"
bushrat011899 marked this conversation as resolved.
Show resolved Hide resolved
path = "examples/ecs/immutable_components.rs"
doc-scrape-examples = true

[package.metadata.example.immutable_components]
name = "Immutable Components"
description = "Demonstrates the creation and utility of immutable components"
category = "ECS (Entity Component System)"
wasm = false

[[example]]
name = "iter_combinations"
path = "examples/ecs/iter_combinations.rs"
Expand Down
12 changes: 6 additions & 6 deletions benches/benches/bevy_ecs/change_detection.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use bevy_ecs::{
component::Component,
component::{Component, Mutable},
entity::Entity,
prelude::{Added, Changed, EntityWorldMut, QueryState},
query::QueryFilter,
Expand Down Expand Up @@ -124,7 +124,7 @@ fn all_added_detection(criterion: &mut Criterion) {
}
}

fn all_changed_detection_generic<T: Component + Default + BenchModify>(
fn all_changed_detection_generic<T: Component<Mutability = Mutable> + Default + BenchModify>(
group: &mut BenchGroup,
entity_count: u32,
) {
Expand Down Expand Up @@ -172,7 +172,7 @@ fn all_changed_detection(criterion: &mut Criterion) {
}
}

fn few_changed_detection_generic<T: Component + Default + BenchModify>(
fn few_changed_detection_generic<T: Component<Mutability = Mutable> + Default + BenchModify>(
group: &mut BenchGroup,
entity_count: u32,
) {
Expand Down Expand Up @@ -222,7 +222,7 @@ fn few_changed_detection(criterion: &mut Criterion) {
}
}

fn none_changed_detection_generic<T: Component + Default>(
fn none_changed_detection_generic<T: Component<Mutability = Mutable> + Default>(
group: &mut BenchGroup,
entity_count: u32,
) {
Expand Down Expand Up @@ -271,7 +271,7 @@ fn insert_if_bit_enabled<const B: u16>(entity: &mut EntityWorldMut, i: u16) {
}
}

fn add_archetypes_entities<T: Component + Default>(
fn add_archetypes_entities<T: Component<Mutability = Mutable> + Default>(
world: &mut World,
archetype_count: u16,
entity_count: u32,
Expand All @@ -298,7 +298,7 @@ fn add_archetypes_entities<T: Component + Default>(
}
}
}
fn multiple_archetype_none_changed_detection_generic<T: Component + Default + BenchModify>(
fn multiple_archetype_none_changed_detection_generic<T: Component<Mutability = Mutable> + Default + BenchModify>(
group: &mut BenchGroup,
archetype_count: u16,
entity_count: u32,
Expand Down
7 changes: 5 additions & 2 deletions crates/bevy_animation/src/animation_curves.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,10 @@ use core::{
marker::PhantomData,
};

use bevy_ecs::{component::Component, world::Mut};
use bevy_ecs::{
component::{Component, Mutable},
world::Mut,
};
use bevy_math::{
curve::{
cores::{UnevenCore, UnevenCoreError},
Expand Down Expand Up @@ -162,7 +165,7 @@ use crate::{
/// [`AnimationClip`]: crate::AnimationClip
pub trait AnimatableProperty: Reflect + TypePath {
/// The type of the component that the property lives on.
type Component: Component;
type Component: Component<Mutability = Mutable>;

/// The type of the property to be animated.
type Property: Animatable + FromReflect + Reflectable + Clone + Sync + Debug;
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_core_pipeline/src/oit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ impl Default for OrderIndependentTransparencySettings {
// we can hook on_add to issue a warning in case `layer_count` is seemingly too high.
impl Component for OrderIndependentTransparencySettings {
const STORAGE_TYPE: StorageType = StorageType::SparseSet;
type Mutability = Mutable;

fn register_component_hooks(hooks: &mut ComponentHooks) {
hooks.on_add(|world, entity, _| {
Expand Down
14 changes: 14 additions & 0 deletions crates/bevy_ecs/macros/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ pub fn derive_event(input: TokenStream) -> TokenStream {

impl #impl_generics #bevy_ecs_path::component::Component for #struct_name #type_generics #where_clause {
const STORAGE_TYPE: #bevy_ecs_path::component::StorageType = #bevy_ecs_path::component::StorageType::SparseSet;
type Mutability = #bevy_ecs_path::component::Mutable;
}
})
}
Expand Down Expand Up @@ -139,12 +140,18 @@ pub fn derive_component(input: TokenStream) -> TokenStream {
}
});

let mutable_type = attrs
.immutable
.then_some(quote! { #bevy_ecs_path::component::Immutable })
.unwrap_or(quote! { #bevy_ecs_path::component::Mutable });

// This puts `register_required` before `register_recursive_requires` to ensure that the constructors of _all_ top
// level components are initialized first, giving them precedence over recursively defined constructors for the same component type
TokenStream::from(quote! {
#required_component_docs
impl #impl_generics #bevy_ecs_path::component::Component for #struct_name #type_generics #where_clause {
const STORAGE_TYPE: #bevy_ecs_path::component::StorageType = #storage;
type Mutability = #mutable_type;
fn register_required_components(
requiree: #bevy_ecs_path::component::ComponentId,
components: &mut #bevy_ecs_path::component::Components,
Expand Down Expand Up @@ -176,13 +183,16 @@ pub const ON_INSERT: &str = "on_insert";
pub const ON_REPLACE: &str = "on_replace";
pub const ON_REMOVE: &str = "on_remove";

pub const IMMUTABLE: &str = "immutable";

struct Attrs {
storage: StorageTy,
requires: Option<Punctuated<Require, Comma>>,
on_add: Option<ExprPath>,
on_insert: Option<ExprPath>,
on_replace: Option<ExprPath>,
on_remove: Option<ExprPath>,
immutable: bool,
}

#[derive(Clone, Copy)]
Expand Down Expand Up @@ -213,6 +223,7 @@ fn parse_component_attr(ast: &DeriveInput) -> Result<Attrs> {
on_replace: None,
on_remove: None,
requires: None,
immutable: false,
bushrat011899 marked this conversation as resolved.
Show resolved Hide resolved
};

let mut require_paths = HashSet::new();
Expand Down Expand Up @@ -242,6 +253,9 @@ fn parse_component_attr(ast: &DeriveInput) -> Result<Attrs> {
} else if nested.path.is_ident(ON_REMOVE) {
attrs.on_remove = Some(nested.value()?.parse::<ExprPath>()?);
Ok(())
} else if nested.path.is_ident(IMMUTABLE) {
attrs.immutable = true;
Ok(())
} else {
Err(nested.error("Unsupported attribute"))
}
Expand Down
97 changes: 97 additions & 0 deletions crates/bevy_ecs/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,26 @@ use derive_more::derive::{Display, Error};
///
/// # Component and data access
///
/// Components can be marked as immutable by adding the `#[component(immutable)]`
/// attribute when using the derive macro.
///
/// ```
/// # use bevy_ecs::component::Component;
/// #
/// #[derive(Component)]
/// #[component(immutable)]
/// struct ImmutableFoo;
/// ```
///
/// Immutable components are guaranteed to never have an exclusive reference,
/// `&mut ...`, created while inserted onto an entity.
/// In all other ways, they are identical to mutable components.
/// This restriction allows hooks to observe all changes made to an immutable
/// component, effectively turning the `OnInsert` and `OnReplace` hooks into a
/// `OnMutate` hook.
/// This is not practical for mutable components, as the runtime cost of invoking
/// a hook for every exclusive reference created would be far too high.
///
/// See the [`entity`] module level documentation to learn how to add or remove components from an entity.
///
/// See the documentation for [`Query`] to learn how to access component data from a system.
Expand Down Expand Up @@ -378,6 +398,14 @@ pub trait Component: Send + Sync + 'static {
/// A constant indicating the storage type used for this component.
const STORAGE_TYPE: StorageType;

/// A marker type to assist Bevy with determining if this component is
/// mutable, or immutable. Mutable components will have [`Component<Mutability = Mutable>`],
/// while immutable components will instead have [`Component<Mutability = Immutable>`].
///
/// * For a component to be mutable, this type must be [`Mutable`].
/// * For a component to be immutable, this type must be [`Immutable`].
type Mutability: ComponentMutability;

/// Called when registering this component, allowing mutable access to its [`ComponentHooks`].
fn register_component_hooks(_hooks: &mut ComponentHooks) {}

Expand All @@ -392,6 +420,35 @@ pub trait Component: Send + Sync + 'static {
}
}

mod private {
pub trait Seal {}
}

/// The mutability option for a [`Component`]. This can either be:
/// * [`Mutable`]
/// * [`Immutable`]
pub trait ComponentMutability: private::Seal + 'static {
/// Boolean to indicate if this mutability setting implies a mutable or immutable
/// component.
const MUTABLE: bool;
}

/// Parameter indicating a [`Component`] is immutable.
pub struct Immutable;

impl private::Seal for Immutable {}
impl ComponentMutability for Immutable {
const MUTABLE: bool = false;
}

/// Parameter indicating a [`Component`] is mutable.
pub struct Mutable;

impl private::Seal for Mutable {}
impl ComponentMutability for Mutable {
const MUTABLE: bool = true;
}

/// The storage used for a specific component type.
///
/// # Examples
Expand Down Expand Up @@ -617,6 +674,12 @@ impl ComponentInfo {
&self.descriptor.name
}

/// Returns `true` if the current component is immutable.
#[inline]
pub fn immutable(&self) -> bool {
self.descriptor.immutable
}

/// Returns the [`TypeId`] of the underlying component type.
/// Returns `None` if the component does not correspond to a Rust type.
#[inline]
Expand Down Expand Up @@ -769,6 +832,7 @@ pub struct ComponentDescriptor {
// this descriptor describes.
// None if the underlying type doesn't need to be dropped
drop: Option<for<'a> unsafe fn(OwningPtr<'a>)>,
immutable: bool,
}

// We need to ignore the `drop` field in our `Debug` impl
Expand All @@ -780,6 +844,7 @@ impl Debug for ComponentDescriptor {
.field("is_send_and_sync", &self.is_send_and_sync)
.field("type_id", &self.type_id)
.field("layout", &self.layout)
.field("immutable", &self.immutable)
.finish()
}
}
Expand All @@ -804,6 +869,7 @@ impl ComponentDescriptor {
type_id: Some(TypeId::of::<T>()),
layout: Layout::new::<T>(),
drop: needs_drop::<T>().then_some(Self::drop_ptr::<T> as _),
immutable: !T::Mutability::MUTABLE,
}
}

Expand All @@ -825,6 +891,29 @@ impl ComponentDescriptor {
type_id: None,
layout,
drop,
immutable: false,
}
}

/// Create a new `ComponentDescriptor` for an immutable [`Component`].
///
/// # Safety
/// - the `drop` fn must be usable on a pointer with a value of the layout `layout`
/// - the component type must be safe to access from any thread (Send + Sync in rust terms)
pub unsafe fn new_immutable_with_layout(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered just adding an immutable: bool parameter to the existing new_with_layout, but this avoids another breaking change in the API.

name: impl Into<Cow<'static, str>>,
storage_type: StorageType,
layout: Layout,
drop: Option<for<'a> unsafe fn(OwningPtr<'a>)>,
) -> Self {
Self {
name: name.into(),
storage_type,
is_send_and_sync: true,
type_id: None,
layout,
drop,
immutable: true,
}
}

Expand All @@ -841,6 +930,7 @@ impl ComponentDescriptor {
type_id: Some(TypeId::of::<T>()),
layout: Layout::new::<T>(),
drop: needs_drop::<T>().then_some(Self::drop_ptr::<T> as _),
immutable: false,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't added immutability to Resources in this PR to keep the scope contained. I suspect it would basically just be a duplication of some of the infrastructure added for immutable Components anyway, so once this PR is merged a followup should be straight-forward. However, without Resource-hooks, I'm not sure how useful it'd be.

}
}

Expand All @@ -852,6 +942,7 @@ impl ComponentDescriptor {
type_id: Some(TypeId::of::<T>()),
layout: Layout::new::<T>(),
drop: needs_drop::<T>().then_some(Self::drop_ptr::<T> as _),
immutable: false,
}
}

Expand All @@ -873,6 +964,12 @@ impl ComponentDescriptor {
pub fn name(&self) -> &str {
self.name.as_ref()
}

/// Returns whether this component is immutable.
#[inline]
pub fn immutable(&self) -> bool {
self.immutable
}
}

/// Stores metadata associated with each kind of [`Component`] in a given [`World`].
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub mod prelude {
pub use crate::{
bundle::Bundle,
change_detection::{DetectChanges, DetectChangesMut, Mut, Ref},
component::Component,
component::{Component, Immutable, Mutable},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little concerned about putting these in the prelude, since they're such generic terms. If any other area of Bevy needs to use similar terms, I think it'd make sense to move these mutability markers into their own module, independent of component (perhaps change_detection?).

Happy to remove them from the prelude if anyone has concerns. They're still publicly accessible regardless.

entity::{Entity, EntityMapper},
event::{Event, EventMutator, EventReader, EventWriter, Events},
observer::{Observer, Trigger},
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_ecs/src/observer/entity_observer.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{
component::{Component, ComponentHooks, StorageType},
component::{Component, ComponentHooks, Mutable, StorageType},
entity::Entity,
observer::ObserverState,
};
Expand All @@ -10,6 +10,7 @@ pub(crate) struct ObservedBy(pub(crate) Vec<Entity>);

impl Component for ObservedBy {
const STORAGE_TYPE: StorageType = StorageType::SparseSet;
type Mutability = Mutable;

fn register_component_hooks(hooks: &mut ComponentHooks) {
hooks.on_remove(|mut world, entity, _| {
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/observer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ impl World {
// Populate ObservedBy for each observed entity.
for watched_entity in &(*observer_state).descriptor.entities {
let mut entity_mut = self.entity_mut(*watched_entity);
let mut observed_by = entity_mut.entry::<ObservedBy>().or_default();
let mut observed_by = entity_mut.entry::<ObservedBy>().or_default().into_mut();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the consequence of changes made to the entry API for components. Ideally, we would return Mut<T> when calling or_default() on a mutable component, and &T on an immutable component (and likewise for other entry methods). The problem is that would require specialisation, since the Rust compiler has no way of knowing that Component<Mutability = Mutable> and Component<Mutability = Immutable> are mutually exclusive traits.

Since we already have the OccupiedEntry type, I decided to return that for all relevant operations instead, since it already has methods to either get a im/mutable reference to the underlying component. The alternatives would either be to not allow the entry API for immutable components (bad), or duplicate all the methods (or_default_immutable(), etc.)

observed_by.0.push(observer_entity);
}
(&*observer_state, &mut self.archetypes, &mut self.observers)
Expand Down
Loading
Loading