Mirrors/bevy
2
0
Fork 0
mirror of https://github.com/bevyengine/bevy synced 2024-11-10 07:04:33 +00:00
Code Issues Projects Releases Packages Wiki Activity

Deprecated Various Component Methods from Query and QueryState (#9920)

Browse source 
# Objective

- (Partially) Fixes #9904
- Acts on #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.
This commit is contained in:
Zachary Harrold 2024-02-04 12:01:59 +11:00 committed by GitHub
parent 21aa5fe2b6
commit 1974723a63
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 121 additions and 63 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
crates/bevy_ecs/src/query/mod.rs
View file

@ -739,6 +739,7 @@ mod tests {
}
}
#[allow(deprecated)]
#[test]
fn mut_to_immut_query_methods_have_immut_item() {
#[derive(Component)]

45
crates/bevy_ecs/src/query/state.rs
View file

@ -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,6 +698,13 @@ 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,
@ -695,18 +712,18 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
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)
}
}

2
crates/bevy_ecs/src/system/mod.rs
View file

@ -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;

46
crates/bevy_ecs/src/system/query.rs
View file

@ -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.

2
crates/bevy_ecs_compile_fail_tests/tests/ui/query_lifetime_safety.rs
View file

@ -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();

80
crates/bevy_ecs_compile_fail_tests/tests/ui/query_lifetime_safety.stderr
View file

@ -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

6
crates/bevy_ui/src/focus.rs
View file

@ -162,7 +162,11 @@ pub fn ui_focus_system(
// reset entities that were both clicked and released in the last frame
for entity in state.entities_to_reset.drain(..) {
if let Ok(mut interaction) = node_query.get_component_mut::<Interaction>(entity) {
if let Ok(NodeQueryItem {
interaction: Some(mut interaction),
..
}) = node_query.get_mut(entity)
{
*interaction = Interaction::None;
}
}

2
examples/ui/size_constraints.rs
View file

@ -366,7 +366,7 @@ fn update_radio_buttons_colors(
children_query: Query<&Children>,
) {
for &ButtonActivatedEvent(button_id) in event_reader.read() {
let target_constraint = button_query.get_component::<Constraint>(button_id).unwrap();
let (_, target_constraint, _) = button_query.get(button_id).unwrap();
for (id, constraint, interaction) in button_query.iter() {
if target_constraint == constraint {
let (border_color, inner_color, text_color) = if id == button_id {