Replaced the BorrowAs trait with ObjectOrVariant trait. (#841)

* replaced the BorrowAs trait with ObjectOrVariant trait; added explicit trait bounds in pool and graph's typed_ref method.

* fixed tests

* fixed restore_integrity test

- it was failing spontaneously due do conflicting resource registries from two tests

* fixed clippy warnings

* install rustfmt + clippy on a CI machine before running the tests

* fixed confusing documentation about batching

* updated changelog

* changelog fixes

* ability to set runs via respective message

* replaced the BorrowAs trait with ObjectOrVariant trait; added explicit trait bounds in pool and graph's typed_ref method.

* removed unused imports

* satisfied cargo doc

---------

Co-authored-by: Dmitry <[email protected]>

fyrox-core/src/pool/mod.rs

@@ -74,23 +74,60 @@ where
     free_stack: Vec<u32>,
 }
 
-pub trait BorrowAs<Object, Container: PayloadContainer<Element = Object>> {
-    type Target;
-    fn borrow_as_ref(self, pool: &Pool<Object, Container>) -> Option<&Self::Target>;
-    fn borrow_as_mut(self, pool: &mut Pool<Object, Container>) -> Option<&mut Self::Target>;
+/// This trait unifies pool objects and their variants.
+///
+/// If T is the pool object type, [`ObjectOrVariant::convert_to_dest_type`] returns the pool object itself.
+///
+/// If T is a variant of the pool object type, [`ObjectOrVariant::convert_to_dest_type`] returns the variant of the pool object.
+///
+/// The [`Pool`] struct uses this trait to unify the logic of retrieving both pool objects and their variants.
+pub trait ObjectOrVariant<T> {
+    fn convert_to_dest_type(object: &T) -> Option<&Self>;
+    fn convert_to_dest_type_mut(object: &mut T) -> Option<&mut Self>;
 }
 
-impl<Object, Container: PayloadContainer<Element = Object> + 'static> BorrowAs<Object, Container>
-    for Handle<Object>
-{
-    type Target = Object;
+/// This trait is a helper trait for implementing [`ObjectOrVariant`] indirectly in child crates.
+///
+/// To implement [`ObjectOrVariant`] for type `U` in a child crate, you need to implement [`ObjectOrVariantHelper`] for [`PhantomData<U>`].
+///
+/// This is necessary because Rust does not support `impl<T> ForeignTrait<LocalType> for T`
+///
+/// But Rust does support `impl<T> ForeignTrait<LocalType> for ForeignType<T>`
+///
+/// Details can be found in [here](https://rust-lang.github.io/rfcs/2451-re-rebalancing-coherence.html#concrete-orphan-rules).
+///
+/// Therefore, we cannot implement [`ObjectOrVariant`] directly using `impl<U: TraitBound> ObjectOrVariant<LocalType> for U`
+///
+/// but we can do `impl<U: TraitBound> ObjectOrVariantHelper<LocalType, U> for PhantomData<U>`
+///
+/// There is an indirection to get rid of [`PhantomData<U>`] because we want to have a clean API for pool methods
+/// where you can pass in `U` directly instead of [`PhantomData<U>`].
+pub trait ObjectOrVariantHelper<T, U> {
+    fn convert_to_dest_type_helper(object: &T) -> Option<&U>;
+    fn convert_to_dest_type_helper_mut(object: &mut T) -> Option<&mut U>;
+}
 
-    fn borrow_as_ref(self, pool: &Pool<Object, Container>) -> Option<&Object> {
-        pool.try_borrow(self)
+// This is the default implementation for pool object types.
+// For pool object variant types, they need to be implemented case by case, since Rust does not support multiple blanket implementations.
+impl<T> ObjectOrVariantHelper<T, T> for PhantomData<T> {
+    fn convert_to_dest_type_helper(object: &T) -> Option<&T> {
+        Some(object)
     }
+    fn convert_to_dest_type_helper_mut(object: &mut T) -> Option<&mut T> {
+        Some(object)
+    }
+}
 
-    fn borrow_as_mut(self, pool: &mut Pool<Object, Container>) -> Option<&mut Object> {
-        pool.try_borrow_mut(self)
+// This blanket implementation wraps types that implement ObjectOrVariantHelper with the ObjectOrVariant trait.
+impl<T, U> ObjectOrVariant<T> for U
+where
+    PhantomData<U>: ObjectOrVariantHelper<T, U>,
+{
+    fn convert_to_dest_type(object: &T) -> Option<&Self> {
+        PhantomData::<U>::convert_to_dest_type_helper(object)
+    }
+    fn convert_to_dest_type_mut(object: &mut T) -> Option<&mut Self> {
+        PhantomData::<U>::convert_to_dest_type_helper_mut(object)
     }
 }
 
@@ -414,16 +451,15 @@ where
     }
 
     #[inline]
-    pub fn typed_ref<Ref>(&self, handle: impl BorrowAs<T, P, Target = Ref>) -> Option<&Ref> {
-        handle.borrow_as_ref(self)
+    pub fn typed_ref<U: ObjectOrVariant<T>>(&self, handle: Handle<U>) -> Option<&U> {
+        let pool_object = self.try_borrow(handle.transmute())?;
+        U::convert_to_dest_type(pool_object)
     }
 
     #[inline]
-    pub fn typed_mut<Ref>(
-        &mut self,
-        handle: impl BorrowAs<T, P, Target = Ref>,
-    ) -> Option<&mut Ref> {
-        handle.borrow_as_mut(self)
+    pub fn typed_mut<U: ObjectOrVariant<T>>(&mut self, handle: Handle<U>) -> Option<&mut U> {
+        let pool_object = self.try_borrow_mut(handle.transmute())?;
+        U::convert_to_dest_type_mut(pool_object)
     }
 
     #[inline]
@@ -1367,28 +1403,27 @@ where
     }
 }
 
-impl<Object, Container, Borrow, Ref> Index<Borrow> for Pool<Object, Container>
+impl<T, U, Container> Index<Handle<U>> for Pool<T, Container>
 where
-    Object: 'static,
-    Container: PayloadContainer<Element = Object> + 'static,
-    Borrow: BorrowAs<Object, Container, Target = Ref>,
+    T: 'static,
+    U: ObjectOrVariant<T>,
+    Container: PayloadContainer<Element = T> + 'static,
 {
-    type Output = Ref;
-
+    type Output = U;
     #[inline]
-    fn index(&self, index: Borrow) -> &Self::Output {
+    fn index(&self, index: Handle<U>) -> &Self::Output {
         self.typed_ref(index).expect("The handle must be valid!")
     }
 }
 
-impl<Object, Container, Borrow, Ref> IndexMut<Borrow> for Pool<Object, Container>
+impl<T, U, Container> IndexMut<Handle<U>> for Pool<T, Container>
 where
-    Object: 'static,
-    Container: PayloadContainer<Element = Object> + 'static,
-    Borrow: BorrowAs<Object, Container, Target = Ref>,
+    T: 'static,
+    U: ObjectOrVariant<T>,
+    Container: PayloadContainer<Element = T> + 'static,
 {
     #[inline]
-    fn index_mut(&mut self, index: Borrow) -> &mut Self::Output {
+    fn index_mut(&mut self, index: Handle<U>) -> &mut Self::Output {
         self.typed_mut(index).expect("The handle must be valid!")
     }
 }

fyrox-graph/src/lib.rs

@@ -25,7 +25,7 @@
 pub mod constructor;
 
 use fxhash::FxHashMap;
-use fyrox_core::pool::{BorrowAs, ErasedHandle, PayloadContainer};
+use fyrox_core::pool::{ErasedHandle, ObjectOrVariant, PayloadContainer};
 use fyrox_core::reflect::ReflectHandle;
 use fyrox_core::{
     log::{Log, MessageKind},
@@ -757,7 +757,7 @@ pub trait BaseSceneGraph: AbstractSceneGraph {
     ///   |_A_
     ///   |   |_B
     ///   |_Root
-    /// ```    
+    /// ```
     ///
     /// This method returns an instance of [`LinkScheme`], that could be used to revert the hierarchy
     /// back to its original. See [`Self::apply_link_scheme`] for more info.
@@ -827,6 +827,7 @@ pub trait BaseSceneGraph: AbstractSceneGraph {
 }
 
 pub trait SceneGraph: BaseSceneGraph {
+    type ObjectType: Sized;
     /// Creates new iterator that iterates over internal collection giving (handle; node) pairs.
     fn pair_iter(&self) -> impl Iterator<Item = (Handle<Self::Node>, &Self::Node)>;
 
@@ -838,15 +839,12 @@ pub trait SceneGraph: BaseSceneGraph {
     /// of nodes. It does *not* perform any tree traversal!
     fn linear_iter_mut(&mut self) -> impl Iterator<Item = &mut Self::Node>;
 
-    fn typed_ref<Ref>(
-        &self,
-        handle: impl BorrowAs<Self::Node, Self::NodeContainer, Target = Ref>,
-    ) -> Option<&Ref>;
+    fn typed_ref<U: ObjectOrVariant<Self::ObjectType>>(&self, handle: Handle<U>) -> Option<&U>;
 
-    fn typed_mut<Ref>(
+    fn typed_mut<U: ObjectOrVariant<Self::ObjectType>>(
         &mut self,
-        handle: impl BorrowAs<Self::Node, Self::NodeContainer, Target = Ref>,
-    ) -> Option<&mut Ref>;
+        handle: Handle<U>,
+    ) -> Option<&mut U>;
 
     /// Tries to borrow a node and fetch its component of specified type.
     #[inline]
@@ -1450,7 +1448,7 @@ mod test {
         AbstractSceneGraph, AbstractSceneNode, BaseSceneGraph, NodeHandleMap, NodeMapping,
         PrefabData, SceneGraph, SceneGraphNode,
     };
-    use fyrox_core::pool::BorrowAs;
+    use fyrox_core::pool::{ObjectOrVariant, ObjectOrVariantHelper};
     use fyrox_core::{
         define_as_any_trait,
         pool::{ErasedHandle, Handle, PayloadContainer, Pool},
@@ -1460,6 +1458,7 @@ mod test {
         NameProvider,
     };
     use fyrox_resource::{untyped::UntypedResource, Resource, ResourceData};
+    use std::marker::PhantomData;
     use std::{
         any::{Any, TypeId},
         error::Error,
@@ -1500,6 +1499,17 @@ mod test {
 
     pub trait NodeTrait: BaseNodeTrait + Reflect + Visit + ComponentProvider + NodeAsAny {}
 
+    // Essentially implements ObjectOrVariant for NodeTrait types.
+    // See ObjectOrVariantHelper for the cause of the indirection.
+    impl<T: NodeTrait> ObjectOrVariantHelper<Node, T> for PhantomData<T> {
+        fn convert_to_dest_type_helper(node: &Node) -> Option<&T> {
+            NodeAsAny::as_any(node.0.deref()).downcast_ref()
+        }
+        fn convert_to_dest_type_helper_mut(node: &mut Node) -> Option<&mut T> {
+            NodeAsAny::as_any_mut(node.0.deref_mut()).downcast_mut()
+        }
+    }
+
     #[derive(ComponentProvider, Debug)]
     pub struct Node(Box<dyn NodeTrait>);
 
@@ -1881,6 +1891,7 @@ mod test {
     }
 
     impl SceneGraph for Graph {
+        type ObjectType = Node;
         fn pair_iter(&self) -> impl Iterator<Item = (Handle<Self::Node>, &Self::Node)> {
             self.nodes.pair_iter()
         }
@@ -1893,17 +1904,11 @@ mod test {
             self.nodes.iter_mut()
         }
 
-        fn typed_ref<Ref>(
-            &self,
-            handle: impl BorrowAs<Self::Node, Self::NodeContainer, Target = Ref>,
-        ) -> Option<&Ref> {
+        fn typed_ref<U: ObjectOrVariant<Node>>(&self, handle: Handle<U>) -> Option<&U> {
             self.nodes.typed_ref(handle)
         }
 
-        fn typed_mut<Ref>(
-            &mut self,
-            handle: impl BorrowAs<Self::Node, Self::NodeContainer, Target = Ref>,
-        ) -> Option<&mut Ref> {
+        fn typed_mut<U: ObjectOrVariant<Node>>(&mut self, handle: Handle<U>) -> Option<&mut U> {
             self.nodes.typed_mut(handle)
         }
     }

fyrox-impl/src/scene/graph/mod.rs

@@ -66,7 +66,7 @@ use crate::{
         },
         mesh::Mesh,
         navmesh,
-        node::{container::NodeContainer, Node, NodeTrait, SyncContext, UpdateContext},
+        node::{container::NodeContainer, Node, SyncContext, UpdateContext},
         pivot::Pivot,
         sound::context::SoundContext,
         transform::TransformBuilder,
@@ -76,9 +76,9 @@ use crate::{
 };
 use bitflags::bitflags;
 use fxhash::{FxHashMap, FxHashSet};
-use fyrox_core::pool::BorrowAs;
+use fyrox_core::pool::ObjectOrVariant;
 use fyrox_graph::SceneGraphNode;
-use std::ops::{Deref, DerefMut};
+use std::ops::Deref;
 use std::{
     any::{Any, TypeId},
     fmt::Debug,
@@ -126,20 +126,6 @@ impl GraphPerformanceStatistics {
 /// A helper type alias for node pool.
 pub type NodePool = Pool<Node, NodeContainer>;
 
-impl<T: NodeTrait> BorrowAs<Node, NodeContainer> for Handle<T> {
-    type Target = T;
-
-    fn borrow_as_ref(self, pool: &NodePool) -> Option<&T> {
-        pool.try_borrow(self.transmute())
-            .and_then(|n| NodeAsAny::as_any(n.0.deref()).downcast_ref::<T>())
-    }
-
-    fn borrow_as_mut(self, pool: &mut NodePool) -> Option<&mut T> {
-        pool.try_borrow_mut(self.transmute())
-            .and_then(|n| NodeAsAny::as_any_mut(n.0.deref_mut()).downcast_mut::<T>())
-    }
-}
-
 /// See module docs.
 #[derive(Debug, Reflect)]
 pub struct Graph {
@@ -1640,20 +1626,20 @@ impl Graph {
     }
 }
 
-impl<T, B: BorrowAs<Node, NodeContainer, Target = T>> Index<B> for Graph {
+impl<T: ObjectOrVariant<Node>> Index<Handle<T>> for Graph {
     type Output = T;
 
     #[inline]
-    fn index(&self, typed_handle: B) -> &Self::Output {
-        self.typed_ref(typed_handle)
+    fn index(&self, index: Handle<T>) -> &Self::Output {
+        self.typed_ref(index)
             .expect("The node handle is invalid or the object it points to has different type.")
     }
 }
 
-impl<T, B: BorrowAs<Node, NodeContainer, Target = T>> IndexMut<B> for Graph {
+impl<T: ObjectOrVariant<Node>> IndexMut<Handle<T>> for Graph {
     #[inline]
-    fn index_mut(&mut self, typed_handle: B) -> &mut Self::Output {
-        self.typed_mut(typed_handle)
+    fn index_mut(&mut self, index: Handle<T>) -> &mut Self::Output {
+        self.typed_mut(index)
             .expect("The node handle is invalid or the object it points to has different type.")
     }
 }
@@ -1842,6 +1828,7 @@ impl BaseSceneGraph for Graph {
 }
 
 impl SceneGraph for Graph {
+    type ObjectType = Node;
     #[inline]
     fn pair_iter(&self) -> impl Iterator<Item = (Handle<Self::Node>, &Self::Node)> {
         self.pool.pair_iter()
@@ -1857,17 +1844,11 @@ impl SceneGraph for Graph {
         self.pool.iter_mut()
     }
 
-    fn typed_ref<Ref>(
-        &self,
-        handle: impl BorrowAs<Self::Node, Self::NodeContainer, Target = Ref>,
-    ) -> Option<&Ref> {
+    fn typed_ref<U: ObjectOrVariant<Node>>(&self, handle: Handle<U>) -> Option<&U> {
         self.pool.typed_ref(handle)
     }
 
-    fn typed_mut<Ref>(
-        &mut self,
-        handle: impl BorrowAs<Self::Node, Self::NodeContainer, Target = Ref>,
-    ) -> Option<&mut Ref> {
+    fn typed_mut<U: ObjectOrVariant<Node>>(&mut self, handle: Handle<U>) -> Option<&mut U> {
         self.pool.typed_mut(handle)
     }
 }

fyrox-impl/src/scene/node/mod.rs

@@ -61,10 +61,11 @@ use crate::{
         Scene,
     },
 };
-use fyrox_core::define_as_any_trait;
+use fyrox_core::{define_as_any_trait, pool::ObjectOrVariantHelper};
 use std::{
     any::{Any, TypeId},
     fmt::Debug,
+    marker::PhantomData,
     ops::{Deref, DerefMut},
 };
 
@@ -235,6 +236,17 @@ pub trait NodeTrait: BaseNodeTrait + Reflect + Visit + ComponentProvider {
     }
 }
 
+// Essentially implements ObjectOrVariant for NodeTrait types.
+// See ObjectOrVariantHelper for the cause of the indirection.
+impl<T: NodeTrait> ObjectOrVariantHelper<Node, T> for PhantomData<T> {
+    fn convert_to_dest_type_helper(node: &Node) -> Option<&T> {
+        NodeAsAny::as_any(node.0.deref()).downcast_ref()
+    }
+    fn convert_to_dest_type_helper_mut(node: &mut Node) -> Option<&mut T> {
+        NodeAsAny::as_any_mut(node.0.deref_mut()).downcast_mut()
+    }
+}
+
 /// Node is the basic building block for 3D scenes. It has multiple variants, but all of them share some
 /// common functionality:
 ///

fyrox-ui/src/control.rs

@@ -26,11 +26,12 @@ use crate::{
     widget::Widget,
     UiNode, UserInterface,
 };
-use fyrox_core::define_as_any_trait;
+use fyrox_core::{define_as_any_trait, pool::ObjectOrVariant};
 
 use fyrox_core::algebra::Matrix3;
 use std::{
     any::Any,
+    marker::PhantomData,
     ops::{Deref, DerefMut},
     sync::mpsc::Sender,
 };
@@ -356,3 +357,14 @@ pub trait Control:
         *self.allow_drop
     }
 }
+
+// Essentially implements ObjectOrVariant for NodeTrait types.
+// See ObjectOrVariantHelper for the cause of the indirection.
+impl<T: Control> ObjectOrVariant<UiNode> for PhantomData<T> {
+    fn convert_to_dest_type(node: &UiNode) -> Option<&Self> {
+        ControlAsAny::as_any(node.0.deref()).downcast_ref()
+    }
+    fn convert_to_dest_type_mut(node: &mut UiNode) -> Option<&mut Self> {
+        ControlAsAny::as_any_mut(node.0.deref_mut()).downcast_mut()
+    }
+}

fyrox-ui/src/lib.rs

@@ -340,8 +340,8 @@ use strum_macros::{AsRefStr, EnumString, VariantNames};
 pub use alignment::*;
 pub use build::*;
 pub use control::*;
-use fyrox_core::futures::future::join_all;
 use fyrox_core::log::Log;
+use fyrox_core::{futures::future::join_all, pool::ObjectOrVariant};
 use fyrox_graph::{
     AbstractSceneGraph, AbstractSceneNode, BaseSceneGraph, NodeHandleMap, NodeMapping, PrefabData,
     SceneGraph, SceneGraphNode,
@@ -355,7 +355,7 @@ use crate::style::resource::{StyleResource, StyleResourceExt};
 use crate::style::{Style, DEFAULT_STYLE};
 use crate::widget::WidgetMaterial;
 pub use fyrox_animation as generic_animation;
-use fyrox_core::pool::{BorrowAs, ErasedHandle};
+use fyrox_core::pool::ErasedHandle;
 use fyrox_resource::untyped::ResourceKind;
 pub use fyrox_texture as texture;
 
@@ -674,20 +674,6 @@ pub struct UiUpdateSwitches {
 
 pub type WidgetPool = Pool<UiNode, WidgetContainer>;
 
-impl<T: Control> BorrowAs<UiNode, WidgetContainer> for Handle<T> {
-    type Target = T;
-
-    fn borrow_as_ref(self, pool: &WidgetPool) -> Option<&T> {
-        pool.try_borrow(self.transmute())
-            .and_then(|n| ControlAsAny::as_any(n.0.deref()).downcast_ref::<T>())
-    }
-
-    fn borrow_as_mut(self, pool: &mut WidgetPool) -> Option<&mut T> {
-        pool.try_borrow_mut(self.transmute())
-            .and_then(|n| ControlAsAny::as_any_mut(n.0.deref_mut()).downcast_mut::<T>())
-    }
-}
-
 #[derive(Reflect, Debug)]
 pub struct UserInterface {
     screen_size: Vector2<f32>,
@@ -3383,6 +3369,7 @@ impl BaseSceneGraph for UserInterface {
 }
 
 impl SceneGraph for UserInterface {
+    type ObjectType = UiNode;
     #[inline]
     fn pair_iter(&self) -> impl Iterator<Item = (Handle<Self::Node>, &Self::Node)> {
         self.nodes.pair_iter()
@@ -3398,17 +3385,11 @@ impl SceneGraph for UserInterface {
         self.nodes.iter_mut()
     }
 
-    fn typed_ref<Ref>(
-        &self,
-        handle: impl BorrowAs<Self::Node, Self::NodeContainer, Target = Ref>,
-    ) -> Option<&Ref> {
+    fn typed_ref<U: ObjectOrVariant<UiNode>>(&self, handle: Handle<U>) -> Option<&U> {
         self.nodes.typed_ref(handle)
     }
 
-    fn typed_mut<Ref>(
-        &mut self,
-        handle: impl BorrowAs<Self::Node, Self::NodeContainer, Target = Ref>,
-    ) -> Option<&mut Ref> {
+    fn typed_mut<U: ObjectOrVariant<UiNode>>(&mut self, handle: Handle<U>) -> Option<&mut U> {
         self.nodes.typed_mut(handle)
     }
 }