List components for QueryEntityError::QueryDoesNotMatch (#15435)

# Objective

Make it easier to debug why an entity doesn't match a query.

## Solution

List the entities components in `QueryEntityError::QueryDoesNotMatch`'s
message, e.g. `The query does not match the entity 0v1, which has
components foo::Bar, foo::Baz`.
This covers most cases as expected components are typically known and
filtering for change detection is rare when assessing a query by entity
id.

## Testing

Added a test confirming the new message matches the entity's components.

## Migration Guide

- `QueryEntityError` now has a lifetime. Convert it to a custom error if
you need to store it.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: poopy <gonesbird@gmail.com>

crates/bevy_ecs/src/query/error.rs

@@ -1,26 +1,93 @@
 use thiserror::Error;
 
-use crate::entity::Entity;
+use crate::{entity::Entity, world::unsafe_world_cell::UnsafeWorldCell};
 
 /// An error that occurs when retrieving a specific [`Entity`]'s query result from [`Query`](crate::system::Query) or [`QueryState`](crate::query::QueryState).
 // TODO: return the type_name as part of this error
-#[derive(Debug, PartialEq, Eq, Clone, Copy, Error)]
-pub enum QueryEntityError {
+#[derive(Clone, Copy)]
+pub enum QueryEntityError<'w> {
  /// The given [`Entity`]'s components do not match the query.
  ///
  /// Either it does not have a requested component, or it has a component which the query filters out.
- #[error("The components of entity {0:?} do not match the query")]
- QueryDoesNotMatch(Entity),
+ QueryDoesNotMatch(Entity, UnsafeWorldCell<'w>),
  /// The given [`Entity`] does not exist.
- #[error("The entity {0:?} does not exist")]
  NoSuchEntity(Entity),
  /// The [`Entity`] was requested mutably more than once.
  ///
  /// See [`QueryState::get_many_mut`](crate::query::QueryState::get_many_mut) for an example.
- #[error("The entity {0:?} was requested mutably more than once")]
  AliasedMutability(Entity),
 }
 
+impl<'w> std::error::Error for QueryEntityError<'w> {}
+
+impl<'w> std::fmt::Display for QueryEntityError<'w> {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ match *self {
+ Self::QueryDoesNotMatch(entity, world) => {
+ write!(
+ f,
+ "The query does not match the entity {entity}, which has components "
+ )?;
+ format_archetype(f, world, entity)
+ }
+ Self::NoSuchEntity(entity) => write!(f, "The entity {entity} does not exist"),
+ Self::AliasedMutability(entity) => write!(
+ f,
+ "The entity {entity} was requested mutably more than once"
+ ),
+ }
+ }
+}
+
+impl<'w> std::fmt::Debug for QueryEntityError<'w> {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ match *self {
+ Self::QueryDoesNotMatch(entity, world) => {
+ write!(f, "QueryDoesNotMatch({entity} with components ")?;
+ format_archetype(f, world, entity)?;
+ write!(f, ")")
+ }
+ Self::NoSuchEntity(entity) => write!(f, "NoSuchEntity({entity})"),
+ Self::AliasedMutability(entity) => write!(f, "AliasedMutability({entity})"),
+ }
+ }
+}
+
+fn format_archetype(
+ f: &mut std::fmt::Formatter<'_>,
+ world: UnsafeWorldCell<'_>,
+ entity: Entity,
+) -> std::fmt::Result {
+ // We know entity is still alive
+ let entity = world
+ .get_entity(entity)
+ .expect("entity does not belong to world");
+ for (i, component_id) in entity.archetype().components().enumerate() {
+ if i > 0 {
+ write!(f, ", ")?;
+ }
+ let name = world
+ .components()
+ .get_name(component_id)
+ .expect("entity does not belong to world");
+ write!(f, "{name}")?;
+ }
+ Ok(())
+}
+
+impl<'w> PartialEq for QueryEntityError<'w> {
+ fn eq(&self, other: &Self) -> bool {
+ match (self, other) {
+ (Self::QueryDoesNotMatch(e1, _), Self::QueryDoesNotMatch(e2, _)) if e1 == e2 => true,
+ (Self::NoSuchEntity(e1), Self::NoSuchEntity(e2)) if e1 == e2 => true,
+ (Self::AliasedMutability(e1), Self::AliasedMutability(e2)) if e1 == e2 => true,
+ _ => false,
+ }
+ }
+}
+
+impl<'w> Eq for QueryEntityError<'w> {}
+
 /// 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)]
