Deprecated Various Component Methods from Query and QueryState (b…

…evyengine#9920) # Objective - (Partially) Fixes bevyengine#9904 - Acts on bevyengine#9910 ## Solution - Deprecated the relevant methods from `Query`, cascading changes as required across Bevy. --- ## Changelog - Deprecated `QueryState::get_component_unchecked_mut` method - Deprecated `Query::get_component` method - Deprecated `Query::get_component_mut` method - Deprecated `Query::component` method - Deprecated `Query::component_mut` method - Deprecated `Query::get_component_unchecked_mut` method ## Migration Guide ### `QueryState::get_component_unchecked_mut` Use `QueryState::get_unchecked_manual` and select for the exact component based on the structure of the exact query as required. ### `Query::(get_)component(_unchecked)(_mut)` Use `Query::get` and select for the exact component based on the structure of the exact query as required. - For mutable access (`_mut`), use `Query::get_mut` - For unchecked access (`_unchecked`), use `Query::get_unchecked` - For panic variants (non-`get_`), add `.unwrap()` ## Notes - `QueryComponentError` can be removed once these deprecated methods are also removed. Due to an interaction with `thiserror`'s derive macro, it is not marked as deprecated.
tjamaan · Feb 6, 2024 · 94ab2ab · 94ab2ab
1 parent f2a8400
commit 94ab2ab
Show file tree

Hide file tree

Showing 8 changed files with 121 additions and 63 deletions.
diff --git a/crates/bevy_ecs/src/query/mod.rs b/crates/bevy_ecs/src/query/mod.rs
@@ -739,6 +739,7 @@ mod tests {
         }
     }
 
+    #[allow(deprecated)]
     #[test]
     fn mut_to_immut_query_methods_have_immut_item() {
         #[derive(Component)]

diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs
@@ -17,8 +17,8 @@ use fixedbitset::FixedBitSet;
 use std::{any::TypeId, borrow::Borrow, fmt, mem::MaybeUninit};
 
 use super::{
-    NopWorldQuery, QueryBuilder, QueryComponentError, QueryData, QueryEntityError, QueryFilter,
-    QueryManyIter, QuerySingleError, ROQueryItem,
+    NopWorldQuery, QueryBuilder, QueryData, QueryEntityError, QueryFilter, QueryManyIter,
+    QuerySingleError, ROQueryItem,
 };
 
 /// Provides scoped access to a [`World`] state according to a given [`QueryData`] and [`QueryFilter`].
@@ -629,32 +629,37 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
     /// 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, QueryComponentError> {
+    ) -> Result<&'w T, super::QueryComponentError> {
         let entity_ref = world
             .get_entity(entity)
-            .ok_or(QueryComponentError::NoSuchEntity)?;
+            .ok_or(super::QueryComponentError::NoSuchEntity)?;
         let component_id = world
             .components()
             .get_id(TypeId::of::<T>())
-            .ok_or(QueryComponentError::MissingComponent)?;
+            .ok_or(super::QueryComponentError::MissingComponent)?;
         let archetype_component = entity_ref
             .archetype()
             .get_archetype_component_id(component_id)
-            .ok_or(QueryComponentError::MissingComponent)?;
+            .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::<T>() }.ok_or(QueryComponentError::MissingComponent)
+            unsafe { entity_ref.get::<T>() }.ok_or(super::QueryComponentError::MissingComponent)
         } else {
-            Err(QueryComponentError::MissingReadAccess)
+            Err(super::QueryComponentError::MissingReadAccess)
         }
     }
 
@@ -663,6 +668,11 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
     /// # 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,
@@ -688,25 +698,32 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
     ///
     /// 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<Mut<'w, T>, QueryComponentError> {
+    ) -> Result<Mut<'w, T>, super::QueryComponentError> {
         let entity_ref = world
             .get_entity(entity)
-            .ok_or(QueryComponentError::NoSuchEntity)?;
+            .ok_or(super::QueryComponentError::NoSuchEntity)?;
         let component_id = world
             .components()
             .get_id(TypeId::of::<T>())
-            .ok_or(QueryComponentError::MissingComponent)?;
+            .ok_or(super::QueryComponentError::MissingComponent)?;
         let archetype_component = entity_ref
             .archetype()
             .get_archetype_component_id(component_id)
-            .ok_or(QueryComponentError::MissingComponent)?;
+            .ok_or(super::QueryComponentError::MissingComponent)?;
         if self
             .archetype_component_access
             .has_write(archetype_component)
@@ -715,9 +732,9 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
             // mutable reference to this entity's component `T`.
             let result = unsafe { entity_ref.get_mut_using_ticks::<T>(last_run, this_run) };
 
-            result.ok_or(QueryComponentError::MissingComponent)
+            result.ok_or(super::QueryComponentError::MissingComponent)
         } else {
-            Err(QueryComponentError::MissingWriteAccess)
+            Err(super::QueryComponentError::MissingWriteAccess)
         }
     }
 

