Skip to content

Commit 5012e72

Browse files
committed
refactor scoped system params
1 parent 586eda2 commit 5012e72

File tree

11 files changed

+510
-213
lines changed

11 files changed

+510
-213
lines changed

assets/scripts/game_of_life.lua

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ function on_click(x,y)
7373
end
7474

7575
function on_update()
76+
info("Lua: updating")
7677
local cells = fetch_life_state().cells
7778
local settings = world.get_resource(Settings)
7879
local dimensions = settings.physical_grid_dimensions

crates/bevy_mod_scripting_core/src/asset.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,14 @@ pub struct AssetPathToLanguageMapper {
161161
pub map: fn(&AssetPath) -> Language,
162162
}
163163

164+
impl Default for AssetPathToLanguageMapper {
165+
fn default() -> Self {
166+
Self {
167+
map: |_| Language::Unknown,
168+
}
169+
}
170+
}
171+
164172
/// A cache of asset id's to their script id's. Necessary since when we drop an asset we won't have the ability to get the path from the asset.
165173
#[derive(Default, Debug, Resource)]
166174
pub struct ScriptMetadataStore {

crates/bevy_mod_scripting_core/src/bindings/world.rs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -185,13 +185,14 @@ impl<'w> WorldAccessGuard<'w> {
185185
let o = f();
186186
(o, self.inner.accesses.count_accesses())
187187
});
188-
if length_start != length_end {
189-
return Err(InteropError::invalid_access_count(
190-
length_end,
191-
length_start,
192-
"Component/Resource/Allocation locks (accesses) were not released correctly in an access scope",
193-
));
194-
}
188+
// TODO: re-enable this when
189+
// if length_start != length_end {
190+
// return Err(InteropError::invalid_access_count(
191+
// length_end,
192+
// length_start,
193+
// "Component/Resource/Allocation locks (accesses) were not released correctly in an access scope",
194+
// ));
195+
// }
195196

196197
Ok(o)
197198
}

crates/bevy_mod_scripting_core/src/commands.rs

Lines changed: 124 additions & 131 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@ use crate::{
55
bindings::WorldGuard,
66
context::ContextBuilder,
77
event::{IntoCallbackLabel, OnScriptLoaded, OnScriptUnloaded},
8-
extractors::{HandlerContext, WithWorldGuard},
8+
extractors::{with_handler_system_state, HandlerContext},
99
handler::{handle_script_errors, CallbackSettings},
1010
script::{Script, ScriptId, StaticScripts},
1111
IntoScriptPluginParams,
1212
};
13-
use bevy::{asset::Handle, ecs::system::SystemState, log::debug, prelude::Command};
13+
use bevy::{asset::Handle, log::debug, prelude::Command};
1414
use std::marker::PhantomData;
1515