@@ -32,3 +99,31 @@ pub enum QuerySingleError {
  #[error("Multiple entities fit the query {0}")]
  MultipleEntities(&'static str),
 }
+
+#[cfg(test)]
+mod test {
+ use crate as bevy_ecs;
+ use crate::prelude::World;
+ use bevy_ecs_macros::Component;
+
+ #[test]
+ fn query_does_not_match() {
+ let mut world = World::new();
+
+ #[derive(Component)]
+ struct Present1;
+ #[derive(Component)]
+ struct Present2;
+ #[derive(Component, Debug)]
+ struct NotPresent;
+
+ let entity = world.spawn((Present1, Present2)).id();
+
+ let err = world
+ .query::<&NotPresent>()
+ .get(&world, entity)
+ .unwrap_err();
+
+ assert_eq!(format!("{err:?}"), "QueryDoesNotMatch(0v1 with components bevy_ecs::query::error::test::query_does_not_match::Present1, bevy_ecs::query::error::test::query_does_not_match::Present2)");
+ }
+}

crates/bevy_ecs/src/query/state.rs

@@ -744,7 +744,7 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
  &mut self,
  world: &'w World,
  entity: Entity,
- ) -> Result<ROQueryItem<'w, D>, QueryEntityError> {
+ ) -> Result<ROQueryItem<'w, D>, QueryEntityError<'w>> {
  self.update_archetypes(world);
  // SAFETY: query is read only
  unsafe {
@@ -794,7 +794,7 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
  &mut self,
  world: &'w World,
  entities: [Entity; N],
- ) -> Result<[ROQueryItem<'w, D>; N], QueryEntityError> {
+ ) -> Result<[ROQueryItem<'w, D>; N], QueryEntityError<'w>> {
  self.update_archetypes(world);
 
  // SAFETY:
@@ -818,7 +818,7 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
  &mut self,
  world: &'w mut World,
  entity: Entity,
- ) -> Result<D::Item<'w>, QueryEntityError> {
+ ) -> Result<D::Item<'w>, QueryEntityError<'w>> {
  self.update_archetypes(world);
  let change_tick = world.change_tick();
  let last_change_tick = world.last_change_tick();
@@ -868,15 +868,15 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
  /// let invalid_entity = world.spawn_empty().id();
  ///
  /// assert_eq!(query_state.get_many_mut(&mut world, [wrong_entity]).unwrap_err(), QueryEntityError::NoSuchEntity(wrong_entity));
- /// assert_eq!(query_state.get_many_mut(&mut world, [invalid_entity]).unwrap_err(), QueryEntityError::QueryDoesNotMatch(invalid_entity));
+ /// assert_eq!(match query_state.get_many_mut(&mut world, [invalid_entity]).unwrap_err() {QueryEntityError::QueryDoesNotMatch(entity, _) => entity, _ => panic!()}, invalid_entity);
  /// assert_eq!(query_state.get_many_mut(&mut world, [entities[0], entities[0]]).unwrap_err(), QueryEntityError::AliasedMutability(entities[0]));
  /// ```
  #[inline]
  pub fn get_many_mut<'w, const N: usize>(
  &mut self,
  world: &'w mut World,
  entities: [Entity; N],
- ) -> Result<[D::Item<'w>; N], QueryEntityError> {
+ ) -> Result<[D::Item<'w>; N], QueryEntityError<'w>> {
  self.update_archetypes(world);
 
  let change_tick = world.change_tick();
@@ -911,7 +911,7 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
  &self,
  world: &'w World,
  entity: Entity,
- ) -> Result<ROQueryItem<'w, D>, QueryEntityError> {
+ ) -> Result<ROQueryItem<'w, D>, QueryEntityError<'w>> {
  self.validate_world(world.id());
  // SAFETY: query is read only and world is validated
  unsafe {
@@ -937,7 +937,7 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
  &mut self,
  world: UnsafeWorldCell<'w>,
  entity: Entity,
- ) -> Result<D::Item<'w>, QueryEntityError> {
+ ) -> Result<D::Item<'w>, QueryEntityError<'w>> {
  self.update_archetypes_unsafe_world_cell(world);
  self.get_unchecked_manual(world, entity, world.last_change_tick(), world.change_tick())
  }
@@ -960,7 +960,7 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
  entity: Entity,
  last_run: Tick,
  this_run: Tick,
- ) -> Result<D::Item<'w>, QueryEntityError> {
+ ) -> Result<D::Item<'w>, QueryEntityError<'w>> {
  let location = world
  .entities()
  .get(entity)
@@ -969,7 +969,7 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
  .matched_archetypes
  .contains(location.archetype_id.index())
  {
- return Err(QueryEntityError::QueryDoesNotMatch(entity));
+ return Err(QueryEntityError::QueryDoesNotMatch(entity, world));
  }
  let archetype = world
  .archetypes()
@@ -989,7 +989,7 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
  if F::filter_fetch(&mut filter, entity, location.table_row) {
  Ok(D::fetch(&mut fetch, entity, location.table_row))
  } else {
- Err(QueryEntityError::QueryDoesNotMatch(entity))
+ Err(QueryEntityError::QueryDoesNotMatch(entity, world))
  }
  }
 
