From a710a4137f28593065414a6a488cb333aa5aa930 Mon Sep 17 00:00:00 2001 From: makspll Date: Mon, 18 Aug 2025 21:54:30 +0100 Subject: [PATCH] refactor: inline `CallbackSettings` into `IntoScriptPluginParam` at compile time --- .github/workflows/pr-titles.yml | 1 + .../src/bindings/script_system.rs | 14 +----- .../bevy_mod_scripting_core/src/extractors.rs | 17 +------ crates/bevy_mod_scripting_core/src/handler.rs | 50 +++++++------------ crates/bevy_mod_scripting_core/src/lib.rs | 13 ++--- .../bevy_mod_scripting_lua/src/lib.rs | 5 +- .../bevy_mod_scripting_rhai/src/lib.rs | 5 +- .../test_utils/src/test_plugin.rs | 10 ++++ release-plz.toml | 3 +- 9 files changed, 48 insertions(+), 70 deletions(-) diff --git a/.github/workflows/pr-titles.yml b/.github/workflows/pr-titles.yml index d5e92a5a6c..5a2c6a2ff6 100644 --- a/.github/workflows/pr-titles.yml +++ b/.github/workflows/pr-titles.yml @@ -28,6 +28,7 @@ jobs: docs refactor perf + changed scopes: | ladfile diff --git a/crates/bevy_mod_scripting_core/src/bindings/script_system.rs b/crates/bevy_mod_scripting_core/src/bindings/script_system.rs index 1b4e4e1509..f1a5b3bbf0 100644 --- a/crates/bevy_mod_scripting_core/src/bindings/script_system.rs +++ b/crates/bevy_mod_scripting_core/src/bindings/script_system.rs @@ -15,7 +15,7 @@ use crate::{ error::{InteropError, ScriptError}, event::CallbackLabel, extractors::get_all_access_ids, - handler::CallbackSettings, + handler::ScriptingHandler, runtime::RuntimeContainer, script::{ScriptAttachment, ScriptContext}, IntoScriptPluginParams, @@ -200,7 +200,6 @@ impl ScriptSystemBuilder { struct DynamicHandlerContext<'w, P: IntoScriptPluginParams> { script_context: &'w ScriptContext

, - callback_settings: &'w CallbackSettings

, context_loading_settings: &'w ContextLoadingSettings

, runtime_container: &'w RuntimeContainer

, } @@ -215,9 +214,6 @@ impl<'w, P: IntoScriptPluginParams> DynamicHandlerContext<'w, P> { let mut access = FilteredAccess::::matches_nothing(); // let scripts_res_id = world // .query::<&Script

>(); - let callback_settings_res_id = world - .resource_id::>() - .expect("CallbackSettings resource not found"); let context_loading_settings_res_id = world .resource_id::>() .expect("ContextLoadingSettings resource not found"); @@ -225,7 +221,6 @@ impl<'w, P: IntoScriptPluginParams> DynamicHandlerContext<'w, P> { .resource_id::>() .expect("RuntimeContainer resource not found"); - access.add_resource_read(callback_settings_res_id); access.add_resource_read(context_loading_settings_res_id); access.add_resource_read(runtime_container_res_id); @@ -240,9 +235,6 @@ impl<'w, P: IntoScriptPluginParams> DynamicHandlerContext<'w, P> { unsafe { Self { script_context: system.get_resource().expect("Scripts resource not found"), - callback_settings: system - .get_resource() - .expect("CallbackSettings resource not found"), context_loading_settings: system .get_resource() .expect("ContextLoadingSettings resource not found"), @@ -268,7 +260,6 @@ impl<'w, P: IntoScriptPluginParams> DynamicHandlerContext<'w, P> { }; // call the script - let handler = self.callback_settings.callback_handler; let pre_handling_initializers = &self .context_loading_settings .context_pre_handling_initializers; @@ -276,8 +267,7 @@ impl<'w, P: IntoScriptPluginParams> DynamicHandlerContext<'w, P> { let mut context = context.lock(); - CallbackSettings::

::call( - handler, + P::handle( payload, context_key, label, diff --git a/crates/bevy_mod_scripting_core/src/extractors.rs b/crates/bevy_mod_scripting_core/src/extractors.rs index e72ea9172e..ca4abd3417 100644 --- a/crates/bevy_mod_scripting_core/src/extractors.rs +++ b/crates/bevy_mod_scripting_core/src/extractors.rs @@ -3,6 +3,7 @@ //! These are designed to be used to pipe inputs into other systems which require them, while handling any configuration erorrs nicely. #![allow(deprecated)] use crate::bindings::pretty_print::DisplayWithWorld; +use crate::handler::ScriptingHandler; use crate::{ bindings::{ access_map::ReflectAccessId, script_value::ScriptValue, WorldAccessGuard, WorldGuard, @@ -10,7 +11,6 @@ use crate::{ context::ContextLoadingSettings, error::{InteropError, ScriptError}, event::{CallbackLabel, IntoCallbackLabel}, - handler::CallbackSettings, runtime::RuntimeContainer, script::{ScriptAttachment, ScriptContext, StaticScripts}, IntoScriptPluginParams, @@ -122,8 +122,6 @@ unsafe impl SystemParam for ResScope<'_, T> { /// Context for systems which handle events for scripts pub struct HandlerContext { - /// Settings for callbacks - pub(crate) callback_settings: CallbackSettings

, /// Settings for loading contexts pub(crate) context_loading_settings: ContextLoadingSettings

, /// The runtime container @@ -139,7 +137,6 @@ impl HandlerContext

{ /// Every call to this function must be paired with a call to [`Self::release`]. pub fn yoink(world: &mut World) -> Self { Self { - callback_settings: world.remove_resource().unwrap_or_default(), context_loading_settings: world.remove_resource().unwrap_or_default(), runtime_container: world.remove_resource().unwrap_or_default(), static_scripts: world.remove_resource().unwrap_or_default(), @@ -151,7 +148,6 @@ impl HandlerContext

{ /// Only call this if you have previously yoinked the handler context from the world. pub fn release(self, world: &mut World) { // insert the handler context back into the world - world.insert_resource(self.callback_settings); world.insert_resource(self.context_loading_settings); world.insert_resource(self.runtime_container); world.insert_resource(self.static_scripts); @@ -165,24 +161,17 @@ impl HandlerContext

{ pub fn destructure( &mut self, ) -> ( - &mut CallbackSettings

, &mut ContextLoadingSettings

, &mut RuntimeContainer

, &mut StaticScripts, ) { ( - &mut self.callback_settings, &mut self.context_loading_settings, &mut self.runtime_container, &mut self.static_scripts, ) } - /// Get the callback settings - pub fn callback_settings(&mut self) -> &mut CallbackSettings

{ - &mut self.callback_settings - } - /// Get the context loading settings pub fn context_loading_settings(&mut self) -> &mut ContextLoadingSettings

{ &mut self.context_loading_settings @@ -226,7 +215,6 @@ impl HandlerContext

{ }; // call the script - let handler = self.callback_settings.callback_handler; let pre_handling_initializers = &self .context_loading_settings .context_pre_handling_initializers; @@ -234,8 +222,7 @@ impl HandlerContext

{ let mut context = context.lock(); - CallbackSettings::

::call( - handler, + P::handle( payload, context_key, label, diff --git a/crates/bevy_mod_scripting_core/src/handler.rs b/crates/bevy_mod_scripting_core/src/handler.rs index f6be314c6e..6408a508b9 100644 --- a/crates/bevy_mod_scripting_core/src/handler.rs +++ b/crates/bevy_mod_scripting_core/src/handler.rs @@ -17,7 +17,6 @@ use crate::{ use bevy::{ ecs::{ event::EventCursor, - resource::Resource, system::{Local, SystemState}, world::{Mut, World}, }, @@ -35,39 +34,26 @@ pub type HandlerFn

= fn( runtime: &

::R, ) -> Result; -/// A resource that holds the settings for the callback handler for a specific combination of type parameters -#[derive(Resource)] -pub struct CallbackSettings { - /// The callback handler function - pub callback_handler: HandlerFn

, -} - -impl Default for CallbackSettings

{ - fn default() -> Self { - Self { - callback_handler: |_, _, _, _, _, _| Ok(ScriptValue::Unit), - } - } -} - -impl Clone for CallbackSettings

{ - fn clone(&self) -> Self { - Self { - callback_handler: self.callback_handler, - } - } +/// A utility trait, implemented for all types implementing `IntoScriptPluginParams`. +/// +/// Calls the underlying handler function with the provided arguments and context. +/// Implementations will handle the necessary thread local context emplacement and retrieval. +pub trait ScriptingHandler { + /// Calls the handler function with the given arguments and context + fn handle( + args: Vec, + context_key: &ScriptAttachment, + callback: &CallbackLabel, + script_ctxt: &mut P::C, + pre_handling_initializers: &[ContextPreHandlingInitializer

], + runtime: &P::R, + world: WorldGuard, + ) -> Result; } -#[profiling::all_functions] -impl CallbackSettings

{ - /// Creates a new callback settings resource with the given handler function - pub fn new(callback_handler: HandlerFn

) -> Self { - Self { callback_handler } - } - +impl ScriptingHandler

for P { /// Calls the handler function while providing the necessary thread local context - pub fn call( - handler: HandlerFn

, + fn handle( args: Vec, context_key: &ScriptAttachment, callback: &CallbackLabel, @@ -78,7 +64,7 @@ impl CallbackSettings

{ ) -> Result { WorldGuard::with_existing_static_guard(world.clone(), |world| { ThreadWorldContainer.set_world(world)?; - (handler)( + Self::handler()( args, context_key, callback, diff --git a/crates/bevy_mod_scripting_core/src/lib.rs b/crates/bevy_mod_scripting_core/src/lib.rs index 8c89cc16ed..74b66651d0 100644 --- a/crates/bevy_mod_scripting_core/src/lib.rs +++ b/crates/bevy_mod_scripting_core/src/lib.rs @@ -20,7 +20,7 @@ use context::{ }; use error::ScriptError; use event::{ScriptCallbackEvent, ScriptCallbackResponseEvent, ScriptEvent}; -use handler::{CallbackSettings, HandlerFn}; +use handler::HandlerFn; use runtime::{initialize_runtime, Runtime, RuntimeContainer, RuntimeInitializer, RuntimeSettings}; use script::{ContextPolicy, ScriptComponent, ScriptContext, StaticScripts}; @@ -70,14 +70,16 @@ pub trait IntoScriptPluginParams: 'static { /// Build the runtime fn build_runtime() -> Self::R; + + /// Returns the handler function for the plugin + fn handler() -> HandlerFn; } /// Bevy plugin enabling scripting within the bevy mod scripting framework pub struct ScriptingPlugin { /// Settings for the runtime pub runtime_settings: RuntimeSettings

, - /// The handler used for executing callbacks in scripts - pub callback_handler: HandlerFn

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

, @@ -103,7 +105,6 @@ where { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("ScriptingPlugin") - .field("callback_handler", &self.callback_handler) .field("context_policy", &self.context_policy) .field("language", &self.language) .field("context_initializers", &self.context_initializers) @@ -120,7 +121,6 @@ impl Default for ScriptingPlugin

{ fn default() -> Self { Self { runtime_settings: Default::default(), - callback_handler: CallbackSettings::

::default().callback_handler, context_builder: Default::default(), context_policy: ContextPolicy::default(), language: Default::default(), @@ -138,9 +138,6 @@ impl Plugin for ScriptingPlugin

{ .insert_resource::>(RuntimeContainer { runtime: P::build_runtime(), }) - .insert_resource::>(CallbackSettings { - callback_handler: self.callback_handler, - }) .insert_resource::>(ContextLoadingSettings { loader: self.context_builder.clone(), context_initializers: self.context_initializers.clone(), diff --git a/crates/languages/bevy_mod_scripting_lua/src/lib.rs b/crates/languages/bevy_mod_scripting_lua/src/lib.rs index a99404d47d..c13b2dab25 100644 --- a/crates/languages/bevy_mod_scripting_lua/src/lib.rs +++ b/crates/languages/bevy_mod_scripting_lua/src/lib.rs @@ -34,6 +34,10 @@ impl IntoScriptPluginParams for LuaScriptingPlugin { const LANGUAGE: Language = Language::Lua; fn build_runtime() -> Self::R {} + + fn handler() -> bevy_mod_scripting_core::handler::HandlerFn { + lua_handler + } } // necessary for automatic config goodies @@ -54,7 +58,6 @@ impl Default for LuaScriptingPlugin { LuaScriptingPlugin { scripting_plugin: ScriptingPlugin { runtime_settings: RuntimeSettings::default(), - callback_handler: lua_handler, context_builder: ContextBuilder:: { load: lua_context_load, reload: lua_context_reload, diff --git a/crates/languages/bevy_mod_scripting_rhai/src/lib.rs b/crates/languages/bevy_mod_scripting_rhai/src/lib.rs index 5eb1b479d8..d023ef86d1 100644 --- a/crates/languages/bevy_mod_scripting_rhai/src/lib.rs +++ b/crates/languages/bevy_mod_scripting_rhai/src/lib.rs @@ -51,6 +51,10 @@ impl IntoScriptPluginParams for RhaiScriptingPlugin { fn build_runtime() -> Self::R { Engine::new().into() } + + fn handler() -> bevy_mod_scripting_core::handler::HandlerFn { + rhai_callback_handler + } } /// The rhai scripting plugin. Used to add rhai scripting to a bevy app within the context of the BMS framework. @@ -79,7 +83,6 @@ impl Default for RhaiScriptingPlugin { Ok(()) }], }, - callback_handler: rhai_callback_handler, context_builder: ContextBuilder { load: rhai_context_load, reload: rhai_context_reload, diff --git a/crates/testing_crates/test_utils/src/test_plugin.rs b/crates/testing_crates/test_utils/src/test_plugin.rs index c3b2a955c8..a8f2da228a 100644 --- a/crates/testing_crates/test_utils/src/test_plugin.rs +++ b/crates/testing_crates/test_utils/src/test_plugin.rs @@ -29,6 +29,16 @@ macro_rules! make_test_plugin { invocations: vec![].into(), } } + + fn handler() -> $ident::HandlerFn { + (|args, context_key, callback, script_ctxt, pre_handling_initializers, runtime| { + runtime + .invocations + .lock() + .push((context_key.entity(), Some(context_key.script().id()))); + Ok($ident::bindings::script_value::ScriptValue::Unit) + }) as $ident::HandlerFn + } } #[derive(Default, std::fmt::Debug)] diff --git a/release-plz.toml b/release-plz.toml index f2ce5e8ce3..94721dffc0 100644 --- a/release-plz.toml +++ b/release-plz.toml @@ -19,10 +19,11 @@ commit_parsers = [ { message = "^chore.*", skip = true }, { message = "^test.*", skip = true }, { message = "^docs.*", skip = true }, - { message = "^.*SKIP_CHANGELOG.*$", skip = true}, + { message = "^.*SKIP_CHANGELOG.*$", skip = true }, { message = "^feat", group = "added" }, { message = "^changed", group = "changed" }, { message = "^deprecated", group = "deprecated" }, + { message = "^refactor", group = "refactored" }, { message = "^fix", group = "fixed" }, { message = "^security", group = "security" }, { message = "^.*", group = "other" },