From 51a5070cd2d3d7262616b2efc8a6cd3b1991d7e6 Mon Sep 17 00:00:00 2001 From: Charles Giguere Date: Fri, 10 Sep 2021 20:23:50 +0000 Subject: [PATCH] add get_single variant (#2793) # Objective The vast majority of `.single()` usage I've seen is immediately followed by a `.unwrap()`. Since it seems most people use it without handling the error, I think making it easier to just get what you want fast while also having a more verbose alternative when you want to handle the error could help. ## Solution Instead of having a lot of `.unwrap()` everywhere, this PR introduces a `try_single()` variant that behaves like the current `.single()` and make the new `.single()` panic on error. --- crates/bevy_ecs/src/system/mod.rs | 10 +-- crates/bevy_ecs/src/system/query.rs | 61 ++++++++++++--- examples/2d/many_sprites.rs | 4 +- examples/game/alien_cake_addict.rs | 2 +- examples/game/breakout.rs | 117 ++++++++++++++-------------- examples/shader/animate_shader.rs | 2 +- 6 files changed, 115 insertions(+), 81 deletions(-) diff --git a/crates/bevy_ecs/src/system/mod.rs b/crates/bevy_ecs/src/system/mod.rs index d4fd0ad799..7bed6a4712 100644 --- a/crates/bevy_ecs/src/system/mod.rs +++ b/crates/bevy_ecs/src/system/mod.rs @@ -574,7 +574,7 @@ mod tests { let (a, query, _) = system_state.get(&world); assert_eq!(*a, A(42), "returned resource matches initial value"); assert_eq!( - *query.single().unwrap(), + *query.single(), B(7), "returned component matches initial value" ); @@ -601,7 +601,7 @@ mod tests { let (a, mut query) = system_state.get_mut(&mut world); assert_eq!(*a, A(42), "returned resource matches initial value"); assert_eq!( - *query.single_mut().unwrap(), + *query.single_mut(), B(7), "returned component matches initial value" ); @@ -618,18 +618,18 @@ mod tests { let mut system_state: SystemState>> = SystemState::new(&mut world); { let query = system_state.get(&world); - assert_eq!(*query.single().unwrap(), A(1)); + assert_eq!(*query.single(), A(1)); } { let query = system_state.get(&world); - assert!(query.single().is_err()); + assert!(query.get_single().is_err()); } world.entity_mut(entity).get_mut::().unwrap().0 = 2; { let query = system_state.get(&world); - assert_eq!(*query.single().unwrap(), A(2)); + assert_eq!(*query.single(), A(2)); } } diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index e550ac6fa0..921454ec2c 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -488,6 +488,33 @@ where } } + /// Gets the result of a single-result query. + /// + /// Assumes this query has only one result and panics if there are no or multiple results. + /// Use [`Self::get_single`] to handle the error cases explicitly + /// + /// # Example + /// + /// ``` + /// # use bevy_ecs::prelude::{IntoSystem, Query, With}; + /// struct Player; + /// struct Position(f32, f32); + /// fn player_system(query: Query<&Position, With>) { + /// let player_position = query.single(); + /// // do something with player_position + /// } + /// # let _check_that_its_a_system = player_system.system(); + /// ``` + /// + /// This can only be called for read-only queries, see [`Self::single_mut`] for write-queries. + #[track_caller] + pub fn single(&'s self) -> >::Item + where + Q::Fetch: ReadOnlyFetch, + { + self.get_single().unwrap() + } + /// Gets the result of a single-result query. /// /// If the query has exactly one result, returns the result inside `Ok` @@ -497,27 +524,28 @@ where /// # Examples /// /// ``` + /// # use bevy_ecs::prelude::{IntoSystem, With}; /// # use bevy_ecs::system::{Query, QuerySingleError}; - /// # use bevy_ecs::prelude::IntoSystem; - /// struct PlayerScore(i32); - /// fn player_scoring_system(query: Query<&PlayerScore>) { - /// match query.single() { - /// Ok(PlayerScore(score)) => { - /// // do something with score + /// struct Player; + /// struct Position(f32, f32); + /// fn player_system(query: Query<&Position, With>) { + /// match query.get_single() { + /// Ok(position) => { + /// // do something with position /// } /// Err(QuerySingleError::NoEntities(_)) => { - /// // no PlayerScore + /// // no position with Player /// } /// Err(QuerySingleError::MultipleEntities(_)) => { - /// // multiple PlayerScore + /// // multiple position with Player /// } /// } /// } - /// # let _check_that_its_a_system = player_scoring_system.system(); + /// # let _check_that_its_a_system = player_system.system(); /// ``` /// - /// This can only be called for read-only queries, see [`Self::single_mut`] for write-queries. - pub fn single(&'s self) -> Result<>::Item, QuerySingleError> + /// This can only be called for read-only queries, see [`Self::get_single_mut`] for write-queries. + pub fn get_single(&'s self) -> Result<>::Item, QuerySingleError> where Q::Fetch: ReadOnlyFetch, { @@ -534,9 +562,18 @@ where } } + /// Gets the query result if it is only a single result, otherwise panics + /// If you want to handle the error case yourself you can use the [`Self::get_single_mut`] variant. + #[track_caller] + pub fn single_mut(&mut self) -> >::Item { + self.get_single_mut().unwrap() + } + /// Gets the query result if it is only a single result, otherwise returns a /// [`QuerySingleError`]. - pub fn single_mut(&mut self) -> Result<>::Item, QuerySingleError> { + pub fn get_single_mut( + &mut self, + ) -> Result<>::Item, QuerySingleError> { let mut query = self.iter_mut(); let first = query.next(); let extra = query.next().is_some(); diff --git a/examples/2d/many_sprites.rs b/examples/2d/many_sprites.rs index a3ca208a34..e6c9b15212 100644 --- a/examples/2d/many_sprites.rs +++ b/examples/2d/many_sprites.rs @@ -75,7 +75,7 @@ fn setup( // System for rotating and translating the camera fn move_camera_system(time: Res