Skip to content

Make Query::single (and friends) return a Result #18082

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Mar 2, 2025
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ fn main() {
}

{
let data: &Foo = query.single();
let mut data2: Mut<Foo> = query.single_mut();
let data: &Foo = query.get_single().unwrap();
let mut data2: Mut<Foo> = query.get_single_mut().unwrap();
//~^ E0502
assert_eq!(data, &mut *data2); // oops UB
}

{
let mut data2: Mut<Foo> = query.single_mut();
let data: &Foo = query.single();
let mut data2: Mut<Foo> = query.get_single_mut().unwrap();
let data: &Foo = query.get_single().unwrap();
//~^ E0502
assert_eq!(data, &mut *data2); // oops UB
}
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/compile_fail/tests/ui/query_to_readonly.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ fn for_loops(mut query: Query<&mut Foo>) {
fn single_mut_query(mut query: Query<&mut Foo>) {
// this should fail to compile
{
let mut mut_foo = query.single_mut();
let mut mut_foo = query.get_single_mut().unwrap();

// This solves "temporary value dropped while borrowed"
let readonly_query = query.as_readonly();
//~^ E0502

let ref_foo = readonly_query.single();
let ref_foo = readonly_query.get_single().unwrap();

*mut_foo = Foo;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ fn main() {
let mut query_a = lens_a.query();
let mut query_b = lens_b.query();

let a = query_a.single_mut();
let b = query_b.single_mut(); // oops 2 mutable references to same Foo
let a = query_a.get_single_mut().unwrap();
let b = query_b.get_single_mut().unwrap(); // oops 2 mutable references to same Foo
assert_eq!(*a, *b);
}

Expand All @@ -34,8 +34,8 @@ fn main() {
let mut query_b = lens.query();
//~^ E0499

let a = query_a.single_mut();
let b = query_b.single_mut(); // oops 2 mutable references to same Foo
let a = query_a.get_single_mut().unwrap();
let b = query_b.get_single_mut().unwrap(); // oops 2 mutable references to same Foo
assert_eq!(*a, *b);
}
}
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/change_detection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1557,7 +1557,7 @@ mod tests {
// Since the world is always ahead, as long as changes can't get older than `u32::MAX` (which we ensure),
// the wrapping difference will always be positive, so wraparound doesn't matter.
let mut query = world.query::<Ref<C>>();
assert!(query.single(&world).is_changed());
assert!(query.get_single(&world).unwrap().is_changed());
}

#[test]
Expand Down
14 changes: 7 additions & 7 deletions crates/bevy_ecs/src/query/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use super::{FilteredAccess, QueryData, QueryFilter};
/// .build();
///
/// // Consume the QueryState
/// let (entity, b) = query.single(&world);
/// let (entity, b) = query.get_single(&world).unwrap();
/// ```
pub struct QueryBuilder<'w, D: QueryData = (), F: QueryFilter = ()> {
access: FilteredAccess<ComponentId>,
Expand Down Expand Up @@ -297,13 +297,13 @@ mod tests {
.with::<A>()
.without::<C>()
.build();
assert_eq!(entity_a, query_a.single(&world));
assert_eq!(entity_a, query_a.get_single(&world).unwrap());
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I prefer to wrap the reference value in Ok(...) rather an unwrap in an assert_eq, as that will print a nicer error message. Has the same end-result of failing the test so it's not that big of a deal though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, mildly agree.


let mut query_b = QueryBuilder::<Entity>::new(&mut world)
.with::<A>()
.without::<B>()
.build();
assert_eq!(entity_b, query_b.single(&world));
assert_eq!(entity_b, query_b.get_single(&world).unwrap());
}

#[test]
Expand All @@ -319,13 +319,13 @@ mod tests {
.with_id(component_id_a)
.without_id(component_id_c)
.build();
assert_eq!(entity_a, query_a.single(&world));
assert_eq!(entity_a, query_a.get_single(&world).unwrap());

let mut query_b = QueryBuilder::<Entity>::new(&mut world)
.with_id(component_id_a)
.without_id(component_id_b)
.build();
assert_eq!(entity_b, query_b.single(&world));
assert_eq!(entity_b, query_b.get_single(&world).unwrap());
}

#[test]
Expand Down Expand Up @@ -385,7 +385,7 @@ mod tests {
.data::<&B>()
.build();

let entity_ref = query.single(&world);
let entity_ref = query.get_single(&world).unwrap();

assert_eq!(entity, entity_ref.id());

Expand All @@ -408,7 +408,7 @@ mod tests {
.ref_id(component_id_b)
.build();

let entity_ref = query.single(&world);
let entity_ref = query.get_single(&world).unwrap();

assert_eq!(entity, entity_ref.id());

Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,7 @@ mod tests {
let _: Option<&Foo> = q.get_manual(&world, e).ok();
let _: Option<[&Foo; 1]> = q.get_many(&world, [e]).ok();
let _: Option<&Foo> = q.get_single(&world).ok();
let _: &Foo = q.single(&world);
let _: &Foo = q.get_single(&world).unwrap();

// system param
let mut q = SystemState::<Query<&mut Foo>>::new(&mut world);
Expand All @@ -778,7 +778,7 @@ mod tests {
let _: Option<[&Foo; 1]> = q.get_many([e]).ok();
let _: Option<&Foo> = q.get_single().ok();
let _: [&Foo; 1] = q.many([e]);
let _: &Foo = q.single();
let _: &Foo = q.get_single().unwrap();
}

// regression test for https://github.com/bevyengine/bevy/pull/8029
Expand Down
100 changes: 86 additions & 14 deletions crates/bevy_ecs/src/query/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1603,8 +1603,12 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
/// [`get_single`](Self::get_single) to return a `Result` instead of panicking.
#[track_caller]
#[inline]
#[deprecated(
since = "0.16.0",
note = "Please use `get_single` instead, which returns a `Result` instead of panicking."
)]
pub fn single<'w>(&mut self, world: &'w World) -> ROQueryItem<'w, D> {
self.query(world).single_inner()
self.query(world).get_single_inner().unwrap()
}

/// Returns a single immutable query result when there is exactly one entity matching
Expand All @@ -1615,6 +1619,66 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
///
/// If the number of query results is not exactly one, a [`QuerySingleError`] is returned
/// instead.
///
/// # Example
///
/// Sometimes, you might want to handle the error in a specific way,
/// generally by spawning the missing entity.
///
/// ```rust
/// use bevy_ecs::prelude::*;
///
/// #[derive(Component)]
/// struct A(usize);
///
/// fn my_system(query: Query<&A>, mut commands: Commands) {
/// match query.get_single() {
/// Some(a) => () // Do something with `a`,
/// None => commands.spawn((A(0),)),
/// }
/// }
/// ```
///
/// However in most cases, this error can simply be handled with a graceful early return.
/// If this is an expected failure mode, you can do this using the `let else` pattern like so:
/// ```rust
/// use bevy_ecs::prelude::*;
///
/// #[derive(Component)]
/// struct A(usize);
///
/// fn my_system(query: Query<&A>) {
/// let Ok(a) = query.get_single() else {
/// return;
/// }
///
/// // Do something with `a`
/// }
/// ```
///
/// If this is unexpected though, you should probably use the `?` operator
/// in combination with Bevy's error handling apparatus.
///
/// ```rust
/// use bevy_ecs::prelude::*;
///
/// #[derive(Component)]
/// struct A(usize);
///
/// fn my_system(query: Query<&A>) -> Result {
/// let a = query.get_single()?;
///
/// // Do something with `a`
/// }
/// ```
///
/// This allows you to globally control how errors are handled in your application,
/// by setting up a custom error handler.
/// See the [`bevy_ecs::result`] module docs for more information!
/// Commonly, you might want to panic on an error during development, but log the error and continue
/// execution in production.
///
/// Simply unwrapping the [`Result`] also works, but should generally be reserved for tests.
#[inline]
pub fn get_single<'w>(
&mut self,
Expand All @@ -1632,15 +1696,23 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
/// [`get_single_mut`](Self::get_single_mut) to return a `Result` instead of panicking.
#[track_caller]
#[inline]
#[deprecated(
since = "0.16.0",
note = "Please use `get_single_mut` instead, which returns a `Result` instead of panicking."
)]
pub fn single_mut<'w>(&mut self, world: &'w mut World) -> D::Item<'w> {
self.query_mut(world).single_inner()
self.query_mut(world).get_single_inner().unwrap()
}

