diff --git a/crates/bevy_mod_scripting_core/src/asset.rs b/crates/bevy_mod_scripting_core/src/asset.rs index 44b93378f0..96544bdc09 100644 --- a/crates/bevy_mod_scripting_core/src/asset.rs +++ b/crates/bevy_mod_scripting_core/src/asset.rs @@ -250,6 +250,7 @@ fn handle_script_events( let handle = Handle::Weak(*id); let attachment = ScriptAttachment::StaticScript(handle.clone()); + let script_contexts = script_contexts.read(); for (resident, _) in script_contexts .residents(&attachment) .filter(|(r, _)| r.script() == handle && r.is_static()) 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 e54fca8503..c8d50ac20e 100644 --- a/crates/bevy_mod_scripting_core/src/bindings/script_system.rs +++ b/crates/bevy_mod_scripting_core/src/bindings/script_system.rs @@ -7,15 +7,14 @@ use super::{ access_map::ReflectAccessId, function::{from::Val, into::IntoScript, script_function::AppScriptFunctionRegistry}, schedule::AppScheduleRegistry, - script_value::ScriptValue, }; +use crate::extractors::CallContext; use crate::{ IntoScriptPluginParams, bindings::pretty_print::DisplayWithWorld, - error::{InteropError, ScriptError}, + error::InteropError, event::CallbackLabel, extractors::get_all_access_ids, - handler::ScriptingHandler, script::{ScriptAttachment, ScriptContext}, }; use ::{ @@ -39,10 +38,8 @@ use bevy_ecs::{ }; use bevy_log::{error, info, warn_once}; use bevy_system_reflection::{ReflectSchedule, ReflectSystem}; -use parking_lot::Mutex; use std::{ any::TypeId, borrow::Cow, collections::HashSet, hash::Hash, marker::PhantomData, ops::Deref, - sync::Arc, }; #[derive(Clone, Hash, PartialEq, Eq)] /// a system set for script systems. @@ -198,64 +195,8 @@ impl ScriptSystemBuilder { } } -struct DynamicHandlerContext<'w, P: IntoScriptPluginParams> { - script_context: &'w ScriptContext

, -} - -#[profiling::all_functions] -impl<'w, P: IntoScriptPluginParams> DynamicHandlerContext<'w, P> { - #[allow( - clippy::expect_used, - reason = "cannot avoid panicking inside init_param due to Bevy API structure" - )] - pub fn init_param(world: &mut World, system: &mut FilteredAccessSet) { - let mut access = FilteredAccess::::matches_nothing(); - - let script_context_res_id = world - .resource_id::>() - .expect("Scripts resource not found"); - - access.add_resource_read(script_context_res_id); - - system.add(access); - } - - #[allow( - clippy::expect_used, - reason = "cannot avoid panicking inside get_param due to Bevy API structure" - )] - pub fn get_param(system: &UnsafeWorldCell<'w>) -> Self { - unsafe { - Self { - script_context: system.get_resource().expect("Scripts resource not found"), - } - } - } - - /// Call a dynamic label on a script - pub fn call_dynamic_label( - &self, - label: &CallbackLabel, - context_key: &ScriptAttachment, - context: Option>>, - payload: Vec, - guard: WorldGuard<'_>, - ) -> Result { - // find script - let Some(context) = context.or_else(|| self.script_context.get(context_key)) else { - return Err(InteropError::missing_context(context_key.clone()).into()); - }; - - // call the script - - let mut context = context.lock(); - - P::handle(payload, context_key, label, &mut context, guard) - } -} - /// TODO: inline world guard into the system state, we should be able to re-use it -struct ScriptSystemState { +struct ScriptSystemState { type_registry: AppTypeRegistry, function_registry: AppScriptFunctionRegistry, schedule_registry: AppScheduleRegistry, @@ -264,6 +205,7 @@ struct ScriptSystemState { subset: HashSet, callback_label: CallbackLabel, system_params: Vec, + script_contexts: ScriptContext

, } /// Equivalent of [`SystemParam`] but for dynamic systems, these are the kinds of things @@ -308,7 +250,7 @@ pub struct DynamicScriptSystem { target_attachment: ScriptAttachment, archetype_generation: ArchetypeGeneration, system_param_descriptors: Vec, - state: Option, + state: Option>, _marker: std::marker::PhantomData P>, } @@ -453,16 +395,17 @@ impl System for DynamicScriptSystem

