refactor(ecs): make FromLockedOptional not take Lock

4 files changed, 139 insertions, 94 deletions

diff --git a/ecs/src/component.rs b/ecs/src/component.rs
index 9ec962d..c01fd75 100644
--- a/ecs/src/component.rs
+++ b/ecs/src/component.rs
@@ -1,4 +1,5 @@
 use std::any::{type_name, Any};
+use std::error::Error;
 use std::fmt::Debug;
 use std::ops::{Deref, DerefMut};
 
@@ -11,7 +12,6 @@ use crate::event::component::{
 };
 use crate::lock::{
     Error as LockError,
-    Lock,
     MappedReadGuard,
     MappedWriteGuard,
     ReadGuard,
@@ -20,7 +20,7 @@ use crate::lock::{
 use crate::system::Input as SystemInput;
 use crate::uid::Uid;
 use crate::util::Array;
-use crate::World;
+use crate::{EntityComponentRef, World};
 
 pub mod local;
 
@@ -33,11 +33,11 @@ pub trait Component: SystemInput + Any
     where
         Self: Sized;
 
-    type HandleMut<'component>: FromLockedOptional<'component>
+    type HandleMut<'component>: HandleFromEntityComponentRef<'component>
     where
         Self: Sized;
 
-    type Handle<'component>: FromLockedOptional<'component>
+    type Handle<'component>: HandleFromEntityComponentRef<'component>
     where
         Self: Sized;
 
@@ -97,8 +97,8 @@ impl Debug for dyn Component
 impl<ComponentT> Component for Option<ComponentT>
 where
     ComponentT: Component,
-    for<'a> Option<ComponentT::Handle<'a>>: FromLockedOptional<'a>,
-    for<'a> Option<ComponentT::HandleMut<'a>>: FromLockedOptional<'a>,
+    for<'a> Option<ComponentT::Handle<'a>>: HandleFromEntityComponentRef<'a>,
+    for<'a> Option<ComponentT::HandleMut<'a>>: HandleFromEntityComponentRef<'a>,
 {
     type Component = ComponentT;
     type Handle<'component> = Option<ComponentT::Handle<'component>>;
@@ -155,7 +155,7 @@ pub trait Sequence
 pub trait Ref
 {
     type Component: Component;
-    type Handle<'component>: FromLockedOptional<'component>;
+    type Handle<'component>: HandleFromEntityComponentRef<'component>;
 }
 
 impl<ComponentT> Ref for &ComponentT
@@ -201,16 +201,15 @@ impl Metadata
     }
 }
 
-pub trait FromLockedOptional<'comp>: Sized
+pub trait HandleFromEntityComponentRef<'comp>: Sized
 {
-    /// Converts a reference to a optional locked boxed component to a instance of `Self`.
-    ///
-    /// # Errors
-    /// Returns `Err` if taking the lock (in a non-blocking way) fails.
-    fn from_locked_optional_component(
-        optional_component: Option<&'comp Lock<Box<dyn Component>>>,
+    type Error: Error;
+
+    /// Creates a new handle instance from a [`EntityComponentRef`].
+    fn from_entity_component_ref(
+        entity_component_ref: Option<EntityComponentRef<'comp>>,
         world: &'comp World,
-    ) -> Result<Self, LockError>;
+    ) -> Result<Self, Self::Error>;
 }
 
 #[derive(Debug)]
@@ -236,36 +235,49 @@ impl<'a, ComponentT: Component> Handle<'a, ComponentT>
     }
 }
 
