ecs: implement Bundle traits for structs

crates/lyra-ecs/lyra-ecs-derive/src/lib.rs

@@ -1,5 +1,5 @@
 use quote::quote;
-use syn::{parse_macro_input, DeriveInput};
+use syn::{parse_macro_input, spanned::Spanned, DeriveInput};
 
 #[proc_macro_derive(Component)]
 pub fn derive_component(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
@@ -17,3 +17,53 @@ pub fn derive_component(input: proc_macro::TokenStream) -> proc_macro::TokenStre
         }
     })
 }
+
+#[proc_macro_derive(Bundle)]
+pub fn derive_bundle(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
+    let input = parse_macro_input!(input as DeriveInput);
+    let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl();
+
+    let type_ident = &input.ident;
+
+    let s = match &input.data {
+        syn::Data::Struct(s) => {
+            s
+        },
+        _ => {
+            return syn::Error::new(input.span(), "Bundle derive macro only supports Structs")
+                .into_compile_error().into();
+        }
+    };
+
+    let field_names: Vec<_> = s.fields.iter().map(|f| f.ident.as_ref().unwrap()).collect();
+
+    proc_macro::TokenStream::from(quote! {
+        impl #impl_generics lyra_engine::ecs::Bundle for #type_ident #ty_generics #where_clause {
+            fn type_ids(&self) -> Vec<lyra_engine::ecs::DynTypeId> {
+                let mut v = vec![];
+                #(
+                    v.extend(self.#field_names.type_ids().into_iter());
+                )*
+                v
+            }
+        
+            fn info(&self) -> Vec<lyra_engine::ecs::ComponentInfo> {
+                let mut v = vec![];
+                #(
+                    v.extend(self.#field_names.info().into_iter());
+                )*
+                v
+            }
+        
+            fn take(self, f: &mut impl FnMut(std::ptr::NonNull<u8>, lyra_engine::ecs::DynTypeId, lyra_engine::ecs::ComponentInfo)) {
+                #(
+                    self.#field_names.take(f);
+                )*
+            }
+        
+            fn is_dynamic(&self) -> bool {
+                false
+            }
+        }
+    })
+}

crates/lyra-ecs/src/archetype.rs

