feat: optimize get and set functions, add MagicFunctions sub-registry (#397)

makspll · web-flow · commit 56d62b805f4d · 2025-03-29T12:12:47.000Z
# Summary
Attempts to speed up the get and set operations by:
- No longer looking for `get` and `set` overrides on
`ReflectReference`'s
- Extracting those functions from the function registry and keeping them
as `MagicFunctions` which do not require script value conversions
diff --git a/crates/bevy_mod_scripting_core/Cargo.toml b/crates/bevy_mod_scripting_core/Cargo.toml
@@ -25,7 +25,9 @@ mlua_impls = ["mlua"]
 rhai_impls = ["rhai"]
 
 [dependencies]
-mlua = { version = "0.10", default-features = false, optional = true }
+mlua = { version = "0.10", default-features = false, optional = true, features = [
+    "lua54",
+] }
 rhai = { version = "1.21", default-features = false, features = [
     "sync",
 ], optional = true }
diff --git a/crates/bevy_mod_scripting_core/src/bindings/function/magic_functions.rs b/crates/bevy_mod_scripting_core/src/bindings/function/magic_functions.rs
@@ -0,0 +1,119 @@
+//! All the switchable special functions used by language implementors
+use super::{FromScriptRef, FunctionCallContext, IntoScriptRef};
+use crate::{
+    bindings::{ReflectReference, ReflectionPathExt, ScriptValue},
+    error::InteropError,
+    reflection_extensions::TypeIdExtensions,
+};
+use bevy::reflect::{ParsedPath, PartialReflect};
+
+/// A list of magic methods, these only have one replacable implementation, and apply to all `ReflectReferences`.
+/// It's up to the language implementer to call these in the right order (after any type specific overrides).
+///
+/// These live in a separate mini registry since they are so commonly needed. This helps us avoid needless hashing and lookups as well as script value conversions
+#[derive(Debug)]
+pub struct MagicFunctions {
+    /// Indexer function
+    pub get:
+        fn(FunctionCallContext, ReflectReference, ScriptValue) -> Result<ScriptValue, InteropError>,
+    /// Indexer setter function
+    pub set: fn(
+        FunctionCallContext,
+        ReflectReference,
+        ScriptValue,
+        ScriptValue,
+    ) -> Result<(), InteropError>,
+}
+
+impl MagicFunctions {
+    /// Calls the currently set `get` function with the given arguments.
+    pub fn get(
+        &self,
+        ctxt: FunctionCallContext,
+        reference: ReflectReference,
+        key: ScriptValue,
+    ) -> Result<ScriptValue, InteropError> {
+        (self.get)(ctxt, reference, key)
+    }
+
+    /// Calls the currently set `set` function with the given arguments.
+    pub fn set(
+        &self,
+        ctxt: FunctionCallContext,
+        reference: ReflectReference,
+        key: ScriptValue,
+        value: ScriptValue,
+    ) -> Result<(), InteropError> {
+        (self.set)(ctxt, reference, key, value)
+    }
+
+    /// Indexes into the given reference and if the nested type is a reference type, returns a deeper reference, otherwise
+    /// returns the concrete value.
+    ///
+    /// Does not support map types at the moment, for maps see `map_get`
+    ///
+    /// Arguments:
+    /// * `ctxt`: The function call context.
+    /// * `reference`: The reference to index into.
+    /// * `key`: The key to index with.
+    ///
+    /// Returns:
+    /// * `value`: The value at the key, if the reference is indexable.
+    pub fn default_get(
+        ctxt: FunctionCallContext,
+        mut reference: ReflectReference,
+        key: ScriptValue,
+    ) -> Result<ScriptValue, InteropError> {
+        let mut path: ParsedPath = key.try_into()?;
+        if ctxt.convert_to_0_indexed() {
+            path.convert_to_0_indexed();
+        }
+        reference.index_path(path);
+        let world = ctxt.world()?;
+        ReflectReference::into_script_ref(reference, world)
+    }
+
+    /// Sets the value under the specified path on the underlying value.
+    ///
+    /// Arguments:
+    /// * `ctxt`: The function call context.
+    /// * `reference`: The reference to set the value on.
+    /// * `key`: The key to set the value at.
+    /// * `value`: The value to set.
+    ///
+    /// Returns:
+    /// * `result`: Nothing if the value was set successfully.
+    pub fn default_set(
+        ctxt: FunctionCallContext,
+        mut reference: ReflectReference,
+        key: ScriptValue,
+        value: ScriptValue,
+    ) -> Result<(), InteropError> {
+        let world = ctxt.world()?;
+        let mut path: ParsedPath = key.try_into()?;
+        if ctxt.convert_to_0_indexed() {
+            path.convert_to_0_indexed();
+        }
+        reference.index_path(path);
+        reference.with_reflect_mut(world.clone(), |r| {
+            let target_type_id = r
+                .get_represented_type_info()
+                .map(|i| i.type_id())
+                .or_fake_id();
+            let other =
+                <Box<dyn PartialReflect>>::from_script_ref(target_type_id, value, world.clone())?;
+            r.try_apply(other.as_partial_reflect())
+                .map_err(|e| InteropError::external_error(Box::new(e)))?;
+            Ok::<_, InteropError>(())
+        })?
+    }
+}
+
+impl Default for MagicFunctions {
+    fn default() -> Self {
+        Self {
+            get: MagicFunctions::default_get,
+            set: MagicFunctions::default_set,
+        }
+    }
+}
diff --git a/crates/bevy_mod_scripting_core/src/bindings/function/mod.rs b/crates/bevy_mod_scripting_core/src/bindings/function/mod.rs
@@ -1,14 +1,15 @@
 //! Abstractions to do with dynamic script functions
 
-crate::private::export_all_in_modules!{
+crate::private::export_all_in_modules! {
     arg_meta,
     from,
     from_ref,
     into,
     into_ref,
     namespace,
     script_function,
-    type_dependencies
+    type_dependencies,
+    magic_functions
 }
 
 #[cfg(test)]
@@ -18,12 +19,13 @@ mod test {
     use bevy_mod_scripting_derive::script_bindings;
 
     use crate::bindings::{
-            function::{
-                from::{Ref, Union, Val},
-                namespace::IntoNamespace,
-                script_function::AppScriptFunctionRegistry,
-            }, script_value::ScriptValue
-        };
+        function::{
+            from::{Ref, Union, Val},
+            namespace::IntoNamespace,
+            script_function::AppScriptFunctionRegistry,
+        },
+        script_value::ScriptValue,
+    };
 
     use super::arg_meta::{ScriptArgument, ScriptReturn, TypedScriptArgument, TypedScriptReturn};
 
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
@@ -1,5 +1,6 @@
 //! Implementations of the [`ScriptFunction`] and [`ScriptFunctionMut`] traits for functions with up to 13 arguments.
 
+use super::MagicFunctions;
 use super::{from::FromScript, into::IntoScript, namespace::Namespace};
 use crate::asset::Language;
 use crate::bindings::function::arg_meta::ArgMeta;
@@ -16,7 +17,6 @@ use std::collections::{HashMap, VecDeque};
 use std::hash::Hash;
 use std::ops::{Deref, DerefMut};
 use std::sync::Arc;
-
 #[diagnostic::on_unimplemented(
     message = "This function does not fulfil the requirements to be a script callable function. All arguments must implement the ScriptArgument trait and all return values must implement the ScriptReturn trait",
     note = "If you're trying to return a non-primitive type, you might need to use Val<T> Ref<T> or Mut<T> wrappers"
@@ -315,6 +315,8 @@ pub struct FunctionKey {
 /// A registry of dynamic script functions
 pub struct ScriptFunctionRegistry {
     functions: HashMap<FunctionKey, DynamicScriptFunction>,
+    /// A registry of magic functions
+    pub magic_functions: MagicFunctions,
 }
 
 #[profiling::all_functions]
diff --git a/crates/bevy_mod_scripting_core/src/bindings/reference.rs b/crates/bevy_mod_scripting_core/src/bindings/reference.rs
@@ -28,7 +28,9 @@ use std::{any::TypeId, fmt::Debug};
 ///
 /// References are composed of two parts:
 /// - The base kind, which specifies where the reference points to
-/// - The path, which specifies how to access the value from the base
+/// - The path, which specifies how to access the value from the base.
+///
+/// Bindings defined on this type, apply to ALL references.
 #[derive(Debug, Clone, PartialEq, Eq, Reflect)]
 #[reflect(Default, opaque)]
 pub struct ReflectReference {
diff --git a/crates/bevy_mod_scripting_functions/src/core.rs b/crates/bevy_mod_scripting_functions/src/core.rs
@@ -2,7 +2,7 @@
 
 use std::collections::HashMap;
 
-use bevy::{prelude::*, reflect::ParsedPath};
+use bevy::prelude::*;
 use bevy_mod_scripting_core::{
     bindings::{
         function::{
@@ -24,9 +24,8 @@ use bindings::{
     },
     pretty_print::DisplayWithWorld,
     script_value::ScriptValue,
-    ReflectReference, ReflectionPathExt, ScriptComponentRegistration, ScriptQueryBuilder,
-    ScriptQueryResult, ScriptResourceRegistration, ScriptTypeRegistration, ThreadWorldContainer,
-    WorldContainer,
+    ReflectReference, ScriptComponentRegistration, ScriptQueryBuilder, ScriptQueryResult,
+    ScriptResourceRegistration, ScriptTypeRegistration, ThreadWorldContainer, WorldContainer,
 };
 use error::InteropError;
 use reflection_extensions::{PartialReflectExt, TypeIdExtensions};
@@ -568,73 +567,6 @@ impl ReflectReference {
         })?
     }
 
-    /// Indexes into the given reference and if the nested type is a reference type, returns a deeper reference, otherwise
-    /// returns the concrete value.
-    ///
-    /// Does not support map types at the moment, for maps see `map_get`
-    ///
-    /// Arguments:
-    /// * `ctxt`: The function call context.
-    /// * `reference`: The reference to index into.
-    /// * `key`: The key to index with.
-    /// Returns:
-    /// * `value`: The value at the key, if the reference is indexable.
-    fn get(
-        ctxt: FunctionCallContext,
-        mut reference: ReflectReference,
-        key: ScriptValue,
-    ) -> Result<ScriptValue, InteropError> {
-        profiling::function_scope!("get");
-        let mut path: ParsedPath = key.try_into()?;
-        if ctxt.convert_to_0_indexed() {
-            path.convert_to_0_indexed();
-        }
-        reference.index_path(path);
-        let world = ctxt.world()?;
-        ReflectReference::into_script_ref(reference, world)
-    }
-
-    /// Sets the value under the specified path on the underlying value.
-    ///
-    /// Arguments:
-    /// * `ctxt`: The function call context.
-    /// * `reference`: The reference to set the value on.
-    /// * `key`: The key to set the value at.
-    /// * `value`: The value to set.
-    /// Returns:
-    /// * `result`: Nothing if the value was set successfully.
-    fn set(
-        ctxt: FunctionCallContext,
-        reference: ScriptValue,
-        key: ScriptValue,
-        value: ScriptValue,
-    ) -> Result<(), InteropError> {
-        profiling::function_scope!("set");
-        if let ScriptValue::Reference(mut self_) = reference {
-            let world = ctxt.world()?;
-            let mut path: ParsedPath = key.try_into()?;
-            if ctxt.convert_to_0_indexed() {
-                path.convert_to_0_indexed();
-            }
-            self_.index_path(path);
-            self_.with_reflect_mut(world.clone(), |r| {
-                let target_type_id = r
-                    .get_represented_type_info()
-                    .map(|i| i.type_id())
-                    .or_fake_id();
-                let other = <Box<dyn PartialReflect>>::from_script_ref(
-                    target_type_id,
-                    value,
-                    world.clone(),
-                )?;
-                r.try_apply(other.as_partial_reflect())
-                    .map_err(|e| InteropError::external_error(Box::new(e)))?;
-                Ok::<_, InteropError>(())
-            })??;
-        }
-        Ok(())
-    }
-
     /// Pushes the value into the reference, if the reference is an appropriate container type.
     ///
     /// Arguments:
diff --git a/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs b/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs
@@ -57,15 +57,13 @@ impl UserData for LuaReflectReference {
                     Err(key) => key,
                 };
 
-                let func = world
-                    .lookup_function([type_id, TypeId::of::<ReflectReference>()], "get")
-                    .map_err(|f| {
-                        InteropError::missing_function(TypeId::of::<ReflectReference>(), f)
-                    })?;
+                // call the default magic getter
+                let registry = world.script_function_registry();
+                let registry = registry.read();
 
-                // call the function with the key
-                let out =
-                    func.call(vec![ScriptValue::Reference(self_), key], LUA_CALLER_CONTEXT)?;
+                let out = registry
+                    .magic_functions
+                    .get(LUA_CALLER_CONTEXT, self_, key)?;
                 Ok(LuaScriptValue(out))
             },
         );
@@ -78,20 +76,15 @@ impl UserData for LuaReflectReference {
                 let self_: ReflectReference = self_.into();
                 let key: ScriptValue = key.into();
                 let value: ScriptValue = value.into();
-                let type_id = self_.tail_type_id(world.clone())?.or_fake_id();
 
-                let func = world
-                    .lookup_function([type_id, TypeId::of::<ReflectReference>()], "set")
-                    .map_err(|f| {
-                        InteropError::missing_function(TypeId::of::<ReflectReference>(), f)
-                    })?;
+                let registry = world.script_function_registry();
+                let registry = registry.read();
 
-                let out = func.call(
-                    vec![ScriptValue::Reference(self_), key, value],
-                    LUA_CALLER_CONTEXT,
-                )?;
+                registry
+                    .magic_functions
+                    .set(LUA_CALLER_CONTEXT, self_, key, value)?;
 
-                Ok(LuaScriptValue(out))
+                Ok(())
             },
         );
 
diff --git a/crates/languages/bevy_mod_scripting_rhai/src/bindings/reference.rs b/crates/languages/bevy_mod_scripting_rhai/src/bindings/reference.rs
diff --git a/docs/src/Development/AddingLanguages/necessary-features.md b/docs/src/Development/AddingLanguages/necessary-features.md
diff --git a/docs/src/Summary/controlling-script-bindings.md b/docs/src/Summary/controlling-script-bindings.md
