From 05a7e712406a64aeaa75d4d8093660b980c2fa3e Mon Sep 17 00:00:00 2001 From: makspll Date: Mon, 18 Aug 2025 22:22:30 +0100 Subject: [PATCH] refactor: inline `CallbackBuilder` into `IntoScriptPluginParams` at compile time --- .../bevy_mod_scripting_core/src/commands.rs | 8 +-- crates/bevy_mod_scripting_core/src/context.rs | 68 ++++++++----------- crates/bevy_mod_scripting_core/src/lib.rs | 22 +++--- .../bevy_mod_scripting_lua/src/lib.rs | 14 ++-- .../bevy_mod_scripting_rhai/src/lib.rs | 14 ++-- .../test_utils/src/test_plugin.rs | 20 ++++++ 6 files changed, 82 insertions(+), 64 deletions(-) diff --git a/crates/bevy_mod_scripting_core/src/commands.rs b/crates/bevy_mod_scripting_core/src/commands.rs index f7f187d3d7..59a63e1010 100644 --- a/crates/bevy_mod_scripting_core/src/commands.rs +++ b/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 CreateOrUpdateScript

{ ) -> Result<(), ScriptError> { bevy::log::debug!("{}: reloading context {}", P::LANGUAGE, attachment); // reload context - (ContextBuilder::

::reload)( - handler_ctxt.context_loading_settings.loader.reload, + P::reload( attachment, content, context, @@ -172,8 +171,7 @@ impl CreateOrUpdateScript

{ handler_ctxt: &HandlerContext

, ) -> Result { bevy::log::debug!("{}: loading context {}", P::LANGUAGE, attachment); - let context = (ContextBuilder::

::load)( - handler_ctxt.context_loading_settings.loader.load, + let context = P::load( attachment, content, &handler_ctxt.context_loading_settings.context_initializers, diff --git a/crates/bevy_mod_scripting_core/src/context.rs b/crates/bevy_mod_scripting_core/src/context.rs index 9bf6649c6d..61166335f2 100644 --- a/crates/bevy_mod_scripting_core/src/context.rs +++ b/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 { /// 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

, /// Initializers run once after creating a context but before executing it for the first time pub context_initializers: Vec>, /// Initializers run every time before executing or loading a script @@ -41,7 +39,6 @@ impl Default for ContextLoadingSettings

{ 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 Clone for ContextLoadingSettings { 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

= fn( runtime: &

::R, ) -> Result<(), ScriptError>; -/// A strategy for loading and reloading contexts -pub struct ContextBuilder { - /// The function to load a context - pub load: ContextLoadFn

, - /// The function to reload a context - pub reload: ContextReloadFn

, -} +/// 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 { + /// Loads a script context using the provided loader function + fn load( + attachment: &ScriptAttachment, + content: &[u8], + context_initializers: &[ContextInitializer

], + pre_handling_initializers: &[ContextPreHandlingInitializer

], + world: WorldGuard, + runtime: &P::R, + ) -> Result; -impl Default for ContextBuilder

{ - 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

], + pre_handling_initializers: &[ContextPreHandlingInitializer

], + world: WorldGuard, + runtime: &P::R, + ) -> Result<(), ScriptError>; } -impl ContextBuilder

{ - /// load a context - pub fn load( - loader: ContextLoadFn

, +impl ScriptingLoader

for P { + fn load( attachment: &ScriptAttachment, content: &[u8], context_initializers: &[ContextInitializer

], @@ -109,7 +110,7 @@ impl ContextBuilder

{ ) -> Result { 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 ContextBuilder

{ }) } - /// reload a context - pub fn reload( - reloader: ContextReloadFn

, + fn reload( attachment: &ScriptAttachment, content: &[u8], previous_context: &mut P::C, @@ -132,7 +131,7 @@ impl ContextBuilder

{ ) -> 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 ContextBuilder

{ }) } } - -impl Clone for ContextBuilder

{ - fn clone(&self) -> Self { - Self { - load: self.load, - reload: self.reload, - } - } -} diff --git a/crates/bevy_mod_scripting_core/src/lib.rs b/crates/bevy_mod_scripting_core/src/lib.rs index 74b66651d0..f87320ffde 100644 --- a/crates/bevy_mod_scripting_core/src/lib.rs +++ b/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,6 +74,12 @@ pub trait IntoScriptPluginParams: 'static { /// Returns the handler function for the plugin fn handler() -> HandlerFn; + + /// Returns the context loader function for the plugin + fn context_loader() -> ContextLoadFn; + + /// Returns the context reloader function for the plugin + fn context_reloader() -> ContextReloadFn; } /// Bevy plugin enabling scripting within the bevy mod scripting framework @@ -80,9 +87,6 @@ pub struct ScriptingPlugin { /// Settings for the runtime pub runtime_settings: RuntimeSettings

, - /// The context builder for loading contexts - pub context_builder: ContextBuilder

, - /// The strategy used to assign contexts to scripts pub context_policy: ContextPolicy, @@ -121,7 +125,6 @@ impl Default for ScriptingPlugin

{ 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 Plugin for ScriptingPlugin

{ runtime: P::build_runtime(), }) .insert_resource::>(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, diff --git a/crates/languages/bevy_mod_scripting_lua/src/lib.rs b/crates/languages/bevy_mod_scripting_lua/src/lib.rs index c13b2dab25..a2f0487f46 100644 --- a/crates/languages/bevy_mod_scripting_lua/src/lib.rs +++ b/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 { lua_handler } + + fn context_loader() -> bevy_mod_scripting_core::context::ContextLoadFn { + lua_context_load + } + + fn context_reloader() -> bevy_mod_scripting_core::context::ContextReloadFn { + 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:: { - load: lua_context_load, - reload: lua_context_reload, - }, context_initializers: vec![ |_script_id, context| { // set the world global diff --git a/crates/languages/bevy_mod_scripting_rhai/src/lib.rs b/crates/languages/bevy_mod_scripting_rhai/src/lib.rs index d023ef86d1..0d92298cbc 100644 --- a/crates/languages/bevy_mod_scripting_rhai/src/lib.rs +++ b/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 { rhai_callback_handler } + + fn context_loader() -> bevy_mod_scripting_core::context::ContextLoadFn { + rhai_context_load + } + + fn context_reloader() -> bevy_mod_scripting_core::context::ContextReloadFn { + 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( diff --git a/crates/testing_crates/test_utils/src/test_plugin.rs b/crates/testing_crates/test_utils/src/test_plugin.rs index a8f2da228a..335f2bc203 100644 --- a/crates/testing_crates/test_utils/src/test_plugin.rs +++ b/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 } + + fn context_loader() -> $ident::ContextLoadFn { + (|attachment, content, context_initializers, pre_handling_initializers, runtime| { + Ok(TestContext { + invocations: vec![], + }) + }) + } + + fn context_reloader() -> $ident::ContextReloadFn { + (|attachment, + content, + previous_context, + context_initializers, + pre_handling_initializers, + runtime| { + previous_context.invocations.clear(); + Ok(()) + }) + } } #[derive(Default, std::fmt::Debug)]