Skip to content

Commit 3f9a87e

Browse files
authored
refactor: inline CallbackSettings<P> into IntoScriptPluginParam at compile time (#455)
1 parent a4e4b21 commit 3f9a87e

File tree

9 files changed

+48
-70
lines changed

9 files changed

+48
-70
lines changed

.github/workflows/pr-titles.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ jobs:
2828
docs
2929
refactor
3030
perf
31+
changed
3132
3233
scopes: |
3334
ladfile

crates/bevy_mod_scripting_core/src/bindings/script_system.rs

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use crate::{
1515
error::{InteropError, ScriptError},
1616
event::CallbackLabel,
1717
extractors::get_all_access_ids,
18-
handler::CallbackSettings,
18+
handler::ScriptingHandler,
1919
runtime::RuntimeContainer,
2020
script::{ScriptAttachment, ScriptContext},
2121
IntoScriptPluginParams,
@@ -200,7 +200,6 @@ impl ScriptSystemBuilder {
200200

201201
struct DynamicHandlerContext<'w, P: IntoScriptPluginParams> {
202202
script_context: &'w ScriptContext<P>,
203-
callback_settings: &'w CallbackSettings<P>,
204203
context_loading_settings: &'w ContextLoadingSettings<P>,
205204
runtime_container: &'w RuntimeContainer<P>,
206205
}
@@ -215,17 +214,13 @@ impl<'w, P: IntoScriptPluginParams> DynamicHandlerContext<'w, P> {
215214
let mut access = FilteredAccess::<ComponentId>::matches_nothing();
216215
// let scripts_res_id = world
217216
// .query::<&Script<P>>();
218-
let callback_settings_res_id = world
219-
.resource_id::<CallbackSettings<P>>()
220-
.expect("CallbackSettings resource not found");
221217
let context_loading_settings_res_id = world
222218
.resource_id::<ContextLoadingSettings<P>>()
223219
.expect("ContextLoadingSettings resource not found");
224220
let runtime_container_res_id = world
225221
.resource_id::<RuntimeContainer<P>>()
226222
.expect("RuntimeContainer resource not found");
227223

228-
access.add_resource_read(callback_settings_res_id);
229224
access.add_resource_read(context_loading_settings_res_id);
230225
access.add_resource_read(runtime_container_res_id);
231226

@@ -240,9 +235,6 @@ impl<'w, P: IntoScriptPluginParams> DynamicHandlerContext<'w, P> {
240235
unsafe {
241236
Self {
242237
script_context: system.get_resource().expect("Scripts resource not found"),
243-
callback_settings: system
244-
.get_resource()
245-
.expect("CallbackSettings resource not found"),
246238
context_loading_settings: system
247239
.get_resource()
248240
.expect("ContextLoadingSettings resource not found"),
@@ -268,16 +260,14 @@ impl<'w, P: IntoScriptPluginParams> DynamicHandlerContext<'w, P> {
268260
};
269261

270262
// call the script
271-
let handler = self.callback_settings.callback_handler;
272263
let pre_handling_initializers = &self
273264
.context_loading_settings
274265
.context_pre_handling_initializers;
275266
let runtime = &self.runtime_container.runtime;
276267

277268
let mut context = context.lock();
278269

279-
CallbackSettings::<P>::call(
280-
handler,
270+
P::handle(
281271
payload,
282272
context_key,
283273
label,

crates/bevy_mod_scripting_core/src/extractors.rs

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,14 @@
33
//! These are designed to be used to pipe inputs into other systems which require them, while handling any configuration erorrs nicely.
44
#![allow(deprecated)]
55
use crate::bindings::pretty_print::DisplayWithWorld;
6+
use crate::handler::ScriptingHandler;
67
use crate::{
78
bindings::{
89
access_map::ReflectAccessId, script_value::ScriptValue, WorldAccessGuard, WorldGuard,
910
},
1011
context::ContextLoadingSettings,
1112
error::{InteropError, ScriptError},
1213
event::{CallbackLabel, IntoCallbackLabel},
13-
handler::CallbackSettings,
1414
runtime::RuntimeContainer,
1515
script::{ScriptAttachment, ScriptContext, StaticScripts},
1616
IntoScriptPluginParams,
@@ -122,8 +122,6 @@ unsafe impl<T: Resource + Default> SystemParam for ResScope<'_, T> {
122122

123123
/// Context for systems which handle events for scripts
124124
pub struct HandlerContext<P: IntoScriptPluginParams> {
125-
/// Settings for callbacks
126-
pub(crate) callback_settings: CallbackSettings<P>,
127125
/// Settings for loading contexts
128126
pub(crate) context_loading_settings: ContextLoadingSettings<P>,
129127
/// The runtime container
@@ -139,7 +137,6 @@ impl<P: IntoScriptPluginParams> HandlerContext<P> {
139137
/// Every call to this function must be paired with a call to [`Self::release`].
140138
pub fn yoink(world: &mut World) -> Self {
141139
Self {
142-
callback_settings: world.remove_resource().unwrap_or_default(),
143140
context_loading_settings: world.remove_resource().unwrap_or_default(),
144141
runtime_container: world.remove_resource().unwrap_or_default(),
145142
static_scripts: world.remove_resource().unwrap_or_default(),
@@ -151,7 +148,6 @@ impl<P: IntoScriptPluginParams> HandlerContext<P> {
151148
/// Only call this if you have previously yoinked the handler context from the world.
152149
pub fn release(self, world: &mut World) {
153150
// insert the handler context back into the world
154-
world.insert_resource(self.callback_settings);
155151
world.insert_resource(self.context_loading_settings);
156152
world.insert_resource(self.runtime_container);
157153
world.insert_resource(self.static_scripts);
@@ -165,24 +161,17 @@ impl<P: IntoScriptPluginParams> HandlerContext<P> {
165161
pub fn destructure(
166162
&mut self,
167163
) -> (
168-
&mut CallbackSettings<P>,
169164
&mut ContextLoadingSettings<P>,
170165
&mut RuntimeContainer<P>,
171166
&mut StaticScripts,
172167
) {
173168
(
174-
&mut self.callback_settings,
175169
&mut self.context_loading_settings,
176170
&mut self.runtime_container,
177171
&mut self.static_scripts,
178172
)
179173
}
180174

181-
/// Get the callback settings
182-
pub fn callback_settings(&mut self) -> &mut CallbackSettings<P> {
183-
&mut self.callback_settings
184-
}
185-
186175
/// Get the context loading settings
187176
pub fn context_loading_settings(&mut self) -> &mut ContextLoadingSettings<P> {
188177
&mut self.context_loading_settings
@@ -226,16 +215,14 @@ impl<P: IntoScriptPluginParams> HandlerContext<P> {
226215
};
227216

228217
// call the script
229-
let handler = self.callback_settings.callback_handler;
230218
let pre_handling_initializers = &self
231219
.context_loading_settings
232220
.context_pre_handling_initializers;
233221
let runtime = &self.runtime_container.runtime;
234222

235223
let mut context = context.lock();
236224

237-
CallbackSettings::<P>::call(
238-
handler,
225+
P::handle(
239226
payload,
240227
context_key,
241228
label,

crates/bevy_mod_scripting_core/src/handler.rs

Lines changed: 18 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ use crate::{
1717
use bevy::{
1818
ecs::{
1919
event::EventCursor,
20-
resource::Resource,
2120
system::{Local, SystemState},
2221
world::{Mut, World},
2322
},
@@ -35,39 +34,26 @@ pub type HandlerFn<P> = fn(
3534
runtime: &<P as IntoScriptPluginParams>::R,
3635
) -> Result<ScriptValue, ScriptError>;
3736

38-
/// A resource that holds the settings for the callback handler for a specific combination of type parameters
39-
#[derive(Resource)]
40-
pub struct CallbackSettings<P: IntoScriptPluginParams> {
41-
/// The callback handler function
42-
pub callback_handler: HandlerFn<P>,
43-
}
44-
45-
impl<P: IntoScriptPluginParams> Default for CallbackSettings<P> {
46-
fn default() -> Self {
47-
Self {
48-
callback_handler: |_, _, _, _, _, _| Ok(ScriptValue::Unit),
49-
}
50-
}
51-
}
52-
53-
impl<P: IntoScriptPluginParams> Clone for CallbackSettings<P> {
54-
fn clone(&self) -> Self {
55-
Self {
56-
callback_handler: self.callback_handler,
57-
}
58-
}
37+
/// A utility trait, implemented for all types implementing `IntoScriptPluginParams`.
38+
///
39+
/// Calls the underlying handler function with the provided arguments and context.
40+
/// Implementations will handle the necessary thread local context emplacement and retrieval.
41+
pub trait ScriptingHandler<P: IntoScriptPluginParams> {
42+
/// Calls the handler function with the given arguments and context
43+
fn handle(
44+
args: Vec<ScriptValue>,
45+
context_key: &ScriptAttachment,
46+
callback: &CallbackLabel,
47+
script_ctxt: &mut P::C,
48+
pre_handling_initializers: &[ContextPreHandlingInitializer<P>],
49+
runtime: &P::R,
50+
world: WorldGuard,
51+
) -> Result<ScriptValue, ScriptError>;
5952
}
6053

61-
#[profiling::all_functions]
62-
impl<P: IntoScriptPluginParams> CallbackSettings<P> {
63-
/// Creates a new callback settings resource with the given handler function
64-
pub fn new(callback_handler: HandlerFn<P>) -> Self {
65-
Self { callback_handler }
66-
}
67-
54+
impl<P: IntoScriptPluginParams> ScriptingHandler<P> for P {
6855
/// Calls the handler function while providing the necessary thread local context
69-
pub fn call(
70-
handler: HandlerFn<P>,
56+
fn handle(
7157
args: Vec<ScriptValue>,
7258
context_key: &ScriptAttachment,
7359
callback: &CallbackLabel,
@@ -78,7 +64,7 @@ impl<P: IntoScriptPluginParams> CallbackSettings<P> {
7864
) -> Result<ScriptValue, ScriptError> {
7965
WorldGuard::with_existing_static_guard(world.clone(), |world| {
8066
ThreadWorldContainer.set_world(world)?;
81-
(handler)(
67+
Self::handler()(
8268
args,
8369
context_key,
8470
callback,

crates/bevy_mod_scripting_core/src/lib.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use context::{
2020
};
2121
use error::ScriptError;
2222
use event::{ScriptCallbackEvent, ScriptCallbackResponseEvent, ScriptEvent};
23-
use handler::{CallbackSettings, HandlerFn};
23+
use handler::HandlerFn;
2424
use runtime::{initialize_runtime, Runtime, RuntimeContainer, RuntimeInitializer, RuntimeSettings};
2525
use script::{ContextPolicy, ScriptComponent, ScriptContext, StaticScripts};
2626

@@ -70,14 +70,16 @@ pub trait IntoScriptPluginParams: 'static {
7070

7171
/// Build the runtime
7272
fn build_runtime() -> Self::R;
73+
74+
/// Returns the handler function for the plugin
75+
fn handler() -> HandlerFn<Self>;
7376
}
7477

7578
/// Bevy plugin enabling scripting within the bevy mod scripting framework
7679
pub struct ScriptingPlugin<P: IntoScriptPluginParams> {
7780
/// Settings for the runtime
7881
pub runtime_settings: RuntimeSettings<P>,
79-
/// The handler used for executing callbacks in scripts
80-
pub callback_handler: HandlerFn<P>,
82+
8183
/// The context builder for loading contexts
8284
pub context_builder: ContextBuilder<P>,
8385

@@ -103,7 +105,6 @@ where
103105
{
104106
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
105107
f.debug_struct("ScriptingPlugin")
106-
.field("callback_handler", &self.callback_handler)
107108
.field("context_policy", &self.context_policy)
108109
.field("language", &self.language)
109110
.field("context_initializers", &self.context_initializers)
@@ -120,7 +121,6 @@ impl<P: IntoScriptPluginParams> Default for ScriptingPlugin<P> {
120121
fn default() -> Self {
121122
Self {
122123
runtime_settings: Default::default(),
123-
callback_handler: CallbackSettings::<P>::default().callback_handler,
124124
context_builder: Default::default(),
125125
context_policy: ContextPolicy::default(),
126126
language: Default::default(),
@@ -138,9 +138,6 @@ impl<P: IntoScriptPluginParams> Plugin for ScriptingPlugin<P> {
138138
.insert_resource::<RuntimeContainer<P>>(RuntimeContainer {
139139
runtime: P::build_runtime(),
140140
})
141-
.insert_resource::<CallbackSettings<P>>(CallbackSettings {
142-
callback_handler: self.callback_handler,
143-
})
144141
.insert_resource::<ContextLoadingSettings<P>>(ContextLoadingSettings {
145142
loader: self.context_builder.clone(),
146143
context_initializers: self.context_initializers.clone(),

crates/languages/bevy_mod_scripting_lua/src/lib.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ impl IntoScriptPluginParams for LuaScriptingPlugin {
3434
const LANGUAGE: Language = Language::Lua;
3535

3636
fn build_runtime() -> Self::R {}
37+
38+
fn handler() -> bevy_mod_scripting_core::handler::HandlerFn<Self> {
39+
lua_handler
40+
}
3741
}
3842

3943
// necessary for automatic config goodies
@@ -54,7 +58,6 @@ impl Default for LuaScriptingPlugin {
5458
LuaScriptingPlugin {
5559
scripting_plugin: ScriptingPlugin {
5660
runtime_settings: RuntimeSettings::default(),
57-
callback_handler: lua_handler,
5861
context_builder: ContextBuilder::<LuaScriptingPlugin> {
5962
load: lua_context_load,
6063
reload: lua_context_reload,

crates/languages/bevy_mod_scripting_rhai/src/lib.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ impl IntoScriptPluginParams for RhaiScriptingPlugin {
5151
fn build_runtime() -> Self::R {
5252
Engine::new().into()
5353
}
54+
55+
fn handler() -> bevy_mod_scripting_core::handler::HandlerFn<Self> {
56+
rhai_callback_handler
57+
}
5458
}
5559

5660
/// 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 {
7983
Ok(())
8084
}],
8185
},
82-
callback_handler: rhai_callback_handler,
8386
context_builder: ContextBuilder {
8487
load: rhai_context_load,
8588
reload: rhai_context_reload,

crates/testing_crates/test_utils/src/test_plugin.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,16 @@ macro_rules! make_test_plugin {
2929
invocations: vec![].into(),
3030
}
3131
}
32+
33+
fn handler() -> $ident::HandlerFn<Self> {
34+
(|args, context_key, callback, script_ctxt, pre_handling_initializers, runtime| {
35+
runtime
36+
.invocations
37+
.lock()
38+
.push((context_key.entity(), Some(context_key.script().id())));
39+
Ok($ident::bindings::script_value::ScriptValue::Unit)
40+
}) as $ident::HandlerFn<Self>
41+
}
3242
}
3343

3444
#[derive(Default, std::fmt::Debug)]

release-plz.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,11 @@ commit_parsers = [
1919
{ message = "^chore.*", skip = true },
2020
{ message = "^test.*", skip = true },
2121
{ message = "^docs.*", skip = true },
22-
{ message = "^.*SKIP_CHANGELOG.*$", skip = true},
22+
{ message = "^.*SKIP_CHANGELOG.*$", skip = true },
2323
{ message = "^feat", group = "added" },
2424
{ message = "^changed", group = "changed" },
2525
{ message = "^deprecated", group = "deprecated" },
26+
{ message = "^refactor", group = "refactored" },
2627
{ message = "^fix", group = "fixed" },
2728
{ message = "^security", group = "security" },
2829
{ message = "^.*", group = "other" },

0 commit comments

Comments
 (0)