feat!: Remove WorldCallbackAccess & unify dynamic function context …

…arguments Reduces indirection and simplifies the entire interface, speeds up compile time for functions
makspll · Jan 20, 2025 · fabf5a7 · fabf5a7
1 parent 6c92a68
commit fabf5a7
Show file tree

Hide file tree

Showing 16 changed files with 692 additions and 612 deletions.
diff --git a/crates/bevy_mod_scripting_core/src/bindings/function/script_function.rs b/crates/bevy_mod_scripting_core/src/bindings/function/script_function.rs
@@ -2,10 +2,10 @@ use super::{from::FromScript, into::IntoScript, namespace::Namespace};
 use crate::{
     bindings::{
         function::from::{Mut, Ref, Val},
-        ReflectReference, WorldGuard,
+        ReflectReference, ThreadWorldContainer, WorldContainer, WorldGuard,
     },
     error::InteropError,
-    ScriptValue, WorldCallbackAccess,
+    ScriptValue,
 };
 use bevy::{
     prelude::{Reflect, Resource},
@@ -96,7 +96,8 @@ macro_rules! register_tuple_dependencies {
 }
 
 no_type_dependencies!(InteropError);
-self_type_dependency_only!(WorldCallbackAccess, CallerContext, ReflectReference);
+no_type_dependencies!(WorldGuard<'static>);
+self_type_dependency_only!(FunctionCallContext, ReflectReference);
 
 recursive_type_dependencies!(
     (Val<T> where T: GetTypeRegistration),
@@ -118,9 +119,21 @@ pub trait GetFunctionTypeDependencies<Marker> {
 /// Functions can choose to react to caller preferences such as converting 1-indexed numbers to 0-indexed numbers
 #[derive(Clone, Copy, Debug, Reflect, Default)]
 #[reflect(opaque)]
-pub struct CallerContext {
+pub struct FunctionCallContext {
     pub convert_to_0_indexed: bool,
 }
+impl FunctionCallContext {
+    pub fn new(convert_to_0_indexed: bool) -> Self {
+        Self {
+            convert_to_0_indexed,
+        }
+    }
+
+    /// Tries to access the world, returning an error if the world is not available
+    pub fn world(&self) -> Result<WorldGuard<'static>, InteropError> {
+        ThreadWorldContainer.try_get_world()
+    }
+}
 
 #[derive(Clone, Debug, PartialEq, Default)]
 pub struct FunctionInfo {
@@ -146,12 +159,7 @@ impl FunctionInfo {
 pub struct DynamicScriptFunction {
     pub info: FunctionInfo,
     // TODO: info about the function, this is hard right now because of non 'static lifetimes in wrappers, we can't use TypePath etc
-    func: Arc<
-        dyn Fn(CallerContext, WorldCallbackAccess, Vec<ScriptValue>) -> ScriptValue
-            + Send
-            + Sync
-            + 'static,
-    >,
+    func: Arc<dyn Fn(FunctionCallContext, Vec<ScriptValue>) -> ScriptValue + Send + Sync + 'static>,
 }
 
 impl PartialEq for DynamicScriptFunction {
@@ -167,10 +175,7 @@ pub struct DynamicScriptFunctionMut {
     func: Arc<
         RwLock<
             // I'd rather consume an option or something instead of having the RWLock but I just wanna get this release out
-            dyn FnMut(CallerContext, WorldCallbackAccess, Vec<ScriptValue>) -> ScriptValue
-                + Send
-                + Sync
-                + 'static,
+            dyn FnMut(FunctionCallContext, Vec<ScriptValue>) -> ScriptValue + Send + Sync + 'static,
         >,
     >,
 }
@@ -188,13 +193,11 @@ impl DynamicScriptFunction {
     pub fn call<I: IntoIterator<Item = ScriptValue>>(
         &self,
         args: I,
-        world: WorldGuard,
-        context: CallerContext,
+        context: FunctionCallContext,
     ) -> Result<ScriptValue, InteropError> {
         let args = args.into_iter().collect::<Vec<_>>();
-        let world_callback_access = WorldCallbackAccess::from_guard(world.clone());
         // should we be inlining call errors into the return value?
-        let return_val = (self.func)(context, world_callback_access, args);
+        let return_val = (self.func)(context, args);
         match return_val {
             ScriptValue::Error(e) => Err(InteropError::function_interop_error(
                 self.name(),
@@ -237,14 +240,12 @@ impl DynamicScriptFunctionMut {
     pub fn call<I: IntoIterator<Item = ScriptValue>>(
         &self,
         args: I,
-        world: WorldGuard,
-        context: CallerContext,
+        context: FunctionCallContext,
     ) -> Result<ScriptValue, InteropError> {
         let args = args.into_iter().collect::<Vec<_>>();
-        let world_callback_access = WorldCallbackAccess::from_guard(world.clone());
         // should we be inlining call errors into the return value?
         let mut write = self.func.write();
-        let return_val = (write)(context, world_callback_access, args);
+        let return_val = (write)(context, args);
         match return_val {
             ScriptValue::Error(e) => Err(InteropError::function_interop_error(
                 self.name(),
@@ -297,10 +298,7 @@ impl std::fmt::Debug for DynamicScriptFunctionMut {
 
 impl<F> From<F> for DynamicScriptFunction
 where
-    F: Fn(CallerContext, WorldCallbackAccess, Vec<ScriptValue>) -> ScriptValue
-        + Send
-        + Sync
-        + 'static,
+    F: Fn(FunctionCallContext, Vec<ScriptValue>) -> ScriptValue + Send + Sync + 'static,
 {
     fn from(fn_: F) -> Self {
         DynamicScriptFunction {
@@ -313,10 +311,7 @@ where
 
 impl<F> From<F> for DynamicScriptFunctionMut
 where
-    F: FnMut(CallerContext, WorldCallbackAccess, Vec<ScriptValue>) -> ScriptValue
-        + Send
-        + Sync
-        + 'static,
+    F: FnMut(FunctionCallContext, Vec<ScriptValue>) -> ScriptValue + Send + Sync + 'static,
 {
     fn from(fn_: F) -> Self {
         DynamicScriptFunctionMut {
@@ -527,52 +522,43 @@ macro_rules! impl_script_function {
         // FnMut(T1...Tn) -> O
         impl_script_function!(@ ScriptFunctionMut FnMut DynamicScriptFunctionMut into_dynamic_script_function_mut $( $param ),* : -> O => O );
 
-        // Fn(WorldCallbackAccess, T1...Tn) -> O
-        impl_script_function!(@ ScriptFunction Fn DynamicScriptFunction into_dynamic_script_function $( $param ),* : ,(callback: WorldCallbackAccess) -> O => O);
-        // FnMut(WorldCallbackAccess, T1...Tn) -> O
-        impl_script_function!(@ ScriptFunctionMut FnMut DynamicScriptFunctionMut into_dynamic_script_function_mut $( $param ),* : ,(callback: WorldCallbackAccess) -> O => O);
-
-        // Fn(CallerContext, WorldCallbackAccess, T1...Tn) -> O
-        impl_script_function!(@ ScriptFunction Fn DynamicScriptFunction into_dynamic_script_function $( $param ),* : (context: CallerContext),(callback: WorldCallbackAccess) -> O => O);
-        // FnMut(CallerContext, WorldCallbackAccess, T1...Tn) -> O
-        impl_script_function!(@ ScriptFunctionMut FnMut DynamicScriptFunctionMut into_dynamic_script_function_mut $( $param ),* : (context: CallerContext),(callback: WorldCallbackAccess) -> O => O);
+        // Fn(CallerContext, T1...Tn) -> O
+        impl_script_function!(@ ScriptFunction Fn DynamicScriptFunction into_dynamic_script_function $( $param ),* : (context: FunctionCallContext) -> O => O);
+        // FnMut(FunctionCallContext, T1...Tn) -> O
+        impl_script_function!(@ ScriptFunctionMut FnMut DynamicScriptFunctionMut into_dynamic_script_function_mut $( $param ),* : (context: FunctionCallContext) -> O => O);
 
         // Fn(T1...Tn) -> Result<O, InteropError>
         impl_script_function!(@ ScriptFunction Fn DynamicScriptFunction into_dynamic_script_function $( $param ),* : -> O => Result<O, InteropError> where s);
         // FnMut(T1...Tn) -> Result<O, InteropError>
         impl_script_function!(@ ScriptFunctionMut FnMut DynamicScriptFunctionMut into_dynamic_script_function_mut $( $param ),* : -> O => Result<O, InteropError> where s);
 
-        // Fn(WorldCallbackAccess, T1...Tn) -> Result<O, InteropError>
-        impl_script_function!(@ ScriptFunction Fn DynamicScriptFunction into_dynamic_script_function $( $param ),* : ,(callback: WorldCallbackAccess) -> O => Result<O, InteropError> where s);
-        // FnMut(WorldCallbackAccess, T1...Tn) -> Result<O, InteropError>
-        impl_script_function!(@ ScriptFunctionMut FnMut DynamicScriptFunctionMut into_dynamic_script_function_mut $( $param ),* : ,(callback: WorldCallbackAccess) -> O => Result<O, InteropError> where s);
-
-        // Fn(CallerContext, WorldCallbackAccess, T1...Tn) -> Result<O, InteropError>
-        impl_script_function!(@ ScriptFunction Fn DynamicScriptFunction into_dynamic_script_function $( $param ),* : (context: CallerContext),(callback: WorldCallbackAccess) -> O => Result<O, InteropError> where s);
-        // FnMut(CallerContext, WorldCallbackAccess, T1...Tn) -> Result<O, InteropError>
-        impl_script_function!(@ ScriptFunctionMut FnMut DynamicScriptFunctionMut into_dynamic_script_function_mut $( $param ),* : (context: CallerContext),(callback: WorldCallbackAccess) -> O => Result<O, InteropError> where s);
+        // Fn(FunctionCallContext, WorldGuard<'w>, T1...Tn) -> Result<O, InteropError>
+        impl_script_function!(@ ScriptFunction Fn DynamicScriptFunction into_dynamic_script_function $( $param ),* : (context: FunctionCallContext)-> O => Result<O, InteropError> where s);
+        // FnMut(FunctionCallContext, WorldGuard<'w>, T1...Tn) -> Result<O, InteropError>
+        impl_script_function!(@ ScriptFunctionMut FnMut DynamicScriptFunctionMut into_dynamic_script_function_mut $( $param ),* : (context: FunctionCallContext) -> O => Result<O, InteropError> where s);
 
 
     };
 
-    (@ $trait_type:ident $fn_type:ident $dynamic_type:ident $trait_fn_name:ident $( $param:ident ),* :  $(($context:ident: $contextty:ty))? $(,($callback:ident: $callbackty:ty))? -> O => $res:ty $(where $out:ident)?) => {
+    (@ $trait_type:ident $fn_type:ident $dynamic_type:ident $trait_fn_name:ident $( $param:ident ),* :  $(($context:ident: $contextty:ty))? -> O => $res:ty $(where $out:ident)?) => {
         #[allow(non_snake_case)]
         impl<
             'env,
+            'w : 'static,
             $( $param: FromScript, )*
             O,
             F
         > $trait_type<'env,
-            fn( $($contextty,)? $( $callbackty, )? $($param ),* ) -> $res
+            fn( $($contextty,)? $($param ),* ) -> $res
         > for F
         where
             O: IntoScript + TypePath + GetOwnership,
-            F: $fn_type(  $($contextty,)? $( $callbackty, )? $($param ),* ) -> $res + Send + Sync + 'static,
-            $( $param::This<'env>: Into<$param>,)*
+            F: $fn_type(  $($contextty,)? $($param ),* ) -> $res + Send + Sync + 'static ,
+            $( $param::This<'w>: Into<$param>,)*
         {
             #[allow(unused_mut,unused_variables)]
             fn $trait_fn_name(mut self) -> $dynamic_type {
-                let func = (move |caller_context: CallerContext, world: WorldCallbackAccess, args: Vec<ScriptValue> | {
+                let func = (move |caller_context: FunctionCallContext, args: Vec<ScriptValue> | {
                     let res: Result<ScriptValue, InteropError> = (|| {
                         let expected_arg_count = count!($($param )*);
                         if args.len() < expected_arg_count {
@@ -582,8 +568,7 @@ macro_rules! impl_script_function {
                             }));
                         }
                         $( let $context = caller_context; )?
-                        $( let $callback = world.clone(); )?
-                        let world = world.try_read()?;
+                        let world = caller_context.world()?;
                         world.begin_access_scope()?;
                         let ret = {
                             let mut current_arg = 0;
@@ -593,7 +578,7 @@ macro_rules! impl_script_function {
                                 let $param = <$param>::from_script(arg_iter.next().expect("invariant"), world.clone())
                                     .map_err(|e| InteropError::function_arg_conversion_error(current_arg.to_string(), e))?;
                             )*
-                            let out = self( $( $context,)? $( $callback, )? $( $param.into(), )* );
+                            let out = self( $( $context,)?  $( $param.into(), )* );
                             $(
                                 let $out = out?;
                                 let out = $out;

diff --git a/crates/bevy_mod_scripting_core/src/bindings/pretty_print.rs b/crates/bevy_mod_scripting_core/src/bindings/pretty_print.rs
@@ -373,8 +373,8 @@ impl DisplayWithWorld for ComponentId {
     fn display_with_world(&self, world: WorldGuard) -> String {
         let component_name = world
             .as_unsafe_world_cell()
-            .components()
-            .get_info(*self)
+            .ok()
+            .and_then(|c| c.components().get_info(*self))
             .map(|info| info.name());
 
         match component_name {
@@ -543,7 +543,7 @@ mod test {
 
     use crate::bindings::{
         function::script_function::AppScriptFunctionRegistry, AppReflectAllocator,
-        ReflectAllocationId, WorldAccessGuard,
+        ReflectAllocationId,
     };
 
     use super::*;
@@ -566,7 +566,7 @@ mod test {
     #[test]
     fn test_type_id() {
         let mut world = setup_world();
-        let world = WorldGuard::new(WorldAccessGuard::new(&mut world));
+        let world = WorldGuard::new(&mut world);
 
         let type_id = TypeId::of::<usize>();
         assert_eq!(type_id.display_with_world(world.clone()), "usize");
@@ -585,7 +585,7 @@ mod test {
     #[test]
     fn test_reflect_base_type() {
         let mut world = setup_world();
-        let world = WorldGuard::new(WorldAccessGuard::new(&mut world));
+        let world = WorldGuard::new(&mut world);
 
         let type_id = TypeId::of::<usize>();
 
@@ -621,7 +621,7 @@ mod test {
     fn test_reflect_reference() {
         let mut world = setup_world();
 
-        let world = WorldGuard::new(WorldAccessGuard::new(&mut world));
+        let world = WorldGuard::new(&mut world);
 
         let type_id = TypeId::of::<usize>();
 

diff --git a/crates/bevy_mod_scripting_core/src/bindings/query.rs b/crates/bevy_mod_scripting_core/src/bindings/query.rs
@@ -1,4 +1,4 @@
-use super::{ReflectReference, WorldAccessGuard, WorldCallbackAccess};
+use super::{ReflectReference, WorldAccessGuard};
 use crate::{error::InteropError, with_global_access};
 use bevy::{
     ecs::{component::ComponentId, entity::Entity},
@@ -152,23 +152,23 @@ pub struct ScriptQueryResult {
     pub components: Vec<ReflectReference>,
 }
 
-impl WorldCallbackAccess {
-    pub fn query(
-        &self,
-        query: ScriptQueryBuilder,
-    ) -> Result<VecDeque<ScriptQueryResult>, InteropError> {
-        // find the set of components
-        self.try_read().and_then(|world| world.query(query))
-    }
-}
+// impl WorldCallbackAccess {
+//     pub fn query(
+//         &self,
+//         query: ScriptQueryBuilder,
+//     ) -> Result<VecDeque<ScriptQueryResult>, InteropError> {
+//         // find the set of components
+//         self.try_read().and_then(|world| world.query(query))
+//     }
+// }
 
 impl WorldAccessGuard<'_> {
     pub fn query(
         &self,
         query: ScriptQueryBuilder,
     ) -> Result<VecDeque<ScriptQueryResult>, InteropError> {
         with_global_access!(self.0.accesses, "Could not query", {
-            let world = unsafe { self.as_unsafe_world_cell().world_mut() };
+            let world = unsafe { self.as_unsafe_world_cell()?.world_mut() };
             let mut dynamic_query = QueryBuilder::<EntityRef>::new(world);
 
             // we don't actually want to fetch the data for components now, only figure out

diff --git a/crates/bevy_mod_scripting_core/src/bindings/reference.rs b/crates/bevy_mod_scripting_core/src/bindings/reference.rs
@@ -139,7 +139,7 @@ impl ReflectReference {
     }
 
     pub fn new_resource_ref<T: Resource>(world: WorldGuard) -> Result<Self, InteropError> {
-        let reflect_id = ReflectAccessId::for_resource::<T>(&world.as_unsafe_world_cell())?;
+        let reflect_id = ReflectAccessId::for_resource::<T>(&world.as_unsafe_world_cell()?)?;
         Ok(Self {
             base: ReflectBaseType {
                 type_id: TypeId::of::<T>(),
@@ -153,7 +153,7 @@ impl ReflectReference {
         entity: Entity,
         world: WorldGuard,
     ) -> Result<Self, InteropError> {
-        let reflect_id = ReflectAccessId::for_component::<T>(&world.as_unsafe_world_cell())?;
+        let reflect_id = ReflectAccessId::for_component::<T>(&world.as_unsafe_world_cell()?)?;
         Ok(Self {
             base: ReflectBaseType {
                 type_id: TypeId::of::<T>(),
@@ -323,7 +323,7 @@ impl ReflectReference {
             .base
             .base_id
             .clone()
-            .into_ptr(world.as_unsafe_world_cell())
+            .into_ptr(world.as_unsafe_world_cell()?)
             .ok_or_else(|| InteropError::unregistered_base(self.base.clone()))?;
 
         // (Ptr) Safety: we use the same type_id to both
@@ -377,7 +377,7 @@ impl ReflectReference {
             .base
             .base_id
             .clone()
-            .into_ptr_mut(world.as_unsafe_world_cell())
+            .into_ptr_mut(world.as_unsafe_world_cell()?)
             .ok_or_else(|| InteropError::unregistered_base(self.base.clone()))?;
 
         // (Ptr) Safety: we use the same type_id to both
@@ -569,7 +569,7 @@ mod test {
     use bevy::prelude::{AppTypeRegistry, World};
 
     use crate::bindings::{
-        function::script_function::AppScriptFunctionRegistry, AppReflectAllocator, WorldAccessGuard,
+        function::script_function::AppScriptFunctionRegistry, AppReflectAllocator,
     };
 
     use super::*;
@@ -609,7 +609,7 @@ mod test {
             .spawn(Component(vec!["hello".to_owned(), "world".to_owned()]))
             .id();
 
-        let world_guard = WorldGuard::new(WorldAccessGuard::new(&mut world));
+        let world_guard = WorldGuard::new(&mut world);
 
         let mut component_ref =
             ReflectReference::new_component_ref::<Component>(entity, world_guard.clone())
@@ -689,7 +689,7 @@ mod test {
 
         world.insert_resource(Resource(vec!["hello".to_owned(), "world".to_owned()]));
 
-        let world_guard = WorldGuard::new(WorldAccessGuard::new(&mut world));
+        let world_guard = WorldGuard::new(&mut world);
 
         let mut resource_ref = ReflectReference::new_resource_ref::<Resource>(world_guard.clone())
             .expect("could not create resource reference");
@@ -768,7 +768,7 @@ mod test {
 
         let value = Component(vec!["hello".to_owned(), "world".to_owned()]);
 
-        let world_guard = WorldGuard::new(WorldAccessGuard::new(&mut world));
+        let world_guard = WorldGuard::new(&mut world);
         let allocator = world_guard.allocator();
         let mut allocator_write = allocator.write();
         let mut allocation_ref = ReflectReference::new_allocated(value, &mut allocator_write);