feat(engine-ecs): improve various error log messages

2 files changed, 105 insertions, 30 deletions

diff --git a/engine-ecs/src/component/storage.rs b/engine-ecs/src/component/storage.rs
index 4a1c035..b56d6ba 100644
--- a/engine-ecs/src/component/storage.rs
+++ b/engine-ecs/src/component/storage.rs
@@ -817,16 +817,16 @@ impl<const TERM_CAP: usize> ArchetypeRefIter<'_, '_, TERM_CAP>
 #[derive(Debug, thiserror::Error)]
 pub enum Error
 {
-    #[error("Entity with ID {0:?} does not exist")]
+    #[error("Entity {0} does not exist")]
     EntityDoesNotExist(Uid),
 
-    #[error("Entity with ID {entity:?} already has component with ID {component:?}")]
+    #[error("Entity {entity} already has component {component}")]
     ComponentAlreadyInEntity
     {
         entity: Uid, component: Uid
     },
 
-    #[error("Entity with ID {entity:?} does not have component with ID {component:?}")]
+    #[error("Entity {entity} does not have component {component}")]
     ComponentNotFoundInEntity
     {
         entity: Uid, component: Uid
@@ -834,7 +834,7 @@ pub enum Error
 }
 
 #[derive(Debug, thiserror::Error)]
-#[error("Entity with already exists")]
+#[error("Entity already exists")]
 pub struct EntityAlreadyExistsError;
 
 #[derive(Debug)]
diff --git a/engine-ecs/src/lib.rs b/engine-ecs/src/lib.rs
index fcb9a8d..b987f65 100644
--- a/engine-ecs/src/lib.rs
+++ b/engine-ecs/src/lib.rs
@@ -1,6 +1,6 @@
 #![deny(clippy::all, clippy::pedantic)]
 
-use std::any::{Any, TypeId, type_name};
+use std::any::{Any, TypeId};
 use std::borrow::Cow;
 use std::fmt::Debug;
 use std::hint::cold_path;
@@ -10,7 +10,11 @@ use std::time::Duration;
 
 use crate::actions::Action;
 use crate::component::storage::archetype::EntityComponent as ArchetypeEntityComponent;
-use crate::component::storage::{EntityAlreadyExistsError, Storage as ComponentStorage};
+use crate::component::storage::{
+    EntityAlreadyExistsError,
+    Error as ComponentStorageError,
+    Storage as ComponentStorage,
+};
 use crate::component::{
     Component,
     Info as ComponentInfo,
@@ -570,26 +574,31 @@ impl World
 
         for action in action_queue_lock.drain(..) {
             match action {
-                Action::Spawn(new_entity_uid, components) => {
-                    if let Err(err) =
-                        self.data.component_storage.create_entity(new_entity_uid)
+                Action::Spawn(entity_id, components) => {
+                    if let Err(err) = self.data.component_storage.create_entity(entity_id)
                     {
-                        tracing::warn!("Failed to create entity: {err}");
+                        tracing::error!(
+                            entity = %entity_id,
+                            "Failed to create entity: {err}"
+                        );
+
                         continue;
                     }
 
                     Self::add_entity_components(
-                        new_entity_uid,
+                        entity_id,
                         components,
                         &mut self.data.component_storage,
                         &EventSubmitter::new(&self.data.new_events),
                     );
                 }
-                Action::Despawn(entity_uid) => {
-                    if let Err(err) =
-                        self.data.component_storage.remove_entity(entity_uid)
+                Action::Despawn(entity_id) => {
+                    if let Err(err) = self.data.component_storage.remove_entity(entity_id)
                     {
-                        tracing::error!("Failed to despawn entity: {err}");
+                        tracing::error!(
+                            entity = %entity_id,
+                            "Failed to despawn entity: {err}"
+                        );
                     }
                 }
                 Action::AddComponents(entity_uid, components) => {
@@ -655,12 +664,26 @@ impl World
                 entity_uid,
                 (comp_id, comp_name, component_parts.data),
             ) {
-                tracing::error!("Failed to add component {comp_name} to entity: {err}");
+                let ent_name = Self::get_component_of_entity::<EntityName>(
+                    component_storage,
+                    entity_uid,
+                );
+
+                tracing::error!(
+                    entity = %entity_uid,
+                    entity_name = ent_name.as_ref().map(|ent_name| ent_name.name.as_ref()),
+                    component = %comp_id,
+                    component_name = comp_name,
+                    "Failed to add component to entity: {err}"
+                );
+
                 continue;
             }
 
             if comp_id.is_pair() {
-                if let Some(pair_relation_metadata) = &component_parts.pair_relation_metadata {
+                if let Some(pair_relation_metadata) =
+                    &component_parts.pair_relation_metadata
+                {
                     Self::create_component_info_entity_if_missing(
                         comp_id.relation(),
                         pair_relation_metadata.name,
@@ -670,7 +693,8 @@ impl World
                     );
                 }
 
-                if let Some(pair_target_metadata) = &component_parts.pair_target_metadata {
+                if let Some(pair_target_metadata) = &component_parts.pair_target_metadata
+                {
                     Self::create_component_info_entity_if_missing(
                         comp_id.target(),
                         pair_target_metadata.name,
@@ -729,18 +753,32 @@ impl World
         }
         .into_parts();
 
-        if let Err(err) = component_storage
-            .add_entity_component(comp_id, (comp_info_parts.id, comp_info_parts.name, comp_info_parts.data))
-        {
-            tracing::error!(
-                component = %type_name::<ComponentInfo>(),
-                "Failed to add component to entity: {err}"
-            );
-        }
+        match component_storage.add_entity_component(
+            comp_id,
+            (
+                comp_info_parts.id,
+                comp_info_parts.name,
+                comp_info_parts.data,
+            ),
+        ) {
+            Ok(()) => {}
+            Err(ComponentStorageError::EntityDoesNotExist(_)) => {
+                // Entity is created if it does not exist
+                unreachable!();
+            }
+            Err(ComponentStorageError::ComponentAlreadyInEntity { .. }) => {
+                // This function returns early if ComponentInfo is already present
+                unreachable!();
+            }
+            Err(ComponentStorageError::ComponentNotFoundInEntity { .. }) => {
+                // This error is never returned by add_entity_component
+                unreachable!();
+            }
+        };
     }
 
     fn remove_entity_components(
-        entity_uid: Uid,
+        entity_id: Uid,
         component_ids: impl IntoIterator<Item = Uid>,
         component_storage: &mut ComponentStorage,
     )
@@ -749,17 +787,54 @@ impl World
 
         for component_id in component_id_iter {
             if let Err(err) =
-                component_storage.remove_entity_component(entity_uid, component_id)
+                component_storage.remove_entity_component(entity_id, component_id)
             {
+                let ent_name = Self::get_component_of_entity::<EntityName>(
+                    component_storage,
+                    entity_id,
+                );
+
+                let component_info = Self::get_component_of_entity::<ComponentInfo>(
+                    component_storage,
+                    component_id,
+                );
+
                 tracing::error!(
-                    entity=%entity_uid,
-                    component=%component_id,
+                    entity = %entity_id,
+                    entity_name = ent_name.as_ref().map(|ent_name| ent_name.name.as_ref()),
+                    component = %component_id,
+                    component_name = component_info
+                        .as_ref()
+                        .map(|component_info| component_info.name),
                     "Failed to remove component from entity: {err}"
                 );
             }
         }
     }
 
+    pub fn get_component_of_entity<ComponentT: Component>(
+        component_storage: &ComponentStorage,
+        entity_id: Uid,
+    ) -> Option<crate::component::Handle<'_, ComponentT>>
+    {
+        let archetype = component_storage.get_entity_archetype(entity_id)?;
+
+        let Some(entity) = archetype.get_entity_by_id(entity_id) else {
+            unreachable!("Should exist since archetype was found by entity id");
+        };
+
+        let comp_index = archetype.get_index_for_component(ComponentT::id())?;
+
+        let comp = &entity.components()[comp_index];
+
+        crate::component::Handle::from_entity_component_ref(&EntityComponentRef {
+            component_id: ComponentT::id(),
+            component: comp,
+            entity_id,
+        })
+        .ok()
+    }
+
     fn emit_event_observers(&self, event_id: Uid, emitted_event: &EmittedEvent<'_>)
     {
         assert!(event_id.is_pair());