add support for list

Author: makspll

assets/scripts/bevy_api.lua

@@ -15,6 +15,8 @@ function on_event()
     print(script)
     print(world)
 
+    print(world:test_vec({entity, entity})[1])
+
 
     local my_component_type = world:get_type_by_name("MyComponent")
 

crates/bevy_mod_scripting_core/src/bindings/allocator.rs

@@ -5,6 +5,7 @@ use std::any::{Any, TypeId};
 use std::cell::UnsafeCell;
 use std::collections::HashMap;
 use std::fmt::{Display, Formatter};
+use std::io::Read;
 use std::sync::Arc;
 
 #[derive(Clone, PartialEq, Eq, Hash, Debug, PartialOrd, Ord)]
@@ -18,6 +19,10 @@ impl ReflectAllocationId {
     pub(crate) fn new(id: usize) -> Self {
         Self(Arc::new(id))
     }
+
+    pub fn strong_count(&self) -> usize {
+        Arc::strong_count(&self.0)
+    }
 }
 
 /// Pointer which owns the value it points to, and will deallocate it when dropped
@@ -45,29 +50,28 @@ impl<T: ?Sized> Drop for OwningPtr<T> {
 }
 
 // yikes, the indirection. I need this to store boxed values too though
-#[derive(Clone, Debug)]
-pub enum ReflectAllocation {
-    Double(Arc<OwningPtr<dyn PartialReflect>>),
-    Single(Arc<UnsafeCell<dyn PartialReflect>>),
-}
+#[derive(Debug)]
+pub struct ReflectAllocation(Box<UnsafeCell<dyn PartialReflect>>);
 