diff --git a/crates/bevy_ecs/src/system/mod.rs b/crates/bevy_ecs/src/system/mod.rs
@@ -395,6 +395,7 @@ mod tests {
         schedule.run(world);
     }
 
+    #[allow(deprecated)]
     #[test]
     fn query_system_gets() {
         fn query_system(
@@ -1554,6 +1555,7 @@ mod tests {
         });
     }
 
+    #[allow(deprecated)]
     #[test]
     fn readonly_query_get_mut_component_fails() {
         use crate::query::QueryComponentError;

diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs
@@ -2,9 +2,9 @@ use crate::{
     component::{Component, Tick},
     entity::Entity,
     query::{
-        BatchingStrategy, QueryCombinationIter, QueryComponentError, QueryData, QueryEntityError,
-        QueryFilter, QueryIter, QueryManyIter, QueryParIter, QuerySingleError, QueryState,
-        ROQueryItem, ReadOnlyQueryData,
+        BatchingStrategy, QueryCombinationIter, QueryData, QueryEntityError, QueryFilter,
+        QueryIter, QueryManyIter, QueryParIter, QuerySingleError, QueryState, ROQueryItem,
+        ReadOnlyQueryData,
     },
     world::{unsafe_world_cell::UnsafeWorldCell, Mut},
 };
@@ -1132,8 +1132,16 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> {
     ///
     /// - [`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<T: Component>(&self, entity: Entity) -> Result<&T, QueryComponentError> {
+    pub fn get_component<T: Component>(
+        &self,
+        entity: Entity,
+    ) -> Result<&T, crate::query::QueryComponentError> {
         self.state.get_component(self.world, entity)
     }
 
@@ -1165,11 +1173,18 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> {
     ///
     /// - [`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<T: Component>(
         &mut self,
         entity: Entity,
-    ) -> Result<Mut<'_, T>, QueryComponentError> {
+    ) -> Result<Mut<'_, T>, crate::query::QueryComponentError> {
         // SAFETY: unique access to query (preventing aliased access)
         unsafe { self.get_component_unchecked_mut(entity) }
     }
@@ -1184,6 +1199,11 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> {
     ///
     /// - [`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<T: Component>(&self, entity: Entity) -> &T {
@@ -1200,6 +1220,11 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> {
     ///
     /// - [`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<T: Component>(&mut self, entity: Entity) -> Mut<'_, T> {
@@ -1226,15 +1251,22 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> {
     /// # 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<T: Component>(
         &self,
         entity: Entity,
-    ) -> Result<Mut<'_, T>, QueryComponentError> {
+    ) -> Result<Mut<'_, T>, 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(QueryComponentError::MissingWriteAccess);
+            return Err(crate::query::QueryComponentError::MissingWriteAccess);
         }
 
         // SAFETY: The above check ensures we are not a readonly query.

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
@@ -24,12 +24,14 @@ fn main() {
             assert_eq!(data, &mut *data2); // oops UB
         }
 
+        #[allow(deprecated)]
         {
             let data: &Foo = query.get_component::<Foo>(e).unwrap();
             let mut data2: Mut<Foo> = query.get_component_mut(e).unwrap();
             assert_eq!(data, &mut *data2); // oops UB
         }
 
