From 0f17607192589c47c8091fc173ad8add65a03563 Mon Sep 17 00:00:00 2001 From: plof27 Date: Sat, 2 Jul 2022 10:42:15 -0400 Subject: [PATCH] Improve error messages when archetypes fail Now, the Rust type names are fetched and nicely formatted, where possible. It would be good to shorten the type names, but there is a separate PR for this. --- crates/bevy_ecs/src/archetype.rs | 17 +++-- crates/bevy_ecs/src/bundle.rs | 1 + .../src/world/archetype_invariants.rs | 66 +++++++++++++++++-- crates/bevy_ecs/src/world/entity_ref.rs | 1 + 4 files changed, 74 insertions(+), 11 deletions(-) diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index a2bd4d4332ba19..d08ed711ad7cff 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -3,7 +3,7 @@ use crate::{ bundle::BundleId, - component::{ComponentId, StorageType}, + component::{ComponentId, Components, StorageType}, entity::{Entity, EntityLocation}, storage::{Column, SparseArray, SparseSet, SparseSetIndex, TableId}, world::ArchetypeInvariants, @@ -154,13 +154,14 @@ impl Archetype { table_archetype_components: Vec, sparse_set_archetype_components: Vec, archetype_invariants: &ArchetypeInvariants, + components: &Components, ) -> Self { - let mut components = + let mut component_set = SparseSet::with_capacity(table_components.len() + sparse_set_components.len()); for (component_id, archetype_component_id) in table_components.iter().zip(table_archetype_components) { - components.insert( + component_set.insert( *component_id, ArchetypeComponentInfo { storage_type: StorageType::Table, @@ -173,7 +174,7 @@ impl Archetype { .iter() .zip(sparse_set_archetype_components) { - components.insert( + component_set.insert( *component_id, ArchetypeComponentInfo { storage_type: StorageType::SparseSet, @@ -182,7 +183,7 @@ impl Archetype { ); } - archetype_invariants.test_archetype(components.indices()); + archetype_invariants.test_archetype(component_set.indices(), components); Self { id, @@ -190,7 +191,7 @@ impl Archetype { id: table_id, entity_rows: Default::default(), }, - components, + components: component_set, table_components, sparse_set_components, unique_components: SparseSet::new(), @@ -401,6 +402,7 @@ impl Default for Archetypes { Vec::new(), Vec::new(), &ArchetypeInvariants::default(), + &Components::default(), ); // adds the resource archetype. it is "special" in that it is inaccessible via a "hash", @@ -413,6 +415,7 @@ impl Default for Archetypes { Vec::new(), Vec::new(), &ArchetypeInvariants::default(), + &Components::default(), )); archetypes } @@ -500,6 +503,7 @@ impl Archetypes { table_components: Vec, sparse_set_components: Vec, archetype_invariants: &ArchetypeInvariants, + components: &Components, ) -> ArchetypeId { let table_components = table_components.into_boxed_slice(); let sparse_set_components = sparse_set_components.into_boxed_slice(); @@ -534,6 +538,7 @@ impl Archetypes { table_archetype_components, sparse_set_archetype_components, archetype_invariants, + components, )); id }) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 96422b810117a6..02b187060201b2 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -393,6 +393,7 @@ impl BundleInfo { table_components, sparse_set_components, archetype_invariants, + components, ); // add an edge from the old archetype to the new archetype archetypes[archetype_id].edges_mut().insert_add_bundle( diff --git a/crates/bevy_ecs/src/world/archetype_invariants.rs b/crates/bevy_ecs/src/world/archetype_invariants.rs index 5d6336a263325c..e77a8540aa8032 100644 --- a/crates/bevy_ecs/src/world/archetype_invariants.rs +++ b/crates/bevy_ecs/src/world/archetype_invariants.rs @@ -2,7 +2,11 @@ use std::marker::PhantomData; use bevy_utils::{tracing::warn, HashSet}; -use crate::{component::ComponentId, prelude::Bundle, world::World}; +use crate::{ + component::{ComponentId, Components}, + prelude::Bundle, + world::World, +}; /// A rule about which [`Component`](crate::component::Component)s can coexist on entities. /// @@ -236,6 +240,15 @@ impl UntypedArchetypeInvariant { ); } } + + /// Returns formatted string describing this archetype invariant + pub fn display(&self, components: &Components) -> String { + format!( + "{{Premise: {}, Consequence: {}}}", + self.premise.display(components), + self.consequence.display(components) + ) + } } /// A type-erased version of [`ArchetypeStatement`]. @@ -270,6 +283,31 @@ impl UntypedArchetypeStatement { } } + /// Returns formatted string describing this archetype invariant + /// + /// For Rust types, the names should match the type name. + /// If any [`ComponentId`]s in the invariant have not been registered in the world, + /// then the raw component id will be returned instead. + pub fn display(&self, components: &Components) -> String { + let component_names: String = self + .component_ids() + .iter() + .map(|id| match components.get_info(*id) { + Some(info) => info.name().to_owned(), + None => format!("{:?}", id), + }) + .reduce(|acc, s| format!("{}, {}", acc, s)) + .unwrap_or("".to_owned()); + + match self { + UntypedArchetypeStatement::AllOf(_) => format!("AllOf({component_names})"), + UntypedArchetypeStatement::AnyOf(_) => format!("AnyOf({component_names})"), + UntypedArchetypeStatement::AtMostOneOf(_) => format!("AtMostOneOf({component_names})"), + UntypedArchetypeStatement::NoneOf(_) => format!("NoneOf({component_names})"), + UntypedArchetypeStatement::Only(_) => format!("Only({component_names})"), + } + } + /// Test if this statement is true for the provided set of [`ComponentId`]s. pub fn test(&self, component_ids: &HashSet) -> bool { match self { @@ -342,7 +380,11 @@ impl ArchetypeInvariants { /// # Panics /// /// Panics if any archetype invariant is violated. - pub fn test_archetype(&self, component_ids_of_archetype: impl Iterator) { + pub fn test_archetype( + &self, + component_ids_of_archetype: impl Iterator, + components: &Components, + ) { let component_ids_of_archetype: HashSet = component_ids_of_archetype.collect(); for invariant in &self.raw_list { @@ -359,10 +401,24 @@ impl ArchetypeInvariants { } } + let archetype_component_names: Vec = component_ids_of_archetype + .into_iter() + .map(|id| match components.get_info(id) { + Some(info) => info.name().to_owned(), + None => format!("{:?}", id), + }) + .collect(); + + let failed_invariant_names: String = failed_invariants + .into_iter() + .map(|invariant| invariant.display(components)) + .reduce(|acc, s| format!("{}\n{}", acc, s)) + .unwrap(); + panic!( - "Archetype invariant violated! The following invariants were violated for archetype {:?}:\n{:?}", - component_ids_of_archetype, - failed_invariants, + "Archetype invariant violated! The following invariants were violated for archetype {:?}:\n{}", + archetype_component_names, + failed_invariant_names, ) } } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 2cb0a26396a798..d31f2e9bd222e1 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -820,6 +820,7 @@ unsafe fn remove_bundle_from_archetype( next_table_components, next_sparse_set_components, archetype_invariants, + components, ); Some(new_archetype_id) };