1616
/// Deletes a script with the given ID
@@ -33,67 +33,62 @@ impl<P: IntoScriptPluginParams> DeleteScript<P> {
3333

3434
impl<P: IntoScriptPluginParams> Command for DeleteScript<P> {
3535
fn apply(self, world: &mut bevy::prelude::World) {
36-
let mut system_state: SystemState<WithWorldGuard<HandlerContext<P>>> =
37-
SystemState::new(world);
38-
39-
let mut with_guard = system_state.get_mut(world);
40-
41-
let (guard, handler_ctxt) = with_guard.get_mut();
42-
43-
if let Some(script) = handler_ctxt.scripts.scripts.remove(&self.id) {
44-
debug!("Deleting script with id: {}", self.id);
45-
46-
match handler_ctxt.script_contexts.get_mut(script.context_id) {
47-
Some(context) => {
48-
// first let the script uninstall itself
49-
match (CallbackSettings::<P>::call)(
50-
handler_ctxt.callback_settings.callback_handler,
51-
vec![],
52-
bevy::ecs::entity::Entity::from_raw(0),
53-
&self.id,
54-
&OnScriptUnloaded::into_callback_label(),
55-
context,
56-
&handler_ctxt
57-
.context_loading_settings
58-
.context_pre_handling_initializers,
59-
&mut handler_ctxt.runtime_container.runtime,
60-
guard.clone(),
61-
) {
62-
Ok(_) => {}
63-
Err(e) => {
64-
handle_script_errors(
65-
guard,
66-
[e.with_context(format!(
67-
"Running unload hook for script with id: {}. Language: {}",
68-
self.id,
69-
P::LANGUAGE
70-
))]
71-
.into_iter(),
72-
);
36+
with_handler_system_state(world, |guard, handler_ctxt: &mut HandlerContext<P>| {
37+
if let Some(script) = handler_ctxt.scripts.scripts.remove(&self.id) {
38+
debug!("Deleting script with id: {}", self.id);
39+
40+
match handler_ctxt.script_contexts.get_mut(script.context_id) {
41+
Some(context) => {
42+
// first let the script uninstall itself
43+
match (CallbackSettings::<P>::call)(
44+
handler_ctxt.callback_settings.callback_handler,
45+
vec![],
46+
bevy::ecs::entity::Entity::from_raw(0),
47+
&self.id,
48+
&OnScriptUnloaded::into_callback_label(),
49+
context,
50+
&handler_ctxt
51+
.context_loading_settings
52+
.context_pre_handling_initializers,
53+
&mut handler_ctxt.runtime_container.runtime,
54+
guard.clone(),
55+
) {
56+
Ok(_) => {}
57+
Err(e) => {
58+
handle_script_errors(
59+
guard,
60+
[e.with_context(format!(
61+
"Running unload hook for script with id: {}. Language: {}",
62+
self.id,
63+
P::LANGUAGE
64+
))]
65+
.into_iter(),
66+
);
67+
}
7368
}
74-
}
7569

76-
debug!("Removing script with id: {}", self.id);
77-
(handler_ctxt.context_loading_settings.assigner.remove)(
78-
script.context_id,
79-
&script,
80-
&mut handler_ctxt.script_contexts,
81-
)
82-
}
83-
None => {
84-
bevy::log::error!(
85-
"Could not find context with id: {} corresponding to script with id: {}. Removing script without running callbacks.",
70+
debug!("Removing script with id: {}", self.id);
71+
(handler_ctxt.context_loading_settings.assigner.remove)(
8672
script.context_id,
87-
self.id
88-
);
89-
}
90-
};
91-
} else {
92-
bevy::log::error!(
93-
"Attempted to delete script with id: {} but it does not exist, doing nothing!",
94-
self.id
95-
);
96-
}
73+
&script,
74+
&mut handler_ctxt.script_contexts,
75+
)
76+
}
77+
None => {
78+
bevy::log::error!(
79+
"Could not find context with id: {} corresponding to script with id: {}. Removing script without running callbacks.",
80+
script.context_id,
81+
self.id
82+
);
83+
}
84+
};
85+
} else {
86+
bevy::log::error!(
87+
"Attempted to delete script with id: {} but it does not exist, doing nothing!",
88+
self.id
89+
);
90+
}
91+
})
9792
}
9893
}
9994

@@ -207,86 +202,84 @@ impl<P: IntoScriptPluginParams> CreateOrUpdateScript<P> {
207202

208203
impl<P: IntoScriptPluginParams> Command for CreateOrUpdateScript<P> {
209204
fn apply(self, world: &mut bevy::prelude::World) {
210-
let mut system_state: SystemState<WithWorldGuard<HandlerContext<P>>> =
211-
SystemState::new(world);
212-
213-
let mut with_guard = system_state.get_mut(world);
214-
215-
let (guard, handler_ctxt) = with_guard.get_mut();
205+
with_handler_system_state(world, |guard, handler_ctxt: &mut HandlerContext<P>| {
206+
let script = handler_ctxt.scripts.scripts.get(&self.id);
207+
let previous_context_id = script.as_ref().map(|s| s.context_id);
208+
debug!(
209+
"{}: CreateOrUpdateScript command applying (script_id: {}, previous_context_id: {:?})",
210+
P::LANGUAGE,
211+
self.id,
212+
previous_context_id
213+
);
216214

217-
let script = handler_ctxt.scripts.scripts.get(&self.id);
218-
let previous_context_id = script.as_ref().map(|s| s.context_id);
219-
debug!(
220-
"{}: CreateOrUpdateScript command applying (script_id: {}, previous_context_id: {:?})",
221-
P::LANGUAGE,
222-
self.id,
223-
previous_context_id
224-
);
215+
match previous_context_id {
216+
Some(previous_context_id) => {
217+
bevy::log::debug!(
218+
"{}: script with id already has a context: {}",
219+
P::LANGUAGE,
220+
self.id
221+
);
222+
self.reload_context(guard.clone(), handler_ctxt, previous_context_id);
223+
}
224+
None => {
225+
let log_context = format!("{}: Loading script: {}", P::LANGUAGE, self.id);
225226

226-
match previous_context_id {
227-
Some(previous_context_id) => {
228-
bevy::log::debug!(
229-
"{}: script with id already has a context: {}",
230-
P::LANGUAGE,
231-
self.id
232-
);
233-
self.reload_context(guard.clone(), handler_ctxt, previous_context_id);
234-
}
235-
None => {
236-
let log_context = format!("{}: Loading script: {}", P::LANGUAGE, self.id);
237-
238-
let new_context_id = (handler_ctxt.context_loading_settings.assigner.assign)(
239-
&self.id,
240-
&self.content,
241-
&handler_ctxt.script_contexts,
242-
)
243-
.unwrap_or_else(|| handler_ctxt.script_contexts.allocate_id());
244-
if handler_ctxt.script_contexts.contains(new_context_id) {
245-
self.reload_context(guard, handler_ctxt, new_context_id);
246-
} else {
247-
// load new context
248-
bevy::log::debug!("{}", log_context);
249-
let ctxt = (ContextBuilder::<P>::load)(
250-
handler_ctxt.context_loading_settings.loader.load,
227+
let new_context_id = (handler_ctxt.context_loading_settings.assigner.assign)(
251228
&self.id,
252229
&self.content,
253-
&handler_ctxt.context_loading_settings.context_initializers,
254-
&handler_ctxt
255-
.context_loading_settings
256-
.context_pre_handling_initializers,
257-
guard.clone(),
258-
&mut handler_ctxt.runtime_container.runtime,
259-
);
230+
&handler_ctxt.script_contexts,
231+
)
232+
.unwrap_or_else(|| handler_ctxt.script_contexts.allocate_id());
233+
if handler_ctxt.script_contexts.contains(new_context_id) {
234+
self.reload_context(guard, handler_ctxt, new_context_id);
235+
} else {
236+
// load new context
237+
bevy::log::debug!("{}", log_context);
238+
let ctxt = (ContextBuilder::<P>::load)(
239+
handler_ctxt.context_loading_settings.loader.load,
240+
&self.id,
241+
&self.content,
242+
&handler_ctxt.context_loading_settings.context_initializers,
243+
&handler_ctxt
244+
.context_loading_settings
245+
.context_pre_handling_initializers,
246+
guard.clone(),
247+
&mut handler_ctxt.runtime_container.runtime,
248+
);
260249

261-
let mut ctxt = match ctxt {
262-
Ok(ctxt) => ctxt,
263-
Err(e) => {
264-
handle_script_errors(guard, [e.with_context(log_context)].into_iter());
265-
return;
250+
let mut ctxt = match ctxt {
251+
Ok(ctxt) => ctxt,
252+
Err(e) => {
253+
handle_script_errors(
254+
guard,
255+
[e.with_context(log_context)].into_iter(),
256+
);
257+
return;
258+
}
259+
};
260+
261+
self.run_on_load_callback(handler_ctxt, guard, &mut ctxt);
262+
263+
if handler_ctxt
264+
.script_contexts
265+
.insert_with_id(new_context_id, ctxt)
266+
.is_some()
267+
{
268+
bevy::log::warn!("{}: Context with id {} was not expected to exist. Overwriting it with a new context. This might happen if a script is not completely removed.", P::LANGUAGE, new_context_id);
266269
}
267-
};
268-
269-
self.run_on_load_callback(handler_ctxt, guard, &mut ctxt);
270-
271-
if handler_ctxt
272-
.script_contexts
273-
.insert_with_id(new_context_id, ctxt)
274-
.is_some()
275-
{
276-
bevy::log::warn!("{}: Context with id {} was not expected to exist. Overwriting it with a new context. This might happen if a script is not completely removed.", P::LANGUAGE, new_context_id);
277270
}
278-
}
279271

280-
handler_ctxt.scripts.scripts.insert(
281-
self.id.clone(),
282-
Script {
283-
id: self.id,
284-
asset: self.asset,
285-
context_id: new_context_id,
286-
},
287-
);
272+
handler_ctxt.scripts.scripts.insert(
273+
self.id.clone(),
274+
Script {
275+
id: self.id,
276+
asset: self.asset,
277+
context_id: new_context_id,
278+
},
279+
);
280+
}
288281
}
289-
}
282+
})
290283
}
291284
}
292285

crates/bevy_mod_scripting_core/src/context.rs

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
33
use crate::{
44
bindings::{ThreadWorldContainer, WorldContainer, WorldGuard},
5-
error::ScriptError,
5+
error::{InteropError, ScriptError},
66
script::{Script, ScriptId},
77
IntoScriptPluginParams,
88
};
@@ -92,6 +92,17 @@ pub struct ContextLoadingSettings<P: IntoScriptPluginParams> {
9292
pub context_pre_handling_initializers: Vec<ContextPreHandlingInitializer<P>>,
9393
}
9494

95+
impl<P: IntoScriptPluginParams> Default for ContextLoadingSettings<P> {
96+
fn default() -> Self {
97+
Self {
98+
loader: ContextBuilder::default(),
99+
assigner: ContextAssigner::default(),
100+
context_initializers: Default::default(),
101+
context_pre_handling_initializers: Default::default(),
102+
}
103+
}
104+
}
105+
95106
impl<T: IntoScriptPluginParams> Clone for ContextLoadingSettings<T> {
96107
fn clone(&self) -> Self {
97108
Self {
@@ -129,6 +140,17 @@ pub struct ContextBuilder<P: IntoScriptPluginParams> {
129140
pub reload: ContextReloadFn<P>,
130141
}
131142

143+
impl<P: IntoScriptPluginParams> Default for ContextBuilder<P> {
144+
fn default() -> Self {
145+
Self {
146+
load: |_, _, _, _, _| Err(InteropError::invariant("no context loader set").into()),
147+
reload: |_, _, _, _, _, _| {
148+
Err(InteropError::invariant("no context reloader set").into())
149+
},
150+
}
151+
}
152+
}
153+
132154
impl<P: IntoScriptPluginParams> ContextBuilder<P> {
133155
/// load a context
134156
pub fn load(

0 commit comments

Comments
 (0)