@@ -1008,7 +1008,7 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
  entities: [Entity; N],
  last_run: Tick,
  this_run: Tick,
- ) -> Result<[ROQueryItem<'w, D>; N], QueryEntityError> {
+ ) -> Result<[ROQueryItem<'w, D>; N], QueryEntityError<'w>> {
  let mut values = [(); N].map(|_| MaybeUninit::uninit());
 
  for (value, entity) in std::iter::zip(&mut values, entities) {
@@ -1042,7 +1042,7 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
  entities: [Entity; N],
  last_run: Tick,
  this_run: Tick,
- ) -> Result<[D::Item<'w>; N], QueryEntityError> {
+ ) -> Result<[D::Item<'w>; N], QueryEntityError<'w>> {
  // Verify that all entities are unique
  for i in 0..N {
  for j in 0..i {
@@ -1442,7 +1442,7 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
  /// # let invalid_entity = world.spawn_empty().id();
  ///
  /// # assert_eq!(query_state.get_many_mut(&mut world, [wrong_entity]).unwrap_err(), QueryEntityError::NoSuchEntity(wrong_entity));
- /// # assert_eq!(query_state.get_many_mut(&mut world, [invalid_entity]).unwrap_err(), QueryEntityError::QueryDoesNotMatch(invalid_entity));
+ /// assert_eq!(match query_state.get_many_mut(&mut world, [invalid_entity]).unwrap_err() {QueryEntityError::QueryDoesNotMatch(entity, _) => entity, _ => panic!()}, invalid_entity);
  /// # assert_eq!(query_state.get_many_mut(&mut world, [entities[0], entities[0]]).unwrap_err(), QueryEntityError::AliasedMutability(entities[0]));
  /// ```
  ///

crates/bevy_ecs/src/world/mod.rs

@@ -723,7 +723,7 @@ impl World {
  /// # Errors
  ///
  /// If any entities are duplicated.
- fn verify_unique_entities(entities: &[Entity]) -> Result<(), QueryEntityError> {
+ fn verify_unique_entities(entities: &[Entity]) -> Result<(), QueryEntityError<'static>> {
  for i in 0..entities.len() {
  for j in 0..i {
  if entities[i] == entities[j] {

crates/bevy_render/src/render_phase/draw.rs

@@ -325,7 +325,8 @@ where
  Ok(view) => view,
  Err(err) => match err {
  QueryEntityError::NoSuchEntity(_) => return Err(DrawError::ViewEntityNotFound),
- QueryEntityError::QueryDoesNotMatch(_) | QueryEntityError::AliasedMutability(_) => {
+ QueryEntityError::QueryDoesNotMatch(_, _)
+ | QueryEntityError::AliasedMutability(_) => {
  return Err(DrawError::InvalidViewQuery)
  }
  },

crates/bevy_transform/src/helper.rs

@@ -51,7 +51,7 @@ impl<'w, 's> TransformHelper<'w, 's> {
 fn map_error(err: QueryEntityError, ancestor: bool) -> ComputeGlobalTransformError {
  use ComputeGlobalTransformError::*;
  match err {
- QueryEntityError::QueryDoesNotMatch(entity) => MissingTransform(entity),
+ QueryEntityError::QueryDoesNotMatch(entity, _) => MissingTransform(entity),
  QueryEntityError::NoSuchEntity(entity) => {
  if ancestor {
  MalformedHierarchy(entity)