refactor: inline CallbackBuilder<P> into IntoScriptPluginParams at compile time (#456)

Author: makspll

crates/bevy_mod_scripting_core/src/commands.rs

@@ -3,7 +3,7 @@
 use crate::{
     asset::ScriptAsset,
     bindings::{ScriptValue, WorldGuard},
-    context::ContextBuilder,
+    context::ScriptingLoader,
     error::{InteropError, ScriptError},
     event::{
         CallbackLabel, IntoCallbackLabel, OnScriptLoaded, OnScriptReloaded, OnScriptUnloaded,
@@ -151,8 +151,7 @@ impl<P: IntoScriptPluginParams> CreateOrUpdateScript<P> {
     ) -> Result<(), ScriptError> {
         bevy::log::debug!("{}: reloading context {}", P::LANGUAGE, attachment);
         // reload context
-        (ContextBuilder::<P>::reload)(
-            handler_ctxt.context_loading_settings.loader.reload,
+        P::reload(
             attachment,
             content,
             context,
@@ -172,8 +171,7 @@ impl<P: IntoScriptPluginParams> CreateOrUpdateScript<P> {
         handler_ctxt: &HandlerContext<P>,
     ) -> Result<P::C, ScriptError> {
         bevy::log::debug!("{}: loading context {}", P::LANGUAGE, attachment);
-        let context = (ContextBuilder::<P>::load)(
-            handler_ctxt.context_loading_settings.loader.load,
+        let context = P::load(
             attachment,
             content,
             &handler_ctxt.context_loading_settings.context_initializers,

crates/bevy_mod_scripting_core/src/context.rs

@@ -2,7 +2,7 @@
 
 use crate::{
     bindings::{ThreadWorldContainer, WorldContainer, WorldGuard},
-    error::{InteropError, ScriptError},
+    error::ScriptError,
     script::ScriptAttachment,
     IntoScriptPluginParams,
 };
@@ -29,8 +29,6 @@ pub struct ContextLoadingSettings<P: IntoScriptPluginParams> {
     /// Whether to emit responses from core script_callbacks like `on_script_loaded` or `on_script_unloaded`.
     /// By default, this is `false` and responses are not emitted.
     pub emit_responses: bool,
-    /// Defines the strategy used to load and reload contexts
-    pub loader: ContextBuilder<P>,
     /// Initializers run once after creating a context but before executing it for the first time
     pub context_initializers: Vec<ContextInitializer<P>>,
     /// Initializers run every time before executing or loading a script
@@ -41,7 +39,6 @@ impl<P: IntoScriptPluginParams> Default for ContextLoadingSettings<P> {
     fn default() -> Self {
         Self {
             emit_responses: false,
-            loader: ContextBuilder::default(),
             context_initializers: Default::default(),
             context_pre_handling_initializers: Default::default(),
         }
@@ -52,7 +49,6 @@ impl<T: IntoScriptPluginParams> Clone for ContextLoadingSettings<T> {
     fn clone(&self) -> Self {
         Self {
             emit_responses: self.emit_responses,
-            loader: self.loader.clone(),
             context_initializers: self.context_initializers.clone(),
             context_pre_handling_initializers: self.context_pre_handling_initializers.clone(),
         }
@@ -77,29 +73,34 @@ pub type ContextReloadFn<P> = fn(
     runtime: &<P as IntoScriptPluginParams>::R,
 ) -> Result<(), ScriptError>;
 
-/// A strategy for loading and reloading contexts
-pub struct ContextBuilder<P: IntoScriptPluginParams> {
-    /// The function to load a context
-    pub load: ContextLoadFn<P>,
-    /// The function to reload a context
-    pub reload: ContextReloadFn<P>,
-}
+/// A utility trait for types implementing `IntoScriptPluginParams`.
+///
+/// Provides methods for initializing and reloading script contexts using the plugin's context loader and reloader functions.
+pub trait ScriptingLoader<P: IntoScriptPluginParams> {
+    /// Loads a script context using the provided loader function
+    fn load(
+        attachment: &ScriptAttachment,
+        content: &[u8],
+        context_initializers: &[ContextInitializer<P>],
+        pre_handling_initializers: &[ContextPreHandlingInitializer<P>],
+        world: WorldGuard,
+        runtime: &P::R,
+    ) -> Result<P::C, ScriptError>;
 
-impl<P: IntoScriptPluginParams> Default for ContextBuilder<P> {
-    fn default() -> Self {
-        Self {
-            load: |_, _, _, _, _| Err(InteropError::invariant("no context loader set").into()),
-            reload: |_, _, _, _, _, _| {
-                Err(InteropError::invariant("no context reloader set").into())
-            },
-        }
-    }
+    /// Reloads a script context using the provided reloader function
+    fn reload(
+        attachment: &ScriptAttachment,
+        content: &[u8],
+        previous_context: &mut P::C,
+        context_initializers: &[ContextInitializer<P>],
+        pre_handling_initializers: &[ContextPreHandlingInitializer<P>],
+        world: WorldGuard,
+        runtime: &P::R,
+    ) -> Result<(), ScriptError>;
 }
 
-impl<P: IntoScriptPluginParams> ContextBuilder<P> {
-    /// load a context
-    pub fn load(
-        loader: ContextLoadFn<P>,
+impl<P: IntoScriptPluginParams> ScriptingLoader<P> for P {
+    fn load(
         attachment: &ScriptAttachment,
         content: &[u8],
         context_initializers: &[ContextInitializer<P>],
@@ -109,7 +110,7 @@ impl<P: IntoScriptPluginParams> ContextBuilder<P> {
     ) -> Result<P::C, ScriptError> {
         WorldGuard::with_existing_static_guard(world.clone(), |world| {
             ThreadWorldContainer.set_world(world)?;
-            (loader)(
+            Self::context_loader()(
                 attachment,
                 content,
                 context_initializers,
@@ -119,9 +120,7 @@ impl<P: IntoScriptPluginParams> ContextBuilder<P> {
         })
     }
 
-    /// reload a context
-    pub fn reload(
-        reloader: ContextReloadFn<P>,
+    fn reload(
         attachment: &ScriptAttachment,
         content: &[u8],
         previous_context: &mut P::C,
@@ -132,7 +131,7 @@ impl<P: IntoScriptPluginParams> ContextBuilder<P> {
     ) -> Result<(), ScriptError> {
         WorldGuard::with_existing_static_guard(world, |world| {
             ThreadWorldContainer.set_world(world)?;
-            (reloader)(
+            Self::context_reloader()(
                 attachment,
                 content,
                 previous_context,
@@ -143,12 +142,3 @@ impl<P: IntoScriptPluginParams> ContextBuilder<P> {
         })
     }
 }
-
-impl<P: IntoScriptPluginParams> Clone for ContextBuilder<P> {
-    fn clone(&self) -> Self {
-        Self {
-            load: self.load,
-            reload: self.reload,
-        }
-    }
-}

crates/bevy_mod_scripting_core/src/lib.rs

@@ -2,7 +2,11 @@
 //!
 //! Contains language agnostic systems and types for handling scripting in bevy.
 
-use crate::{bindings::MarkAsCore, event::ScriptErrorEvent};
+use crate::{
+    bindings::MarkAsCore,
+    context::{ContextLoadFn, ContextReloadFn},
+    event::ScriptErrorEvent,
+};
 use asset::{
     configure_asset_systems, configure_asset_systems_for_plugin, Language, ScriptAsset,
     ScriptAssetLoader,
@@ -14,10 +18,7 @@ use bindings::{
     DynamicScriptComponentPlugin, ReflectAllocator, ReflectReference, ScriptTypeRegistration,
 };
 use commands::{AddStaticScript, RemoveStaticScript};
-use context::{
-    Context, ContextBuilder, ContextInitializer, ContextLoadingSettings,
-    ContextPreHandlingInitializer,
-};
+use context::{Context, ContextInitializer, ContextLoadingSettings, ContextPreHandlingInitializer};
 use error::ScriptError;
 use event::{ScriptCallbackEvent, ScriptCallbackResponseEvent, ScriptEvent};
 use handler::HandlerFn;
@@ -73,16 +74,19 @@ pub trait IntoScriptPluginParams: 'static {
 
     /// Returns the handler function for the plugin
     fn handler() -> HandlerFn<Self>;
+
+    /// Returns the context loader function for the plugin
+    fn context_loader() -> ContextLoadFn<Self>;
+
+    /// Returns the context reloader function for the plugin
+    fn context_reloader() -> ContextReloadFn<Self>;
 }
 
 /// Bevy plugin enabling scripting within the bevy mod scripting framework
 pub struct ScriptingPlugin<P: IntoScriptPluginParams> {
     /// Settings for the runtime
     pub runtime_settings: RuntimeSettings<P>,
 
-    /// The context builder for loading contexts
-    pub context_builder: ContextBuilder<P>,
-
     /// The strategy used to assign contexts to scripts
     pub context_policy: ContextPolicy,
 
@@ -121,7 +125,6 @@ impl<P: IntoScriptPluginParams> Default for ScriptingPlugin<P> {
     fn default() -> Self {
         Self {
             runtime_settings: Default::default(),
-            context_builder: Default::default(),
             context_policy: ContextPolicy::default(),
             language: Default::default(),
             context_initializers: Default::default(),
@@ -139,7 +142,6 @@ impl<P: IntoScriptPluginParams> Plugin for ScriptingPlugin<P> {
                 runtime: P::build_runtime(),
             })
             .insert_resource::<ContextLoadingSettings<P>>(ContextLoadingSettings {
-                loader: self.context_builder.clone(),
                 context_initializers: self.context_initializers.clone(),
                 context_pre_handling_initializers: self.context_pre_handling_initializers.clone(),
                 emit_responses: self.emit_responses,

crates/languages/bevy_mod_scripting_lua/src/lib.rs

@@ -10,7 +10,7 @@ use bevy_mod_scripting_core::{
         function::namespace::Namespace, globals::AppScriptGlobalsRegistry,
         script_value::ScriptValue, ThreadWorldContainer, WorldContainer,
     },
-    context::{ContextBuilder, ContextInitializer, ContextPreHandlingInitializer},
+    context::{ContextInitializer, ContextPreHandlingInitializer},
     error::ScriptError,
     event::CallbackLabel,
     reflection_extensions::PartialReflectExt,
@@ -38,6 +38,14 @@ impl IntoScriptPluginParams for LuaScriptingPlugin {
     fn handler() -> bevy_mod_scripting_core::handler::HandlerFn<Self> {
         lua_handler
     }
+
+    fn context_loader() -> bevy_mod_scripting_core::context::ContextLoadFn<Self> {
+        lua_context_load
+    }
+
+    fn context_reloader() -> bevy_mod_scripting_core::context::ContextReloadFn<Self> {
+        lua_context_reload
+    }
 }
 
 // necessary for automatic config goodies
@@ -58,10 +66,6 @@ impl Default for LuaScriptingPlugin {
         LuaScriptingPlugin {
             scripting_plugin: ScriptingPlugin {
                 runtime_settings: RuntimeSettings::default(),
-                context_builder: ContextBuilder::<LuaScriptingPlugin> {
-                    load: lua_context_load,
-                    reload: lua_context_reload,
-                },
                 context_initializers: vec![
                     |_script_id, context| {
                         // set the world global

crates/languages/bevy_mod_scripting_rhai/src/lib.rs

@@ -13,7 +13,7 @@ use bevy_mod_scripting_core::{
         function::namespace::Namespace, globals::AppScriptGlobalsRegistry,
         script_value::ScriptValue, ThreadWorldContainer, WorldContainer,
     },
-    context::{ContextBuilder, ContextInitializer, ContextPreHandlingInitializer},
+    context::{ContextInitializer, ContextPreHandlingInitializer},
     error::ScriptError,
     event::CallbackLabel,
     reflection_extensions::PartialReflectExt,
@@ -55,6 +55,14 @@ impl IntoScriptPluginParams for RhaiScriptingPlugin {
     fn handler() -> bevy_mod_scripting_core::handler::HandlerFn<Self> {
         rhai_callback_handler
     }
+
+    fn context_loader() -> bevy_mod_scripting_core::context::ContextLoadFn<Self> {
+        rhai_context_load
+    }
+
+    fn context_reloader() -> bevy_mod_scripting_core::context::ContextReloadFn<Self> {
+        rhai_context_reload
+    }
 }
 
 /// The rhai scripting plugin. Used to add rhai scripting to a bevy app within the context of the BMS framework.
@@ -83,10 +91,6 @@ impl Default for RhaiScriptingPlugin {
                         Ok(())
                     }],
                 },
-                context_builder: ContextBuilder {
-                    load: rhai_context_load,
-                    reload: rhai_context_reload,
-                },
                 context_initializers: vec![
                     |_, context| {
                         context.scope.set_or_push(

crates/testing_crates/test_utils/src/test_plugin.rs

@@ -39,6 +39,26 @@ macro_rules! make_test_plugin {
                     Ok($ident::bindings::script_value::ScriptValue::Unit)
                 }) as $ident::HandlerFn<Self>
             }
+
+            fn context_loader() -> $ident::ContextLoadFn<Self> {
+                (|attachment, content, context_initializers, pre_handling_initializers, runtime| {
+                    Ok(TestContext {
+                        invocations: vec![],
+                    })
+                })
+            }
+
+            fn context_reloader() -> $ident::ContextReloadFn<Self> {
+                (|attachment,
+                  content,
+                  previous_context,
+                  context_initializers,
+                  pre_handling_initializers,
+                  runtime| {
+                    previous_context.invocations.clear();
+                    Ok(())
+                })
+            }
         }
 
         #[derive(Default, std::fmt::Debug)]