{ // targetted scripts. Let's start with just calling the one targetted // script. - let handler_ctxt = DynamicHandlerContext::

::get_param(&world); + let script_context = &state.script_contexts.read(); - if let Some(context) = handler_ctxt.script_context.get(&self.target_attachment) { - let result = handler_ctxt.call_dynamic_label( - &state.callback_label, + if let Some(context) = script_context.get_context(&self.target_attachment) { + let mut context = context.lock(); + let result = context.call_context_dynamic( &self.target_attachment, - Some(context), + &state.callback_label, payload, guard.clone(), ); + drop(context); // TODO: Emit error events via commands, maybe accumulate in state // instead and use apply. match result { @@ -552,9 +495,6 @@ impl System for DynamicScriptSystem

{ } } - // TODO: access to internal resources, i.e. handler state - DynamicHandlerContext::

::init_param(world, &mut self.component_access_set); - self.state = Some(ScriptSystemState { type_registry: world.get_resource_or_init::().clone(), function_registry: world @@ -568,6 +508,7 @@ impl System for DynamicScriptSystem

{ subset, callback_label: self.name.to_string().into(), system_params, + script_contexts: world.get_resource_or_init::>().clone(), }) } diff --git a/crates/bevy_mod_scripting_core/src/commands.rs b/crates/bevy_mod_scripting_core/src/commands.rs index 998c351c23..5630322f1e 100644 --- a/crates/bevy_mod_scripting_core/src/commands.rs +++ b/crates/bevy_mod_scripting_core/src/commands.rs @@ -1,6 +1,6 @@ //! Commands for creating, updating and deleting scripts -use std::marker::PhantomData; +use std::{marker::PhantomData, sync::Arc}; use crate::{ IntoScriptPluginParams, ScriptContext, @@ -12,16 +12,17 @@ use crate::{ CallbackLabel, IntoCallbackLabel, OnScriptLoaded, OnScriptReloaded, OnScriptUnloaded, ScriptCallbackResponseEvent, ScriptEvent, }, - extractors::{HandlerContext, with_handler_system_state}, + extractors::CallContext, handler::{handle_script_errors, send_callback_response}, script::{DisplayProxy, ScriptAttachment}, }; use bevy_ecs::{system::Command, world::World}; use bevy_log::{error, info, trace}; +use parking_lot::Mutex; use { bevy_asset::{Assets, Handle}, bevy_ecs::event::Events, - bevy_log::{debug, warn}, + bevy_log::debug, }; /// Detaches a script, invoking the `on_script_unloaded` callback if it exists, and removes the script from the static scripts collection. @@ -55,10 +56,11 @@ impl Command for DeleteScript

{ fn apply(mut self, world: &mut World) { // we demote to weak from here on out, so as not to hold the asset hostage self.context_key = self.context_key.into_weak(); + let script_contexts = world.get_resource_or_init::>().clone(); // first check the script exists, if it does not it could have been deleted by another command { - let script_contexts = world.get_resource_or_init::>(); + let script_contexts = script_contexts.read(); if !script_contexts.contains(&self.context_key) { debug!( "{}: No context found for {}, not deleting.", @@ -80,7 +82,7 @@ impl Command for DeleteScript

{ world, ); - let mut script_contexts = world.get_resource_or_init::>(); + let mut script_contexts = script_contexts.write(); let residents_count = script_contexts.residents_len(&self.context_key); let delete_context = residents_count == 1; let script_id = self.context_key.script(); @@ -164,7 +166,7 @@ impl CreateOrUpdateScript

{ fn before_reload( attachment: ScriptAttachment, world: WorldGuard, - handler_ctxt: &HandlerContext

, + ctxt: Arc>, emit_responses: bool, ) -> Option { // if something goes wrong, the error will be handled in the command @@ -177,14 +179,14 @@ impl CreateOrUpdateScript

{ ) .with_context(P::LANGUAGE) .with_context("saving reload state") - .run_with_handler(world, handler_ctxt) + .run_with_context(world, ctxt) .ok() } fn after_load( attachment: ScriptAttachment, world: WorldGuard, - handler_ctxt: &HandlerContext

, + script_context: Arc>, script_state: Option, emit_responses: bool, is_reload: bool, @@ -197,7 +199,7 @@ impl CreateOrUpdateScript

{ ) .with_context(P::LANGUAGE) .with_context("on loaded callback") - .run_with_handler(world.clone(), handler_ctxt); + .run_with_context(world.clone(), script_context.clone()); if is_reload { let _ = RunScriptCallback::

::new( @@ -208,108 +210,164 @@ impl CreateOrUpdateScript

{ ) .with_context(P::LANGUAGE) .with_context("on reloaded callback") - .run_with_handler(world, handler_ctxt); + .run_with_context(world, script_context); } } - pub(crate) fn create_or_update_script( + /// when a brand new script is created with no prior context + pub(crate) fn create_new_script_and_context( attachment: &ScriptAttachment, content: &[u8], guard: WorldGuard, - handler_ctxt: &mut HandlerContext

, + script_context: ScriptContext

, emit_responses: bool, ) -> Result<(), ScriptError> { - // we demote to weak from here on out, so as not to hold the asset hostage - let attachment = attachment.clone().into_weak(); + let ctxt = Self::load_context(attachment, content, guard.clone())?; - let script_id = attachment.script(); + let ctxt = Arc::new(Mutex::new(ctxt)); + let mut script_context = script_context.write(); - let phrase; - let success; - let mut script_state = None; - // what callbacks we invoke depends whether or not this attachment - // was already present in the context or not - let is_reload = handler_ctxt.script_context.contains(&attachment); - if is_reload { - phrase = "reloading"; - success = "reloaded"; - script_state = Self::before_reload( - attachment.clone(), - guard.clone(), - handler_ctxt, - emit_responses, - ); - } else { - phrase = "loading"; - success = "loaded"; - }; + script_context + .insert_arc(attachment, ctxt.clone()) + .map_err(|_| { + ScriptError::new(String::from("No context policy applied")) + .with_context("creating new script and context") + })?; - // whether or not we actually load vs reload the context (i.e. scrap the old one and create a new one) - // depends on whether the context is already present in the script context - let context = handler_ctxt.script_context.get(&attachment); - let result_context_to_insert = match context { - Some(context) => { - let mut context = context.lock(); + Self::after_load( + attachment.clone(), + guard, + ctxt, + None, // no prior script state + emit_responses, + false, // first ever load + ); - Self::reload_context(&attachment, content, &mut context, guard.clone()) - .map(|_| None) - } - None => Self::load_context(&attachment, content, guard.clone()).map(Some), - }; + Ok(()) + } - match result_context_to_insert { - Ok(maybe_context) => { - if let Some(context) = maybe_context - && handler_ctxt - .script_context - .insert(&attachment, context) - .is_err() - { - warn!("Unable to insert script context for {}.", attachment); - } + /// when a script is created in an existing context, e.g. when using shared contexts + /// and we load a script into that context the first time (although not a reload from it's POV) + pub(crate) fn create_script_in_existing_context( + attachment: &ScriptAttachment, + content: &[u8], + guard: WorldGuard, + context: Arc>, + script_context: ScriptContext

, + emit_responses: bool, + ) -> Result<(), ScriptError> { + let mut context_guard = context.lock(); + Self::reload_context(attachment, content, &mut context_guard, guard.clone())?; + drop(context_guard); - // mark as resident in the context - handler_ctxt - .script_context - .insert_resident(attachment.clone()) - .map_err(|err| { - ScriptError::new(InteropError::invariant(format!( - "expected context to be present, could not mark attachment as resident in context, {err:?}" - ))) - })?; + let mut script_context = script_context.write(); - debug!( - "{}: script {} successfully {}", - P::LANGUAGE, - attachment, - success, - ); + script_context.insert_resident(attachment.clone()).map_err(|err| { + ScriptError::new(InteropError::invariant(format!( + "expected context to be present, could not mark attachment as resident in context, {err:?}" + ))) + })?; + + Self::after_load( + attachment.clone(), + guard, + context.clone(), + None, // no prior script state + emit_responses, + false, // first ever load + ); + Ok(()) + } + + /// Reloads a script in an already existing context + pub(crate) fn reload_script_in_context( + attachment: &ScriptAttachment, + content: &[u8], + guard: WorldGuard, + context: Arc>, + emit_responses: bool, + ) -> Result<(), ScriptError> { + let script_state = Self::before_reload( + attachment.clone(), + guard.clone(), + context.clone(), + emit_responses, + ); + + let mut context_guard = context.lock(); + Self::reload_context(attachment, content, &mut context_guard, guard.clone())?; + drop(context_guard); + + Self::after_load( + attachment.clone(), + guard, + context.clone(), + script_state, + emit_responses, + true, // this is definitely a reload + ); - Self::after_load( + Ok(()) + } + + pub(crate) fn create_or_update_script( + attachment: &ScriptAttachment, + content: &[u8], + guard: WorldGuard, + script_context: ScriptContext

, + emit_responses: bool, + ) -> Result<(), ScriptError> { + // determine if + // - context exists + // - script is RESIDENT in context + + let script_context_guard = script_context.read(); + let (context, is_resident) = script_context_guard.get_context_and_residency(attachment); + drop(script_context_guard); + + let res = match (context, is_resident) { + (None, _) => { + // no context exists, create new context and script + Self::create_new_script_and_context( attachment, + content, guard, - handler_ctxt, - script_state, + script_context, emit_responses, - is_reload, - ); - - Ok(()) + ) + .map_err(|err| { + err.with_context(format!("creating new context for script {attachment}")) + }) } - Err(err) => { - handle_script_errors( + (Some(context), false) => { + // context exists, but script is not resident in it, add script to existing context + Self::create_script_in_existing_context( + attachment, + content, guard, - vec![ - err.clone() - .with_script(script_id.display()) - .with_context(P::LANGUAGE) - .with_context(phrase), - ] - .into_iter(), - ); - Err(err) + context, + script_context, + emit_responses, + ) + .map_err(|err| { + err.with_context(format!("creating script {attachment} in existing context")) + }) } - } + (Some(context), true) => { + // context exists, and script is resident in it, reload script in existing context + Self::reload_script_in_context(attachment, content, guard, context, emit_responses) + .map_err(|err| { + err.with_context(format!( + "reloading script {attachment} in existing context" + )) + }) + } + }; + + res.map_err(|err| { + err.with_script(attachment.script().display()) + .with_context(P::LANGUAGE) + }) } } @@ -334,16 +392,19 @@ impl Command for CreateOrUpdateScript

{ } }, }; + let script_context = world.get_resource_or_init::>().clone(); + let guard = WorldGuard::new_exclusive(world); + let res = Self::create_or_update_script( + &self.attachment, + &content, + guard, + script_context, + self.emit_responses, + ); - with_handler_system_state(world, |guard, handler_ctxt: &mut HandlerContext

| { - let _ = Self::create_or_update_script( - &self.attachment, - &content, - guard.clone(), - handler_ctxt, - self.emit_responses, - ); - }); + if let Err(err) = res { + handle_script_errors(WorldGuard::new_exclusive(world), vec![err].into_iter()); + } } } @@ -387,16 +448,18 @@ impl RunScriptCallback

{ self } - /// Equivalent to [`Self::run`], but usable in the case where you already have a [`HandlerContext`]. - pub fn run_with_handler( + /// Run the command on the given context. + /// + /// Assumes this context matches the attachment for the command. + pub fn run_with_context( self, guard: WorldGuard, - handler_ctxt: &HandlerContext

, + ctxt: Arc>, ) -> Result { - let result = handler_ctxt.call_dynamic_label( - &self.callback, + let mut ctxt_guard = ctxt.lock(); + let result = ctxt_guard.call_context_dynamic( &self.attachment, - None, + &self.callback, self.args, guard.clone(), ); @@ -433,13 +496,35 @@ impl RunScriptCallback

{ result } + /// Equivalent to [`Self::run`], but usable in the case where you already have a [`HandlerContext`]. + pub fn run_with_contexts( + self, + guard: WorldGuard, + script_contexts: ScriptContext

, + ) -> Result { + let script_contexts = script_contexts.read(); + let ctxt = script_contexts.get_context(&self.attachment); + let ctxt = match ctxt { + Some(ctxt) => ctxt, + None => { + let err = ScriptError::new(InteropError::missing_context(self.attachment.clone())) + .with_script(self.attachment.script().display()) + .with_context(P::LANGUAGE); + handle_script_errors(guard, vec![err.clone()].into_iter()); + return Err(err); + } + }; + + self.run_with_context(guard, ctxt.clone()) + } + /// Equivalent to running the command, but also returns the result of the callback. /// /// The returned error will already be handled and logged. pub fn run(self, world: &mut World) -> Result { - with_handler_system_state(world, |guard, handler_ctxt: &mut HandlerContext

| { - self.run_with_handler(guard, handler_ctxt) - }) + let script_contexts = world.get_resource_or_init::>().clone(); + let guard = WorldGuard::new_exclusive(world); + self.run_with_contexts(guard, script_contexts) } } diff --git a/crates/bevy_mod_scripting_core/src/context.rs b/crates/bevy_mod_scripting_core/src/context.rs index b9b65b9131..47355c6b37 100644 --- a/crates/bevy_mod_scripting_core/src/context.rs +++ b/crates/bevy_mod_scripting_core/src/context.rs @@ -6,6 +6,7 @@ use crate::{ IntoScriptPluginParams, bindings::{ThreadWorldContainer, WorldContainer, WorldGuard}, error::ScriptError, + extractors::GetPluginFor, script::ScriptAttachment, }; @@ -13,8 +14,8 @@ use crate::{ /// /// Contexts are not required to be `Sync` as they are internally stored behind a `Mutex` but they must satisfy `Send` so they can be /// freely sent between threads. -pub trait Context: 'static + Send {} -impl Context for T {} +pub trait Context: 'static + Send + GetPluginFor {} +impl Context for T {} /// Initializer run once after creating a context but before executing it for the first time as well as after re-loading the script pub type ContextInitializer

= diff --git a/crates/bevy_mod_scripting_core/src/event.rs b/crates/bevy_mod_scripting_core/src/event.rs index 4a44df8b7b..60da689fa5 100644 --- a/crates/bevy_mod_scripting_core/src/event.rs +++ b/crates/bevy_mod_scripting_core/src/event.rs @@ -206,15 +206,16 @@ impl Recipients { /// Retrieves all the recipients of the event based on existing scripts pub fn get_recipients( &self, - script_context: &ScriptContext

, + script_context: ScriptContext

, ) -> Vec<(ScriptAttachment, Arc>)> { + let script_context = script_context.read(); match self { Recipients::AllScripts => script_context.all_residents().collect(), Recipients::AllContexts => script_context.first_resident_from_each_context().collect(), Recipients::ScriptEntity(script, entity) => { let attachment = ScriptAttachment::EntityScript(*entity, Handle::Weak(*script)); script_context - .get(&attachment) + .get_context(&attachment) .into_iter() .map(|entry| (attachment.clone(), entry)) .collect() @@ -222,7 +223,7 @@ impl Recipients { Recipients::StaticScript(script) => { let attachment = ScriptAttachment::StaticScript(Handle::Weak(*script)); script_context - .get(&attachment) + .get_context(&attachment) .into_iter() .map(|entry| (attachment.clone(), entry)) .collect() @@ -484,7 +485,8 @@ mod test { /// - StaticScriptB (AssetId::from_bits(5)) fn make_test_contexts() -> ScriptContext { let policy = ContextPolicy::per_entity(); - let mut script_context = ScriptContext::::new(policy); + let script_context = ScriptContext::::new(policy); + let mut script_context_guard = script_context.write(); let context_a = TestContext { invocations: vec![ScriptValue::String("a".to_string().into())], }; @@ -506,41 +508,42 @@ mod test { let static_script_a = Handle::Weak(AssetId::from(AssetIndex::from_bits(4))); let static_script_b = Handle::Weak(AssetId::from(AssetIndex::from_bits(5))); - script_context + script_context_guard .insert( &ScriptAttachment::EntityScript(Entity::from_raw(0), entity_script_a), context_a, ) .unwrap(); - script_context + script_context_guard .insert_resident(ScriptAttachment::EntityScript( Entity::from_raw(0), entity_script_b, )) .unwrap(); - script_context + script_context_guard .insert( &ScriptAttachment::EntityScript(Entity::from_raw(1), entity_script_c), context_b, ) .unwrap(); - script_context + script_context_guard .insert_resident(ScriptAttachment::EntityScript( Entity::from_raw(1), entity_script_d, )) .unwrap(); - script_context + script_context_guard .insert(&ScriptAttachment::StaticScript(static_script_a), context_c) .unwrap(); - script_context + script_context_guard .insert(&ScriptAttachment::StaticScript(static_script_b), context_d) .unwrap(); + drop(script_context_guard); script_context } @@ -575,7 +578,7 @@ mod test { #[test] fn test_all_scripts_recipients() { let script_context = make_test_contexts(); - let recipients = Recipients::AllScripts.get_recipients(&script_context); + let recipients = Recipients::AllScripts.get_recipients(script_context); assert_eq!(recipients.len(), 6); let mut id_context_pairs = recipients_to_asset_ids(&recipients); @@ -597,7 +600,7 @@ mod test { #[test] fn test_all_contexts_recipients() { let script_context = make_test_contexts(); - let recipients = Recipients::AllContexts.get_recipients(&script_context); + let recipients = Recipients::AllContexts.get_recipients(script_context); assert_eq!(recipients.len(), 4); let mut id_context_pairs = recipients_to_asset_ids(&recipients); id_context_pairs.sort_by_key(|(id, _)| *id); @@ -623,7 +626,7 @@ mod test { let script_context = make_test_contexts(); let recipients = Recipients::ScriptEntity(AssetId::from(AssetIndex::from_bits(0)), Entity::from_raw(0)) - .get_recipients(&script_context); + .get_recipients(script_context); assert_eq!(recipients.len(), 1); let id_context_pairs = recipients_to_asset_ids(&recipients); @@ -634,7 +637,7 @@ mod test { fn test_static_script_recipients() { let script_context = make_test_contexts(); let recipients = Recipients::StaticScript(AssetId::from(AssetIndex::from_bits(4))) - .get_recipients(&script_context); + .get_recipients(script_context); assert_eq!(recipients.len(), 1); let id_context_pairs = recipients_to_asset_ids(&recipients); diff --git a/crates/bevy_mod_scripting_core/src/extractors.rs b/crates/bevy_mod_scripting_core/src/extractors.rs index b3059cfe12..b0280efa0c 100644 --- a/crates/bevy_mod_scripting_core/src/extractors.rs +++ b/crates/bevy_mod_scripting_core/src/extractors.rs @@ -1,8 +1,6 @@ //! Systems which are used to extract the various resources and components used by BMS. //! //! These are designed to be used to pipe inputs into other systems which require them, while handling any configuration erorrs nicely. -#![allow(deprecated)] -use std::sync::Arc; use bevy_ecs::{ archetype::Archetype, @@ -13,7 +11,6 @@ use bevy_ecs::{ world::{DeferredWorld, World, unsafe_world_cell::UnsafeWorldCell}, }; use fixedbitset::FixedBitSet; -use parking_lot::Mutex; use crate::{ IntoScriptPluginParams, @@ -21,98 +18,54 @@ use crate::{ WorldAccessGuard, WorldGuard, access_map::ReflectAccessId, pretty_print::DisplayWithWorld, script_value::ScriptValue, }, - error::{InteropError, ScriptError}, + context::Context, + error::ScriptError, event::{CallbackLabel, IntoCallbackLabel}, handler::ScriptingHandler, - script::{ScriptAttachment, ScriptContext}, + script::ScriptAttachment, }; -/// Executes `system_state.get_mut` followed by `system_state.apply` after running the given closure, makes sure state is correctly handled in the context of an exclusive system. -/// Using system state with a handler ctxt without applying the state after will leave the world in an inconsistent state. -pub fn with_handler_system_state< - P: IntoScriptPluginParams, - F: FnOnce(WorldGuard, &mut HandlerContext

) -> O, - O, ->( - world: &mut World, - f: F, -) -> O { - let mut handler_ctxt = HandlerContext::

::yoink(world); - let guard = WorldGuard::new_exclusive(world); - let o = f(guard, &mut handler_ctxt); - handler_ctxt.release(world); - - o +/// A reverse mapping from plugin context types to their plugin types. +/// Useful in implementing generic traits on context types. +pub trait GetPluginFor { + /// The plugin type associated with this context type + type P: IntoScriptPluginParams; } -/// Context for systems which handle events for scripts -pub struct HandlerContext { - /// Script context - pub(crate) script_context: ScriptContext

, -} - -impl HandlerContext

{ - /// Yoink the handler context from the world, this will remove the matching resource from the world. - /// Every call to this function must be paired with a call to [`Self::release`]. - pub fn yoink(world: &mut World) -> Self { - Self { - script_context: world.remove_resource().unwrap_or_default(), - } - } - - /// Releases the current handler context back into the world, restoring the resources it contains. - /// 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.script_context); - } - - /// Get the static scripts - pub fn script_context(&mut self) -> &mut ScriptContext

{ - &mut self.script_context - } - - /// checks if the script is loaded such that it can be executed. - /// - /// since the mapping between scripts and contexts is not one-to-one, will map the context key using the - /// context policy to find the script context, if one is found then the script is loaded. - pub fn is_script_fully_loaded(&self, key: &ScriptAttachment) -> bool { - self.script_context.contains(key) - } - - /// Equivalent to [`Self::call`] but with a dynamically passed in label - pub fn call_dynamic_label( - &self, +/// A utility trait extending arbitrary script contexts with the ability to call callbacks on themselves using their +/// plugin handler function. +pub trait CallContext { + /// Invoke a callback on this context + fn call_context_dynamic( + &mut self, + context_key: &ScriptAttachment, label: &CallbackLabel, + payload: Vec, + guard: WorldGuard<'_>, + ) -> Result; + + /// Invoke a callback on this context + fn call_context( + &mut self, context_key: &ScriptAttachment, - context: Option>>, payload: Vec, guard: WorldGuard<'_>, ) -> Result { - // find script - let Some(context) = context.or_else(|| self.script_context.get(context_key)) else { - return Err(InteropError::missing_context(context_key.clone()).into()); - }; - - let mut context = context.lock(); - - P::handle(payload, context_key, label, &mut context, guard) + self.call_context_dynamic(context_key, &C::into_callback_label(), payload, guard) } +} - /// Invoke a callback in a script immediately. - /// - /// This will return [`crate::error::InteropErrorInner::MissingScript`] or [`crate::error::InteropErrorInner::MissingContext`] errors while the script is loading. - /// Run [`Self::is_script_fully_loaded`] before calling the script to ensure the script and context were loaded ahead of time. - pub fn call( - &self, +impl CallContext for C { + fn call_context_dynamic( + &mut self, context_key: &ScriptAttachment, + label: &CallbackLabel, payload: Vec, guard: WorldGuard<'_>, ) -> Result { - self.call_dynamic_label(&C::into_callback_label(), context_key, None, payload, guard) + C::P::handle(payload, context_key, label, self, guard) } } - /// A wrapper around a world which pre-populates access, to safely co-exist with other system params, /// acts exactly like `&mut World` so this should be your only top-level system param /// diff --git a/crates/bevy_mod_scripting_core/src/handler.rs b/crates/bevy_mod_scripting_core/src/handler.rs index df6478db85..94b96c8443 100644 --- a/crates/bevy_mod_scripting_core/src/handler.rs +++ b/crates/bevy_mod_scripting_core/src/handler.rs @@ -1,16 +1,7 @@ //! Contains the logic for handling script callback events use bevy_ecs::world::WorldId; -use { - bevy_ecs::{ - event::EventCursor, - event::Events, - system::{Local, SystemState}, - world::{Mut, World}, - }, - bevy_log::error, -}; - +use crate::extractors::CallContext; use crate::{ IntoScriptPluginParams, Language, bindings::{ @@ -22,8 +13,17 @@ use crate::{ CallbackLabel, IntoCallbackLabel, ScriptCallbackEvent, ScriptCallbackResponseEvent, ScriptErrorEvent, }, - extractors::{HandlerContext, WithWorldGuard}, - script::ScriptAttachment, + extractors::WithWorldGuard, + script::{ScriptAttachment, ScriptContext}, +}; +use { + bevy_ecs::{ + event::EventCursor, + event::Events, + system::{Local, SystemState}, + world::{Mut, World}, + }, + bevy_log::error, }; /// A function that handles a callback event @@ -77,12 +77,15 @@ pub fn event_handler( ) { // we wrap the inner event handler, so that we can guarantee that the handler context is released statically { - let handler_ctxt = HandlerContext::

::yoink(world); + let script_context = world.get_resource_or_init::>().clone(); let (event_cursor, mut guard) = state.get_mut(world); let (guard, _) = guard.get_mut(); - let handler_ctxt = - event_handler_inner::

(L::into_callback_label(), event_cursor, handler_ctxt, guard); - handler_ctxt.release(world); + event_handler_inner::

( + L::into_callback_label(), + event_cursor, + script_context, + guard, + ); } } @@ -96,11 +99,10 @@ type EventHandlerSystemState<'w, 's> = SystemState<( pub(crate) fn event_handler_inner( callback_label: CallbackLabel, mut event_cursor: Local>, - handler_ctxt: HandlerContext

, + script_context: ScriptContext

, guard: WorldAccessGuard, -) -> HandlerContext

{ +) { let mut errors = Vec::default(); - // let events = guard.with_resour events.read().cloned().collect::>(); let events = guard.with_resource(|events: &Events| { event_cursor .read(events) @@ -116,25 +118,24 @@ pub(crate) fn event_handler_inner( "Failed to read script callback events: {}", err.display_with_world(guard) ); - return handler_ctxt; + return; } }; for event in events.into_iter().filter(|e| { e.label == callback_label && e.language.as_ref().is_none_or(|l| l == &P::LANGUAGE) }) { - let recipients = event - .recipients - .get_recipients(&handler_ctxt.script_context); + let recipients = event.recipients.get_recipients(script_context.clone()); for (attachment, ctxt) in recipients { - let call_result = handler_ctxt.call_dynamic_label( - &callback_label, + let mut ctxt = ctxt.lock(); + let call_result = ctxt.call_context_dynamic( &attachment, - Some(ctxt), + &callback_label, event.args.clone(), guard.clone(), ); + drop(ctxt); if event.trigger_response { send_callback_response( @@ -151,7 +152,6 @@ pub(crate) fn event_handler_inner( } } handle_script_errors(guard, errors.into_iter()); - return handler_ctxt; } fn collect_errors( diff --git a/crates/bevy_mod_scripting_core/src/script/script_context.rs b/crates/bevy_mod_scripting_core/src/script/script_context.rs index 3d42763519..1cef84fbda 100644 --- a/crates/bevy_mod_scripting_core/src/script/script_context.rs +++ b/crates/bevy_mod_scripting_core/src/script/script_context.rs @@ -1,6 +1,6 @@ use std::{hash::Hash, sync::Arc}; -use parking_lot::Mutex; +use parking_lot::{Mutex, RwLock}; use super::*; use crate::IntoScriptPluginParams; @@ -191,14 +191,46 @@ struct ContextEntry { #[derive(Resource)] /// Keeps track of script contexts and enforces the context selection policy. -pub struct ScriptContext { +pub struct ScriptContext(Arc>>); + +impl ScriptContext

{ + /// Construct a new ScriptContext with the given policy. + pub fn new(policy: ContextPolicy) -> Self { + Self(Arc::new(RwLock::new(ScriptContextInner::new(policy)))) + } + + /// Read the inner data with a read lock. + pub fn read(&self) -> parking_lot::RwLockReadGuard<'_, ScriptContextInner

> { + self.0.read() + } + + /// Write to the inner data with a write lock. + pub fn write(&self) -> parking_lot::RwLockWriteGuard<'_, ScriptContextInner

> { + self.0.write() + } +} + +impl Clone for ScriptContext

{ + fn clone(&self) -> Self { + Self(self.0.clone()) + } +} + +impl Default for ScriptContext

{ + fn default() -> Self { + Self::new(ContextPolicy::default()) + } +} + +/// Inner data carried by the Arc proxy in ScriptContext +pub struct ScriptContextInner { /// script contexts and the counts of how many scripts are associated with them. map: HashMap>, /// The policy used to determine the context key. pub policy: ContextPolicy, } -impl ScriptContext

{ +impl ScriptContextInner

{ /// Construct a new ScriptContext with the given policy. pub fn new(policy: ContextPolicy) -> Self { Self { @@ -219,12 +251,43 @@ impl ScriptContext

{ .and_then(|key| self.map.get_mut(&key)) } - /// Get the context. - pub fn get(&self, context_key: &ScriptAttachment) -> Option>> { + /// Get the context containing the attachment. + /// This is a weaker assertion than `get_resident`. + /// as it will return the context even if the attachment is not resident in it. + /// for example if sharing contexts and the attachment is not the first to create the context. + pub fn get_context(&self, context_key: &ScriptAttachment) -> Option>> { self.get_entry(context_key) .map(|entry| entry.context.clone()) } + /// Gets the context containing the attachment only if it is resident. + /// i.e. if `contains` would return true. + pub fn get_resident(&self, context_key: &ScriptAttachment) -> Option>> { + self.get_entry(context_key).and_then(|entry| { + if entry.residents.contains(context_key) { + Some(entry.context.clone()) + } else { + None + } + }) + } + + /// Get the context if it exists, and checks if the attachment is resident in it. + /// Returns `None` if no context exists for the attachment. + pub fn get_context_and_residency( + &self, + context_key: &ScriptAttachment, + ) -> (Option>>, bool) { + self.get_entry(context_key) + .map(|entry| { + ( + Some(entry.context.clone()), + entry.residents.contains(context_key), + ) + }) + .unwrap_or((None, false)) + } + /// Insert a context. /// /// If the context cannot be inserted, it is returned as an `Err`. @@ -244,6 +307,29 @@ impl ScriptContext

{ } } + /// Insert a context. + /// + /// If the context cannot be inserted, it is returned as an `Err`. + /// + /// The attachment is also inserted as resident into the context. + pub fn insert_arc( + &mut self, + context_key: &ScriptAttachment, + context: Arc>, + ) -> Result<(), Arc>> { + match self.policy.select(context_key) { + Some(key) => { + let entry = ContextEntry { + context, + residents: HashSet::from_iter([context_key.clone()]), // context with a residency of one + }; + self.map.insert(key.into_weak(), entry); + Ok(()) + } + None => Err(context), + } + } + /// Mark a context as resident. /// This needs to be called when a script is added to a context. /// @@ -317,7 +403,9 @@ impl ScriptContext

{ .map_or(0, |entry| entry.residents.len()) } - /// Returns true if a context contains this given attachment + /// Returns true if a context contains this given attachment, note this is + /// different to `get` which returns true if the context simply exists. + /// i.e. `contains` checks if the attachment is resident in the context. pub fn contains(&self, context_key: &ScriptAttachment) -> bool { self.get_entry(context_key) .is_some_and(|entry| entry.residents.contains(context_key)) @@ -335,7 +423,7 @@ impl ScriptContext

{ /// Use one script context per entity and script by default; see /// [ScriptContext::per_entity_and_script]. -impl Default for ScriptContext

{ +impl Default for ScriptContextInner

{ fn default() -> Self { Self { map: HashMap::default(), @@ -359,7 +447,8 @@ mod tests { fn test_insertion_per_script_policy() { let policy = ContextPolicy::per_script(); - let mut script_context = ScriptContext::::new(policy.clone()); + let script_context = ScriptContext::::new(policy.clone()); + let mut script_context = script_context.write(); let context_key = ScriptAttachment::EntityScript( Entity::from_raw(1), Handle::Weak(AssetIndex::from_bits(1).into()), @@ -378,7 +467,7 @@ mod tests { assert_eq!(script_context.residents_len(&context_key), 1); let resident = script_context.residents(&context_key).next().unwrap(); assert_eq!(resident.0, context_key); - assert!(script_context.get(&context_key).is_some()); + assert!(script_context.get_context(&context_key).is_some()); // insert another into the same context assert!( diff --git a/crates/languages/bevy_mod_scripting_lua/src/lib.rs b/crates/languages/bevy_mod_scripting_lua/src/lib.rs index e884889352..3ea7b51af4 100644 --- a/crates/languages/bevy_mod_scripting_lua/src/lib.rs +++ b/crates/languages/bevy_mod_scripting_lua/src/lib.rs @@ -1,4 +1,6 @@ //! Lua integration for the bevy_mod_scripting system. +use std::ops::{Deref, DerefMut}; + use ::{ bevy_app::Plugin, bevy_asset::Handle, @@ -17,6 +19,7 @@ use bevy_mod_scripting_core::{ config::{GetPluginThreadConfig, ScriptingPluginConfiguration}, error::ScriptError, event::CallbackLabel, + extractors::GetPluginFor, make_plugin_config_static, reflection_extensions::PartialReflectExt, script::{ContextPolicy, ScriptAttachment}, @@ -33,8 +36,30 @@ pub mod bindings; make_plugin_config_static!(LuaScriptingPlugin); +/// A newtype around a lua context. +#[derive(Debug, Clone)] +pub struct LuaContext(Lua); + +impl Deref for LuaContext { + type Target = Lua; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl DerefMut for LuaContext { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} + +impl GetPluginFor for LuaContext { + type P = LuaScriptingPlugin; +} + impl IntoScriptPluginParams for LuaScriptingPlugin { - type C = Lua; + type C = LuaContext; type R = (); const LANGUAGE: Language = Language::Lua; @@ -174,7 +199,7 @@ impl Plugin for LuaScriptingPlugin { } fn load_lua_content_into_context( - context: &mut Lua, + context: &mut LuaContext, context_key: &ScriptAttachment, content: &[u8], world_id: WorldId, @@ -204,11 +229,11 @@ pub fn lua_context_load( context_key: &ScriptAttachment, content: &[u8], world_id: WorldId, -) -> Result { +) -> Result { #[cfg(feature = "unsafe_lua_modules")] - let mut context = unsafe { Lua::unsafe_new() }; + let mut context = LuaContext(unsafe { Lua::unsafe_new() }); #[cfg(not(feature = "unsafe_lua_modules"))] - let mut context = Lua::new(); + let mut context = LuaContext(Lua::new()); load_lua_content_into_context(&mut context, context_key, content, world_id)?; Ok(context) @@ -219,7 +244,7 @@ pub fn lua_context_load( pub fn lua_context_reload( context_key: &ScriptAttachment, content: &[u8], - old_ctxt: &mut Lua, + old_ctxt: &mut LuaContext, world_id: WorldId, ) -> Result<(), ScriptError> { load_lua_content_into_context(old_ctxt, context_key, content, world_id)?; @@ -233,7 +258,7 @@ pub fn lua_handler( args: Vec, context_key: &ScriptAttachment, callback_label: &CallbackLabel, - context: &mut Lua, + context: &mut LuaContext, world_id: WorldId, ) -> Result { let config = LuaScriptingPlugin::readonly_configuration(world_id); @@ -276,7 +301,7 @@ mod test { #[test] fn test_reload_doesnt_overwrite_old_context() { let lua = Lua::new(); - let mut old_ctxt = lua.clone(); + let mut old_ctxt = LuaContext(lua.clone()); let handle = Handle::Weak(AssetId::from(AssetIndex::from_bits(0))); let context_key = ScriptAttachment::EntityScript(Entity::from_raw(1), handle); let world_id = WorldId::new().unwrap(); diff --git a/crates/languages/bevy_mod_scripting_rhai/src/lib.rs b/crates/languages/bevy_mod_scripting_rhai/src/lib.rs index f25023f7a6..1ff56cab44 100644 --- a/crates/languages/bevy_mod_scripting_rhai/src/lib.rs +++ b/crates/languages/bevy_mod_scripting_rhai/src/lib.rs @@ -20,6 +20,7 @@ use bevy_mod_scripting_core::{ config::{GetPluginThreadConfig, ScriptingPluginConfiguration}, error::ScriptError, event::CallbackLabel, + extractors::GetPluginFor, make_plugin_config_static, reflection_extensions::PartialReflectExt, script::{ContextPolicy, DisplayProxy, ScriptAttachment}, @@ -46,6 +47,10 @@ pub struct RhaiScriptContext { pub scope: Scope<'static>, } +impl GetPluginFor for RhaiScriptContext { + type P = RhaiScriptingPlugin; +} + make_plugin_config_static!(RhaiScriptingPlugin); impl IntoScriptPluginParams for RhaiScriptingPlugin { diff --git a/crates/testing_crates/script_integration_test_harness/src/lib.rs b/crates/testing_crates/script_integration_test_harness/src/lib.rs index d378a67913..e1704664d4 100644 --- a/crates/testing_crates/script_integration_test_harness/src/lib.rs +++ b/crates/testing_crates/script_integration_test_harness/src/lib.rs @@ -28,8 +28,7 @@ use bevy_mod_scripting_core::{ }, commands::CreateOrUpdateScript, error::ScriptError, - extractors::HandlerContext, - script::{DisplayProxy, ScriptAttachment, ScriptComponent, ScriptId}, + script::{DisplayProxy, ScriptAttachment, ScriptComponent, ScriptContext, ScriptId}, }; use bevy_mod_scripting_functions::ScriptFunctionsPlugin; use criterion::{BatchSize, measurement::Measurement}; @@ -69,7 +68,7 @@ pub fn make_test_lua_plugin() -> bevy_mod_scripting_lua::LuaScriptingPlugin { use bevy_mod_scripting_lua::{LuaScriptingPlugin, mlua}; LuaScriptingPlugin::default().add_context_initializer( - |_, ctxt: &mut bevy_mod_scripting_lua::mlua::Lua| { + |_, ctxt: &mut bevy_mod_scripting_lua::LuaContext| { let globals = ctxt.globals(); globals.set( "assert_throws", @@ -306,12 +305,17 @@ where app.update(); - let mut context = HandlerContext::

::yoink(app.world_mut()); + let script_contexts = app + .world_mut() + .get_resource_or_init::>() + .clone(); let guard = WorldGuard::new_exclusive(app.world_mut()); let context_key = ScriptAttachment::EntityScript(entity, Handle::Weak(script_id)); - let ctxt_arc = context.script_context().get(&context_key).unwrap(); + let script_contexts = script_contexts.read(); + let ctxt_arc = script_contexts.get_context(&context_key).unwrap(); + drop(script_contexts); let mut ctxt_locked = ctxt_arc.lock(); let runtime = P::readonly_configuration(guard.id()).runtime; @@ -324,7 +328,6 @@ where // Pass the locked context to the closure for benchmarking its Lua (or generic) part bench_fn(&mut ctxt_locked, runtime, label, criterion) }); - context.release(app.world_mut()); Ok(()) } diff --git a/crates/testing_crates/script_integration_test_harness/src/scenario.rs b/crates/testing_crates/script_integration_test_harness/src/scenario.rs index b734b726c1..3c49ce1a93 100644 --- a/crates/testing_crates/script_integration_test_harness/src/scenario.rs +++ b/crates/testing_crates/script_integration_test_harness/src/scenario.rs @@ -753,10 +753,12 @@ impl ScenarioStep { #[cfg(feature = "lua")] Some(Language::Lua) => world .resource::>() + .read() .residents_len(&script), #[cfg(feature = "rhai")] Some(Language::Rhai) => world .resource::>() + .read() .residents_len(&script), _ => { return Err(anyhow!( diff --git a/crates/testing_crates/test_utils/src/test_plugin.rs b/crates/testing_crates/test_utils/src/test_plugin.rs index 670c9deccc..e76cfaa02e 100644 --- a/crates/testing_crates/test_utils/src/test_plugin.rs +++ b/crates/testing_crates/test_utils/src/test_plugin.rs @@ -20,6 +20,10 @@ macro_rules! make_test_plugin { $ident::make_plugin_config_static!(TestPlugin); + impl $ident::extractors::GetPluginFor for TestContext { + type P = TestPlugin; + } + impl $ident::IntoScriptPluginParams for TestPlugin { type C = TestContext; type R = TestRuntime;