From bc82749012f8d1557de4b6c2f80b24f3b4a262c9 Mon Sep 17 00:00:00 2001 From: James Liu Date: Mon, 19 Feb 2024 11:04:47 -0800 Subject: [PATCH] Remove APIs deprecated in 0.13 (#11974) # Objective We deprecated quite a few APIs in 0.13. 0.13 has shipped already. It should be OK to remove them in 0.14's release. Fixes #4059. Fixes #9011. ## Solution Remove them. --- benches/benches/bevy_ecs/world/entity_hash.rs | 2 +- benches/benches/bevy_ecs/world/mod.rs | 2 - benches/benches/bevy_ecs/world/world_get.rs | 99 ------ crates/bevy_ecs/src/entity/map_entities.rs | 9 - crates/bevy_ecs/src/query/error.rs | 70 ---- crates/bevy_ecs/src/query/mod.rs | 2 - crates/bevy_ecs/src/query/state.rs | 167 +-------- crates/bevy_ecs/src/schedule/condition.rs | 61 ---- crates/bevy_ecs/src/system/mod.rs | 76 ---- crates/bevy_ecs/src/system/query.rs | 334 ++---------------- crates/bevy_ecs/src/system/system_param.rs | 2 +- .../tests/ui/query_lifetime_safety.rs | 14 - .../tests/ui/query_lifetime_safety.stderr | 84 ++--- crates/bevy_render/src/lib.rs | 2 +- crates/bevy_render/src/mesh/mesh/mod.rs | 2 +- crates/bevy_render/src/mesh/mod.rs | 2 - crates/bevy_render/src/mesh/shape/capsule.rs | 49 --- crates/bevy_render/src/mesh/shape/cylinder.rs | 41 --- .../bevy_render/src/mesh/shape/icosphere.rs | 32 -- crates/bevy_render/src/mesh/shape/mod.rs | 306 ---------------- .../src/mesh/shape/regular_polygon.rs | 85 ----- crates/bevy_render/src/mesh/shape/torus.rs | 38 -- crates/bevy_render/src/mesh/shape/uvsphere.rs | 37 -- 23 files changed, 74 insertions(+), 1442 deletions(-) delete mode 100644 crates/bevy_render/src/mesh/shape/capsule.rs delete mode 100644 crates/bevy_render/src/mesh/shape/cylinder.rs delete mode 100644 crates/bevy_render/src/mesh/shape/icosphere.rs delete mode 100644 crates/bevy_render/src/mesh/shape/mod.rs delete mode 100644 crates/bevy_render/src/mesh/shape/regular_polygon.rs delete mode 100644 crates/bevy_render/src/mesh/shape/torus.rs delete mode 100644 crates/bevy_render/src/mesh/shape/uvsphere.rs diff --git a/benches/benches/bevy_ecs/world/entity_hash.rs b/benches/benches/bevy_ecs/world/entity_hash.rs index 03faf268330ca..c337e9d86a401 100644 --- a/benches/benches/bevy_ecs/world/entity_hash.rs +++ b/benches/benches/bevy_ecs/world/entity_hash.rs @@ -1,4 +1,4 @@ -use bevy_ecs::entity::{Entity, EntityHashMap, EntityHashSet}; +use bevy_ecs::entity::{Entity, EntityHashSet}; use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion, Throughput}; use rand::{Rng, SeedableRng}; use rand_chacha::ChaCha8Rng; diff --git a/benches/benches/bevy_ecs/world/mod.rs b/benches/benches/bevy_ecs/world/mod.rs index 421f7a06ad239..8b12a08fcd783 100644 --- a/benches/benches/bevy_ecs/world/mod.rs +++ b/benches/benches/bevy_ecs/world/mod.rs @@ -28,8 +28,6 @@ criterion_group!( world_query_iter, world_query_for_each, world_spawn, - query_get_component_simple, - query_get_component, query_get, query_get_many::<2>, query_get_many::<5>, diff --git a/benches/benches/bevy_ecs/world/world_get.rs b/benches/benches/bevy_ecs/world/world_get.rs index 5daef6cd88f0d..4812d44f6bca6 100644 --- a/benches/benches/bevy_ecs/world/world_get.rs +++ b/benches/benches/bevy_ecs/world/world_get.rs @@ -2,7 +2,6 @@ use bevy_ecs::{ bundle::Bundle, component::Component, entity::Entity, - prelude::*, system::{Query, SystemState}, world::World, }; @@ -257,104 +256,6 @@ pub fn world_query_for_each(criterion: &mut Criterion) { group.finish(); } -pub fn query_get_component_simple(criterion: &mut Criterion) { - #[derive(Component)] - struct A(f32); - - let mut group = criterion.benchmark_group("query_get_component_simple"); - group.warm_up_time(std::time::Duration::from_millis(500)); - group.measurement_time(std::time::Duration::from_secs(4)); - - group.bench_function("unchecked", |bencher| { - let mut world = World::new(); - - let entity = world.spawn(A(0.0)).id(); - let mut query = world.query::<&mut A>(); - - let world_cell = world.as_unsafe_world_cell(); - bencher.iter(|| { - for _x in 0..100000 { - let mut a = unsafe { query.get_unchecked(world_cell, entity).unwrap() }; - a.0 += 1.0; - } - }); - }); - group.bench_function("system", |bencher| { - let mut world = World::new(); - - let entity = world.spawn(A(0.0)).id(); - fn query_system(In(entity): In, mut query: Query<&mut A>) { - for _ in 0..100_000 { - let mut a = query.get_mut(entity).unwrap(); - a.0 += 1.0; - } - } - - let mut system = IntoSystem::into_system(query_system); - system.initialize(&mut world); - system.update_archetype_component_access(world.as_unsafe_world_cell()); - - bencher.iter(|| system.run(entity, &mut world)); - }); - - group.finish(); -} - -pub fn query_get_component(criterion: &mut Criterion) { - let mut group = criterion.benchmark_group("query_get_component"); - group.warm_up_time(std::time::Duration::from_millis(500)); - group.measurement_time(std::time::Duration::from_secs(4)); - - for entity_count in RANGE.map(|i| i * 10_000) { - group.bench_function(format!("{}_entities_table", entity_count), |bencher| { - let mut world = World::default(); - let mut entities: Vec<_> = world - .spawn_batch((0..entity_count).map(|_| Table::default())) - .collect(); - entities.shuffle(&mut deterministic_rand()); - let mut query = SystemState::>::new(&mut world); - let query = query.get(&world); - - bencher.iter(|| { - let mut count = 0; - for comp in entities - .iter() - .flat_map(|&e| query.get_component::(e)) - { - black_box(comp); - count += 1; - black_box(count); - } - assert_eq!(black_box(count), entity_count); - }); - }); - group.bench_function(format!("{}_entities_sparse", entity_count), |bencher| { - let mut world = World::default(); - let mut entities: Vec<_> = world - .spawn_batch((0..entity_count).map(|_| Sparse::default())) - .collect(); - entities.shuffle(&mut deterministic_rand()); - let mut query = SystemState::>::new(&mut world); - let query = query.get(&world); - - bencher.iter(|| { - let mut count = 0; - for comp in entities - .iter() - .flat_map(|&e| query.get_component::(e)) - { - black_box(comp); - count += 1; - black_box(count); - } - assert_eq!(black_box(count), entity_count); - }); - }); - } - - group.finish(); -} - pub fn query_get(criterion: &mut Criterion) { let mut group = criterion.benchmark_group("query_get"); group.warm_up_time(std::time::Duration::from_millis(500)); diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index 1b8a2943707dc..083b7a5075b2e 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -121,15 +121,6 @@ pub struct SceneEntityMapper<'m> { } impl<'m> SceneEntityMapper<'m> { - #[deprecated( - since = "0.13.0", - note = "please use `EntityMapper::map_entity` instead" - )] - /// Returns the corresponding mapped entity or reserves a new dead entity ID in the current world if it is absent. - pub fn get_or_reserve(&mut self, entity: Entity) -> Entity { - self.map_entity(entity) - } - /// Gets a reference to the underlying [`EntityHashMap`]. pub fn get_map(&'m self) -> &'m EntityHashMap { self.map diff --git a/crates/bevy_ecs/src/query/error.rs b/crates/bevy_ecs/src/query/error.rs index 1df1417910359..1f1f33f2c4d56 100644 --- a/crates/bevy_ecs/src/query/error.rs +++ b/crates/bevy_ecs/src/query/error.rs @@ -21,76 +21,6 @@ pub enum QueryEntityError { AliasedMutability(Entity), } -/// An error that occurs when retrieving a specific [`Entity`]'s component from a [`Query`](crate::system::Query). -#[derive(Debug, PartialEq, Eq, Error)] -pub enum QueryComponentError { - /// The [`Query`](crate::system::Query) does not have read access to the requested component. - /// - /// This error occurs when the requested component is not included in the original query. - /// - /// # Example - /// - /// ``` - /// # use bevy_ecs::{prelude::*, query::QueryComponentError}; - /// # - /// # #[derive(Component)] - /// # struct OtherComponent; - /// # - /// # #[derive(Component, PartialEq, Debug)] - /// # struct RequestedComponent; - /// # - /// # #[derive(Resource)] - /// # struct SpecificEntity { - /// # entity: Entity, - /// # } - /// # - /// fn get_missing_read_access_error(query: Query<&OtherComponent>, res: Res) { - /// assert_eq!( - /// query.get_component::(res.entity), - /// Err(QueryComponentError::MissingReadAccess), - /// ); - /// println!("query doesn't have read access to RequestedComponent because it does not appear in Query<&OtherComponent>"); - /// } - /// # bevy_ecs::system::assert_is_system(get_missing_read_access_error); - /// ``` - #[error("This query does not have read access to the requested component")] - MissingReadAccess, - /// The [`Query`](crate::system::Query) does not have write access to the requested component. - /// - /// This error occurs when the requested component is not included in the original query, or the mutability of the requested component is mismatched with the original query. - /// - /// # Example - /// - /// ``` - /// # use bevy_ecs::{prelude::*, query::QueryComponentError}; - /// # - /// # #[derive(Component, PartialEq, Debug)] - /// # struct RequestedComponent; - /// # - /// # #[derive(Resource)] - /// # struct SpecificEntity { - /// # entity: Entity, - /// # } - /// # - /// fn get_missing_write_access_error(mut query: Query<&RequestedComponent>, res: Res) { - /// assert_eq!( - /// query.get_component::(res.entity), - /// Err(QueryComponentError::MissingWriteAccess), - /// ); - /// println!("query doesn't have write access to RequestedComponent because it doesn't have &mut in Query<&RequestedComponent>"); - /// } - /// # bevy_ecs::system::assert_is_system(get_missing_write_access_error); - /// ``` - #[error("This query does not have write access to the requested component")] - MissingWriteAccess, - /// The given [`Entity`] does not have the requested component. - #[error("The given entity does not have the requested component")] - MissingComponent, - /// The requested [`Entity`] does not exist. - #[error("The requested entity does not exist")] - NoSuchEntity, -} - /// An error that occurs when evaluating a [`Query`](crate::system::Query) or [`QueryState`](crate::query::QueryState) as a single expected result via /// [`get_single`](crate::system::Query::get_single) or [`get_single_mut`](crate::system::Query::get_single_mut). #[derive(Debug, Error)] diff --git a/crates/bevy_ecs/src/query/mod.rs b/crates/bevy_ecs/src/query/mod.rs index 249485d005b9e..4cc2b9f3458a8 100644 --- a/crates/bevy_ecs/src/query/mod.rs +++ b/crates/bevy_ecs/src/query/mod.rs @@ -739,7 +739,6 @@ mod tests { } } - #[allow(deprecated)] #[test] fn mut_to_immut_query_methods_have_immut_item() { #[derive(Component)] @@ -771,7 +770,6 @@ mod tests { q.iter().for_each(|_: &Foo| ()); let _: Option<&Foo> = q.get(e).ok(); - let _: Option<&Foo> = q.get_component(e).ok(); let _: Option<[&Foo; 1]> = q.get_many([e]).ok(); let _: Option<&Foo> = q.get_single().ok(); let _: [&Foo; 1] = q.many([e]); diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index d0c79a3a72ded..f32ba11b7686f 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -1,9 +1,8 @@ use crate::{ archetype::{Archetype, ArchetypeComponentId, ArchetypeGeneration, ArchetypeId}, - change_detection::Mut, component::{ComponentId, Tick}, entity::Entity, - prelude::{Component, FromWorld}, + prelude::FromWorld, query::{ Access, BatchingStrategy, DebugCheckedUnwrap, FilteredAccess, QueryCombinationIter, QueryIter, QueryParIter, @@ -14,7 +13,7 @@ use crate::{ #[cfg(feature = "trace")] use bevy_utils::tracing::Span; use fixedbitset::FixedBitSet; -use std::{any::TypeId, borrow::Borrow, fmt, mem::MaybeUninit, ptr}; +use std::{borrow::Borrow, fmt, mem::MaybeUninit, ptr}; use super::{ NopWorldQuery, QueryBuilder, QueryData, QueryEntityError, QueryFilter, QueryManyIter, @@ -646,118 +645,6 @@ impl QueryState { } } - /// Returns a shared reference to the component `T` of the given [`Entity`]. - /// - /// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is returned instead. - #[deprecated( - since = "0.13.0", - note = "Please use `get` and select for the exact component based on the structure of the exact query as required." - )] - #[allow(deprecated)] - #[inline] - pub(crate) fn get_component<'w, T: Component>( - &self, - world: UnsafeWorldCell<'w>, - entity: Entity, - ) -> Result<&'w T, super::QueryComponentError> { - let entity_ref = world - .get_entity(entity) - .ok_or(super::QueryComponentError::NoSuchEntity)?; - let component_id = world - .components() - .get_id(TypeId::of::()) - .ok_or(super::QueryComponentError::MissingComponent)?; - let archetype_component = entity_ref - .archetype() - .get_archetype_component_id(component_id) - .ok_or(super::QueryComponentError::MissingComponent)?; - if self - .archetype_component_access - .has_read(archetype_component) - { - // SAFETY: `world` must have access to the component `T` for this entity, - // since it was registered in `self`'s archetype component access set. - unsafe { entity_ref.get::() }.ok_or(super::QueryComponentError::MissingComponent) - } else { - Err(super::QueryComponentError::MissingReadAccess) - } - } - - /// Returns a shared reference to the component `T` of the given [`Entity`]. - /// - /// # Panics - /// - /// If given a nonexisting entity or mismatched component, this will panic. - #[deprecated( - since = "0.13.0", - note = "Please use `get` and select for the exact component based on the structure of the exact query as required." - )] - #[allow(deprecated)] - #[inline] - pub(crate) fn component<'w, T: Component>( - &self, - world: UnsafeWorldCell<'w>, - entity: Entity, - ) -> &'w T { - match self.get_component(world, entity) { - Ok(component) => component, - Err(error) => { - panic!( - "Cannot get component `{:?}` from {entity:?}: {error}", - TypeId::of::() - ) - } - } - } - - /// Returns a mutable reference to the component `T` of the given entity. - /// - /// In case of a nonexisting entity or mismatched component, a [`QueryComponentError`] is returned instead. - /// - /// # Safety - /// - /// This function makes it possible to violate Rust's aliasing guarantees. - /// You must make sure this call does not result in multiple mutable references to the same component. - /// - /// [`QueryComponentError`]: super::QueryComponentError - #[deprecated( - since = "0.13.0", - note = "Please use QueryState::get_unchecked_manual and select for the exact component based on the structure of the exact query as required." - )] - #[allow(deprecated)] - #[inline] - pub unsafe fn get_component_unchecked_mut<'w, T: Component>( - &self, - world: UnsafeWorldCell<'w>, - entity: Entity, - last_run: Tick, - this_run: Tick, - ) -> Result, super::QueryComponentError> { - let entity_ref = world - .get_entity(entity) - .ok_or(super::QueryComponentError::NoSuchEntity)?; - let component_id = world - .components() - .get_id(TypeId::of::()) - .ok_or(super::QueryComponentError::MissingComponent)?; - let archetype_component = entity_ref - .archetype() - .get_archetype_component_id(component_id) - .ok_or(super::QueryComponentError::MissingComponent)?; - if self - .archetype_component_access - .has_write(archetype_component) - { - // SAFETY: It is the responsibility of the caller to ensure it is sound to get a - // mutable reference to this entity's component `T`. - let result = unsafe { entity_ref.get_mut_using_ticks::(last_run, this_run) }; - - result.ok_or(super::QueryComponentError::MissingComponent) - } else { - Err(super::QueryComponentError::MissingWriteAccess) - } - } - /// Gets the read-only query results for the given [`World`] and array of [`Entity`], where the last change and /// the current change tick are given. /// @@ -1133,56 +1020,6 @@ impl QueryState { QueryCombinationIter::new(world, self, last_run, this_run) } - /// Runs `func` on each query result for the given [`World`]. This is faster than the equivalent - /// `iter()` method, but cannot be chained like a normal [`Iterator`]. - /// - /// This can only be called for read-only queries, see [`Self::for_each_mut`] for write-queries. - /// - /// Shorthand for `query.iter(world).for_each(..)`. - #[inline] - #[deprecated( - since = "0.13.0", - note = "QueryState::for_each was not idiomatic Rust and has been moved to query.iter().for_each()" - )] - pub fn for_each<'w, FN: FnMut(ROQueryItem<'w, D>)>(&mut self, world: &'w World, func: FN) { - self.iter(world).for_each(func); - } - - /// Runs `func` on each query result for the given [`World`]. This is faster than the equivalent - /// `iter_mut()` method, but cannot be chained like a normal [`Iterator`]. - /// - /// Shorthand for `query.iter_mut(world).for_each(..)`. - #[inline] - #[deprecated( - since = "0.13.0", - note = "QueryState::for_each_mut was not idiomatic Rust and has been moved to query.iter_mut().for_each()" - )] - pub fn for_each_mut<'w, FN: FnMut(D::Item<'w>)>(&mut self, world: &'w mut World, func: FN) { - self.iter_mut(world).for_each(func); - } - - /// Runs `func` on each query result for the given [`World`]. This is faster than the equivalent - /// `iter()` method, but cannot be chained like a normal [`Iterator`]. - /// - /// # Safety - /// - /// This does not check for mutable query correctness. To be safe, make sure mutable queries - /// have unique access to the components they query. - #[inline] - #[deprecated( - since = "0.13.0", - note = "QueryState::for_each_unchecked was not idiomatic Rust and has been moved to query.iter_unchecked_manual().for_each()" - )] - pub unsafe fn for_each_unchecked<'w, FN: FnMut(D::Item<'w>)>( - &mut self, - world: UnsafeWorldCell<'w>, - func: FN, - ) { - self.update_archetypes_unsafe_world_cell(world); - self.iter_unchecked_manual(world, world.last_change_tick(), world.change_tick()) - .for_each(func); - } - /// Returns a parallel iterator over the query results for the given [`World`]. /// /// This can only be called for read-only queries, see [`par_iter_mut`] for write-queries. diff --git a/crates/bevy_ecs/src/schedule/condition.rs b/crates/bevy_ecs/src/schedule/condition.rs index 4c29a5994b60e..1be8e4b7edded 100644 --- a/crates/bevy_ecs/src/schedule/condition.rs +++ b/crates/bevy_ecs/src/schedule/condition.rs @@ -766,67 +766,6 @@ pub mod common_conditions { } } - /// Identical to [`in_state`] - use that instead. - /// - /// Generates a [`Condition`](super::Condition)-satisfying closure that returns `true` - /// if the state machine exists and is currently in `state`. - /// - /// The condition will return `false` if the state does not exist. - /// - /// # Example - /// - /// ``` - /// # use bevy_ecs::prelude::*; - /// # #[derive(Resource, Default)] - /// # struct Counter(u8); - /// # let mut app = Schedule::default(); - /// # let mut world = World::new(); - /// # world.init_resource::(); - /// #[derive(States, Clone, Copy, Default, Eq, PartialEq, Hash, Debug)] - /// enum GameState { - /// #[default] - /// Playing, - /// Paused, - /// } - /// - /// app.add_systems(( - /// // `state_exists_and_equals` will only return true if the - /// // given state exists and equals the given value - /// play_system.run_if(state_exists_and_equals(GameState::Playing)), - /// pause_system.run_if(state_exists_and_equals(GameState::Paused)), - /// )); - /// - /// fn play_system(mut counter: ResMut) { - /// counter.0 += 1; - /// } - /// - /// fn pause_system(mut counter: ResMut) { - /// counter.0 -= 1; - /// } - /// - /// // `GameState` does not yet exists so neither system will run - /// app.run(&mut world); - /// assert_eq!(world.resource::().0, 0); - /// - /// world.init_resource::>(); - /// - /// // We default to `GameState::Playing` so `play_system` runs - /// app.run(&mut world); - /// assert_eq!(world.resource::().0, 1); - /// - /// *world.resource_mut::>() = State::new(GameState::Paused); - /// - /// // Now that we are in `GameState::Pause`, `pause_system` will run - /// app.run(&mut world); - /// assert_eq!(world.resource::().0, 0); - /// ``` - #[deprecated(since = "0.13.0", note = "use `in_state` instead.")] - pub fn state_exists_and_equals( - state: S, - ) -> impl FnMut(Option>>) -> bool + Clone { - in_state(state) - } - /// A [`Condition`](super::Condition)-satisfying system that returns `true` /// if the state machine changed state. /// diff --git a/crates/bevy_ecs/src/system/mod.rs b/crates/bevy_ecs/src/system/mod.rs index a89e189670fc0..775163f4fc2b9 100644 --- a/crates/bevy_ecs/src/system/mod.rs +++ b/crates/bevy_ecs/src/system/mod.rs @@ -401,65 +401,6 @@ mod tests { schedule.run(world); } - #[allow(deprecated)] - #[test] - fn query_system_gets() { - fn query_system( - mut ran: ResMut, - entity_query: Query>, - b_query: Query<&B>, - a_c_query: Query<(&A, &C)>, - d_query: Query<&D>, - ) { - let entities = entity_query.iter().collect::>(); - assert!( - b_query.get_component::(entities[0]).is_err(), - "entity 0 should not have B" - ); - assert!( - b_query.get_component::(entities[1]).is_ok(), - "entity 1 should have B" - ); - assert!( - b_query.get_component::(entities[1]).is_err(), - "entity 1 should have A, but b_query shouldn't have access to it" - ); - assert!( - b_query.get_component::(entities[3]).is_err(), - "entity 3 should have D, but it shouldn't be accessible from b_query" - ); - assert!( - b_query.get_component::(entities[2]).is_err(), - "entity 2 has C, but it shouldn't be accessible from b_query" - ); - assert!( - a_c_query.get_component::(entities[2]).is_ok(), - "entity 2 has C, and it should be accessible from a_c_query" - ); - assert!( - a_c_query.get_component::(entities[3]).is_err(), - "entity 3 should have D, but it shouldn't be accessible from b_query" - ); - assert!( - d_query.get_component::(entities[3]).is_ok(), - "entity 3 should have D" - ); - - *ran = SystemRan::Yes; - } - - let mut world = World::default(); - world.insert_resource(SystemRan::No); - world.spawn(A); - world.spawn((A, B)); - world.spawn((A, C)); - world.spawn((A, D)); - - run_system(&mut world, query_system); - - assert_eq!(*world.resource::(), SystemRan::Yes); - } - #[test] fn get_many_is_ordered() { use crate::system::Resource; @@ -1561,22 +1502,6 @@ mod tests { }); } - #[allow(deprecated)] - #[test] - fn readonly_query_get_mut_component_fails() { - use crate::query::QueryComponentError; - - let mut world = World::new(); - let entity = world.spawn(W(42u32)).id(); - run_system(&mut world, move |q: Query<&mut W>| { - let mut rq = q.to_readonly(); - assert_eq!( - QueryComponentError::MissingWriteAccess, - rq.get_component_mut::>(entity).unwrap_err(), - ); - }); - } - #[test] #[should_panic = "Encountered a mismatched World."] fn query_validates_world_id() { @@ -1590,7 +1515,6 @@ mod tests { &qstate, Tick::new(0), Tick::new(0), - false, ) }; query.iter(); diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index f13b38024451c..7689c459c06b1 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1,14 +1,14 @@ use crate::{ - component::{Component, Tick}, + component::Tick, entity::Entity, query::{ BatchingStrategy, QueryCombinationIter, QueryData, QueryEntityError, QueryFilter, QueryIter, QueryManyIter, QueryParIter, QuerySingleError, QueryState, ROQueryItem, ReadOnlyQueryData, }, - world::{unsafe_world_cell::UnsafeWorldCell, Mut}, + world::unsafe_world_cell::UnsafeWorldCell, }; -use std::{any::TypeId, borrow::Borrow}; +use std::borrow::Borrow; /// [System parameter] that provides selective access to the [`Component`] data stored in a [`World`]. /// @@ -239,7 +239,7 @@ use std::{any::TypeId, borrow::Borrow}; /// |Query methods|Effect| /// |:---:|---| /// |[`iter`]\[[`_mut`][`iter_mut`]]|Returns an iterator over all query items.| -/// |[`for_each`]\[[`_mut`][`for_each_mut`]],
[`par_iter`]\[[`_mut`][`par_iter_mut`]]|Runs a specified function for each query item.| +/// |[[`iter().for_each()`][`for_each`]\[[`iter_mut().for_each()`][`for_each`]],
[`par_iter`]\[[`_mut`][`par_iter_mut`]]|Runs a specified function for each query item.| /// |[`iter_many`]\[[`_mut`][`iter_many_mut`]]|Iterates or runs a specified function over query items generated by a list of entities.| /// |[`iter_combinations`]\[[`_mut`][`iter_combinations_mut`]]|Returns an iterator over all combinations of a specified number of query items.| /// |[`get`]\[[`_mut`][`get_mut`]]|Returns the query item for the specified entity.| @@ -275,7 +275,7 @@ use std::{any::TypeId, borrow::Borrow}; /// |Query operation|Computational complexity| /// |:---:|:---:| /// |[`iter`]\[[`_mut`][`iter_mut`]]|O(n)| -/// |[`for_each`]\[[`_mut`][`for_each_mut`]],
[`par_iter`]\[[`_mut`][`par_iter_mut`]]|O(n)| +/// |[[`iter().for_each()`][`for_each`]\[[`iter_mut().for_each()`][`for_each`]],
[`par_iter`]\[[`_mut`][`par_iter_mut`]]|O(n)| /// |[`iter_many`]\[[`_mut`][`iter_many_mut`]]|O(k)| /// |[`iter_combinations`]\[[`_mut`][`iter_combinations_mut`]]|O(nCr)| /// |[`get`]\[[`_mut`][`get_mut`]]|O(1)| @@ -285,12 +285,34 @@ use std::{any::TypeId, borrow::Borrow}; /// |Archetype based filtering ([`With`], [`Without`], [`Or`])|O(a)| /// |Change detection filtering ([`Added`], [`Changed`])|O(a + n)| /// -/// `for_each` methods are seen to be generally faster than their `iter` version on worlds with high archetype fragmentation. -/// As iterators are in general more flexible and better integrated with the rest of the Rust ecosystem, -/// it is advised to use `iter` methods over `for_each`. -/// It is strongly advised to only use `for_each` if it tangibly improves performance: -/// be sure profile or benchmark both before and after the change. +/// # `Iterator::for_each` /// +/// `for_each` methods are seen to be generally faster than directly iterating through `iter` on worlds with high archetype +/// fragmentation, and may enable additional optimizations like [autovectorization]. It is strongly advised to only use +/// [`Iterator::for_each`] if it tangibly improves performance. *Always* be sure profile or benchmark both before and +/// after the change! +/// +/// ```rust +/// # use bevy_ecs::prelude::*; +/// # #[derive(Component)] +/// # struct ComponentA; +/// # fn system( +/// # query: Query<&ComponentA>, +/// # ) { +/// // This might be result in better performance... +/// query.iter().for_each(|component| { +/// // do things with the component +/// }); +/// // ...than this. Always be sure to benchmark to validate the difference! +/// for component in query.iter() { +/// // do things with the component +/// } +/// # } +/// # bevy_ecs::system::assert_system_does_not_conflict(system); +/// ``` +/// +/// [`Component`]: crate::component::Component +/// [autovectorization]: https://en.wikipedia.org/wiki/Automatic_vectorization /// [`Added`]: crate::query::Added /// [`AnyOf`]: crate::query::AnyOf /// [binomial coefficient]: https://en.wikipedia.org/wiki/Binomial_coefficient @@ -298,8 +320,7 @@ use std::{any::TypeId, borrow::Borrow}; /// [components]: crate::component::Component /// [entity identifiers]: Entity /// [`EntityRef`]: crate::world::EntityRef -/// [`for_each`]: Self::for_each -/// [`for_each_mut`]: Self::for_each_mut +/// [`for_each`]: #iterator-for-each /// [`get`]: Self::get /// [`get_many`]: Self::get_many /// [`get_many_mut`]: Self::get_many_mut @@ -331,12 +352,6 @@ pub struct Query<'world, 'state, D: QueryData, F: QueryFilter = ()> { state: &'state QueryState, last_run: Tick, this_run: Tick, - // SAFETY: This is used to ensure that `get_component_mut::` properly fails when a Query writes C - // and gets converted to a read-only query using `to_readonly`. Without checking this, `get_component_mut` relies on - // QueryState's archetype_component_access, which will continue allowing write access to C after being cast to - // the read-only variant. This whole situation is confusing and error prone. Ideally this is a temporary hack - // until we sort out a cleaner alternative. - force_read_only_component_access: bool, } impl std::fmt::Debug for Query<'_, '_, D, F> { @@ -368,12 +383,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { state: &'s QueryState, last_run: Tick, this_run: Tick, - force_read_only_component_access: bool, ) -> Self { state.validate_world(world.id()); Self { - force_read_only_component_access, world, state, last_run, @@ -389,17 +402,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { pub fn to_readonly(&self) -> Query<'_, 's, D::ReadOnly, F> { let new_state = self.state.as_readonly(); // SAFETY: This is memory safe because it turns the query immutable. - unsafe { - Query::new( - self.world, - new_state, - self.last_run, - self.this_run, - // SAFETY: this must be set to true or `get_component_mut` will be unsound. See the comments - // on this field for more details - true, - ) - } + unsafe { Query::new(self.world, new_state, self.last_run, self.this_run) } } /// Returns an [`Iterator`] over the read-only query items. @@ -424,8 +427,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// /// # See also /// - /// - [`iter_mut`](Self::iter_mut) for mutable query items. - /// - [`for_each`](Self::for_each) for the closure based alternative. + /// [`iter_mut`](Self::iter_mut) for mutable query items. #[inline] pub fn iter(&self) -> QueryIter<'_, 's, D::ReadOnly, F> { // SAFETY: @@ -460,8 +462,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// /// # See also /// - /// - [`iter`](Self::iter) for read-only query items. - /// - [`for_each_mut`](Self::for_each_mut) for the closure based alternative. + /// [`iter`](Self::iter) for read-only query items. #[inline] pub fn iter_mut(&mut self) -> QueryIter<'_, 's, D, F> { // SAFETY: `self.world` has permission to access the required components. @@ -710,89 +711,6 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { .iter_many_unchecked_manual(entities, self.world, self.last_run, self.this_run) } - /// Runs `f` on each read-only query item. - /// - /// Shorthand for `query.iter().for_each(..)`. - /// - /// # Example - /// - /// Here, the `report_names_system` iterates over the `Player` component of every entity that contains it: - /// - /// ``` - /// # use bevy_ecs::prelude::*; - /// # - /// # #[derive(Component)] - /// # struct Player { name: String } - /// # - /// fn report_names_system(query: Query<&Player>) { - /// query.for_each(|player| { - /// println!("Say hello to {}!", player.name); - /// }); - /// } - /// # bevy_ecs::system::assert_is_system(report_names_system); - /// ``` - /// - /// # See also - /// - /// - [`for_each_mut`](Self::for_each_mut) to operate on mutable query items. - /// - [`iter`](Self::iter) for the iterator based alternative. - #[inline] - #[deprecated( - since = "0.13.0", - note = "Query::for_each was not idiomatic Rust and has been moved to query.iter().for_each()" - )] - pub fn for_each<'this>(&'this self, f: impl FnMut(ROQueryItem<'this, D>)) { - // SAFETY: - // - `self.world` has permission to access the required components. - // - The query is read-only, so it can be aliased even if it was originally mutable. - unsafe { - self.state - .as_readonly() - .iter_unchecked_manual(self.world, self.last_run, self.this_run) - .for_each(f); - }; - } - - /// Runs `f` on each query item. - /// - /// Shorthand for `query.iter_mut().for_each(..)`. - /// - /// # Example - /// - /// Here, the `gravity_system` updates the `Velocity` component of every entity that contains it: - /// - /// ``` - /// # use bevy_ecs::prelude::*; - /// # - /// # #[derive(Component)] - /// # struct Velocity { x: f32, y: f32, z: f32 } - /// fn gravity_system(mut query: Query<&mut Velocity>) { - /// const DELTA: f32 = 1.0 / 60.0; - /// query.for_each_mut(|mut velocity| { - /// velocity.y -= 9.8 * DELTA; - /// }); - /// } - /// # bevy_ecs::system::assert_is_system(gravity_system); - /// ``` - /// - /// # See also - /// - /// - [`for_each`](Self::for_each) to operate on read-only query items. - /// - [`iter_mut`](Self::iter_mut) for the iterator based alternative. - #[inline] - #[deprecated( - since = "0.13.0", - note = "Query::for_each_mut was not idiomatic Rust and has been moved to query.iter_mut().for_each()" - )] - pub fn for_each_mut<'a>(&'a mut self, f: impl FnMut(D::Item<'a>)) { - // SAFETY: `self.world` has permission to access the required components. - unsafe { - self.state - .iter_unchecked_manual(self.world, self.last_run, self.this_run) - .for_each(f); - }; - } - /// Returns a parallel iterator over the query results for the given [`World`]. /// /// This can only be called for read-only queries, see [`par_iter_mut`] for write-queries. @@ -1100,183 +1018,6 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { .get_unchecked_manual(self.world, entity, self.last_run, self.this_run) } - /// Returns a shared reference to the component `T` of the given [`Entity`]. - /// - /// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is returned instead. - /// - /// # Example - /// - /// Here, `get_component` is used to retrieve the `Character` component of the entity specified by the `SelectedCharacter` resource. - /// - /// ``` - /// # use bevy_ecs::prelude::*; - /// # - /// # #[derive(Resource)] - /// # struct SelectedCharacter { entity: Entity } - /// # #[derive(Component)] - /// # struct Character { name: String } - /// # - /// fn print_selected_character_name_system( - /// query: Query<&Character>, - /// selection: Res - /// ) - /// { - /// if let Ok(selected_character) = query.get_component::(selection.entity) { - /// println!("{}", selected_character.name); - /// } - /// } - /// # bevy_ecs::system::assert_is_system(print_selected_character_name_system); - /// ``` - /// - /// # See also - /// - /// - [`component`](Self::component) a panicking version of this function. - /// - [`get_component_mut`](Self::get_component_mut) to get a mutable reference of a component. - #[deprecated( - since = "0.13.0", - note = "Please use `get` and select for the exact component based on the structure of the exact query as required." - )] - #[allow(deprecated)] - #[inline] - pub fn get_component( - &self, - entity: Entity, - ) -> Result<&T, crate::query::QueryComponentError> { - self.state.get_component(self.world, entity) - } - - /// Returns a mutable reference to the component `T` of the given entity. - /// - /// In case of a nonexisting entity, mismatched component or missing write access, a [`QueryComponentError`] is returned instead. - /// - /// # Example - /// - /// Here, `get_component_mut` is used to retrieve the `Health` component of the entity specified by the `PoisonedCharacter` resource. - /// - /// ``` - /// # use bevy_ecs::prelude::*; - /// # - /// # #[derive(Resource)] - /// # struct PoisonedCharacter { character_id: Entity } - /// # #[derive(Component)] - /// # struct Health(u32); - /// # - /// fn poison_system(mut query: Query<&mut Health>, poisoned: Res) { - /// if let Ok(mut health) = query.get_component_mut::(poisoned.character_id) { - /// health.0 -= 1; - /// } - /// } - /// # bevy_ecs::system::assert_is_system(poison_system); - /// ``` - /// - /// # See also - /// - /// - [`component_mut`](Self::component_mut) a panicking version of this function. - /// - [`get_component`](Self::get_component) to get a shared reference of a component. - /// - /// [`QueryComponentError`]: crate::query::QueryComponentError - #[deprecated( - since = "0.13.0", - note = "Please use `get_mut` and select for the exact component based on the structure of the exact query as required." - )] - #[allow(deprecated)] - #[inline] - pub fn get_component_mut( - &mut self, - entity: Entity, - ) -> Result, crate::query::QueryComponentError> { - // SAFETY: unique access to query (preventing aliased access) - unsafe { self.get_component_unchecked_mut(entity) } - } - - /// Returns a shared reference to the component `T` of the given [`Entity`]. - /// - /// # Panics - /// - /// Panics in case of a nonexisting entity or mismatched component. - /// - /// # See also - /// - /// - [`get_component`](Self::get_component) a non-panicking version of this function. - /// - [`component_mut`](Self::component_mut) to get a mutable reference of a component. - #[deprecated( - since = "0.13.0", - note = "Please use `get` and select for the exact component based on the structure of the exact query as required." - )] - #[allow(deprecated)] - #[inline] - #[track_caller] - pub fn component(&self, entity: Entity) -> &T { - self.state.component(self.world, entity) - } - - /// Returns a mutable reference to the component `T` of the given entity. - /// - /// # Panics - /// - /// Panics in case of a nonexisting entity, mismatched component or missing write access. - /// - /// # See also - /// - /// - [`get_component_mut`](Self::get_component_mut) a non-panicking version of this function. - /// - [`component`](Self::component) to get a shared reference of a component. - #[deprecated( - since = "0.13.0", - note = "Please use `get_mut` and select for the exact component based on the structure of the exact query as required." - )] - #[allow(deprecated)] - #[inline] - #[track_caller] - pub fn component_mut(&mut self, entity: Entity) -> Mut<'_, T> { - match self.get_component_mut(entity) { - Ok(component) => component, - Err(error) => { - panic!( - "Cannot get component `{:?}` from {entity:?}: {error}", - TypeId::of::() - ) - } - } - } - - /// Returns a mutable reference to the component `T` of the given entity. - /// - /// In case of a nonexisting entity or mismatched component, a [`QueryComponentError`] is returned instead. - /// - /// # Safety - /// - /// This function makes it possible to violate Rust's aliasing guarantees. - /// You must make sure this call does not result in multiple mutable references to the same component. - /// - /// # See also - /// - /// - [`get_component_mut`](Self::get_component_mut) for the safe version. - /// - /// [`QueryComponentError`]: crate::query::QueryComponentError - #[deprecated( - since = "0.13.0", - note = "Please use `get_unchecked` and select for the exact component based on the structure of the exact query as required." - )] - #[allow(deprecated)] - #[inline] - pub unsafe fn get_component_unchecked_mut( - &self, - entity: Entity, - ) -> Result, crate::query::QueryComponentError> { - // This check is required to ensure soundness in the case of `to_readonly().get_component_mut()` - // See the comments on the `force_read_only_component_access` field for more info. - if self.force_read_only_component_access { - return Err(crate::query::QueryComponentError::MissingWriteAccess); - } - - // SAFETY: The above check ensures we are not a readonly query. - // It is the callers responsibility to ensure multiple mutable access is not provided. - unsafe { - self.state - .get_component_unchecked_mut(self.world, entity, self.last_run, self.this_run) - } - } - /// Returns a single read-only query item when there is exactly one entity matching the query. /// /// # Panics @@ -1566,7 +1307,6 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { state, last_run: self.last_run, this_run: self.this_run, - force_read_only_component_access: self.force_read_only_component_access, } } @@ -1684,7 +1424,6 @@ pub struct QueryLens<'w, Q: QueryData, F: QueryFilter = ()> { state: QueryState, last_run: Tick, this_run: Tick, - force_read_only_component_access: bool, } impl<'w, Q: QueryData, F: QueryFilter> QueryLens<'w, Q, F> { @@ -1695,7 +1434,6 @@ impl<'w, Q: QueryData, F: QueryFilter> QueryLens<'w, Q, F> { state: &self.state, last_run: self.last_run, this_run: self.this_run, - force_read_only_component_access: self.force_read_only_component_access, } } } diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 8168654e18749..68415fd6eb08b 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -228,7 +228,7 @@ unsafe impl SystemParam for Qu // SAFETY: We have registered all of the query's world accesses, // so the caller ensures that `world` has permission to access any // world data that the query needs. - Query::new(world, state, system_meta.last_run, change_tick, false) + Query::new(world, state, system_meta.last_run, change_tick) } } diff --git a/crates/bevy_ecs_compile_fail_tests/tests/ui/query_lifetime_safety.rs b/crates/bevy_ecs_compile_fail_tests/tests/ui/query_lifetime_safety.rs index 11e04d46aee55..bec91d9796d2e 100644 --- a/crates/bevy_ecs_compile_fail_tests/tests/ui/query_lifetime_safety.rs +++ b/crates/bevy_ecs_compile_fail_tests/tests/ui/query_lifetime_safety.rs @@ -24,20 +24,6 @@ fn main() { assert_eq!(data, &mut *data2); // oops UB } - #[allow(deprecated)] - { - let data: &Foo = query.get_component::(e).unwrap(); - let mut data2: Mut = query.get_component_mut(e).unwrap(); - assert_eq!(data, &mut *data2); // oops UB - } - - #[allow(deprecated)] - { - let mut data2: Mut = query.get_component_mut(e).unwrap(); - let data: &Foo = query.get_component::(e).unwrap(); - assert_eq!(data, &mut *data2); // oops UB - } - { let data: &Foo = query.single(); let mut data2: Mut = query.single_mut(); diff --git a/crates/bevy_ecs_compile_fail_tests/tests/ui/query_lifetime_safety.stderr b/crates/bevy_ecs_compile_fail_tests/tests/ui/query_lifetime_safety.stderr index 5d425c1e9b6b0..4564db26e044e 100644 --- a/crates/bevy_ecs_compile_fail_tests/tests/ui/query_lifetime_safety.stderr +++ b/crates/bevy_ecs_compile_fail_tests/tests/ui/query_lifetime_safety.stderr @@ -19,101 +19,81 @@ error[E0502]: cannot borrow `query` as immutable because it is also borrowed as | ----- mutable borrow later used here error[E0502]: cannot borrow `query` as mutable because it is also borrowed as immutable - --> tests/ui/query_lifetime_safety.rs:30:39 + --> tests/ui/query_lifetime_safety.rs:29:39 | -29 | let data: &Foo = query.get_component::(e).unwrap(); +28 | let data: &Foo = query.single(); | ----- immutable borrow occurs here -30 | let mut data2: Mut = query.get_component_mut(e).unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here -31 | assert_eq!(data, &mut *data2); // oops UB - | ----------------------------- immutable borrow later used here - -error[E0502]: cannot borrow `query` as immutable because it is also borrowed as mutable - --> tests/ui/query_lifetime_safety.rs:37:30 - | -36 | let mut data2: Mut = query.get_component_mut(e).unwrap(); - | ----- mutable borrow occurs here -37 | let data: &Foo = query.get_component::(e).unwrap(); - | ^^^^^ immutable borrow occurs here -38 | assert_eq!(data, &mut *data2); // oops UB - | ----- mutable borrow later used here - -error[E0502]: cannot borrow `query` as mutable because it is also borrowed as immutable - --> tests/ui/query_lifetime_safety.rs:43:39 - | -42 | let data: &Foo = query.single(); - | ----- immutable borrow occurs here -43 | let mut data2: Mut = query.single_mut(); +29 | let mut data2: Mut = query.single_mut(); | ^^^^^^^^^^^^^^^^^^ mutable borrow occurs here -44 | assert_eq!(data, &mut *data2); // oops UB +30 | assert_eq!(data, &mut *data2); // oops UB | ----------------------------- immutable borrow later used here error[E0502]: cannot borrow `query` as immutable because it is also borrowed as mutable - --> tests/ui/query_lifetime_safety.rs:49:30 + --> tests/ui/query_lifetime_safety.rs:35:30 | -48 | let mut data2: Mut = query.single_mut(); +34 | let mut data2: Mut = query.single_mut(); | ----- mutable borrow occurs here -49 | let data: &Foo = query.single(); +35 | let data: &Foo = query.single(); | ^^^^^ immutable borrow occurs here -50 | assert_eq!(data, &mut *data2); // oops UB +36 | assert_eq!(data, &mut *data2); // oops UB | ----- mutable borrow later used here error[E0502]: cannot borrow `query` as mutable because it is also borrowed as immutable - --> tests/ui/query_lifetime_safety.rs:55:39 + --> tests/ui/query_lifetime_safety.rs:41:39 | -54 | let data: &Foo = query.get_single().unwrap(); +40 | let data: &Foo = query.get_single().unwrap(); | ----- immutable borrow occurs here -55 | let mut data2: Mut = query.get_single_mut().unwrap(); +41 | let mut data2: Mut = query.get_single_mut().unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here -56 | assert_eq!(data, &mut *data2); // oops UB +42 | assert_eq!(data, &mut *data2); // oops UB | ----------------------------- immutable borrow later used here error[E0502]: cannot borrow `query` as immutable because it is also borrowed as mutable - --> tests/ui/query_lifetime_safety.rs:61:30 + --> tests/ui/query_lifetime_safety.rs:47:30 | -60 | let mut data2: Mut = query.get_single_mut().unwrap(); +46 | let mut data2: Mut = query.get_single_mut().unwrap(); | ----- mutable borrow occurs here -61 | let data: &Foo = query.get_single().unwrap(); +47 | let data: &Foo = query.get_single().unwrap(); | ^^^^^ immutable borrow occurs here -62 | assert_eq!(data, &mut *data2); // oops UB +48 | assert_eq!(data, &mut *data2); // oops UB | ----- mutable borrow later used here error[E0502]: cannot borrow `query` as mutable because it is also borrowed as immutable - --> tests/ui/query_lifetime_safety.rs:67:39 + --> tests/ui/query_lifetime_safety.rs:53:39 | -66 | let data: &Foo = query.iter().next().unwrap(); +52 | let data: &Foo = query.iter().next().unwrap(); | ----- immutable borrow occurs here -67 | let mut data2: Mut = query.iter_mut().next().unwrap(); +53 | let mut data2: Mut = query.iter_mut().next().unwrap(); | ^^^^^^^^^^^^^^^^ mutable borrow occurs here -68 | assert_eq!(data, &mut *data2); // oops UB +54 | assert_eq!(data, &mut *data2); // oops UB | ----------------------------- immutable borrow later used here error[E0502]: cannot borrow `query` as immutable because it is also borrowed as mutable - --> tests/ui/query_lifetime_safety.rs:73:30 + --> tests/ui/query_lifetime_safety.rs:59:30 | -72 | let mut data2: Mut = query.iter_mut().next().unwrap(); +58 | let mut data2: Mut = query.iter_mut().next().unwrap(); | ----- mutable borrow occurs here -73 | let data: &Foo = query.iter().next().unwrap(); +59 | let data: &Foo = query.iter().next().unwrap(); | ^^^^^ immutable borrow occurs here -74 | assert_eq!(data, &mut *data2); // oops UB +60 | assert_eq!(data, &mut *data2); // oops UB | ----- mutable borrow later used here error[E0502]: cannot borrow `query` as mutable because it is also borrowed as immutable - --> tests/ui/query_lifetime_safety.rs:81:13 + --> tests/ui/query_lifetime_safety.rs:67:13 | -80 | query.iter().for_each(|data| opt_data = Some(data)); +66 | query.iter().for_each(|data| opt_data = Some(data)); | ----- immutable borrow occurs here -81 | query.iter_mut().for_each(|data| opt_data_2 = Some(data)); +67 | query.iter_mut().for_each(|data| opt_data_2 = Some(data)); | ^^^^^^^^^^^^^^^^ mutable borrow occurs here -82 | assert_eq!(opt_data.unwrap(), &mut *opt_data_2.unwrap()); // oops UB +68 | assert_eq!(opt_data.unwrap(), &mut *opt_data_2.unwrap()); // oops UB | -------- immutable borrow later used here error[E0502]: cannot borrow `query` as immutable because it is also borrowed as mutable - --> tests/ui/query_lifetime_safety.rs:89:13 + --> tests/ui/query_lifetime_safety.rs:75:13 | -88 | query.iter_mut().for_each(|data| opt_data_2 = Some(data)); +74 | query.iter_mut().for_each(|data| opt_data_2 = Some(data)); | ----- mutable borrow occurs here -89 | query.iter().for_each(|data| opt_data = Some(data)); +75 | query.iter().for_each(|data| opt_data = Some(data)); | ^^^^^ immutable borrow occurs here -90 | assert_eq!(opt_data.unwrap(), &mut *opt_data_2.unwrap()); // oops UB +76 | assert_eq!(opt_data.unwrap(), &mut *opt_data_2.unwrap()); // oops UB | ---------- mutable borrow later used here diff --git a/crates/bevy_render/src/lib.rs b/crates/bevy_render/src/lib.rs index 045119267e94d..c95a4168d4e3c 100644 --- a/crates/bevy_render/src/lib.rs +++ b/crates/bevy_render/src/lib.rs @@ -37,7 +37,7 @@ pub mod prelude { Projection, }, color::Color, - mesh::{morph::MorphWeights, primitives::Meshable, shape, Mesh}, + mesh::{morph::MorphWeights, primitives::Meshable, Mesh}, render_resource::Shader, spatial_bundle::SpatialBundle, texture::{Image, ImagePlugin}, diff --git a/crates/bevy_render/src/mesh/mesh/mod.rs b/crates/bevy_render/src/mesh/mesh/mod.rs index 07e3dd0af00d6..aee822155fadc 100644 --- a/crates/bevy_render/src/mesh/mesh/mod.rs +++ b/crates/bevy_render/src/mesh/mesh/mod.rs @@ -32,7 +32,7 @@ pub const VERTEX_ATTRIBUTE_BUFFER_ID: u64 = 10; /// with "attribute" values for each vertex. /// /// Meshes can be automatically generated by a bevy `AssetLoader` (generally by loading a `Gltf` file), -/// or by converting a primitive [`shape`](crate::mesh::shape) using [`into`](Into). +/// or by converting a [primitive](bevy_math::primitives) using [`into`](Into). /// It is also possible to create one manually. /// They can be edited after creation. /// diff --git a/crates/bevy_render/src/mesh/mod.rs b/crates/bevy_render/src/mesh/mod.rs index 0a070d0388b24..172565ef1db5b 100644 --- a/crates/bevy_render/src/mesh/mod.rs +++ b/crates/bevy_render/src/mesh/mod.rs @@ -2,8 +2,6 @@ mod mesh; pub mod morph; pub mod primitives; -/// Generation for some primitive shape meshes. -pub mod shape; pub use mesh::*; pub use primitives::*; diff --git a/crates/bevy_render/src/mesh/shape/capsule.rs b/crates/bevy_render/src/mesh/shape/capsule.rs deleted file mode 100644 index 25fbd7c160e32..0000000000000 --- a/crates/bevy_render/src/mesh/shape/capsule.rs +++ /dev/null @@ -1,49 +0,0 @@ -use crate::mesh::{Capsule3dMeshBuilder, CapsuleUvProfile, Mesh}; - -/// A cylinder with hemispheres at the top and bottom -#[deprecated( - since = "0.13.0", - note = "please use the `Capsule3d` primitive in `bevy_math` instead" -)] -#[derive(Debug, Copy, Clone)] -pub struct Capsule { - /// Radius on the `XZ` plane. - pub radius: f32, - /// Number of sections in cylinder between hemispheres. - pub rings: usize, - /// Height of the middle cylinder on the `Y` axis, excluding the hemispheres. - pub depth: f32, - /// Number of latitudes, distributed by inclination. Must be even. - pub latitudes: usize, - /// Number of longitudes, or meridians, distributed by azimuth. - pub longitudes: usize, - /// Manner in which UV coordinates are distributed vertically. - pub uv_profile: CapsuleUvProfile, -} -impl Default for Capsule { - fn default() -> Self { - Capsule { - radius: 0.5, - rings: 0, - depth: 1.0, - latitudes: 16, - longitudes: 32, - uv_profile: CapsuleUvProfile::Aspect, - } - } -} - -impl From for Mesh { - #[allow(clippy::needless_range_loop)] - fn from(capsule: Capsule) -> Self { - Capsule3dMeshBuilder::new( - capsule.radius, - capsule.depth, - capsule.longitudes, - capsule.latitudes, - ) - .rings(capsule.rings) - .uv_profile(capsule.uv_profile) - .build() - } -} diff --git a/crates/bevy_render/src/mesh/shape/cylinder.rs b/crates/bevy_render/src/mesh/shape/cylinder.rs deleted file mode 100644 index 5c898036e2566..0000000000000 --- a/crates/bevy_render/src/mesh/shape/cylinder.rs +++ /dev/null @@ -1,41 +0,0 @@ -use crate::mesh::{CylinderMeshBuilder, Mesh}; - -/// A cylinder which stands on the XZ plane -#[deprecated( - since = "0.13.0", - note = "please use the `Cylinder` primitive in `bevy_math` instead" -)] -#[derive(Clone, Copy, Debug)] -pub struct Cylinder { - /// Radius in the XZ plane. - pub radius: f32, - /// Height of the cylinder in the Y axis. - pub height: f32, - /// The number of vertices around each horizontal slice of the cylinder. If you are looking at the cylinder from - /// above, this is the number of points you will see on the circle. - /// A higher number will make it appear more circular. - pub resolution: u32, - /// The number of segments between the two ends. Setting this to 1 will have triangles spanning the full - /// height of the cylinder. Setting it to 2 will have two sets of triangles with a horizontal slice in the middle of - /// cylinder. Greater numbers increase triangles/slices in the same way. - pub segments: u32, -} - -impl Default for Cylinder { - fn default() -> Self { - Self { - radius: 0.5, - height: 1.0, - resolution: 16, - segments: 1, - } - } -} - -impl From for Mesh { - fn from(c: Cylinder) -> Self { - CylinderMeshBuilder::new(c.radius, c.height, c.resolution) - .segments(c.segments) - .build() - } -} diff --git a/crates/bevy_render/src/mesh/shape/icosphere.rs b/crates/bevy_render/src/mesh/shape/icosphere.rs deleted file mode 100644 index 68fa4ca16a045..0000000000000 --- a/crates/bevy_render/src/mesh/shape/icosphere.rs +++ /dev/null @@ -1,32 +0,0 @@ -use crate::mesh::{IcosphereError, Mesh, Meshable}; -use bevy_math::primitives::Sphere; - -/// A sphere made from a subdivided Icosahedron. -#[deprecated( - since = "0.13.0", - note = "please use the `Sphere` primitive in `bevy_math` instead" -)] -#[derive(Debug, Clone, Copy)] -pub struct Icosphere { - /// The radius of the sphere. - pub radius: f32, - /// The number of subdivisions applied. - pub subdivisions: usize, -} - -impl Default for Icosphere { - fn default() -> Self { - Self { - radius: 1.0, - subdivisions: 5, - } - } -} - -impl TryFrom for Mesh { - type Error = IcosphereError; - - fn try_from(sphere: Icosphere) -> Result { - Sphere::new(sphere.radius).mesh().ico(sphere.subdivisions) - } -} diff --git a/crates/bevy_render/src/mesh/shape/mod.rs b/crates/bevy_render/src/mesh/shape/mod.rs deleted file mode 100644 index b49f51a359296..0000000000000 --- a/crates/bevy_render/src/mesh/shape/mod.rs +++ /dev/null @@ -1,306 +0,0 @@ -#![allow(deprecated)] - -use crate::render_asset::RenderAssetUsages; - -use super::{Indices, Mesh}; -use bevy_math::*; - -#[deprecated( - since = "0.13.0", - note = "please use the `Cuboid` primitive for meshing or `Aabb2d` for a bounding volume" -)] -#[derive(Debug, Copy, Clone)] -pub struct Cube { - pub size: f32, -} - -impl Cube { - pub fn new(size: f32) -> Cube { - Cube { size } - } -} - -impl Default for Cube { - fn default() -> Self { - Cube { size: 1.0 } - } -} - -impl From for Mesh { - fn from(cube: Cube) -> Self { - Box::new(cube.size, cube.size, cube.size).into() - } -} - -/// An axis-aligned box defined by its minimum and maximum point. -#[deprecated( - since = "0.13.0", - note = "please use the `Cuboid` primitive for meshing or `Aabb3d` for a bounding volume" -)] -#[derive(Debug, Copy, Clone)] -pub struct Box { - pub min_x: f32, - pub max_x: f32, - - pub min_y: f32, - pub max_y: f32, - - pub min_z: f32, - pub max_z: f32, -} - -impl Box { - /// Creates a new box centered at the origin with the supplied side lengths. - pub fn new(x_length: f32, y_length: f32, z_length: f32) -> Box { - Box { - max_x: x_length / 2.0, - min_x: -x_length / 2.0, - max_y: y_length / 2.0, - min_y: -y_length / 2.0, - max_z: z_length / 2.0, - min_z: -z_length / 2.0, - } - } - - /// Creates a new box given the coordinates of two opposing corners. - pub fn from_corners(a: Vec3, b: Vec3) -> Box { - let max = a.max(b); - let min = a.min(b); - Box { - max_x: max.x, - min_x: min.x, - max_y: max.y, - min_y: min.y, - max_z: max.z, - min_z: min.z, - } - } -} - -impl Default for Box { - fn default() -> Self { - Box::new(2.0, 1.0, 1.0) - } -} - -impl From for Mesh { - fn from(sp: Box) -> Self { - // suppose Y-up right hand, and camera look from +z to -z - let vertices = &[ - // Front - ([sp.min_x, sp.min_y, sp.max_z], [0., 0., 1.0], [0., 0.]), - ([sp.max_x, sp.min_y, sp.max_z], [0., 0., 1.0], [1.0, 0.]), - ([sp.max_x, sp.max_y, sp.max_z], [0., 0., 1.0], [1.0, 1.0]), - ([sp.min_x, sp.max_y, sp.max_z], [0., 0., 1.0], [0., 1.0]), - // Back - ([sp.min_x, sp.max_y, sp.min_z], [0., 0., -1.0], [1.0, 0.]), - ([sp.max_x, sp.max_y, sp.min_z], [0., 0., -1.0], [0., 0.]), - ([sp.max_x, sp.min_y, sp.min_z], [0., 0., -1.0], [0., 1.0]), - ([sp.min_x, sp.min_y, sp.min_z], [0., 0., -1.0], [1.0, 1.0]), - // Right - ([sp.max_x, sp.min_y, sp.min_z], [1.0, 0., 0.], [0., 0.]), - ([sp.max_x, sp.max_y, sp.min_z], [1.0, 0., 0.], [1.0, 0.]), - ([sp.max_x, sp.max_y, sp.max_z], [1.0, 0., 0.], [1.0, 1.0]), - ([sp.max_x, sp.min_y, sp.max_z], [1.0, 0., 0.], [0., 1.0]), - // Left - ([sp.min_x, sp.min_y, sp.max_z], [-1.0, 0., 0.], [1.0, 0.]), - ([sp.min_x, sp.max_y, sp.max_z], [-1.0, 0., 0.], [0., 0.]), - ([sp.min_x, sp.max_y, sp.min_z], [-1.0, 0., 0.], [0., 1.0]), - ([sp.min_x, sp.min_y, sp.min_z], [-1.0, 0., 0.], [1.0, 1.0]), - // Top - ([sp.max_x, sp.max_y, sp.min_z], [0., 1.0, 0.], [1.0, 0.]), - ([sp.min_x, sp.max_y, sp.min_z], [0., 1.0, 0.], [0., 0.]), - ([sp.min_x, sp.max_y, sp.max_z], [0., 1.0, 0.], [0., 1.0]), - ([sp.max_x, sp.max_y, sp.max_z], [0., 1.0, 0.], [1.0, 1.0]), - // Bottom - ([sp.max_x, sp.min_y, sp.max_z], [0., -1.0, 0.], [0., 0.]), - ([sp.min_x, sp.min_y, sp.max_z], [0., -1.0, 0.], [1.0, 0.]), - ([sp.min_x, sp.min_y, sp.min_z], [0., -1.0, 0.], [1.0, 1.0]), - ([sp.max_x, sp.min_y, sp.min_z], [0., -1.0, 0.], [0., 1.0]), - ]; - - let positions: Vec<_> = vertices.iter().map(|(p, _, _)| *p).collect(); - let normals: Vec<_> = vertices.iter().map(|(_, n, _)| *n).collect(); - let uvs: Vec<_> = vertices.iter().map(|(_, _, uv)| *uv).collect(); - - let indices = Indices::U32(vec![ - 0, 1, 2, 2, 3, 0, // front - 4, 5, 6, 6, 7, 4, // back - 8, 9, 10, 10, 11, 8, // right - 12, 13, 14, 14, 15, 12, // left - 16, 17, 18, 18, 19, 16, // top - 20, 21, 22, 22, 23, 20, // bottom - ]); - - Mesh::new( - PrimitiveTopology::TriangleList, - RenderAssetUsages::default(), - ) - .with_inserted_attribute(Mesh::ATTRIBUTE_POSITION, positions) - .with_inserted_attribute(Mesh::ATTRIBUTE_NORMAL, normals) - .with_inserted_attribute(Mesh::ATTRIBUTE_UV_0, uvs) - .with_inserted_indices(indices) - } -} - -/// A rectangle on the `XY` plane centered at the origin. -#[deprecated( - since = "0.13.0", - note = "please use the `Rectangle` primitive in `bevy_math` instead" -)] -#[derive(Debug, Copy, Clone)] -pub struct Quad { - /// Full width and height of the rectangle. - pub size: Vec2, - /// Horizontally-flip the texture coordinates of the resulting mesh. - pub flip: bool, -} - -impl Default for Quad { - fn default() -> Self { - Quad::new(Vec2::ONE) - } -} - -impl Quad { - pub fn new(size: Vec2) -> Self { - Self { size, flip: false } - } - - pub fn flipped(size: Vec2) -> Self { - Self { size, flip: true } - } -} - -impl From for Mesh { - fn from(quad: Quad) -> Self { - let extent_x = quad.size.x / 2.0; - let extent_y = quad.size.y / 2.0; - - let (u_left, u_right) = if quad.flip { (1.0, 0.0) } else { (0.0, 1.0) }; - let vertices = [ - ([-extent_x, -extent_y, 0.0], [0.0, 0.0, 1.0], [u_left, 1.0]), - ([-extent_x, extent_y, 0.0], [0.0, 0.0, 1.0], [u_left, 0.0]), - ([extent_x, extent_y, 0.0], [0.0, 0.0, 1.0], [u_right, 0.0]), - ([extent_x, -extent_y, 0.0], [0.0, 0.0, 1.0], [u_right, 1.0]), - ]; - - let indices = Indices::U32(vec![0, 2, 1, 0, 3, 2]); - - let positions: Vec<_> = vertices.iter().map(|(p, _, _)| *p).collect(); - let normals: Vec<_> = vertices.iter().map(|(_, n, _)| *n).collect(); - let uvs: Vec<_> = vertices.iter().map(|(_, _, uv)| *uv).collect(); - - Mesh::new( - PrimitiveTopology::TriangleList, - RenderAssetUsages::default(), - ) - .with_inserted_indices(indices) - .with_inserted_attribute(Mesh::ATTRIBUTE_POSITION, positions) - .with_inserted_attribute(Mesh::ATTRIBUTE_NORMAL, normals) - .with_inserted_attribute(Mesh::ATTRIBUTE_UV_0, uvs) - } -} - -/// A square on the `XZ` plane centered at the origin. -#[deprecated( - since = "0.13.0", - note = "please use the `Plane3d` primitive in `bevy_math` instead" -)] -#[derive(Debug, Copy, Clone)] -pub struct Plane { - /// The total side length of the square. - pub size: f32, - /// The number of subdivisions in the mesh. - /// - /// 0 - is the original plane geometry, the 4 points in the XZ plane. - /// - /// 1 - is split by 1 line in the middle of the plane on both the X axis and the Z axis, resulting in a plane with 4 quads / 8 triangles. - /// - /// 2 - is a plane split by 2 lines on both the X and Z axes, subdividing the plane into 3 equal sections along each axis, resulting in a plane with 9 quads / 18 triangles. - /// - /// and so on... - pub subdivisions: u32, -} - -impl Default for Plane { - fn default() -> Self { - Plane { - size: 1.0, - subdivisions: 0, - } - } -} - -impl Plane { - /// Creates a new plane centered at the origin with the supplied side length and zero subdivisions. - pub fn from_size(size: f32) -> Self { - Self { - size, - subdivisions: 0, - } - } -} - -impl From for Mesh { - fn from(plane: Plane) -> Self { - // here this is split in the z and x directions if one ever needs asymmetrical subdivision - // two Plane struct fields would need to be added instead of the single subdivisions field - let z_vertex_count = plane.subdivisions + 2; - let x_vertex_count = plane.subdivisions + 2; - let num_vertices = (z_vertex_count * x_vertex_count) as usize; - let num_indices = ((z_vertex_count - 1) * (x_vertex_count - 1) * 6) as usize; - let up = Vec3::Y.to_array(); - - let mut positions: Vec<[f32; 3]> = Vec::with_capacity(num_vertices); - let mut normals: Vec<[f32; 3]> = Vec::with_capacity(num_vertices); - let mut uvs: Vec<[f32; 2]> = Vec::with_capacity(num_vertices); - let mut indices: Vec = Vec::with_capacity(num_indices); - - for z in 0..z_vertex_count { - for x in 0..x_vertex_count { - let tx = x as f32 / (x_vertex_count - 1) as f32; - let tz = z as f32 / (z_vertex_count - 1) as f32; - positions.push([(-0.5 + tx) * plane.size, 0.0, (-0.5 + tz) * plane.size]); - normals.push(up); - uvs.push([tx, tz]); - } - } - - for y in 0..z_vertex_count - 1 { - for x in 0..x_vertex_count - 1 { - let quad = y * x_vertex_count + x; - indices.push(quad + x_vertex_count + 1); - indices.push(quad + 1); - indices.push(quad + x_vertex_count); - indices.push(quad); - indices.push(quad + x_vertex_count); - indices.push(quad + 1); - } - } - - Mesh::new( - PrimitiveTopology::TriangleList, - RenderAssetUsages::default(), - ) - .with_inserted_indices(Indices::U32(indices)) - .with_inserted_attribute(Mesh::ATTRIBUTE_POSITION, positions) - .with_inserted_attribute(Mesh::ATTRIBUTE_NORMAL, normals) - .with_inserted_attribute(Mesh::ATTRIBUTE_UV_0, uvs) - } -} - -mod capsule; -mod cylinder; -mod icosphere; -mod regular_polygon; -mod torus; -mod uvsphere; - -pub use capsule::Capsule; -pub use cylinder::Cylinder; -pub use icosphere::Icosphere; -pub use regular_polygon::{Circle, RegularPolygon}; -pub use torus::Torus; -pub use uvsphere::UVSphere; -use wgpu::PrimitiveTopology; diff --git a/crates/bevy_render/src/mesh/shape/regular_polygon.rs b/crates/bevy_render/src/mesh/shape/regular_polygon.rs deleted file mode 100644 index 1d50060d48195..0000000000000 --- a/crates/bevy_render/src/mesh/shape/regular_polygon.rs +++ /dev/null @@ -1,85 +0,0 @@ -use crate::mesh::{Mesh, Meshable}; - -/// A regular polygon in the `XY` plane -#[deprecated( - since = "0.13.0", - note = "please use the `RegularPolygon` primitive in `bevy_math` instead" -)] -#[derive(Debug, Copy, Clone)] -pub struct RegularPolygon { - /// Circumscribed radius in the `XY` plane. - /// - /// In other words, the vertices of this polygon will all touch a circle of this radius. - pub radius: f32, - /// Number of sides. - pub sides: usize, -} - -impl Default for RegularPolygon { - fn default() -> Self { - Self { - radius: 0.5, - sides: 6, - } - } -} - -impl RegularPolygon { - /// Creates a regular polygon in the `XY` plane - pub fn new(radius: f32, sides: usize) -> Self { - Self { radius, sides } - } -} - -impl From for Mesh { - fn from(polygon: RegularPolygon) -> Self { - bevy_math::primitives::RegularPolygon::new(polygon.radius, polygon.sides).mesh() - } -} - -/// A circle in the `XY` plane -#[deprecated( - since = "0.13.0", - note = "please use the `Circle` primitive in `bevy_math` instead" -)] -#[derive(Debug, Copy, Clone)] -pub struct Circle { - /// Inscribed radius in the `XY` plane. - pub radius: f32, - /// The number of vertices used. - pub vertices: usize, -} - -impl Default for Circle { - fn default() -> Self { - Self { - radius: 0.5, - vertices: 64, - } - } -} - -impl Circle { - /// Creates a circle in the `XY` plane - pub fn new(radius: f32) -> Self { - Self { - radius, - ..Default::default() - } - } -} - -impl From for RegularPolygon { - fn from(circle: Circle) -> Self { - Self { - radius: circle.radius, - sides: circle.vertices, - } - } -} - -impl From for Mesh { - fn from(circle: Circle) -> Self { - Mesh::from(RegularPolygon::from(circle)) - } -} diff --git a/crates/bevy_render/src/mesh/shape/torus.rs b/crates/bevy_render/src/mesh/shape/torus.rs deleted file mode 100644 index 6c4a146030fe6..0000000000000 --- a/crates/bevy_render/src/mesh/shape/torus.rs +++ /dev/null @@ -1,38 +0,0 @@ -use crate::mesh::{Mesh, Meshable}; - -/// A torus (donut) shape. -#[deprecated( - since = "0.13.0", - note = "please use the `Torus` primitive in `bevy_math` instead" -)] -#[derive(Debug, Clone, Copy)] -pub struct Torus { - pub radius: f32, - pub ring_radius: f32, - pub subdivisions_segments: usize, - pub subdivisions_sides: usize, -} - -impl Default for Torus { - fn default() -> Self { - Torus { - radius: 1.0, - ring_radius: 0.5, - subdivisions_segments: 32, - subdivisions_sides: 24, - } - } -} - -impl From for Mesh { - fn from(torus: Torus) -> Self { - bevy_math::primitives::Torus { - minor_radius: torus.ring_radius, - major_radius: torus.radius, - } - .mesh() - .minor_resolution(torus.subdivisions_sides) - .major_resolution(torus.subdivisions_segments) - .build() - } -} diff --git a/crates/bevy_render/src/mesh/shape/uvsphere.rs b/crates/bevy_render/src/mesh/shape/uvsphere.rs deleted file mode 100644 index abf4de21eb5cf..0000000000000 --- a/crates/bevy_render/src/mesh/shape/uvsphere.rs +++ /dev/null @@ -1,37 +0,0 @@ -use bevy_math::primitives::Sphere; - -use crate::mesh::{Mesh, Meshable}; - -/// A sphere made of sectors and stacks. -#[deprecated( - since = "0.13.0", - note = "please use the `Sphere` primitive in `bevy_math` instead" -)] -#[allow(clippy::upper_case_acronyms)] -#[derive(Debug, Clone, Copy)] -pub struct UVSphere { - /// The radius of the sphere. - pub radius: f32, - /// Longitudinal sectors - pub sectors: usize, - /// Latitudinal stacks - pub stacks: usize, -} - -impl Default for UVSphere { - fn default() -> Self { - Self { - radius: 1.0, - sectors: 36, - stacks: 18, - } - } -} - -impl From for Mesh { - fn from(sphere: UVSphere) -> Self { - Sphere::new(sphere.radius) - .mesh() - .uv(sphere.sectors, sphere.stacks) - } -}