/// Returns a single mutable query result when there is exactly one entity matching
/// the query.
///
/// If the number of query results is not exactly one, a [`QuerySingleError`] is returned
/// instead.
///
/// # Examples
///
/// Please see [`Query::get_single`] for advice on handling the error.
#[inline]
pub fn get_single_mut<'w>(
&mut self,
Expand Down Expand Up @@ -1753,7 +1825,7 @@ mod tests {
let query_state = world.query::<(&A, &B)>();
let mut new_query_state = query_state.transmute::<&A>(&world);
assert_eq!(new_query_state.iter(&world).len(), 1);
let a = new_query_state.single(&world);
let a = new_query_state.get_single(&world).unwrap();

assert_eq!(a.0, 1);
}
Expand All @@ -1767,7 +1839,7 @@ mod tests {
let query_state = world.query_filtered::<(&A, &B), Without<C>>();
let mut new_query_state = query_state.transmute::<&A>(&world);
// even though we change the query to not have Without<C>, we do not get the component with C.
let a = new_query_state.single(&world);
let a = new_query_state.get_single(&world).unwrap();

assert_eq!(a.0, 0);
}
Expand All @@ -1780,7 +1852,7 @@ mod tests {

let q = world.query::<()>();
let mut q = q.transmute::<Entity>(&world);
assert_eq!(q.single(&world), entity);
assert_eq!(q.get_single(&world).unwrap(), entity);
}