-unsafe impl Send for ReflectAllocation {}
+// unsafe impl Send for ReflectAllocation {}
 unsafe impl Sync for ReflectAllocation {}
 
 impl ReflectAllocation {
     pub fn get_ptr(&self) -> *mut dyn PartialReflect {
-        match self {
-            ReflectAllocation::Double(v) => v.ptr,
-            ReflectAllocation::Single(v) => v.get(),
-        }
+        self.0.as_ref().get()
     }
-    pub fn new(value: Arc<UnsafeCell<dyn PartialReflect>>) -> Self {
-        Self::Single(value)
+
+    pub fn new(value: Box<dyn PartialReflect>) -> Self {
+        let value: Box<UnsafeCell<dyn PartialReflect>> = unsafe { std::mem::transmute(value) };
+        Self(value)
     }
 
-    pub fn new_boxed(value: Box<dyn PartialReflect>) -> Self {
-        let ptr = Box::leak::<'static>(value);
-        Self::Double(Arc::new(unsafe { OwningPtr::new(ptr) }))
+    /// Takes the value out of the allocation.
+    ///
+    /// # Safety
+    /// - Must only be done if no other references to this allocation exist at the same time
+    pub unsafe fn take(self) -> Box<dyn PartialReflect> {
+        std::mem::transmute(self.0)
     }
 }
 
@@ -113,36 +117,21 @@ pub struct ReflectAllocator {
 impl ReflectAllocator {
     /// Allocates a new [`Reflect`] value and returns an [`AllocationId`] which can be used to access it later.
     /// Use [`Self::allocate_boxed`] if you already have an allocated boxed value.
-    pub fn allocate<T: PartialReflect>(
-        &mut self,
-        value: T,
-    ) -> (ReflectAllocationId, ReflectAllocation) {
-        let type_id = value.get_represented_type_info().map(|i| i.type_id());
-
-        let id = ReflectAllocationId::new(self.allocations.len());
-        let index = id.id();
-        let value = ReflectAllocation::new(Arc::new(UnsafeCell::new(value)));
-        self.allocations.insert(id.clone(), value.clone());
-        if let Some(type_id) = type_id {
-            self.types.insert(index, type_id);
-        }
-        (id, value)
+    pub fn allocate<T: PartialReflect>(&mut self, value: T) -> ReflectAllocationId {
+        self.allocate_boxed(Box::new(value))
     }
 
-    pub fn allocate_boxed(
-        &mut self,
-        value: Box<dyn PartialReflect>,
-    ) -> (ReflectAllocationId, ReflectAllocation) {
+    pub fn allocate_boxed(&mut self, value: Box<dyn PartialReflect>) -> ReflectAllocationId {
         let type_id = value.get_represented_type_info().map(|i| i.type_id());
 
         let id = ReflectAllocationId::new(self.allocations.len());
         let index = id.id();
-        let value = ReflectAllocation::new_boxed(value);
-        self.allocations.insert(id.clone(), value.clone());
+        let value = ReflectAllocation::new(value);
+        self.allocations.insert(id.clone(), value);
         if let Some(type_id) = type_id {
             self.types.insert(index, type_id);
         }
-        (id, value)
+        id
     }
 
     // /// Moves the given boxed [`PartialReflect`] value into the allocator, returning an [`AllocationId`] which can be used to access it later
@@ -168,16 +157,33 @@ impl ReflectAllocator {
     //     (id, allocation)
     // }
 
-    pub fn get(&self, id: &ReflectAllocationId) -> Option<ReflectAllocation> {
-        self.allocations.get(id).cloned()
+    // pub fn get(&self, id: &ReflectAllocationId) -> Option<ReflectAllocation> {
+    //     self.allocations.get(id).cloned()
+    // }
+
+    pub fn insert(
+        &mut self,
+        id: ReflectAllocationId,
+        value: ReflectAllocation,
+    ) -> Option<ReflectAllocation> {
+        self.allocations.insert(id, value)
+    }
+
+    pub fn remove(&mut self, id: &ReflectAllocationId) -> Option<ReflectAllocation> {
+        println!("removing {:?}", id);
+        self.allocations.remove(id)
     }
 
     pub fn get_type_id(&self, id: &ReflectAllocationId) -> Option<TypeId> {
         self.types.get(&id.id()).cloned()
     }
 
-    pub fn get_mut(&self, id: &ReflectAllocationId) -> Option<ReflectAllocation> {
-        self.allocations.get(id).cloned()
+    pub fn get_mut(&mut self, id: &ReflectAllocationId) -> Option<&mut ReflectAllocation> {
+        self.allocations.get_mut(id)
+    }
+
+    pub fn get(&self, id: &ReflectAllocationId) -> Option<&ReflectAllocation> {
+        self.allocations.get(id)
     }
 
     /// Deallocates the [`PartialReflect`] value with the given [`AllocationId`]
@@ -199,7 +205,7 @@ mod test {
     #[test]
     fn test_reflect_allocator_garbage_clean() {
         let mut allocator = ReflectAllocator::default();
-        let (id, _) = allocator.allocate(0);
+        let id = allocator.allocate(0);
         assert_eq!(allocator.allocations.len(), 1);
         drop(id);
         allocator.clean_garbage_allocations();
@@ -209,7 +215,7 @@ mod test {
     #[test]
     fn test_reflect_allocator_garbage_clean_no_garbage() {
         let mut allocator = ReflectAllocator::default();
-        let (_id, _) = allocator.allocate(0);
+        let _id = allocator.allocate(0);
         assert_eq!(allocator.allocations.len(), 1);
         allocator.clean_garbage_allocations();
         assert_eq!(allocator.allocations.len(), 1);

crates/bevy_mod_scripting_core/src/bindings/function.rs

@@ -119,7 +119,7 @@ impl CallableWithAccess for DynamicFunction<'_> {
             Return::Ref(ref_) => ref_.into_script_value(world),
             Return::Mut(mut_ref) => mut_ref.into_script_value(world),
         })?
-        .map_err(|e| InteropError::function_interop_error(self.info(), None, e).into())
+        .map_err(|e| InteropError::function_interop_error(self.info(), None, e))
     }
 }
 
@@ -159,8 +159,12 @@ impl<I: Iterator<Item = ScriptValue>> IntoArgsListWithAccess for I {
             match value {
                 // TODO: I'd see the logic be a bit cleaner, this if is needed to support 'Raw' ScriptValue arguments
                 // as we do not want to claim any access since we're not using the value yet
+
+                // for owned values the from_script_value impl for ReflectReferences will deal with the case
+                // here we try to make an actual reference, or otherwise clone the value
                 ScriptValue::Reference(arg_ref)
-                    if argument_info.type_id() != TypeId::of::<ScriptValue>() =>
+                    if argument_info.type_id() != TypeId::of::<ScriptValue>()
+                        && matches!(argument_info.ownership(), Ownership::Mut | Ownership::Ref) =>
                 {
                     let access_id = ReflectAccessId::for_reference(arg_ref.base.base_id.clone())
                         .ok_or_else(|| {
@@ -171,30 +175,39 @@ impl<I: Iterator<Item = ScriptValue>> IntoArgsListWithAccess for I {
                             )
                         })?;
 
-                    let is_write = matches!(argument_info.ownership(), Ownership::Mut);
-
-                    let success = if is_write {
-                        world.claim_write_access(access_id)
-                    } else {
-                        world.claim_read_access(access_id)
+                    let arg = match argument_info.ownership() {
+                        Ownership::Ref => {
+                            if world.claim_read_access(access_id) {
+                                accesses.push((access_id, Ownership::Ref));
+                                let ref_ = unsafe { arg_ref.reflect_unsafe(world.clone()) };
+                                if let Ok(ref_) = ref_ {
+                                    Ok(ArgValue::Ref(ref_))
+                                } else {
+                                    Err(ref_.expect_err("invariant"))
+                                }
+                            } else {
+                                Err(InteropError::cannot_claim_access(arg_ref.base.clone()))
+                            }
+                        }
+                        Ownership::Mut => {
+                            if world.claim_write_access(access_id) {
+                                accesses.push((access_id, Ownership::Mut));
+                                let mut_ref = unsafe { arg_ref.reflect_mut_unsafe(world.clone()) };
+                                if let Ok(mut_ref) = mut_ref {
+                                    Ok(ArgValue::Mut(mut_ref))
+                                } else {
+                                    Err(mut_ref.expect_err("invariant"))
+                                }
+                            } else {
+                                Err(InteropError::cannot_claim_access(arg_ref.base.clone()))
+                            }
+                        }
+                        _ => unreachable!(),
                     };
 
-                    if !success {
-                        release_accesses(&mut accesses);
-                        return Err(InteropError::function_interop_error(
-                            function_info,
-                            Some(argument_info),
-                            InteropError::cannot_claim_access(arg_ref.base.clone()),
-                        )
-                        .into());
-                    }
-
-                    accesses.push((access_id, argument_info.ownership()));
+                    // TODO: check if the value is actually a `dyn Reflect` and not a dynamic
 
-                    let val =
-                        unsafe { arg_ref.clone().into_arg_value(world.clone(), argument_info) };
-                    let val = match val {
-                        Ok(v) => v,
+                    match arg {
                         Err(e) => {
                             release_accesses(&mut accesses);
                             return Err(InteropError::function_interop_error(
@@ -203,8 +216,8 @@ impl<I: Iterator<Item = ScriptValue>> IntoArgsListWithAccess for I {
                                 e,
                             ));
                         }
-                    };
-                    arg_list.push(val);
+                        Ok(arg) => arg_list.push(arg),
+                    }
                 }
                 ScriptValue::World => {
                     arg_list.push(ArgValue::Owned(Box::new(WorldCallbackAccess::from_guard(

crates/bevy_mod_scripting_core/src/bindings/pretty_print.rs

@@ -293,10 +293,10 @@ impl ReflectReferencePrinter {
             ReflectRef::Function(f) => {
                 output.push_str("Function(");
                 output.push_str(
-                    &f.info()
+                    f.info()
                         .name()
                         .unwrap_or(&Cow::Borrowed("<unnamed function>"))
-                        .to_string(),
+                        .as_ref(),
                 );
                 output.push(')');
             }
@@ -327,7 +327,7 @@ macro_rules! impl_dummy_display (
     ($t:ty) => {
         impl std::fmt::Display for $t {
             fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-                write!(f, "Displaying {} without world access: {:#?}", stringify!($t), self);
+                write!(f, "Displaying {} without world access: {:#?}", stringify!($t), self)?;
                 Ok(())
             }
         }
@@ -399,6 +399,16 @@ impl DisplayWithWorld for ScriptValue {
             ScriptValue::String(cow) => cow.to_string(),
             ScriptValue::World => "World".to_owned(),
             ScriptValue::Error(script_error) => script_error.to_string(),
+            ScriptValue::List(vec) => {
+                let mut string = String::new();
+                ReflectReferencePrinter::pretty_print_key_values(
+                    BracketType::Square,
+                    vec.iter()
+                        .map(|v| (None::<String>, v.display_value_with_world(world.clone()))),
+                    &mut string,
+                );
+                string
+            }
         }
     }
 }