-impl<'component, ComponentT: Component> FromLockedOptional<'component>
-    for Handle<'component, ComponentT>
+impl<'comp, ComponentT: Component> HandleFromEntityComponentRef<'comp>
+    for Handle<'comp, ComponentT>
 {
-    fn from_locked_optional_component(
-        optional_component: Option<&'component crate::lock::Lock<Box<dyn Component>>>,
-        _world: &'component World,
-    ) -> Result<Self, LockError>
-    {
-        let component = optional_component.unwrap_or_else(|| {
-            panic!(
-                "Component {} was not found in entity",
-                type_name::<ComponentT>()
-            );
-        });
+    type Error = HandleError;
 
-        Ok(Self::new(component.read_nonblock()?))
+    fn from_entity_component_ref(
+        entity_component_ref: Option<EntityComponentRef<'comp>>,
+        _world: &'comp World,
+    ) -> Result<Self, Self::Error>
+    {
+        let entity_comp =
+            entity_component_ref.ok_or(HandleError::ComponentDoesNotExist)?;
+
+        Ok(Self::new(
+            entity_comp
+                .component()
+                .read_nonblock()
+                .map_err(AcquireComponentLockFailed)?,
+        ))
     }
 }
 
-impl<'comp, ComponentT> FromLockedOptional<'comp> for Option<Handle<'comp, ComponentT>>
+impl<'comp, ComponentT> HandleFromEntityComponentRef<'comp>
+    for Option<Handle<'comp, ComponentT>>
 where
     ComponentT: Component,
 {
-    fn from_locked_optional_component(
-        optional_component: Option<&'comp Lock<Box<dyn Component>>>,
+    type Error = HandleError;
+
+    fn from_entity_component_ref(
+        entity_component_ref: Option<EntityComponentRef<'comp>>,
         _world: &'comp World,
-    ) -> Result<Self, LockError>
+    ) -> Result<Self, Self::Error>
     {
-        optional_component
-            .map(|lock| Ok(Handle::new(lock.read_nonblock()?)))
+        entity_component_ref
+            .map(|entity_comp| {
+                Ok(Handle::new(
+                    entity_comp
+                        .component()
+                        .read_nonblock()
+                        .map_err(AcquireComponentLockFailed)?,
+                ))
+            })
             .transpose()
     }
 }
@@ -303,36 +315,49 @@ impl<'a, ComponentT: Component> HandleMut<'a, ComponentT>
     }
 }
 
-impl<'component, ComponentT: Component> FromLockedOptional<'component>
-    for HandleMut<'component, ComponentT>
+impl<'comp, ComponentT: Component> HandleFromEntityComponentRef<'comp>
+    for HandleMut<'comp, ComponentT>
 {
-    fn from_locked_optional_component(
-        optional_component: Option<&'component Lock<Box<dyn Component>>>,
-        _world: &'component World,
-    ) -> Result<Self, LockError>
-    {
-        let component = optional_component.unwrap_or_else(|| {
-            panic!(
-                "Component {} was not found in entity",
-                type_name::<ComponentT>()
-            );
-        });
+    type Error = HandleError;
 
-        Ok(Self::new(component.write_nonblock()?))
+    fn from_entity_component_ref(
+        entity_component_ref: Option<EntityComponentRef<'comp>>,
+        _world: &'comp World,
+    ) -> Result<Self, Self::Error>
+    {
+        let entity_comp =
+            entity_component_ref.ok_or(HandleError::ComponentDoesNotExist)?;
+
+        Ok(Self::new(
+            entity_comp
+                .component()
+                .write_nonblock()
+                .map_err(AcquireComponentLockFailed)?,
+        ))
     }
 }
 
-impl<'comp, ComponentT> FromLockedOptional<'comp> for Option<HandleMut<'comp, ComponentT>>
+impl<'comp, ComponentT> HandleFromEntityComponentRef<'comp>
+    for Option<HandleMut<'comp, ComponentT>>
 where
     ComponentT: Component,
 {
-    fn from_locked_optional_component(
-        optional_component: Option<&'comp Lock<Box<dyn Component>>>,
+    type Error = HandleError;
+
+    fn from_entity_component_ref(
+        entity_component_ref: Option<EntityComponentRef<'comp>>,
         _world: &'comp World,
-    ) -> Result<Self, LockError>
+    ) -> Result<Self, Self::Error>
     {
-        optional_component
-            .map(|lock| Ok(HandleMut::new(lock.write_nonblock()?)))
+        entity_component_ref
+            .map(|entity_comp| {
+                Ok(HandleMut::new(
+                    entity_comp
+                        .component()
+                        .write_nonblock()
+                        .map_err(AcquireComponentLockFailed)?,
+                ))
+            })
             .transpose()
     }
 }
@@ -355,6 +380,20 @@ impl<ComponentT: Component> DerefMut for HandleMut<'_, ComponentT>
     }
 }
 