#[test]
Expand All @@ -1790,7 +1862,7 @@ mod tests {

let q = world.query::<&A>();
let mut new_q = q.transmute::<Ref<A>>(&world);
assert!(new_q.single(&world).is_added());
assert!(new_q.get_single(&world).unwrap().is_added());

let q = world.query::<Ref<A>>();
let _ = q.transmute::<&A>(&world);
Expand Down Expand Up @@ -1861,7 +1933,7 @@ mod tests {

let query_state = world.query::<Option<&A>>();
let mut new_query_state = query_state.transmute::<&A>(&world);
let x = new_query_state.single(&world);
let x = new_query_state.get_single(&world).unwrap();
assert_eq!(x.0, 1234);
}

Expand All @@ -1886,7 +1958,7 @@ mod tests {

let mut query = query;
// Our result is completely untyped
let entity_ref = query.single(&world);
let entity_ref = query.get_single(&world).unwrap();

assert_eq!(entity, entity_ref.id());
assert_eq!(0, entity_ref.get::<A>().unwrap().0);
Expand All @@ -1901,12 +1973,12 @@ mod tests {
let mut query = QueryState::<(Entity, &A, Has<B>)>::new(&mut world)
.transmute_filtered::<(Entity, Has<B>), Added<A>>(&world);

assert_eq!((entity_a, false), query.single(&world));
assert_eq!((entity_a, false), query.get_single(&world).unwrap());

world.clear_trackers();

let entity_b = world.spawn((A(0), B(0))).id();
assert_eq!((entity_b, true), query.single(&world));
assert_eq!((entity_b, true), query.get_single(&world).unwrap());

world.clear_trackers();

Expand All @@ -1922,15 +1994,15 @@ mod tests {
.transmute_filtered::<Entity, Changed<A>>(&world);

let mut change_query = QueryState::<&mut A>::new(&mut world);
assert_eq!(entity_a, detection_query.single(&world));
assert_eq!(entity_a, detection_query.get_single(&world).unwrap());

world.clear_trackers();

assert!(detection_query.get_single(&world).is_err());

change_query.single_mut(&mut world).0 = 1;
change_query.get_single_mut(&mut world).unwrap().0 = 1;

assert_eq!(entity_a, detection_query.single(&world));
assert_eq!(entity_a, detection_query.get_single(&world).unwrap());
}

#[test]
Expand Down Expand Up @@ -2017,7 +2089,7 @@ mod tests {
let query_2 = QueryState::<&B, Without<C>>::new(&mut world);
let mut new_query: QueryState<Entity, ()> = query_1.join_filtered(&world, &query_2);

assert_eq!(new_query.single(&world), entity_ab);
assert_eq!(new_query.get_single(&world).unwrap(), entity_ab);
}

#[test]
Expand Down
Loading
Loading