+        #[allow(deprecated)]
         {
             let mut data2: Mut<Foo> = query.get_component_mut(e).unwrap();
             let data: &Foo = query.get_component::<Foo>(e).unwrap();

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
@@ -19,101 +19,101 @@ 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:29:39
+  --> tests/ui/query_lifetime_safety.rs:30:39
    |
-28 |             let data: &Foo = query.get_component::<Foo>(e).unwrap();
+29 |             let data: &Foo = query.get_component::<Foo>(e).unwrap();
    |                              ----- immutable borrow occurs here
-29 |             let mut data2: Mut<Foo> = query.get_component_mut(e).unwrap();
+30 |             let mut data2: Mut<Foo> = query.get_component_mut(e).unwrap();
    |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here
-30 |             assert_eq!(data, &mut *data2); // oops UB
+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:35:30
+  --> tests/ui/query_lifetime_safety.rs:37:30
    |
-34 |             let mut data2: Mut<Foo> = query.get_component_mut(e).unwrap();
+36 |             let mut data2: Mut<Foo> = query.get_component_mut(e).unwrap();
    |                                       ----- mutable borrow occurs here
-35 |             let data: &Foo = query.get_component::<Foo>(e).unwrap();
+37 |             let data: &Foo = query.get_component::<Foo>(e).unwrap();
    |                              ^^^^^ immutable borrow occurs here
-36 |             assert_eq!(data, &mut *data2); // oops UB
+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:41:39
+  --> tests/ui/query_lifetime_safety.rs:43:39
    |
-40 |             let data: &Foo = query.single();
+42 |             let data: &Foo = query.single();
    |                              ----- immutable borrow occurs here
-41 |             let mut data2: Mut<Foo> = query.single_mut();
+43 |             let mut data2: Mut<Foo> = query.single_mut();
    |                                       ^^^^^^^^^^^^^^^^^^ mutable borrow occurs here
-42 |             assert_eq!(data, &mut *data2); // oops UB
+44 |             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:47:30
+  --> tests/ui/query_lifetime_safety.rs:49:30
    |
-46 |             let mut data2: Mut<Foo> = query.single_mut();
+48 |             let mut data2: Mut<Foo> = query.single_mut();
    |                                       ----- mutable borrow occurs here
-47 |             let data: &Foo = query.single();
+49 |             let data: &Foo = query.single();
    |                              ^^^^^ immutable borrow occurs here
-48 |             assert_eq!(data, &mut *data2); // oops UB
+50 |             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:53:39
+  --> tests/ui/query_lifetime_safety.rs:55:39
    |
-52 |             let data: &Foo = query.get_single().unwrap();
+54 |             let data: &Foo = query.get_single().unwrap();
    |                              ----- immutable borrow occurs here
-53 |             let mut data2: Mut<Foo> = query.get_single_mut().unwrap();
+55 |             let mut data2: Mut<Foo> = query.get_single_mut().unwrap();
    |                                       ^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here
-54 |             assert_eq!(data, &mut *data2); // oops UB
+56 |             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:59:30
+  --> tests/ui/query_lifetime_safety.rs:61:30
    |
-58 |             let mut data2: Mut<Foo> = query.get_single_mut().unwrap();
+60 |             let mut data2: Mut<Foo> = query.get_single_mut().unwrap();
    |                                       ----- mutable borrow occurs here
-59 |             let data: &Foo = query.get_single().unwrap();
+61 |             let data: &Foo = query.get_single().unwrap();
    |                              ^^^^^ immutable borrow occurs here
-60 |             assert_eq!(data, &mut *data2); // oops UB
+62 |             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:65:39
+  --> tests/ui/query_lifetime_safety.rs:67:39
    |
-64 |             let data: &Foo = query.iter().next().unwrap();
+66 |             let data: &Foo = query.iter().next().unwrap();
    |                              ----- immutable borrow occurs here
-65 |             let mut data2: Mut<Foo> = query.iter_mut().next().unwrap();
+67 |             let mut data2: Mut<Foo> = query.iter_mut().next().unwrap();
    |                                       ^^^^^^^^^^^^^^^^ mutable borrow occurs here
-66 |             assert_eq!(data, &mut *data2); // oops UB
+68 |             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:71:30
+  --> tests/ui/query_lifetime_safety.rs:73:30
    |
-70 |             let mut data2: Mut<Foo> = query.iter_mut().next().unwrap();
+72 |             let mut data2: Mut<Foo> = query.iter_mut().next().unwrap();
    |                                       ----- mutable borrow occurs here
-71 |             let data: &Foo = query.iter().next().unwrap();
+73 |             let data: &Foo = query.iter().next().unwrap();
    |                              ^^^^^ immutable borrow occurs here
-72 |             assert_eq!(data, &mut *data2); // oops UB
+74 |             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:79:13
+  --> tests/ui/query_lifetime_safety.rs:81:13
    |
-78 |             query.iter().for_each(|data| opt_data = Some(data));
+80 |             query.iter().for_each(|data| opt_data = Some(data));
    |             ----- immutable borrow occurs here
-79 |             query.iter_mut().for_each(|data| opt_data_2 = Some(data));
+81 |             query.iter_mut().for_each(|data| opt_data_2 = Some(data));
    |             ^^^^^^^^^^^^^^^^ mutable borrow occurs here
-80 |             assert_eq!(opt_data.unwrap(), &mut *opt_data_2.unwrap()); // oops UB
+82 |             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:87:13
+  --> tests/ui/query_lifetime_safety.rs:89:13
    |
-86 |             query.iter_mut().for_each(|data| opt_data_2 = Some(data));
+88 |             query.iter_mut().for_each(|data| opt_data_2 = Some(data));
    |             ----- mutable borrow occurs here
-87 |             query.iter().for_each(|data| opt_data = Some(data));
+89 |             query.iter().for_each(|data| opt_data = Some(data));
    |             ^^^^^ immutable borrow occurs here
-88 |             assert_eq!(opt_data.unwrap(), &mut *opt_data_2.unwrap()); // oops UB
+90 |             assert_eq!(opt_data.unwrap(), &mut *opt_data_2.unwrap()); // oops UB
    |                                                 ---------- mutable borrow later used here
-Original file line number
+Diff line change
@@ Expand Up / @@ -739,6 +739,7 @@ mod tests { @@
             }
         }
+        #[allow(deprecated)]
         #[test]
         fn mut_to_immut_query_methods_have_immut_item() {
             #[derive(Component)]
@@ Expand Down @@