+#[derive(Debug, thiserror::Error)]
+pub enum HandleError
+{
+    #[error(transparent)]
+    AcquireComponentLockFailed(#[from] AcquireComponentLockFailed),
+
+    #[error("Component does not exist")]
+    ComponentDoesNotExist,
+}
+
+#[derive(Debug, thiserror::Error)]
+#[error(transparent)]
+pub struct AcquireComponentLockFailed(LockError);
+
 macro_rules! inner {
     ($c: tt) => {
         seq!(I in 0..=$c {
diff --git a/ecs/src/lib.rs b/ecs/src/lib.rs
index 2c653ee..254ee12 100644
--- a/ecs/src/lib.rs
+++ b/ecs/src/lib.rs
@@ -618,7 +618,7 @@ pub struct EntityComponentRef<'a>
 
 impl<'a> EntityComponentRef<'a>
 {
-    pub fn component(&self) -> &'a Lock<Box<dyn Component>>
+    fn component(&self) -> &'a Lock<Box<dyn Component>>
     {
         self.comp.component()
     }
diff --git a/ecs/src/query.rs b/ecs/src/query.rs
index b29db3d..f642156 100644
--- a/ecs/src/query.rs
+++ b/ecs/src/query.rs
@@ -3,7 +3,7 @@ use std::marker::PhantomData;
 
 use seq_macro::seq;
 
-use crate::component::{Component, FromLockedOptional, Ref as ComponentRef};
+use crate::component::{Component, HandleFromEntityComponentRef, Ref as ComponentRef};
 use crate::entity::Handle as EntityHandle;
 use crate::query::flexible::{Iter as FlexibleQueryIter, Query as FlexibleQuery};
 use crate::system::{Param as SystemParam, System};
@@ -314,15 +314,13 @@ impl<ComponentRefT: ComponentRef> TermWithField for ComponentRefT
         world: &'world World,
     ) -> Self::Field<'world>
     {
-        Self::Field::from_locked_optional_component(
-            entity_handle
-                .get_component(ComponentRefT::Component::id())
-                .map(|component| component.component()),
+        Self::Field::from_entity_component_ref(
+            entity_handle.get_component(ComponentRefT::Component::id()),
             world,
         )
         .unwrap_or_else(|err| {
             panic!(
-                "Taking component {} lock failed: {err}",
+                "Creating handle to component {} failed: {err}",
                 type_name::<ComponentRefT::Component>()
             );
         })
diff --git a/ecs/src/relationship.rs b/ecs/src/relationship.rs
index 45fa265..c5399e4 100644
--- a/ecs/src/relationship.rs
+++ b/ecs/src/relationship.rs
@@ -6,13 +6,14 @@ use ecs_macros::Component;
 use crate::component::storage::Storage as ComponentStorage;
 use crate::component::{
     Component,
-    FromLockedOptional as FromLockedOptionalComponent,
     Handle as ComponentHandle,
+    HandleError as ComponentHandleError,
+    HandleFromEntityComponentRef,
     HandleMut as ComponentHandleMut,
 };
-use crate::lock::{Error as LockError, Lock, ReadGuard};
+use crate::lock::ReadGuard;
 use crate::uid::{Kind as UidKind, Uid};
-use crate::World;
+use crate::{EntityComponentRef, World};
 
 /// A relationship to one or more targets.
 #[derive(Debug, Component)]
@@ -70,22 +71,24 @@ where
     relationship_comp: ComponentHandleMut<'rel_comp, Relationship<Kind, ComponentT>>,
 }
 
-impl<'rel_comp, Kind, ComponentT> FromLockedOptionalComponent<'rel_comp>
+impl<'rel_comp, Kind, ComponentT> HandleFromEntityComponentRef<'rel_comp>
     for RelationMut<'rel_comp, Kind, ComponentT>
 where
     ComponentT: Component,
 {
-    fn from_locked_optional_component(
-        optional_component: Option<&'rel_comp crate::lock::Lock<Box<dyn Component>>>,
+    type Error = ComponentHandleError;
+
+    fn from_entity_component_ref(
+        entity_component_ref: Option<EntityComponentRef<'rel_comp>>,
         world: &'rel_comp World,
-    ) -> Result<Self, LockError>
+    ) -> Result<Self, Self::Error>
     {
-        let relationship_comp_handle_from_locked_opt_comp = ComponentHandleMut::<
+        let relationship_comp_handle_from_ent_comp_ref = ComponentHandleMut::<
             Relationship<Kind, ComponentT>,
-        >::from_locked_optional_component;
+        >::from_entity_component_ref;
 
         let relationship_comp =
-            relationship_comp_handle_from_locked_opt_comp(optional_component, world)?;
+            relationship_comp_handle_from_ent_comp_ref(entity_component_ref, world)?;
 
         let component_storage_lock = world
             .data
@@ -100,19 +103,21 @@ where
     }
 }
 
-impl<'rel_comp, Kind, ComponentT> FromLockedOptionalComponent<'rel_comp>
+impl<'rel_comp, Kind, ComponentT> HandleFromEntityComponentRef<'rel_comp>
     for Option<RelationMut<'rel_comp, Kind, ComponentT>>
 where
     ComponentT: Component,
 {
-    fn from_locked_optional_component(
-        optional_component: Option<&'rel_comp Lock<Box<dyn Component>>>,
+    type Error = ComponentHandleError;
+
+    fn from_entity_component_ref(
+        entity_component_ref: Option<EntityComponentRef<'rel_comp>>,
         world: &'rel_comp World,
-    ) -> Result<Self, crate::lock::Error>
+    ) -> Result<Self, Self::Error>
     {
-        optional_component
-            .map(|component| {
-                RelationMut::from_locked_optional_component(Some(component), world)
+        entity_component_ref
+            .map(|entity_comp| {
+                RelationMut::from_entity_component_ref(Some(entity_comp), world)
             })
             .transpose()
     }
@@ -292,22 +297,23 @@ where
     relationship_comp: ComponentHandle<'rel_comp, Relationship<Kind, ComponentT>>,
 }
 
-impl<'rel_comp, Kind, ComponentT> FromLockedOptionalComponent<'rel_comp>
+impl<'rel_comp, Kind, ComponentT> HandleFromEntityComponentRef<'rel_comp>
     for Relation<'rel_comp, Kind, ComponentT>
 where
     ComponentT: Component,
 {
-    fn from_locked_optional_component(
-        optional_component: Option<&'rel_comp Lock<Box<dyn Component>>>,
+    type Error = ComponentHandleError;
+
+    fn from_entity_component_ref(
+        entity_component_ref: Option<EntityComponentRef<'rel_comp>>,
         world: &'rel_comp World,
-    ) -> Result<Self, LockError>
+    ) -> Result<Self, Self::Error>
     {
-        let relationship_comp_handle_from_locked_opt_comp = ComponentHandle::<
-            Relationship<Kind, ComponentT>,
-        >::from_locked_optional_component;
+        let relationship_comp_handle_from_ent_comp_ref =
+            ComponentHandle::<Relationship<Kind, ComponentT>>::from_entity_component_ref;
 
         let relationship_comp =
-            relationship_comp_handle_from_locked_opt_comp(optional_component, world)?;
+            relationship_comp_handle_from_ent_comp_ref(entity_component_ref, world)?;
 
         let component_storage_lock = world
             .data
@@ -322,19 +328,21 @@ where
     }
 }
 
-impl<'rel_comp, Kind, ComponentT> FromLockedOptionalComponent<'rel_comp>
+impl<'rel_comp, Kind, ComponentT> HandleFromEntityComponentRef<'rel_comp>
     for Option<Relation<'rel_comp, Kind, ComponentT>>
 where
     ComponentT: Component,
 {
-    fn from_locked_optional_component(
-        optional_component: Option<&'rel_comp Lock<Box<dyn Component>>>,
+    type Error = ComponentHandleError;
+
+    fn from_entity_component_ref(
+        entity_component_ref: Option<EntityComponentRef<'rel_comp>>,
         world: &'rel_comp World,
-    ) -> Result<Self, crate::lock::Error>
+    ) -> Result<Self, ComponentHandleError>
     {
-        optional_component
-            .map(|component| {
-                Relation::from_locked_optional_component(Some(component), world)
+        entity_component_ref
+            .map(|entity_comp| {
+                Relation::from_entity_component_ref(Some(entity_comp), world)
             })
             .transpose()
     }