From 7c9df9f532831ffeb60eb1ce9980d3c11fd5af3c Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Wed, 24 Aug 2022 18:21:07 -0400 Subject: [PATCH 1/9] Track write access for components --- crates/bevy_ecs/macros/src/component.rs | 15 ++++++++++++++- crates/bevy_ecs/macros/src/lib.rs | 6 ++++++ crates/bevy_ecs/src/bundle.rs | 8 ++++++-- crates/bevy_ecs/src/component.rs | 14 ++++++++++++++ 4 files changed, 40 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/macros/src/component.rs b/crates/bevy_ecs/macros/src/component.rs index a75c4a92b3fe9..4a3d67eb6d159 100644 --- a/crates/bevy_ecs/macros/src/component.rs +++ b/crates/bevy_ecs/macros/src/component.rs @@ -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 { @@ -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("vis"); struct Attrs { + marker: Option, storage: StorageTy, } @@ -69,6 +76,7 @@ fn parse_component_attr(ast: &DeriveInput) -> Result { let meta_items = bevy_macro_utils::parse_attrs(ast, COMPONENT)?; let mut attrs = Attrs { + marker: None, storage: StorageTy::Table, }; @@ -93,6 +101,11 @@ fn parse_component_attr(ast: &DeriveInput) -> Result { } }; } + 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(), diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 98fbe27200b2e..cc18a01c2613a 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -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()) { @@ -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(); @@ -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)* } diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 8eb7730289f49..49e89c5fc78fb 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -6,7 +6,7 @@ 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}, }; @@ -14,6 +14,10 @@ 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 @@ -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 { vec![$(components.init_component::<$name>(storages)),*] diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index eb379934efed4..d6f35e91f3534 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -113,6 +113,20 @@ 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 {} + +/// Asserts at compile time that the specified component has public write access. +pub fn assert_has_write_access() {} + +/// [`Component`]s that have [write access](WriteAccess) - shorthand for `Component + WriteAccess`. +pub trait WriteComponent: Component + WriteAccess {} +impl WriteComponent for T where T: Component + WriteAccess {} + pub struct TableStorage; pub struct SparseStorage; From dbea243356e246e9977de84d0f6d3ae8f72328e1 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Wed, 24 Aug 2022 18:29:43 -0400 Subject: [PATCH 2/9] Protect from mutation via queries --- crates/bevy_ecs/src/query/fetch.rs | 102 +++++++++++++++++++- crates/bevy_render/src/camera/camera.rs | 4 +- crates/bevy_render/src/camera/projection.rs | 6 +- 3 files changed, 104 insertions(+), 8 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 21fab0db52367..2f075fc208de4 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -1,7 +1,9 @@ use crate::{ archetype::{Archetype, ArchetypeComponentId}, change_detection::Ticks, - component::{Component, ComponentId, ComponentStorage, ComponentTicks, StorageType}, + component::{ + Component, ComponentId, ComponentStorage, ComponentTicks, StorageType, WriteComponent, + }, entity::Entity, query::{debug_checked_unreachable, Access, FilteredAccess}, storage::{ComponentSparseSet, Table, Tables}, @@ -746,9 +748,18 @@ pub struct WriteFetch<'w, T> { change_tick: u32, } -/// SAFETY: access of `&T` is a subset of `&mut T` -unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { - type ReadOnly = &'__w T; +/// A [`WorldQuery`] that provides mutable access to a component. +pub struct Write +where + T: WriteComponent, +{ + _ref: PhantomData<&'static mut T>, + _vis: PhantomData Vis>, +} + +/// SAFETY: access of `&'static T` is a subset of `Write` +unsafe impl, Vis> WorldQuery for Write { + type ReadOnly = &'static T; type State = ComponentId; fn shrink<'wlong: 'wshort, 'wshort>(item: Mut<'wlong, T>) -> Mut<'wshort, T> { @@ -908,6 +919,89 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { } } +impl<'w, T: WriteComponent, Vis> WorldQueryGats<'w> for Write { + type Fetch = WriteFetch<'w, T>; + type Item = Mut<'w, T>; +} + +/// SAFETY: access of `&T` is a subset of `Write` +// Note: `&mut T` as a WorldQuery is essentially a shorthand for `Write`. +unsafe impl<'__w, T: WriteComponent> WorldQuery for &'__w mut T { + type ReadOnly = &'__w T; + type State = as WorldQuery>::State; + + fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> { + as WorldQuery>::shrink(item) + } + + unsafe fn init_fetch<'w>( + world: &'w World, + state: &Self::State, + last_change_tick: u32, + change_tick: u32, + ) -> >::Fetch { + as WorldQuery>::init_fetch(world, state, last_change_tick, change_tick) + } + + const IS_DENSE: bool = as WorldQuery>::IS_DENSE; + + const IS_ARCHETYPAL: bool = as WorldQuery>::IS_ARCHETYPAL; + + unsafe fn set_archetype<'w>( + fetch: &mut >::Fetch, + state: &Self::State, + archetype: &'w Archetype, + tables: &'w Tables, + ) { + as WorldQuery>::set_archetype(fetch, state, archetype, tables); + } + + unsafe fn set_table<'w>( + fetch: &mut >::Fetch, + state: &Self::State, + table: &'w Table, + ) { + as WorldQuery>::set_table(fetch, state, table); + } + + unsafe fn archetype_fetch<'w>( + fetch: &mut >::Fetch, + archetype_index: usize, + ) -> >::Item { + as WorldQuery>::archetype_fetch(fetch, archetype_index) + } + + unsafe fn table_fetch<'w>( + fetch: &mut >::Fetch, + table_row: usize, + ) -> >::Item { + as WorldQuery>::table_fetch(fetch, table_row) + } + + fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { + as WorldQuery>::update_component_access(state, access); + } + + fn update_archetype_component_access( + state: &Self::State, + archetype: &Archetype, + access: &mut Access, + ) { + as WorldQuery>::update_archetype_component_access(state, archetype, access); + } + + fn init_state(world: &mut World) -> Self::State { + as WorldQuery>::init_state(world) + } + + fn matches_component_set( + state: &Self::State, + set_contains_id: &impl Fn(ComponentId) -> bool, + ) -> bool { + as WorldQuery>::matches_component_set(state, set_contains_id) + } +} + impl Clone for WriteFetch<'_, T> { fn clone(&self) -> Self { Self { diff --git a/crates/bevy_render/src/camera/camera.rs b/crates/bevy_render/src/camera/camera.rs index 30fd1651f2145..30b2c34c750ea 100644 --- a/crates/bevy_render/src/camera/camera.rs +++ b/crates/bevy_render/src/camera/camera.rs @@ -10,7 +10,7 @@ use bevy_asset::{AssetEvent, Assets, Handle}; use bevy_derive::{Deref, DerefMut}; use bevy_ecs::{ change_detection::DetectChanges, - component::Component, + component::{Component, WriteComponent}, entity::Entity, event::EventReader, query::Added, @@ -305,7 +305,7 @@ impl RenderTarget { } } -pub fn camera_system( +pub fn camera_system( mut window_resized_events: EventReader, mut window_created_events: EventReader, mut image_asset_events: EventReader>, diff --git a/crates/bevy_render/src/camera/projection.rs b/crates/bevy_render/src/camera/projection.rs index 592b4cc8f916b..2145f9ab64fe5 100644 --- a/crates/bevy_render/src/camera/projection.rs +++ b/crates/bevy_render/src/camera/projection.rs @@ -1,7 +1,7 @@ use std::marker::PhantomData; use bevy_app::{App, CoreStage, Plugin, StartupStage}; -use bevy_ecs::{prelude::*, reflect::ReflectComponent}; +use bevy_ecs::{component::WriteComponent, prelude::*, reflect::ReflectComponent}; use bevy_math::Mat4; use bevy_reflect::{ std_traits::ReflectDefault, FromReflect, GetTypeRegistration, Reflect, ReflectDeserialize, @@ -22,7 +22,9 @@ impl Default for CameraProjectionPlugin { #[derive(SystemLabel, Clone, Eq, PartialEq, Hash, Debug)] pub struct CameraUpdateSystem; -impl Plugin for CameraProjectionPlugin { +impl Plugin + for CameraProjectionPlugin +{ fn build(&self, app: &mut App) { app.register_type::() .add_startup_system_to_stage( From 46ce7315f8863a7f9c03fff233035be94dd664b5 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Wed, 24 Aug 2022 19:30:50 -0400 Subject: [PATCH 3/9] Add types for making bundles from protected components --- crates/bevy_ecs/src/component.rs | 143 +++++++++++++++++++++++++++++++ 1 file changed, 143 insertions(+) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index d6f35e91f3534..a0a82a8b4beeb 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -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, @@ -11,6 +12,7 @@ use std::{ alloc::Layout, any::{Any, TypeId}, borrow::Cow, + marker::PhantomData, mem::needs_drop, }; @@ -127,6 +129,147 @@ pub fn assert_has_write_access() {} pub trait WriteComponent: Component + WriteAccess {} impl WriteComponent for T where T: Component + WriteAccess {} +/// 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::>()` to remove protected values. +pub(crate) struct Unlocked(pub T); + +// SAFETY: There is only one ComponentId, so the order does not matter. +unsafe impl Bundle for Unlocked +where + T: Component, +{ + fn component_ids( + components: &mut Components, + storages: &mut crate::storage::Storages, + ) -> Vec { + vec![components.init_component::(storages)] + } + unsafe fn from_components(ctx: &mut U, mut func: F) -> Self + where + F: FnMut(&mut U) -> crate::ptr::OwningPtr, + { + Self(func(ctx).read::()) + } + 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` 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(vis = "Marker")] +/// pub struct Chassis; +/// +/// #[derive(Component)] +/// #[component(vis = "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(vis = "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, +/// #[bundle] +/// axle: ProtectedBundle, +/// #[bundle] +/// tires: ProtectedBundle, +/// } +/// # +/// // `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 +where + T: WriteComponent, +{ + // FIXME: Derive this when we can do #[bundle(ignore)]. + val: Unlocked, + _marker: PhantomData Marker>, +} + +impl ProtectedBundle +where + T: WriteComponent, +{ + pub const fn new(val: T) -> Self { + Self { + val: Unlocked(val), + _marker: PhantomData, + } + } +} + +// SAFETY: Defer to the safety of `Unlocked`. +unsafe impl Bundle for ProtectedBundle +where + T: WriteComponent, +{ + fn component_ids( + components: &mut Components, + storages: &mut crate::storage::Storages, + ) -> Vec { + as Bundle>::component_ids(components, storages) + } + unsafe fn from_components(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; From 040a21242c455d0baaec9a60291b02c11c1714c4 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Thu, 25 Aug 2022 00:21:10 -0400 Subject: [PATCH 4/9] Patch a compile fail test --- .../tests/ui/entity_ref_mut_lifetime_safety.rs | 4 ++-- .../tests/ui/entity_ref_mut_lifetime_safety.stderr | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs_compile_fail_tests/tests/ui/entity_ref_mut_lifetime_safety.rs b/crates/bevy_ecs_compile_fail_tests/tests/ui/entity_ref_mut_lifetime_safety.rs index e1ec8656df6cd..9840410311551 100644 --- a/crates/bevy_ecs_compile_fail_tests/tests/ui/entity_ref_mut_lifetime_safety.rs +++ b/crates/bevy_ecs_compile_fail_tests/tests/ui/entity_ref_mut_lifetime_safety.rs @@ -51,14 +51,14 @@ fn main() { { let gotten: &A = e_mut.get::().unwrap(); - e_mut.insert::(B); + e_mut.insert(B); assert_eq!(gotten, &A(Box::new(16_usize))); // oops UB e_mut.remove::(); } { let mut gotten_mut: Mut = e_mut.get_mut::().unwrap(); - e_mut.insert::(B); + e_mut.insert(B); assert_eq!(&mut *gotten_mut, &mut A(Box::new(16_usize))); // oops UB } } diff --git a/crates/bevy_ecs_compile_fail_tests/tests/ui/entity_ref_mut_lifetime_safety.stderr b/crates/bevy_ecs_compile_fail_tests/tests/ui/entity_ref_mut_lifetime_safety.stderr index e37857d1d3101..275a93a3b18e2 100644 --- a/crates/bevy_ecs_compile_fail_tests/tests/ui/entity_ref_mut_lifetime_safety.stderr +++ b/crates/bevy_ecs_compile_fail_tests/tests/ui/entity_ref_mut_lifetime_safety.stderr @@ -53,8 +53,8 @@ error[E0502]: cannot borrow `e_mut` as mutable because it is also borrowed as im | 53 | let gotten: &A = e_mut.get::().unwrap(); | ---------------- immutable borrow occurs here -54 | e_mut.insert::(B); - | ^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here +54 | e_mut.insert(B); + | ^^^^^^^^^^^^^^^ mutable borrow occurs here 55 | assert_eq!(gotten, &A(Box::new(16_usize))); // oops UB | ------------------------------------------ immutable borrow later used here @@ -63,7 +63,7 @@ error[E0499]: cannot borrow `e_mut` as mutable more than once at a time | 60 | let mut gotten_mut: Mut = e_mut.get_mut::().unwrap(); | -------------------- first mutable borrow occurs here -61 | e_mut.insert::(B); - | ^^^^^^^^^^^^^^^^^^^^ second mutable borrow occurs here +61 | e_mut.insert(B); + | ^^^^^^^^^^^^^^^ second mutable borrow occurs here 62 | assert_eq!(&mut *gotten_mut, &mut A(Box::new(16_usize))); // oops UB | ---------- first borrow later used here From 5d4382a89d6c44e47c4764932104bf1e1b962504 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Wed, 24 Aug 2022 19:31:22 -0400 Subject: [PATCH 5/9] Protect from mutation via direct world access --- benches/benches/bevy_ecs/world/world_get.rs | 4 +-- crates/bevy_ecs/src/reflect.rs | 15 ++++---- crates/bevy_ecs/src/system/commands/mod.rs | 38 +++++++++++++-------- crates/bevy_ecs/src/system/system_param.rs | 2 +- crates/bevy_ecs/src/world/entity_ref.rs | 30 ++++++++++++---- crates/bevy_ecs/src/world/mod.rs | 20 +++++++++-- crates/bevy_render/src/extract_component.rs | 27 +++++++++------ 7 files changed, 93 insertions(+), 43 deletions(-) diff --git a/benches/benches/bevy_ecs/world/world_get.rs b/benches/benches/bevy_ecs/world/world_get.rs index 511864fa27418..3407c0a5cf189 100644 --- a/benches/benches/bevy_ecs/world/world_get.rs +++ b/benches/benches/bevy_ecs/world/world_get.rs @@ -1,6 +1,6 @@ use bevy_ecs::{ bundle::Bundle, - component::Component, + component::WriteComponent, entity::Entity, prelude::*, system::{Query, SystemState}, @@ -29,7 +29,7 @@ fn deterministic_rand() -> ChaCha8Rng { ChaCha8Rng::seed_from_u64(42) } -fn setup(entity_count: u32) -> World { +fn setup(entity_count: u32) -> World { let mut world = World::default(); world.spawn_batch((0..entity_count).map(|_| (T::default(),))); black_box(world) diff --git a/crates/bevy_ecs/src/reflect.rs b/crates/bevy_ecs/src/reflect.rs index b1ad42b4d1899..64d57fa8c0ef1 100644 --- a/crates/bevy_ecs/src/reflect.rs +++ b/crates/bevy_ecs/src/reflect.rs @@ -116,27 +116,28 @@ impl ReflectComponent { impl FromType for ReflectComponent { fn from_type() -> Self { + use crate::component::Unlocked; ReflectComponent { insert: |world, entity, reflected_component| { let mut component = C::from_world(world); component.apply(reflected_component); - world.entity_mut(entity).insert(component); + world.entity_mut(entity).insert_bundle(Unlocked(component)); }, apply: |world, entity, reflected_component| { - let mut component = world.get_mut::(entity).unwrap(); + let mut component = world.get_mut_raw::(entity).unwrap(); component.apply(reflected_component); }, apply_or_insert: |world, entity, reflected_component| { - if let Some(mut component) = world.get_mut::(entity) { + if let Some(mut component) = world.get_mut_raw::(entity) { component.apply(reflected_component); } else { let mut component = C::from_world(world); component.apply(reflected_component); - world.entity_mut(entity).insert(component); + world.entity_mut(entity).insert_bundle(Unlocked(component)); } }, remove: |world, entity| { - world.entity_mut(entity).remove::(); + world.entity_mut(entity).remove_bundle::>(); }, copy: |source_world, destination_world, source_entity, destination_entity| { let source_component = source_world.get::(source_entity).unwrap(); @@ -144,7 +145,7 @@ impl FromType for ReflectComponent { destination_component.apply(source_component); destination_world .entity_mut(destination_entity) - .insert(destination_component); + .insert_bundle(Unlocked(destination_component)); }, reflect: |world, entity| { world @@ -312,7 +313,7 @@ impl FromType for ReflectMapEntities { ReflectMapEntities { map_entities: |world, entity_map| { for entity in entity_map.values() { - if let Some(mut component) = world.get_mut::(entity) { + if let Some(mut component) = world.get_mut_raw::(entity) { component.map_entities(entity_map)?; } } diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 19c057bc4b664..9df3e1cd65107 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -3,7 +3,7 @@ mod parallel_scope; use crate::{ bundle::Bundle, - component::Component, + component::WriteComponent, entity::{Entities, Entity}, world::{FromWorld, World}, }; @@ -507,7 +507,7 @@ impl<'w, 's, 'a> EntityCommands<'w, 's, 'a> { self } - /// Adds a single [`Component`] to the entity. + /// Adds a single [`Component`](crate::component::Component) to the entity. /// /// # Example /// @@ -533,10 +533,11 @@ impl<'w, 's, 'a> EntityCommands<'w, 's, 'a> { /// } /// # bevy_ecs::system::assert_is_system(example_system); /// ``` - pub fn insert(&mut self, component: impl Component) -> &mut Self { + pub fn insert(&mut self, component: impl WriteComponent) -> &mut Self { self.commands.add(Insert { entity: self.entity, component, + phantom: PhantomData, }); self } @@ -596,9 +597,17 @@ impl<'w, 's, 'a> EntityCommands<'w, 's, 'a> { /// ``` pub fn remove(&mut self) -> &mut Self where - T: Component, + T: WriteComponent, { - self.commands.add(Remove:: { + self.remove_protected::() + } + + /// Removes a component that has protected mutability. + pub fn remove_protected(&mut self) -> &mut Self + where + T: WriteComponent, + { + self.commands.add(Remove:: { entity: self.entity, phantom: PhantomData, }); @@ -742,7 +751,7 @@ pub struct InsertBundle { impl Command for InsertBundle where - T: Bundle + 'static, + T: Bundle, { fn write(self, world: &mut World) { if let Some(mut entity) = world.get_entity_mut(self.entity) { @@ -754,14 +763,15 @@ where } #[derive(Debug)] -pub struct Insert { +pub struct Insert { pub entity: Entity, pub component: T, + pub phantom: PhantomData Vis>, } -impl Command for Insert +impl Command for Insert where - T: Component, + T: WriteComponent, { fn write(self, world: &mut World) { if let Some(mut entity) = world.get_entity_mut(self.entity) { @@ -773,18 +783,18 @@ where } #[derive(Debug)] -pub struct Remove { +pub struct Remove { pub entity: Entity, - pub phantom: PhantomData, + pub phantom: PhantomData<(T, fn() -> Vis)>, } -impl Command for Remove +impl Command for Remove where - T: Component, + T: WriteComponent, { fn write(self, world: &mut World) { if let Some(mut entity_mut) = world.get_entity_mut(self.entity) { - entity_mut.remove::(); + entity_mut.remove_protected::(); } } } diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 8762bd64c4255..bd3a209f532ea 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -1447,7 +1447,7 @@ all_tuples!(impl_system_param_tuple, 0, 16, P); pub mod lifetimeless { pub type SQuery = super::Query<'static, 'static, Q, F>; pub type Read = &'static T; - pub type Write = &'static mut T; + pub use crate::query::Write; pub type SRes = super::Res<'static, T>; pub type SResMut = super::ResMut<'static, T>; pub type SCommands = crate::system::Commands<'static, 'static>; diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 2b6314536c6ef..abcc81715f0a5 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -2,7 +2,7 @@ use crate::{ archetype::{Archetype, ArchetypeId, Archetypes}, bundle::{Bundle, BundleInfo}, change_detection::{MutUntyped, Ticks}, - component::{Component, ComponentId, ComponentTicks, Components, StorageType}, + component::{Component, ComponentId, ComponentTicks, Components, StorageType, WriteComponent}, entity::{Entities, Entity, EntityLocation}, storage::{SparseSet, Storages}, world::{Mut, World}, @@ -192,7 +192,16 @@ impl<'w> EntityMut<'w> { } #[inline] - pub fn get_mut(&mut self) -> Option> { + pub fn get_mut(&mut self) -> Option> { + self.get_mut_protected::() + } + + /// Gets mutable access to a component that has protected mutability. + #[inline] + pub fn get_mut_protected(&mut self) -> Option> + where + T: WriteComponent, + { // SAFETY: world access is unique, and lifetimes enforce correct usage of returned borrow unsafe { self.get_unchecked_mut::() } } @@ -442,12 +451,21 @@ impl<'w> EntityMut<'w> { } } - pub fn insert(&mut self, value: T) -> &mut Self { - self.insert_bundle((value,)) + pub fn insert(&mut self, value: impl WriteComponent) -> &mut Self { + self.insert_bundle(crate::component::Unlocked(value)) + } + + pub fn remove(&mut self) -> Option { + self.remove_protected::() } - pub fn remove(&mut self) -> Option { - self.remove_bundle::<(T,)>().map(|v| v.0) + /// Removes a component that has protected mutability. + pub fn remove_protected(&mut self) -> Option + where + T: WriteComponent, + { + self.remove_bundle::>() + .map(|v| v.0) } pub fn despawn(self) { diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 661ef2ddd5813..9a5d60b4832ec 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -13,7 +13,7 @@ use crate::{ change_detection::{MutUntyped, Ticks}, component::{ Component, ComponentDescriptor, ComponentId, ComponentInfo, ComponentTicks, Components, - StorageType, + StorageType, WriteComponent, }, entity::{AllocAtWithoutReplacement, Entities, Entity}, query::{QueryState, WorldQuery}, @@ -478,7 +478,23 @@ impl World { /// position.x = 1.0; /// ``` #[inline] - pub fn get_mut(&mut self, entity: Entity) -> Option> { + pub fn get_mut(&mut self, entity: Entity) -> Option> { + self.get_mut_raw::(entity) + } + + /// Gets mutable access to a component that has protected mutability. + #[inline] + pub fn get_mut_protected, Vis>( + &mut self, + entity: Entity, + ) -> Option> { + self.get_mut_raw::(entity) + } + + /// Allows mutable access to a component while ignoring visibility rules. + /// This is not unsafe, just error-prone. + #[inline] + pub(crate) fn get_mut_raw(&mut self, entity: Entity) -> Option> { // SAFETY: lifetimes enforce correct usage of returned borrow unsafe { get_mut(self, entity, self.get_entity(entity)?.location()) } } diff --git a/crates/bevy_render/src/extract_component.rs b/crates/bevy_render/src/extract_component.rs index f57c07b6a2c00..98761b4084238 100644 --- a/crates/bevy_render/src/extract_component.rs +++ b/crates/bevy_render/src/extract_component.rs @@ -7,7 +7,7 @@ use crate::{ use bevy_app::{App, Plugin}; use bevy_asset::{Asset, Handle}; use bevy_ecs::{ - component::Component, + component::{Component, ProtectedBundle, WriteAccess}, prelude::*, query::{QueryItem, ReadOnlyWorldQuery, WorldQuery}, system::lifetimeless::Read, @@ -133,12 +133,12 @@ fn prepare_uniform_components( /// /// Therefore it sets up the [`RenderStage::Extract`](crate::RenderStage::Extract) step /// for the specified [`ExtractComponent`]. -pub struct ExtractComponentPlugin { +pub struct ExtractComponentPlugin { only_extract_visible: bool, - marker: PhantomData (C, F)>, + marker: PhantomData (C, F, Vis)>, } -impl Default for ExtractComponentPlugin { +impl Default for ExtractComponentPlugin { fn default() -> Self { Self { only_extract_visible: false, @@ -156,14 +156,17 @@ impl ExtractComponentPlugin { } } -impl Plugin for ExtractComponentPlugin { +impl Plugin for ExtractComponentPlugin +where + C: ExtractComponent + WriteAccess, +{ fn build(&self, app: &mut App) { if let Ok(render_app) = app.get_sub_app_mut(RenderApp) { if self.only_extract_visible { render_app - .add_system_to_stage(RenderStage::Extract, extract_visible_components::); + .add_system_to_stage(RenderStage::Extract, extract_visible_components::); } else { - render_app.add_system_to_stage(RenderStage::Extract, extract_components::); + render_app.add_system_to_stage(RenderStage::Extract, extract_components::); } } } @@ -180,21 +183,22 @@ impl ExtractComponent for Handle { } /// This system extracts all components of the corresponding [`ExtractComponent`] type. -fn extract_components( +fn extract_components, Vis: 'static>( mut commands: Commands, mut previous_len: Local, mut query: Extract>, ) { let mut values = Vec::with_capacity(*previous_len); for (entity, query_item) in query.iter_mut() { - values.push((entity, (C::extract_component(query_item),))); + let component = C::extract_component(query_item); + values.push((entity, ProtectedBundle::new(component))); } *previous_len = values.len(); commands.insert_or_spawn_batch(values); } /// This system extracts all visible components of the corresponding [`ExtractComponent`] type. -fn extract_visible_components( +fn extract_visible_components, Vis: 'static>( mut commands: Commands, mut previous_len: Local, mut query: Extract>, @@ -202,7 +206,8 @@ fn extract_visible_components( let mut values = Vec::with_capacity(*previous_len); for (entity, computed_visibility, query_item) in query.iter_mut() { if computed_visibility.is_visible() { - values.push((entity, (C::extract_component(query_item),))); + let component = C::extract_component(query_item); + values.push((entity, ProtectedBundle::new(component))); } } *previous_len = values.len(); From fcc3ea16a396d351beaeeb7498622d47412163e7 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Wed, 24 Aug 2022 20:46:24 -0400 Subject: [PATCH 6/9] Protect hierarchy components --- crates/bevy_hierarchy/src/child_builder.rs | 28 ++++++------ .../bevy_hierarchy/src/components/children.rs | 1 + crates/bevy_hierarchy/src/components/mod.rs | 2 + .../bevy_hierarchy/src/components/parent.rs | 1 + crates/bevy_hierarchy/src/hierarchy.rs | 6 +-- crates/bevy_transform/src/systems.rs | 45 +------------------ 6 files changed, 22 insertions(+), 61 deletions(-) diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index c7334a3d9bac0..78f4ebd857452 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -21,7 +21,7 @@ fn push_events(world: &mut World, events: SmallVec<[HierarchyEvent; 8]>) { fn push_child_unchecked(world: &mut World, parent: Entity, child: Entity) { let mut parent = world.entity_mut(parent); - if let Some(mut children) = parent.get_mut::() { + if let Some(mut children) = parent.get_mut_protected::() { children.0.push(child); } else { parent.insert(Children(smallvec::smallvec![child])); @@ -30,7 +30,7 @@ fn push_child_unchecked(world: &mut World, parent: Entity, child: Entity) { fn update_parent(world: &mut World, child: Entity, new_parent: Entity) -> Option { let mut child = world.entity_mut(child); - if let Some(mut parent) = child.get_mut::() { + if let Some(mut parent) = child.get_mut_protected::() { let previous = parent.0; *parent = Parent(new_parent); Some(previous) @@ -42,10 +42,10 @@ fn update_parent(world: &mut World, child: Entity, new_parent: Entity) -> Option fn remove_from_children(world: &mut World, parent: Entity, child: Entity) { let mut parent = world.entity_mut(parent); - if let Some(mut children) = parent.get_mut::() { + if let Some(mut children) = parent.get_mut_protected::() { children.0.retain(|x| *x != child); if children.is_empty() { - parent.remove::(); + parent.remove_protected::(); } } } @@ -69,7 +69,7 @@ fn update_old_parents(world: &mut World, parent: Entity, children: &[Entity]) { fn remove_children(parent: Entity, children: &[Entity], world: &mut World) { let mut events: SmallVec<[HierarchyEvent; 8]> = SmallVec::new(); for child in children { - world.entity_mut(*child).remove::(); + world.entity_mut(*child).remove_protected::(); events.push(HierarchyEvent::ChildRemoved { child: *child, parent, @@ -77,7 +77,7 @@ fn remove_children(parent: Entity, children: &[Entity], world: &mut World) { } push_events(world, events); - if let Some(mut parent_children) = world.get_mut::(parent) { + if let Some(mut parent_children) = world.get_mut_protected::(parent) { parent_children .0 .retain(|parent_child| !children.contains(parent_child)); @@ -115,7 +115,7 @@ impl Command for AddChild { }); } let mut parent = world.entity_mut(self.parent); - if let Some(mut children) = parent.get_mut::() { + if let Some(mut children) = parent.get_mut_protected::() { if !children.contains(&self.child) { children.0.push(self.child); } @@ -137,7 +137,7 @@ impl Command for InsertChildren { fn write(self, world: &mut World) { update_old_parents(world, self.parent, &self.children); let mut parent = world.entity_mut(self.parent); - if let Some(mut children) = parent.get_mut::() { + if let Some(mut children) = parent.get_mut_protected::() { children.0.retain(|value| !self.children.contains(value)); children.0.insert_from_slice(self.index, &self.children); } else { @@ -157,7 +157,7 @@ impl Command for PushChildren { fn write(mut self, world: &mut World) { update_old_parents(world, self.parent, &self.children); let mut parent = world.entity_mut(self.parent); - if let Some(mut children) = parent.get_mut::() { + if let Some(mut children) = parent.get_mut_protected::() { children.0.retain(|child| !self.children.contains(child)); children.0.append(&mut self.children); } else { @@ -326,7 +326,7 @@ pub struct WorldChildBuilder<'w> { impl<'w> WorldChildBuilder<'w> { /// Spawns an entity with the given bundle and inserts it into the children defined by the [`WorldChildBuilder`] - pub fn spawn_bundle(&mut self, bundle: impl Bundle + Send + Sync + 'static) -> EntityMut<'_> { + pub fn spawn_bundle(&mut self, bundle: impl Bundle) -> EntityMut<'_> { let parent_entity = self.parent_entity(); let entity = self .world @@ -408,7 +408,7 @@ impl<'w> BuildWorldChildren for EntityMut<'w> { // Inserting a bundle in the children entities may change the parent entity's location if they were of the same archetype self.update_location(); } - if let Some(mut children_component) = self.get_mut::() { + if let Some(mut children_component) = self.get_mut_protected::() { children_component .0 .retain(|value| !children.contains(value)); @@ -429,7 +429,7 @@ impl<'w> BuildWorldChildren for EntityMut<'w> { self.update_location(); } - if let Some(mut children_component) = self.get_mut::() { + if let Some(mut children_component) = self.get_mut_protected::() { children_component .0 .retain(|value| !children.contains(value)); @@ -471,7 +471,7 @@ impl<'w> BuildWorldChildren for WorldChildBuilder<'w> { .current_entity .expect("Cannot add children without a parent. Try creating an entity first."); update_old_parents(self.world, parent, children); - if let Some(mut children_component) = self.world.get_mut::(parent) { + if let Some(mut children_component) = self.world.get_mut_protected::(parent) { children_component .0 .retain(|value| !children.contains(value)); @@ -489,7 +489,7 @@ impl<'w> BuildWorldChildren for WorldChildBuilder<'w> { .current_entity .expect("Cannot add children without a parent. Try creating an entity first."); update_old_parents(self.world, parent, children); - if let Some(mut children_component) = self.world.get_mut::(parent) { + if let Some(mut children_component) = self.world.get_mut_protected::(parent) { children_component .0 .retain(|value| !children.contains(value)); diff --git a/crates/bevy_hierarchy/src/components/children.rs b/crates/bevy_hierarchy/src/components/children.rs index e9e3b72654709..0680b111f9b25 100644 --- a/crates/bevy_hierarchy/src/components/children.rs +++ b/crates/bevy_hierarchy/src/components/children.rs @@ -13,6 +13,7 @@ use std::ops::Deref; /// Contains references to the child entities of this entity #[derive(Component, Debug, Reflect)] #[reflect(Component, MapEntities)] +#[component(vis = "crate::Vis")] pub struct Children(pub(crate) SmallVec<[Entity; 8]>); impl MapEntities for Children { diff --git a/crates/bevy_hierarchy/src/components/mod.rs b/crates/bevy_hierarchy/src/components/mod.rs index 3c8b544850382..87253b859bd3a 100644 --- a/crates/bevy_hierarchy/src/components/mod.rs +++ b/crates/bevy_hierarchy/src/components/mod.rs @@ -3,3 +3,5 @@ mod parent; pub use children::Children; pub use parent::Parent; + +pub(crate) struct Vis; diff --git a/crates/bevy_hierarchy/src/components/parent.rs b/crates/bevy_hierarchy/src/components/parent.rs index d4a5e2f3bec70..eff8f48c3ae53 100644 --- a/crates/bevy_hierarchy/src/components/parent.rs +++ b/crates/bevy_hierarchy/src/components/parent.rs @@ -11,6 +11,7 @@ use std::ops::Deref; /// This component should only be present on entities that actually have a parent entity. #[derive(Component, Debug, Eq, PartialEq, Reflect)] #[reflect(Component, MapEntities, PartialEq)] +#[component(vis = "crate::Vis")] pub struct Parent(pub(crate) Entity); impl Parent { diff --git a/crates/bevy_hierarchy/src/hierarchy.rs b/crates/bevy_hierarchy/src/hierarchy.rs index cb4a60597c2ae..1c9bbab0c955d 100644 --- a/crates/bevy_hierarchy/src/hierarchy.rs +++ b/crates/bevy_hierarchy/src/hierarchy.rs @@ -24,7 +24,7 @@ pub struct DespawnChildrenRecursive { pub fn despawn_with_children_recursive(world: &mut World, entity: Entity) { // first, make the entity's own parent forget about it if let Some(parent) = world.get::(entity).map(|parent| parent.0) { - if let Some(mut children) = world.get_mut::(parent) { + if let Some(mut children) = world.get_mut_protected::(parent) { children.0.retain(|c| *c != entity); } } @@ -35,7 +35,7 @@ pub fn despawn_with_children_recursive(world: &mut World, entity: Entity) { // Should only be called by `despawn_with_children_recursive`! fn despawn_with_children_recursive_inner(world: &mut World, entity: Entity) { - if let Some(mut children) = world.get_mut::(entity) { + if let Some(mut children) = world.get_mut_protected::(entity) { for e in std::mem::take(&mut children.0) { despawn_with_children_recursive_inner(world, e); } @@ -47,7 +47,7 @@ fn despawn_with_children_recursive_inner(world: &mut World, entity: Entity) { } fn despawn_children(world: &mut World, entity: Entity) { - if let Some(mut children) = world.get_mut::(entity) { + if let Some(mut children) = world.get_mut_protected::(entity) { for e in std::mem::take(&mut children.0) { despawn_with_children_recursive_inner(world, e); } diff --git a/crates/bevy_transform/src/systems.rs b/crates/bevy_transform/src/systems.rs index 9406149fb0847..54c4779cafcc6 100644 --- a/crates/bevy_transform/src/systems.rs +++ b/crates/bevy_transform/src/systems.rs @@ -103,7 +103,7 @@ mod test { use crate::components::{GlobalTransform, Transform}; use crate::systems::transform_propagate_system; use crate::TransformBundle; - use bevy_hierarchy::{BuildChildren, BuildWorldChildren, Children, Parent}; + use bevy_hierarchy::{BuildChildren, BuildWorldChildren, Children}; #[test] fn did_propagate() { @@ -323,47 +323,4 @@ mod test { assert_eq!(global, &GlobalTransform::from_translation(translation)); } } - - #[test] - #[should_panic] - fn panic_when_hierarchy_cycle() { - // We cannot directly edit Parent and Children, so we use a temp world to break - // the hierarchy's invariants. - let mut temp = World::new(); - let mut app = App::new(); - - app.add_system(transform_propagate_system); - - fn setup_world(world: &mut World) -> (Entity, Entity) { - let mut grandchild = Entity::from_raw(0); - let child = world - .spawn() - .insert_bundle((Transform::identity(), GlobalTransform::default())) - .with_children(|builder| { - grandchild = builder - .spawn() - .insert_bundle((Transform::identity(), GlobalTransform::default())) - .id(); - }) - .id(); - (child, grandchild) - } - - let (temp_child, temp_grandchild) = setup_world(&mut temp); - let (child, grandchild) = setup_world(&mut app.world); - - assert_eq!(temp_child, child); - assert_eq!(temp_grandchild, grandchild); - - app.world - .spawn() - .insert_bundle((Transform::default(), GlobalTransform::default())) - .push_children(&[child]); - std::mem::swap( - &mut *app.world.get_mut::(child).unwrap(), - &mut *temp.get_mut::(grandchild).unwrap(), - ); - - app.update(); - } } From 728f68ba8056d58825517c3edd98fad315593f25 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Fri, 26 Aug 2022 13:52:34 -0400 Subject: [PATCH 7/9] `vis` -> `write_marker` --- crates/bevy_ecs/macros/src/component.rs | 2 +- crates/bevy_ecs/src/component.rs | 6 +++--- crates/bevy_hierarchy/src/components/children.rs | 2 +- crates/bevy_hierarchy/src/components/parent.rs | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/macros/src/component.rs b/crates/bevy_ecs/macros/src/component.rs index 4a3d67eb6d159..24470a1838388 100644 --- a/crates/bevy_ecs/macros/src/component.rs +++ b/crates/bevy_ecs/macros/src/component.rs @@ -55,7 +55,7 @@ pub fn derive_component(input: TokenStream) -> TokenStream { pub const COMPONENT: Symbol = Symbol("component"); pub const STORAGE: Symbol = Symbol("storage"); -pub const VISIBILITY: Symbol = Symbol("vis"); +pub const VISIBILITY: Symbol = Symbol("write_marker"); struct Attrs { marker: Option, diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index a0a82a8b4beeb..506777a679aaa 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -173,11 +173,11 @@ where /// struct Marker; /// /// #[derive(Component)] -/// #[component(vis = "Marker")] +/// #[component(write_marker = "Marker")] /// pub struct Chassis; /// /// #[derive(Component)] -/// #[component(vis = "Marker")] +/// #[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. @@ -185,7 +185,7 @@ where /// } /// /// #[derive(Component)] -/// #[component(vis = "Marker")] +/// #[component(write_marker = "Marker")] /// pub struct Tires { /// pub width: f64, /// } diff --git a/crates/bevy_hierarchy/src/components/children.rs b/crates/bevy_hierarchy/src/components/children.rs index 0680b111f9b25..7fef88163a35d 100644 --- a/crates/bevy_hierarchy/src/components/children.rs +++ b/crates/bevy_hierarchy/src/components/children.rs @@ -13,7 +13,7 @@ use std::ops::Deref; /// Contains references to the child entities of this entity #[derive(Component, Debug, Reflect)] #[reflect(Component, MapEntities)] -#[component(vis = "crate::Vis")] +#[component(write_marker = "crate::Vis")] pub struct Children(pub(crate) SmallVec<[Entity; 8]>); impl MapEntities for Children { diff --git a/crates/bevy_hierarchy/src/components/parent.rs b/crates/bevy_hierarchy/src/components/parent.rs index eff8f48c3ae53..d8c1c1986524a 100644 --- a/crates/bevy_hierarchy/src/components/parent.rs +++ b/crates/bevy_hierarchy/src/components/parent.rs @@ -11,7 +11,7 @@ use std::ops::Deref; /// This component should only be present on entities that actually have a parent entity. #[derive(Component, Debug, Eq, PartialEq, Reflect)] #[reflect(Component, MapEntities, PartialEq)] -#[component(vis = "crate::Vis")] +#[component(write_marker = "crate::Vis")] pub struct Parent(pub(crate) Entity); impl Parent { From d7784516bb7c308a642b39e740c5a8347c3e9663 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Fri, 26 Aug 2022 13:54:42 -0400 Subject: [PATCH 8/9] Link a blocking issue in a comment --- crates/bevy_ecs/src/component.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 506777a679aaa..4125129da35bb 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -229,6 +229,7 @@ where T: WriteComponent, { // FIXME: Derive this when we can do #[bundle(ignore)]. + // https://github.com/bevyengine/bevy/pull/5628 val: Unlocked, _marker: PhantomData Marker>, } From 7385f61f3b6570f3f9434c480149b5c401fccdab Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Fri, 26 Aug 2022 14:18:49 -0400 Subject: [PATCH 9/9] Shorten some trait bounds --- crates/bevy_ecs/src/world/entity_ref.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index abcc81715f0a5..5f67947ce0ade 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -198,10 +198,7 @@ impl<'w> EntityMut<'w> { /// Gets mutable access to a component that has protected mutability. #[inline] - pub fn get_mut_protected(&mut self) -> Option> - where - T: WriteComponent, - { + pub fn get_mut_protected, Vis>(&mut self) -> Option> { // SAFETY: world access is unique, and lifetimes enforce correct usage of returned borrow unsafe { self.get_unchecked_mut::() } } @@ -460,10 +457,7 @@ impl<'w> EntityMut<'w> { } /// Removes a component that has protected mutability. - pub fn remove_protected(&mut self) -> Option - where - T: WriteComponent, - { + pub fn remove_protected, Vis>(&mut self) -> Option { self.remove_bundle::>() .map(|v| v.0) }