@@ -348,7 +348,7 @@ impl Archetype {
         self.entity_ids.insert(entity, entity_index);
         self.entities.push(entity);
 
-        bundle.take(|data, type_id, info| {
+        bundle.take(&mut |data, type_id, info| {
             self.put_component_at(
                 tick,
                 data,
@@ -621,7 +621,7 @@ impl Archetype {
         }
 
         for (eid, bundle) in new_columns.into_iter().enumerate() {
-            bundle.take(|ptr, tyid, _size| unsafe {
+            bundle.take(&mut |ptr, tyid, _size| unsafe {
                 let col = self.get_column_mut(tyid).unwrap();
                 col.insert_entity(eid, ptr, tick.clone());
             });

crates/lyra-ecs/src/bundle.rs

@@ -11,7 +11,7 @@ pub trait Bundle {
 
     /// Take the bundle by calling the closure with pointers to each component, its type and size.
     /// The closure is expected to take ownership of the pointer.
-    fn take(self, f: impl FnMut(NonNull<u8>, DynTypeId, ComponentInfo));
+    fn take(self, f: &mut impl FnMut(NonNull<u8>, DynTypeId, ComponentInfo));
 
     /// Returns a boolean indicating if this Bundle is dynamic. See [`DynamicBundle`]
     fn is_dynamic(&self) -> bool;
@@ -26,7 +26,7 @@ impl Bundle for () {
         vec![ComponentInfo::new::<()>()]
     }
 
-    fn take(self, mut f: impl FnMut(NonNull<u8>, DynTypeId, ComponentInfo)) {
+    fn take(self, f: &mut impl FnMut(NonNull<u8>, DynTypeId, ComponentInfo)) {
         f(NonNull::from(&self).cast(), DynTypeId::of::<()>(), ComponentInfo::new::<()>());
     }
 
@@ -44,7 +44,7 @@ impl<C: Component> Bundle for C {
         vec![ComponentInfo::new::<C>()]
     }
 
-    fn take(self, mut f: impl FnMut(NonNull<u8>, DynTypeId, ComponentInfo)) {
+    fn take(self, f: &mut impl FnMut(NonNull<u8>, DynTypeId, ComponentInfo)) {
         f(NonNull::from(&self).cast(), DynTypeId::of::<C>(), ComponentInfo::new::<C>());
 
         // this must be done to avoid calling drop on heap memory that the component
@@ -59,29 +59,41 @@ impl<C: Component> Bundle for C {
 
 macro_rules! impl_bundle_tuple {
     ( $($name: ident),+ ) => (
+        // these names wont follow rust convention, but its a macro so deal with it
         #[allow(non_snake_case)]
-        impl<$($name: Component),+> Bundle for ($($name,)+) {
+        impl<$($name: Bundle),+> Bundle for ($($name,)+) {
+            #[inline(always)]
             fn type_ids(&self) -> Vec<DynTypeId> {
-                // these names wont follow rust convention, but its a macro so deal with it
+                let ($($name,)+) = self;
-                vec![$(DynTypeId::of::<$name>()),+]
+
+                let mut v = vec![];
+                $(
+                    v.extend($name.type_ids().into_iter());
+                )+
+                v
             }
 
+            #[inline(always)]
             fn info(&self) -> Vec<ComponentInfo> {
-                vec![$(ComponentInfo::new::<$name>()),+]
+                let ($($name,)+) = self;
+
+                let mut v = vec![];
+                $(
+                    v.extend($name.info().into_iter());
+                )+
+                v
             }
 
-            fn take(self, mut f: impl FnMut(NonNull<u8>, DynTypeId, ComponentInfo)) {
+            #[inline(always)]
-                // these names wont follow rust convention, but its a macro so deal with it
+            fn take(self, f: &mut impl FnMut(NonNull<u8>, DynTypeId, ComponentInfo)) {
                 let ($($name,)+) = self;
 
                 $(
-                    f(NonNull::from(&$name).cast(), DynTypeId::of::<$name>(), ComponentInfo::new::<$name>());
+                    $name.take(f);
-                    // this must be done to avoid calling drop on heap memory that the component
-                    // may manage. So something like a Vec, or HashMap, etc.
-                    std::mem::forget($name);
                 )+
             }
 
+            #[inline(always)]
             fn is_dynamic(&self) -> bool {
                 false
             }
@@ -166,7 +178,7 @@ impl DynamicBundle {
     where
         B: Bundle
     {
-        bundle.take(|ptr, _, info| {
+        bundle.take(&mut |ptr, _, info| {
             // unfortunately the components in the bundle must be copied since there is no guarantee that
             // `bundle` lasts for as long as the `DynamicBundle`. If the data wasn't copied, the pointers
             // could be invalid later.
@@ -192,7 +204,7 @@ impl Bundle for DynamicBundle {
         self.bundle.iter().map(|b| b.1).collect()
     }
 
-    fn take(self, mut f: impl FnMut(NonNull<u8>, DynTypeId, ComponentInfo)) {
+    fn take(self, f: &mut impl FnMut(NonNull<u8>, DynTypeId, ComponentInfo)) {
         for (data, info) in self.bundle.into_iter() {
             f(data, info.type_id(), info);
         }
@@ -202,3 +214,70 @@ impl Bundle for DynamicBundle {
         true
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use lyra_ecs_derive::{Bundle, Component};
+    use crate::{lyra_engine, ComponentInfo, World};
+
+    use super::Bundle;
+
+    #[allow(dead_code)]
+    #[derive(Component, PartialEq, Clone, Copy, Debug)]
+    struct Vec2 {
+        x: f32,
+        y: f32,
+    }
+
+    #[allow(dead_code)]
+    #[derive(Component, Debug, PartialEq, Eq, Clone, Copy)]
+    enum SomeFlag {
+        SomethingA,
+        SomethingB,
+    }
+
+    #[derive(Bundle)]
+    struct CompBundle {
+        pos: Vec2,
+        flag: SomeFlag
+    }
+
+    #[test]
+    fn check_bundle_order() {
+        let b = CompBundle {
+            pos: Vec2 {
+                x: 10.0, y: 10.0,
+            },
+            flag: SomeFlag::SomethingA,
+        };
+
+        let info = b.info();
+        let mut info = info.into_iter();
+
+        assert_eq!(info.next().unwrap(), ComponentInfo::new::<Vec2>());
+        assert_eq!(info.next().unwrap(), ComponentInfo::new::<SomeFlag>());
+    }
+
+    #[test]
+    fn check_bundle_spawn() {
+        let b_pos = Vec2 {
+            x: 10.0, y: 10.0,
+        };
+        let b_flag = SomeFlag::SomethingA;
+        let b = CompBundle {
+            pos: b_pos,
+            flag: b_flag,
+        };
+
+        let mut world = World::new();
+        let e = world.spawn(b);
+
+        let pos = world.view_one::<&Vec2>(e).get()
+            .expect("failed to find spawned Vec2 from Bundle on Entity");
+        assert!(pos.x == b_pos.x && pos.y == b_pos.y, "Spawned Vec2 values were not correct, got: {:?}, expected: {:?}", *pos, b_pos);
+        
+        let flag = world.view_one::<&SomeFlag>(e).get()
+            .expect("failed to find spawned SomeFlag from Bundle on Entity");
+        assert_eq!(*flag, b_flag);
+    }
+}

crates/lyra-ecs/src/query/filter/mod.rs

@@ -1,6 +1,4 @@
 mod has;
-use std::marker::PhantomData;
-
 pub use has::*;
 
 mod or;

crates/lyra-ecs/src/query/resource.rs

@@ -112,7 +112,7 @@ impl<'a, T: ResourceObject> Res<'a, T> {
 
     /// Returns a boolean indicating if the resource changed.
     pub fn changed(&self) -> bool {
-        *self.inner.tick >= *self.world_tick - 1
+        self.inner.changed(self.world_tick)
     }
 
     /// The tick that this resource was last modified at
@@ -236,7 +236,7 @@ impl<'a, T: ResourceObject> ResMut<'a, T> {
     }
 
     pub fn changed(&self) -> bool {
-        *self.inner.tick - 1 >= *self.world_tick - 1
+        self.inner.changed(self.world_tick)
     }
 
     /// The tick that this resource was last modified at

crates/lyra-ecs/src/resource.rs

@@ -2,7 +2,7 @@ use std::{any::{Any, TypeId}, sync::Arc};
 
 use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut};
 
-use crate::{Tick, TickTracker};
+use crate::Tick;
 
 /// Shorthand for `Send + Sync + 'static`, so it never needs to be implemented manually.
 pub trait ResourceObject: Send + Sync + Any {
@@ -25,6 +25,14 @@ pub struct TrackedResource<T: ?Sized> {
     pub res: T,
 }
 
+impl<T: ?Sized> TrackedResource<T> {
+    pub fn changed(&self, tick: Tick) -> bool {
+        let tick = tick.checked_sub(1).unwrap_or(0);
+        //println!("self: {}, world: {}", *self.tick, tick);
+        *self.tick >= tick
+    }
+}
+
 /// A type erased storage for a Resource.
 #[derive(Clone)]
 pub struct ResourceData {
@@ -89,6 +97,6 @@ impl ResourceData {
     }
 
     pub fn changed(&self, tick: Tick) -> bool {
-        *self.data.borrow().tick >= *tick - 1
+        self.data.borrow().changed(tick)
     }
 }

crates/lyra-ecs/src/world.rs

@@ -180,7 +180,7 @@ impl World {
             let entry_idx = *current_arch.entity_indexes()
                 .get(&entity).unwrap();
 
-            bundle.take(|ptr, id, _info| {
+            bundle.take(&mut |ptr, id, _info| {
                 let col = current_arch.get_column_mut(id).unwrap();
                 unsafe { col.set_at(entry_idx.0 as _, ptr, tick) };
             });
@@ -459,6 +459,8 @@ impl World {
     }
 
     /// Gets a resource from the World.
+    /// 
+    /// Returns `None` if the resource wasn't found, or is already borrowed.
     pub fn get_resource<T: ResourceObject>(&self) -> Option<Res<T>> {
         self.get_tracked_resource::<T>().map(|r| Res {
             inner: r,
@@ -476,20 +478,24 @@ impl World {
 
     /// Gets a reference to a change tracked resource.
     /// 
+    /// Returns `None` if the resource wasn't found, or is already borrowed.
+    ///
     /// You will have to manually downcast the inner resource. Most people don't need this, see
     /// [`World::get_resource`].
     pub fn get_tracked_resource<T: ResourceObject>(&self) -> Option<AtomicRef<TrackedResource<dyn ResourceObject>>> {
         self.resources.get(&TypeId::of::<T>())
-            .map(|r| r.data.borrow())
+            .and_then(|r| r.data.try_borrow().ok())
     }
 
     /// Gets a mutable borrow to a change tracked resource.
     /// 
+    /// Returns `None` if the resource wasn't found, or is already borrowed.
+    /// 
     /// You will have to manually downcast the inner resource. Most people don't need this, see
     /// [`World::get_resource_mut`].
     pub fn get_tracked_resource_mut<T: ResourceObject>(&self) -> Option<AtomicRefMut<TrackedResource<dyn ResourceObject>>> {
         self.resources.get(&TypeId::of::<T>())
-            .map(|r| r.data.borrow_mut())
+            .and_then(|r| r.data.try_borrow_mut().ok())
     }
 
     /// Returns a boolean indicating if the resource changed.
@@ -791,14 +797,18 @@ mod tests {
         world.add_resource(SimpleCounter(50));
 
         assert!(world.has_resource_changed::<SimpleCounter>());
+        world.tick();
 
         world.spawn(Vec2::new(50.0, 50.0));
+        world.tick();
      
         assert!(!world.has_resource_changed::<SimpleCounter>());
+        world.tick();
 
         let mut counter = world.get_resource_mut::<SimpleCounter>()
             .expect("Counter resource is missing");
         counter.0 += 100;
+        drop(counter);
 
         assert!(world.has_resource_changed::<SimpleCounter>());
     }