diff --git a/crates/bevy_mod_scripting_core/src/bindings/schedule.rs b/crates/bevy_mod_scripting_core/src/bindings/schedule.rs index 4c70179c98..8f798f798a 100644 --- a/crates/bevy_mod_scripting_core/src/bindings/schedule.rs +++ b/crates/bevy_mod_scripting_core/src/bindings/schedule.rs @@ -334,7 +334,7 @@ impl ScriptSystemBuilder { bevy::log::debug_once!("First call to script system {}", name); match handler_ctxt.call_dynamic_label( &name, - self.script_id.clone(), + &self.script_id, Entity::from_raw(0), vec![], guard.clone(), diff --git a/crates/bevy_mod_scripting_core/src/commands.rs b/crates/bevy_mod_scripting_core/src/commands.rs index e14dd14d61..667661f8a8 100644 --- a/crates/bevy_mod_scripting_core/src/commands.rs +++ b/crates/bevy_mod_scripting_core/src/commands.rs @@ -4,14 +4,16 @@ use crate::{ asset::ScriptAsset, bindings::WorldGuard, context::ContextBuilder, + error::{InteropError, ScriptError}, event::{IntoCallbackLabel, OnScriptLoaded, OnScriptUnloaded}, extractors::{with_handler_system_state, HandlerContext}, handler::{handle_script_errors, CallbackSettings}, script::{Script, ScriptId, StaticScripts}, IntoScriptPluginParams, }; -use bevy::{asset::Handle, log::debug, prelude::Command}; -use std::marker::PhantomData; +use bevy::{asset::Handle, ecs::entity::Entity, log::debug, prelude::Command}; +use parking_lot::Mutex; +use std::{marker::PhantomData, sync::Arc}; /// Deletes a script with the given ID pub struct DeleteScript { @@ -37,51 +39,40 @@ impl Command for DeleteScript

{ if let Some(script) = handler_ctxt.scripts.scripts.remove(&self.id) { debug!("Deleting script with id: {}", self.id); - match handler_ctxt.script_contexts.get_mut(script.context_id) { - Some(context) => { - // first let the script uninstall itself - match (CallbackSettings::

::call)( - handler_ctxt.callback_settings.callback_handler, - vec![], - bevy::ecs::entity::Entity::from_raw(0), - &self.id, - &OnScriptUnloaded::into_callback_label(), - context, - &handler_ctxt - .context_loading_settings - .context_pre_handling_initializers, - &mut handler_ctxt.runtime_container.runtime, - guard.clone(), - ) { - Ok(_) => {} - Err(e) => { - handle_script_errors( - guard, - [e.with_context(format!( - "Running unload hook for script with id: {}. Language: {}", - self.id, - P::LANGUAGE - ))] - .into_iter(), - ); - } - } - - debug!("Removing script with id: {}", self.id); - (handler_ctxt.context_loading_settings.assigner.remove)( - script.context_id, - &script, - &mut handler_ctxt.script_contexts, - ) - } - None => { - bevy::log::error!( - "Could not find context with id: {} corresponding to script with id: {}. Removing script without running callbacks.", - script.context_id, - self.id - ); + // first let the script uninstall itself + let mut context = script.context.lock(); + + match (CallbackSettings::

::call)( + handler_ctxt.callback_settings.callback_handler, + vec![], + bevy::ecs::entity::Entity::from_raw(0), + &self.id, + &OnScriptUnloaded::into_callback_label(), + &mut context, + &handler_ctxt + .context_loading_settings + .context_pre_handling_initializers, + &handler_ctxt.runtime_container.runtime, + guard.clone(), + ) { + Ok(_) => {} + Err(e) => { + handle_script_errors( + guard, + [e.with_context(format!( + "Running unload hook for script with id: {}. Language: {}", + self.id, + P::LANGUAGE + ))] + .into_iter(), + ); } - }; + } + + debug!("Removing script with id: {}", self.id); + // since we removed the script and are dropping the context, + // it's going to get de-allocated if it's the last context irrespective if we're + // using a global or individual allocation strategy } else { bevy::log::error!( "Attempted to delete script with id: {} but it does not exist, doing nothing!", @@ -114,168 +105,154 @@ impl CreateOrUpdateScript

{ } } - fn run_on_load_callback( + fn reload_context( &self, - res_ctxt: &mut HandlerContext

, guard: WorldGuard, - ctxt: &mut

::C, - ) { - bevy::log::debug!( - "{}: Running on load callback for script with id: {}", - P::LANGUAGE, - self.id - ); + handler_ctxt: &HandlerContext

, + ) -> Result<(), ScriptError> { + let existing_script = match handler_ctxt.scripts.scripts.get(&self.id) { + Some(script) => script, + None => { + return Err( + InteropError::invariant("Tried to reload script which doesn't exist").into(), + ) + } + }; - match (CallbackSettings::

::call)( - res_ctxt.callback_settings.callback_handler, - vec![], - bevy::ecs::entity::Entity::from_raw(0), + // reload context + let mut context = existing_script.context.lock(); + + (ContextBuilder::

::reload)( + handler_ctxt.context_loading_settings.loader.reload, &self.id, - &OnScriptLoaded::into_callback_label(), - ctxt, - &res_ctxt + &self.content, + &mut context, + &handler_ctxt.context_loading_settings.context_initializers, + &handler_ctxt .context_loading_settings .context_pre_handling_initializers, - &mut res_ctxt.runtime_container.runtime, guard.clone(), - ) { - Ok(_) => {} - Err(e) => { - handle_script_errors( - guard, - [e.with_context(format!( - "{}: Running initialization hook for script with id: {}", - P::LANGUAGE, - self.id - ))] - .into_iter(), - ); - } - } + &handler_ctxt.runtime_container.runtime, + )?; + + Ok(()) } - #[inline(always)] - fn reload_context( + fn load_context( &self, guard: WorldGuard, - res_ctxt: &mut HandlerContext

, - previous_context_id: u32, - ) { - if let Some(mut previous_context) = res_ctxt.script_contexts.remove(previous_context_id) { - match (ContextBuilder::

::reload)( - res_ctxt.context_loading_settings.loader.reload, - &self.id, - &self.content, - &mut previous_context, - &res_ctxt.context_loading_settings.context_initializers, - &res_ctxt - .context_loading_settings - .context_pre_handling_initializers, - guard.clone(), - &mut res_ctxt.runtime_container.runtime, - ) { - Ok(_) => {} - Err(e) => { - handle_script_errors( - guard.clone(), - [e.with_context(format!("reloading script with id: {}", self.id))] - .into_iter(), - ); - } - }; + handler_ctxt: &mut HandlerContext

, + ) -> Result<(), ScriptError> { + let context = (ContextBuilder::

::load)( + handler_ctxt.context_loading_settings.loader.load, + &self.id, + &self.content, + &handler_ctxt.context_loading_settings.context_initializers, + &handler_ctxt + .context_loading_settings + .context_pre_handling_initializers, + guard.clone(), + &handler_ctxt.runtime_container.runtime, + )?; - self.run_on_load_callback(res_ctxt, guard, &mut previous_context); + let context = Arc::new(Mutex::new(context)); - res_ctxt - .script_contexts - .insert_with_id(previous_context_id, previous_context); - } else { - bevy::log::error!( - "{}: Could not reload script with id: {}, as the context with id: {} does not exist.", - P::LANGUAGE, - self.id, - previous_context_id - ); - } + handler_ctxt.scripts.scripts.insert( + self.id.clone(), + Script { + id: self.id.clone(), + asset: self.asset.clone(), + context, + }, + ); + Ok(()) } } impl Command for CreateOrUpdateScript

{ fn apply(self, world: &mut bevy::prelude::World) { with_handler_system_state(world, |guard, handler_ctxt: &mut HandlerContext

| { - let script = handler_ctxt.scripts.scripts.get(&self.id); - let previous_context_id = script.as_ref().map(|s| s.context_id); + let is_new_script = !handler_ctxt.scripts.scripts.contains_key(&self.id); + + let assigned_shared_context = + match handler_ctxt.context_loading_settings.assignment_strategy { + crate::context::ContextAssignmentStrategy::Individual => None, + crate::context::ContextAssignmentStrategy::Global => { + let is_new_context = handler_ctxt.scripts.scripts.is_empty(); + if !is_new_context { + handler_ctxt + .scripts + .scripts + .values() + .next() + .map(|s| s.context.clone()) + } else { + None + } + } + }; + debug!( - "{}: CreateOrUpdateScript command applying (script_id: {}, previous_context_id: {:?})", + "{}: CreateOrUpdateScript command applying (script_id: {}, new context?: {}, new script?: {})", P::LANGUAGE, self.id, - previous_context_id + assigned_shared_context.is_none(), + is_new_script ); - match previous_context_id { - Some(previous_context_id) => { + let result = match &assigned_shared_context { + Some(assigned_shared_context) => { + if is_new_script { + // this will happen when sharing contexts + // make a new script with the shared context + let script = Script { + id: self.id.clone(), + asset: self.asset.clone(), + context: assigned_shared_context.clone(), + }; + // it can potentially be loaded but without a successful script reload but that + // leaves us in an okay state + handler_ctxt.scripts.scripts.insert(self.id.clone(), script); + } + bevy::log::debug!("{}: reloading script with id: {}", P::LANGUAGE, self.id); + self.reload_context(guard.clone(), handler_ctxt) + } + None => { + bevy::log::debug!("{}: loading script with id: {}", P::LANGUAGE, self.id); + self.load_context(guard.clone(), handler_ctxt) + } + }; + + let result = result.and_then(|()| { + handler_ctxt.call::( + &self.id, + Entity::from_raw(0), + vec![], + guard.clone(), + )?; + Ok(()) + }); + + match result { + Ok(_) => { bevy::log::debug!( - "{}: script with id already has a context: {}", + "{}: script with id: {} successfully created or updated", P::LANGUAGE, self.id ); - self.reload_context(guard.clone(), handler_ctxt, previous_context_id); } - None => { - let log_context = format!("{}: Loading script: {}", P::LANGUAGE, self.id); - - let new_context_id = (handler_ctxt.context_loading_settings.assigner.assign)( - &self.id, - &self.content, - &handler_ctxt.script_contexts, - ) - .unwrap_or_else(|| handler_ctxt.script_contexts.allocate_id()); - if handler_ctxt.script_contexts.contains(new_context_id) { - self.reload_context(guard, handler_ctxt, new_context_id); + Err(e) => { + let phrase = if assigned_shared_context.is_some() { + "reloading" } else { - // load new context - bevy::log::debug!("{}", log_context); - let ctxt = (ContextBuilder::

::load)( - handler_ctxt.context_loading_settings.loader.load, - &self.id, - &self.content, - &handler_ctxt.context_loading_settings.context_initializers, - &handler_ctxt - .context_loading_settings - .context_pre_handling_initializers, - guard.clone(), - &mut handler_ctxt.runtime_container.runtime, - ); - - let mut ctxt = match ctxt { - Ok(ctxt) => ctxt, - Err(e) => { - handle_script_errors( - guard, - [e.with_context(log_context)].into_iter(), - ); - return; - } - }; - - self.run_on_load_callback(handler_ctxt, guard, &mut ctxt); - - if handler_ctxt - .script_contexts - .insert_with_id(new_context_id, ctxt) - .is_some() - { - 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); - } - } - - handler_ctxt.scripts.scripts.insert( - self.id.clone(), - Script { - id: self.id, - asset: self.asset, - context_id: new_context_id, - }, + "loading" + }; + handle_script_errors( + guard, + vec![e + .with_script(self.id) + .with_context(format!("{}: {phrase} script", P::LANGUAGE))] + .into_iter(), ); } } @@ -334,7 +311,7 @@ mod test { use crate::{ asset::Language, bindings::script_value::ScriptValue, - context::{ContextAssigner, ContextBuilder, ContextLoadingSettings, ScriptContexts}, + context::{ContextBuilder, ContextLoadingSettings}, handler::CallbackSettings, runtime::RuntimeContainer, script::Scripts, @@ -374,7 +351,7 @@ mod test { Ok(()) }, }, - assigner: Default::default(), + assignment_strategy: Default::default(), context_initializers: vec![|_, c| { c.push_str(" initialized"); Ok(()) @@ -384,9 +361,6 @@ mod test { Ok(()) }], }) - .insert_resource(ScriptContexts:: { - contexts: Default::default(), - }) .insert_resource(RuntimeContainer:: { runtime: "Runtime".to_string(), }) @@ -397,7 +371,7 @@ mod test { Ok(ScriptValue::Unit) }, }) - .insert_resource(Scripts { + .insert_resource(Scripts:: { scripts: Default::default(), }); @@ -416,19 +390,18 @@ mod test { } } - fn assert_context_and_script(world: &World, id: &str, context: &str) { - let contexts = world.get_resource::>().unwrap(); - let scripts = world.get_resource::().unwrap(); + fn assert_context_and_script(world: &World, id: &str, context: &str, message: &str) { + let scripts = world.get_resource::>().unwrap(); - let script = scripts.scripts.get(id).expect("Script not found"); + let script = scripts + .scripts + .get(id) + .unwrap_or_else(|| panic!("Script not found {message}")); assert_eq!(id, script.id); - let found_context = contexts - .contexts - .get(&script.context_id) - .expect("Context not found"); + let found_context = script.context.lock(); - assert_eq!(found_context, context); + assert_eq!(*context, *found_context, "{}", message); } #[test] @@ -445,6 +418,7 @@ mod test { world, "script", "content initialized pre-handling-initialized callback-ran-on_script_loaded", + "Initial script creation failed", ); // update the script @@ -457,6 +431,7 @@ mod test { world, "script", "new content initialized pre-handling-initialized callback-ran-on_script_loaded", + "Script update failed", ); // create second script @@ -471,6 +446,7 @@ mod test { world, "script2", "content2 initialized pre-handling-initialized callback-ran-on_script_loaded", + "Second script creation failed", ); // delete both scripts @@ -480,11 +456,8 @@ mod test { command.apply(world); // check that the scripts are gone - let scripts = world.get_resource::().unwrap(); + let scripts = world.get_resource::>().unwrap(); assert!(scripts.scripts.is_empty()); - - let contexts = world.get_resource::>().unwrap(); - assert!(contexts.contexts.is_empty()); } #[test] @@ -497,7 +470,7 @@ mod test { .get_resource_mut::>() .unwrap(); - settings.assigner = ContextAssigner::new_global_context_assigner(); + settings.assignment_strategy = crate::context::ContextAssignmentStrategy::Global; // create a script let content = "content".as_bytes().to_vec().into_boxed_slice(); @@ -510,6 +483,7 @@ mod test { app.world(), "script", "content initialized pre-handling-initialized callback-ran-on_script_loaded", + "Initial script creation failed", ); // update the script @@ -525,6 +499,7 @@ mod test { app.world(), "script", "new content initialized pre-handling-initialized callback-ran-on_script_loaded", + "Script update failed", ); // create second script @@ -538,22 +513,19 @@ mod test { assert_context_and_script( app.world(), - "script", + "script2", "content2 initialized pre-handling-initialized callback-ran-on_script_loaded", + "second script context was not created correctly", ); - assert_context_and_script( app.world(), - "script2", + "script", "content2 initialized pre-handling-initialized callback-ran-on_script_loaded", + "First script context was not updated on second script insert", ); - // check one context exists only - let context = app - .world() - .get_resource::>() - .unwrap(); - assert!(context.contexts.len() == 1); + let scripts = app.world().get_resource::>().unwrap(); + assert!(scripts.scripts.len() == 2); // delete first script let command = DeleteScript::::new("script".into()); @@ -565,6 +537,7 @@ mod test { app.world(), "script2", "content2 initialized pre-handling-initialized callback-ran-on_script_loaded callback-ran-on_script_unloaded", + "first script unload didn't have the desired effect", ); // delete second script @@ -573,17 +546,14 @@ mod test { command.apply(app.world_mut()); - // check that the scripts are gone, but context is still there + // check that the scripts are gone, and so is the context - let scripts = app.world().get_resource::().unwrap(); + let scripts = app.world().get_resource::>().unwrap(); assert!(scripts.scripts.is_empty()); - let contexts = app - .world() - .get_resource::>() - .unwrap(); + let scripts = app.world().get_resource::>().unwrap(); - assert!(contexts.contexts.len() == 1); + assert_eq!(scripts.scripts.len(), 0, "scripts weren't removed"); } #[test] diff --git a/crates/bevy_mod_scripting_core/src/context.rs b/crates/bevy_mod_scripting_core/src/context.rs index 8d907a4f34..dec993f7be 100644 --- a/crates/bevy_mod_scripting_core/src/context.rs +++ b/crates/bevy_mod_scripting_core/src/context.rs @@ -3,73 +3,17 @@ use crate::{ bindings::{ThreadWorldContainer, WorldContainer, WorldGuard}, error::{InteropError, ScriptError}, - script::{Script, ScriptId}, + script::ScriptId, IntoScriptPluginParams, }; use bevy::ecs::{entity::Entity, system::Resource}; -use std::{collections::HashMap, sync::atomic::AtomicU32}; /// A trait that all script contexts must implement. -pub trait Context: 'static + Send + Sync {} -impl Context for T {} - -/// The type of a context id -pub type ContextId = u32; - -/// Stores script state for a scripting plugin. Scripts are identified by their `ScriptId`, while contexts are identified by their `ContextId`. -#[derive(Resource)] -pub struct ScriptContexts { - /// The contexts of the scripts - pub contexts: HashMap, -} - -impl Default for ScriptContexts

{ - fn default() -> Self { - Self { - contexts: Default::default(), - } - } -} - -static CONTEXT_ID_COUNTER: AtomicU32 = AtomicU32::new(0); -impl ScriptContexts

{ - /// Allocates a new ContextId and inserts the context into the map - pub fn insert(&mut self, ctxt: P::C) -> ContextId { - let id = CONTEXT_ID_COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed); - self.contexts.insert(id, ctxt); - id - } - - /// Inserts a context with a specific id - pub fn insert_with_id(&mut self, id: ContextId, ctxt: P::C) -> Option { - self.contexts.insert(id, ctxt) - } - - /// Allocate new context id without inserting a context - pub fn allocate_id(&self) -> ContextId { - CONTEXT_ID_COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed) - } - - /// Removes a context from the map - pub fn remove(&mut self, id: ContextId) -> Option { - self.contexts.remove(&id) - } - - /// Get a reference to a context - pub fn get(&self, id: ContextId) -> Option<&P::C> { - self.contexts.get(&id) - } - - /// Get a mutable reference to a context - pub fn get_mut(&mut self, id: ContextId) -> Option<&mut P::C> { - self.contexts.get_mut(&id) - } - - /// Check if a context exists - pub fn contains(&self, id: ContextId) -> bool { - self.contexts.contains_key(&id) - } -} +/// +/// 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 {} /// 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

= @@ -85,7 +29,7 @@ pub struct ContextLoadingSettings { /// Defines the strategy used to load and reload contexts pub loader: ContextBuilder

, /// Defines the strategy used to assign contexts to scripts - pub assigner: ContextAssigner

, + pub assignment_strategy: ContextAssignmentStrategy, /// 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 @@ -96,7 +40,7 @@ impl Default for ContextLoadingSettings

{ fn default() -> Self { Self { loader: ContextBuilder::default(), - assigner: ContextAssigner::default(), + assignment_strategy: Default::default(), context_initializers: Default::default(), context_pre_handling_initializers: Default::default(), } @@ -107,7 +51,7 @@ impl Clone for ContextLoadingSettings { fn clone(&self) -> Self { Self { loader: self.loader.clone(), - assigner: self.assigner.clone(), + assignment_strategy: self.assignment_strategy, context_initializers: self.context_initializers.clone(), context_pre_handling_initializers: self.context_pre_handling_initializers.clone(), } @@ -119,7 +63,7 @@ pub type ContextLoadFn

= fn( content: &[u8], context_initializers: &[ContextInitializer

], pre_handling_initializers: &[ContextPreHandlingInitializer

], - runtime: &mut

::R, + runtime: &

::R, ) -> Result<

::C, ScriptError>; /// A strategy for reloading contexts @@ -129,7 +73,7 @@ pub type ContextReloadFn

= fn( previous_context: &mut

::C, context_initializers: &[ContextInitializer

], pre_handling_initializers: &[ContextPreHandlingInitializer

], - runtime: &mut

::R, + runtime: &

::R, ) -> Result<(), ScriptError>; /// A strategy for loading and reloading contexts @@ -160,7 +104,7 @@ impl ContextBuilder

{ context_initializers: &[ContextInitializer

], pre_handling_initializers: &[ContextPreHandlingInitializer

], world: WorldGuard, - runtime: &mut P::R, + runtime: &P::R, ) -> Result { WorldGuard::with_existing_static_guard(world.clone(), |world| { ThreadWorldContainer.set_world(world)?; @@ -183,7 +127,7 @@ impl ContextBuilder

{ context_initializers: &[ContextInitializer

], pre_handling_initializers: &[ContextPreHandlingInitializer

], world: WorldGuard, - runtime: &mut P::R, + runtime: &P::R, ) -> Result<(), ScriptError> { WorldGuard::with_existing_static_guard(world, |world| { ThreadWorldContainer.set_world(world)?; @@ -208,108 +152,12 @@ impl Clone for ContextBuilder

{ } } -/// A strategy for assigning contexts to new and existing but re-loaded scripts as well as for managing old contexts -pub struct ContextAssigner { - /// Assign a context to the script. - /// The assigner can either return `Some(id)` or `None`. - /// Returning None will request the processor to assign a new context id to assign to this script. - /// - /// Regardless, whether a script gets a new context id or not, the processor will check if the given context exists. - /// If it does not exist, it will create a new context and assign it to the script. - /// If it does exist, it will NOT create a new context, but assign the existing one to the script, and re-load the context. - /// - /// This function is only called once for each script, when it is loaded for the first time. - pub assign: fn( - script_id: &ScriptId, - new_content: &[u8], - contexts: &ScriptContexts

, - ) -> Option, - - /// Handle the removal of the script, if any clean up in contexts is necessary perform it here. - /// - /// If you do not clean up the context here, it will stay in the context map! - pub remove: fn(context_id: ContextId, script: &Script, contexts: &mut ScriptContexts

), -} - -impl ContextAssigner

{ - /// Create an assigner which re-uses a single global context for all scripts, only use if you know what you're doing. - /// Will not perform any clean up on removal. - pub fn new_global_context_assigner() -> Self { - Self { - assign: |_, _, _| Some(0), // always use the same id in rotation - remove: |_, _, _| {}, // do nothing - } - } - - /// Create an assigner which assigns a new context to each script. This is the default strategy. - pub fn new_individual_context_assigner() -> Self { - Self { - assign: |_, _, _| None, - remove: |id, _, c| _ = c.remove(id), - } - } -} - -impl Default for ContextAssigner

{ - fn default() -> Self { - Self::new_individual_context_assigner() - } -} - -impl Clone for ContextAssigner

{ - fn clone(&self) -> Self { - Self { - assign: self.assign, - remove: self.remove, - } - } -} - -#[cfg(test)] -mod tests { - use crate::asset::Language; - - use super::*; - - struct DummyParams; - impl IntoScriptPluginParams for DummyParams { - type C = String; - type R = (); - - const LANGUAGE: Language = Language::Lua; - - fn build_runtime() -> Self::R {} - } - - #[test] - fn test_script_contexts_insert_get() { - let mut contexts: ScriptContexts = ScriptContexts::default(); - let id = contexts.insert("context1".to_string()); - assert_eq!(contexts.contexts.get(&id), Some(&"context1".to_string())); - assert_eq!( - contexts.contexts.get_mut(&id), - Some(&mut "context1".to_string()) - ); - } - - #[test] - fn test_script_contexts_allocate_id() { - let contexts: ScriptContexts = ScriptContexts::default(); - let id = contexts.allocate_id(); - let next_id = contexts.allocate_id(); - assert_eq!(next_id, id + 1); - } - - #[test] - fn test_script_contexts_remove() { - let mut contexts: ScriptContexts = ScriptContexts::default(); - let id = contexts.insert("context1".to_string()); - let removed = contexts.remove(id); - assert_eq!(removed, Some("context1".to_string())); - assert!(!contexts.contexts.contains_key(&id)); - - // assert next id is still incremented - let next_id = contexts.allocate_id(); - assert_eq!(next_id, id + 1); - } +/// The strategy used in assigning contexts to scripts +#[derive(Default, Clone, Copy)] +pub enum ContextAssignmentStrategy { + /// Assign a new context to each script + #[default] + Individual, + /// Share contexts with all other scripts + Global, } diff --git a/crates/bevy_mod_scripting_core/src/error.rs b/crates/bevy_mod_scripting_core/src/error.rs index 338b44da5e..f37d6f8fba 100644 --- a/crates/bevy_mod_scripting_core/src/error.rs +++ b/crates/bevy_mod_scripting_core/src/error.rs @@ -8,7 +8,6 @@ use crate::{ script_value::ScriptValue, ReflectBaseType, ReflectReference, }, - context::ContextId, script::ScriptId, }; use bevy::{ @@ -599,9 +598,8 @@ impl InteropError { } /// Thrown if the required context for an operation is missing. - pub fn missing_context(context_id: ContextId, script_id: impl Into) -> Self { + pub fn missing_context(script_id: impl Into) -> Self { Self(Arc::new(InteropErrorInner::MissingContext { - context_id, script_id: script_id.into(), })) } @@ -812,8 +810,6 @@ pub enum InteropErrorInner { }, /// Thrown if the required context for an operation is missing. MissingContext { - /// The context that was missing - context_id: ContextId, /// The script that was attempting to access the context script_id: ScriptId, }, @@ -1053,15 +1049,9 @@ impl PartialEq for InteropErrorInner { }, ) => a == c && b == d, ( - InteropErrorInner::MissingContext { - context_id: a, - script_id: b, - }, - InteropErrorInner::MissingContext { - context_id: c, - script_id: d, - }, - ) => a == c && b == d, + InteropErrorInner::MissingContext { script_id: b }, + InteropErrorInner::MissingContext { script_id: d }, + ) => b == d, ( InteropErrorInner::MissingSchedule { schedule_name: a }, InteropErrorInner::MissingSchedule { schedule_name: b }, @@ -1284,10 +1274,10 @@ macro_rules! argument_count_mismatch_msg { } macro_rules! missing_context_for_callback { - ($context_id:expr, $script_id:expr) => { + ($script_id:expr) => { format!( - "Missing context with id: {} for script with id: {}. Was the script loaded?.", - $context_id, $script_id + "Missing context for script with id: {}. Was the script loaded?.", + $script_id ) }; } @@ -1433,9 +1423,8 @@ impl DisplayWithWorld for InteropErrorInner { InteropErrorInner::MissingScript { script_id } => { missing_script_for_callback!(script_id) }, - InteropErrorInner::MissingContext { context_id, script_id } => { + InteropErrorInner::MissingContext { script_id } => { missing_context_for_callback!( - context_id, script_id ) }, @@ -1580,9 +1569,8 @@ impl DisplayWithWorld for InteropErrorInner { InteropErrorInner::MissingScript { script_id } => { missing_script_for_callback!(script_id) }, - InteropErrorInner::MissingContext { context_id, script_id } => { + InteropErrorInner::MissingContext { script_id } => { missing_context_for_callback!( - context_id, script_id ) }, diff --git a/crates/bevy_mod_scripting_core/src/extractors.rs b/crates/bevy_mod_scripting_core/src/extractors.rs index 0daefff90b..b00a6794dc 100644 --- a/crates/bevy_mod_scripting_core/src/extractors.rs +++ b/crates/bevy_mod_scripting_core/src/extractors.rs @@ -20,7 +20,7 @@ use crate::{ access_map::ReflectAccessId, pretty_print::DisplayWithWorld, script_value::ScriptValue, WorldAccessGuard, WorldGuard, }, - context::{ContextLoadingSettings, ScriptContexts}, + context::ContextLoadingSettings, error::{InteropError, ScriptError}, event::{CallbackLabel, IntoCallbackLabel}, handler::CallbackSettings, @@ -141,11 +141,9 @@ pub struct HandlerContext<'s, P: IntoScriptPluginParams> { /// Settings for loading contexts pub(crate) context_loading_settings: ResScope<'s, ContextLoadingSettings

>, /// Scripts - pub(crate) scripts: ResScope<'s, Scripts>, + pub(crate) scripts: ResScope<'s, Scripts

>, /// The runtime container pub(crate) runtime_container: ResScope<'s, RuntimeContainer

>, - /// The script contexts - pub(crate) script_contexts: ResScope<'s, ScriptContexts

>, /// List of static scripts pub(crate) static_scripts: ResScope<'s, StaticScripts>, } @@ -160,9 +158,8 @@ impl HandlerContext<'_, P> { ) -> ( &mut CallbackSettings

, &mut ContextLoadingSettings

, - &mut Scripts, + &mut Scripts

, &mut RuntimeContainer

, - &mut ScriptContexts

, &mut StaticScripts, ) { ( @@ -170,7 +167,6 @@ impl HandlerContext<'_, P> { &mut self.context_loading_settings, &mut self.scripts, &mut self.runtime_container, - &mut self.script_contexts, &mut self.static_scripts, ) } @@ -186,7 +182,7 @@ impl HandlerContext<'_, P> { } /// Get the scripts - pub fn scripts(&mut self) -> &mut Scripts { + pub fn scripts(&mut self) -> &mut Scripts

{ &mut self.scripts } @@ -195,11 +191,6 @@ impl HandlerContext<'_, P> { &mut self.runtime_container } - /// Get the script contexts - pub fn script_contexts(&mut self) -> &mut ScriptContexts

{ - &mut self.script_contexts - } - /// Get the static scripts pub fn static_scripts(&mut self) -> &mut StaticScripts { &mut self.static_scripts @@ -207,38 +198,22 @@ impl HandlerContext<'_, P> { /// checks if the script is loaded such that it can be executed. pub fn is_script_fully_loaded(&self, script_id: ScriptId) -> bool { - // check script exists in scripts and contexts - let script = match self.scripts.scripts.get(&script_id) { - Some(script) => script, - None => { - return false; - } - }; - - self.script_contexts - .contexts - .contains_key(&script.context_id) + self.scripts.scripts.contains_key(&script_id) } /// Equivalent to [`Self::call`] but with a dynamically passed in label pub fn call_dynamic_label( - &mut self, + &self, label: &CallbackLabel, - script_id: ScriptId, + script_id: &ScriptId, entity: Entity, payload: Vec, guard: WorldGuard<'_>, ) -> Result { // find script - let script = match self.scripts.scripts.get(&script_id) { + let script = match self.scripts.scripts.get(script_id) { Some(script) => script, - None => return Err(InteropError::missing_script(script_id).into()), - }; - - // find context - let context = match self.script_contexts.contexts.get_mut(&script.context_id) { - Some(context) => context, - None => return Err(InteropError::missing_context(script.context_id, script_id).into()), + None => return Err(InteropError::missing_script(script_id.clone()).into()), }; // call the script @@ -246,14 +221,17 @@ impl HandlerContext<'_, P> { let pre_handling_initializers = &self .context_loading_settings .context_pre_handling_initializers; - let runtime = &mut self.runtime_container.runtime; + let runtime = &self.runtime_container.runtime; + + let mut context = script.context.lock(); + CallbackSettings::

::call( handler, payload, entity, - &script_id, + script_id, label, - context, + &mut context, pre_handling_initializers, runtime, guard, @@ -265,8 +243,8 @@ impl HandlerContext<'_, P> { /// 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( - &mut self, - script_id: ScriptId, + &self, + script_id: &ScriptId, entity: Entity, payload: Vec, guard: WorldGuard<'_>, diff --git a/crates/bevy_mod_scripting_core/src/handler.rs b/crates/bevy_mod_scripting_core/src/handler.rs index a53c99659f..9da9d995d3 100644 --- a/crates/bevy_mod_scripting_core/src/handler.rs +++ b/crates/bevy_mod_scripting_core/src/handler.rs @@ -30,7 +30,7 @@ pub type HandlerFn

= fn( callback: &CallbackLabel, context: &mut

::C, pre_handling_initializers: &[ContextPreHandlingInitializer

], - runtime: &mut

::R, + runtime: &

::R, ) -> Result; /// A resource that holds the settings for the callback handler for a specific combination of type parameters @@ -72,7 +72,7 @@ impl CallbackSettings

{ callback: &CallbackLabel, script_ctxt: &mut P::C, pre_handling_initializers: &[ContextPreHandlingInitializer

], - runtime: &mut P::R, + runtime: &P::R, world: WorldGuard, ) -> Result { WorldGuard::with_existing_static_guard(world.clone(), |world| { @@ -194,7 +194,7 @@ pub(crate) fn event_handler_inner( let call_result = handler_ctxt.call_dynamic_label( &callback_label, - script_id.clone(), + script_id, *entity, event.args.clone(), guard.clone(), @@ -206,7 +206,7 @@ pub(crate) fn event_handler_inner( match e.downcast_interop_inner() { Some(InteropErrorInner::MissingScript { script_id }) => { trace_once!( - "{}: Script `{}` on entity `{:?}` is either still loading or doesn't exist, ignoring until the corresponding script is loaded.", + "{}: Script `{}` on entity `{:?}` is either still loading, doesn't exist, or is for another language, ignoring until the corresponding script is loaded.", P::LANGUAGE, script_id, entity ); @@ -256,7 +256,7 @@ pub fn handle_script_errors + Clone>(world: Worl #[cfg(test)] #[allow(clippy::todo)] mod test { - use std::{borrow::Cow, collections::HashMap}; + use std::{borrow::Cow, collections::HashMap, sync::Arc}; use bevy::{ app::{App, Update}, @@ -264,13 +264,13 @@ mod test { diagnostic::DiagnosticsPlugin, ecs::world::FromWorld, }; + use parking_lot::Mutex; use test_utils::make_test_plugin; use crate::{ bindings::script_value::ScriptValue, - context::{ContextAssigner, ContextBuilder, ContextLoadingSettings, ScriptContexts}, + context::{ContextBuilder, ContextLoadingSettings}, event::{CallbackLabel, IntoCallbackLabel, ScriptCallbackEvent, ScriptErrorEvent}, - handler::HandlerFn, runtime::RuntimeContainer, script::{Script, ScriptComponent, ScriptId, Scripts, StaticScripts}, }; @@ -286,33 +286,32 @@ mod test { make_test_plugin!(crate); - fn setup_app( - handler_fn: HandlerFn

, - runtime: P::R, - contexts: HashMap, - scripts: HashMap, + fn setup_app( + runtime: TestRuntime, + scripts: HashMap>, ) -> App { let mut app = App::new(); app.add_event::(); app.add_event::(); - app.insert_resource::>(CallbackSettings { - callback_handler: handler_fn, + app.insert_resource::>(CallbackSettings { + callback_handler: |args, entity, script, _, ctxt, _, runtime| { + ctxt.invocations.extend(args); + let mut runtime = runtime.invocations.lock(); + runtime.push((entity, script.clone())); + Ok(ScriptValue::Unit) + }, }); - app.add_systems(Update, event_handler::); - app.insert_resource::(Scripts { scripts }); - app.insert_resource(RuntimeContainer::

{ runtime }); - app.insert_resource(ScriptContexts::

{ contexts }); + app.add_systems(Update, event_handler::); + app.insert_resource::>(Scripts { scripts }); + app.insert_resource(RuntimeContainer:: { runtime }); app.init_resource::(); - app.insert_resource(ContextLoadingSettings::

{ + app.insert_resource(ContextLoadingSettings:: { loader: ContextBuilder { load: |_, _, _, _, _| todo!(), reload: |_, _, _, _, _, _| todo!(), }, - assigner: ContextAssigner { - assign: |_, _, _| todo!(), - remove: |_, _, _| todo!(), - }, + assignment_strategy: Default::default(), context_initializers: vec![], context_pre_handling_initializers: vec![], }); @@ -324,32 +323,16 @@ mod test { #[test] fn test_handler_called_with_right_args() { let test_script_id = Cow::Borrowed("test_script"); - let test_ctxt_id = 0; let test_script = Script { id: test_script_id.clone(), asset: None, - context_id: test_ctxt_id, + context: Arc::new(Mutex::new(TestContext::default())), }; let scripts = HashMap::from_iter(vec![(test_script_id.clone(), test_script.clone())]); - let contexts = HashMap::from_iter(vec![( - test_ctxt_id, - TestContext { - invocations: vec![], - }, - )]); let runtime = TestRuntime { - invocations: vec![], + invocations: vec![].into(), }; - let mut app = setup_app::( - |args, entity, script, _, ctxt, _, runtime| { - ctxt.invocations.extend(args); - runtime.invocations.push((entity, script.clone())); - Ok(ScriptValue::Unit) - }, - runtime, - contexts, - scripts, - ); + let mut app = setup_app::(runtime, scripts); let test_entity_id = app .world_mut() .spawn(ScriptComponent(vec![test_script_id.clone()])) @@ -361,28 +344,29 @@ mod test { )); app.update(); - let test_context = app + let test_script = app .world() - .get_resource::>() + .get_resource::>() + .unwrap() + .scripts + .get(&test_script_id) .unwrap(); + + let test_context = test_script.context.lock(); + let test_runtime = app .world() .get_resource::>() .unwrap(); assert_eq!( - test_context - .contexts - .get(&test_ctxt_id) - .unwrap() - .invocations, + test_context.invocations, vec![ScriptValue::String("test_args".into())] ); + let runtime_invocations = test_runtime.runtime.invocations.lock(); assert_eq!( - test_runtime - .runtime - .invocations + runtime_invocations .iter() .map(|(e, s)| (*e, s.clone())) .collect::>(), @@ -393,11 +377,10 @@ mod test { #[test] fn test_handler_called_on_right_recipients() { let test_script_id = Cow::Borrowed("test_script"); - let test_ctxt_id = 0; let test_script = Script { id: test_script_id.clone(), asset: None, - context_id: test_ctxt_id, + context: Arc::new(Mutex::new(TestContext::default())), }; let scripts = HashMap::from_iter(vec![ (test_script_id.clone(), test_script.clone()), @@ -406,37 +389,15 @@ mod test { Script { id: "wrong".into(), asset: None, - context_id: 1, - }, - ), - ]); - let contexts = HashMap::from_iter(vec![ - ( - test_ctxt_id, - TestContext { - invocations: vec![], - }, - ), - ( - 1, - TestContext { - invocations: vec![], + context: Arc::new(Mutex::new(TestContext::default())), }, ), ]); + let runtime = TestRuntime { - invocations: vec![], + invocations: vec![].into(), }; - let mut app = setup_app::( - |args, entity, script, _, ctxt, _, runtime| { - ctxt.invocations.extend(args); - runtime.invocations.push((entity, script.clone())); - Ok(ScriptValue::Unit) - }, - runtime, - contexts, - scripts, - ); + let mut app = setup_app::(runtime, scripts); let test_entity_id = app .world_mut() .spawn(ScriptComponent(vec![test_script_id.clone()])) @@ -456,21 +417,16 @@ mod test { app.update(); - let test_context = app - .world() - .get_resource::>() - .unwrap(); + let test_scripts = app.world().get_resource::>().unwrap(); let test_runtime = app .world() .get_resource::>() .unwrap(); - + let test_runtime = test_runtime.runtime.invocations.lock(); + let script_after = test_scripts.scripts.get(&test_script_id).unwrap(); + let context_after = script_after.context.lock(); assert_eq!( - test_context - .contexts - .get(&test_ctxt_id) - .unwrap() - .invocations, + context_after.invocations, vec![ ScriptValue::String("test_args_script".into()), ScriptValue::String("test_args_entity".into()) @@ -479,8 +435,6 @@ mod test { assert_eq!( test_runtime - .runtime - .invocations .iter() .map(|(e, s)| (*e, s.clone())) .collect::>(), @@ -494,35 +448,19 @@ mod test { #[test] fn test_handler_called_for_static_scripts() { let test_script_id = Cow::Borrowed("test_script"); - let test_ctxt_id = 0; let scripts = HashMap::from_iter(vec![( test_script_id.clone(), Script { id: test_script_id.clone(), asset: None, - context_id: test_ctxt_id, - }, - )]); - let contexts = HashMap::from_iter(vec![( - test_ctxt_id, - TestContext { - invocations: vec![], + context: Arc::new(Mutex::new(TestContext::default())), }, )]); let runtime = TestRuntime { - invocations: vec![], + invocations: vec![].into(), }; - let mut app = setup_app::( - |args, entity, script, _, ctxt, _, runtime| { - ctxt.invocations.extend(args); - runtime.invocations.push((entity, script.clone())); - Ok(ScriptValue::Unit) - }, - runtime, - contexts, - scripts, - ); + let mut app = setup_app::(runtime, scripts); app.world_mut().insert_resource(StaticScripts { scripts: vec![test_script_id.clone()].into_iter().collect(), @@ -542,17 +480,16 @@ mod test { app.update(); - let test_context = app - .world() - .get_resource::>() - .unwrap(); + let test_scripts = app.world().get_resource::>().unwrap(); + let test_context = test_scripts + .scripts + .get(&test_script_id) + .unwrap() + .context + .lock(); assert_eq!( - test_context - .contexts - .get(&test_ctxt_id) - .unwrap() - .invocations, + test_context.invocations, vec![ ScriptValue::String("test_args_script".into()), ScriptValue::String("test_script_id".into()) diff --git a/crates/bevy_mod_scripting_core/src/lib.rs b/crates/bevy_mod_scripting_core/src/lib.rs index dcdefea841..331fe3d223 100644 --- a/crates/bevy_mod_scripting_core/src/lib.rs +++ b/crates/bevy_mod_scripting_core/src/lib.rs @@ -18,8 +18,8 @@ use bindings::{ }; use commands::{AddStaticScript, RemoveStaticScript}; use context::{ - Context, ContextAssigner, ContextBuilder, ContextInitializer, ContextLoadingSettings, - ContextPreHandlingInitializer, ScriptContexts, + Context, ContextAssignmentStrategy, ContextBuilder, ContextInitializer, ContextLoadingSettings, + ContextPreHandlingInitializer, }; use error::ScriptError; use event::ScriptCallbackEvent; @@ -83,8 +83,9 @@ pub struct ScriptingPlugin { pub callback_handler: HandlerFn

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

, - /// The context assigner for assigning contexts to scripts. - pub context_assigner: ContextAssigner

, + + /// The strategy for assigning contexts to scripts + pub context_assignment_strategy: ContextAssignmentStrategy, /// The asset path to language mapper for the plugin pub language_mapper: AssetPathToLanguageMapper, @@ -104,7 +105,7 @@ impl Default for ScriptingPlugin

{ runtime_settings: Default::default(), callback_handler: CallbackSettings::

::default().callback_handler, context_builder: Default::default(), - context_assigner: Default::default(), + context_assignment_strategy: Default::default(), language_mapper: Default::default(), context_initializers: Default::default(), context_pre_handling_initializers: Default::default(), @@ -119,16 +120,16 @@ impl Plugin for ScriptingPlugin

{ .insert_resource::>(RuntimeContainer { runtime: P::build_runtime(), }) - .init_resource::>() .insert_resource::>(CallbackSettings { callback_handler: self.callback_handler, }) .insert_resource::>(ContextLoadingSettings { loader: self.context_builder.clone(), - assigner: self.context_assigner.clone(), + assignment_strategy: self.context_assignment_strategy, context_initializers: self.context_initializers.clone(), context_pre_handling_initializers: self.context_pre_handling_initializers.clone(), - }); + }) + .init_resource::>(); register_script_plugin_systems::

(app); @@ -227,7 +228,7 @@ impl>> ConfigureScriptPlugi } fn enable_context_sharing(mut self) -> Self { - self.as_mut().context_assigner = ContextAssigner::new_global_context_assigner(); + self.as_mut().context_assignment_strategy = ContextAssignmentStrategy::Global; self } } @@ -289,7 +290,6 @@ fn once_per_app_init(app: &mut App) { app.add_event::() .add_event::() .init_resource::() - .init_resource::() .init_resource::() .init_asset::() .init_resource::() diff --git a/crates/bevy_mod_scripting_core/src/runtime.rs b/crates/bevy_mod_scripting_core/src/runtime.rs index 44230c944f..6ddb7050b1 100644 --- a/crates/bevy_mod_scripting_core/src/runtime.rs +++ b/crates/bevy_mod_scripting_core/src/runtime.rs @@ -12,8 +12,7 @@ pub trait Runtime: Default + 'static + Send + Sync {} impl Runtime for T {} /// A function that initializes a runtime. -pub type RuntimeInitializer

= - fn(&mut

::R) -> Result<(), ScriptError>; +pub type RuntimeInitializer

= fn(&

::R) -> Result<(), ScriptError>; #[derive(Resource)] /// Resource storing settings for a scripting plugin regarding runtime initialization & configuration. @@ -54,11 +53,11 @@ impl Default for RuntimeContainer

{ } pub(crate) fn initialize_runtime( - mut runtime: ResMut>, + runtime: ResMut>, settings: Res>, ) -> Result<(), ScriptError> { for initializer in settings.initializers.iter() { - (initializer)(&mut runtime.runtime)?; + (initializer)(&runtime.runtime)?; } Ok(()) } diff --git a/crates/bevy_mod_scripting_core/src/script.rs b/crates/bevy_mod_scripting_core/src/script.rs index a69018f240..b616822834 100644 --- a/crates/bevy_mod_scripting_core/src/script.rs +++ b/crates/bevy_mod_scripting_core/src/script.rs @@ -1,8 +1,9 @@ //! Script related types, functions and components -use crate::{asset::ScriptAsset, context::ContextId}; +use crate::{asset::ScriptAsset, IntoScriptPluginParams}; use bevy::{asset::Handle, ecs::system::Resource, reflect::Reflect, utils::HashSet}; -use std::{borrow::Cow, collections::HashMap, ops::Deref}; +use parking_lot::Mutex; +use std::{borrow::Cow, collections::HashMap, ops::Deref, sync::Arc}; /// A unique identifier for a script, by default corresponds to the path of the asset excluding the asset source. /// @@ -32,20 +33,37 @@ impl ScriptComponent { } /// All the scripts which are currently loaded or loading and their mapping to contexts -#[derive(Resource, Default, Clone)] -pub struct Scripts { - pub(crate) scripts: HashMap, +#[derive(Resource)] +pub struct Scripts { + pub(crate) scripts: HashMap>, +} + +impl Default for Scripts

{ + fn default() -> Self { + Self { + scripts: Default::default(), + } + } } /// A script -#[derive(Clone)] -pub struct Script { +pub struct Script { /// The id of the script pub id: ScriptId, /// the asset holding the content of the script if it comes from an asset pub asset: Option>, - /// The id of the context this script is currently assigned to - pub context_id: ContextId, + /// The context of the script, possibly shared with other scripts + pub context: Arc>, +} + +impl Clone for Script

{ + fn clone(&self) -> Self { + Self { + id: self.id.clone(), + asset: self.asset.clone(), + context: self.context.clone(), + } + } } /// A collection of scripts, not associated with any entity. diff --git a/crates/languages/bevy_mod_scripting_lua/src/lib.rs b/crates/languages/bevy_mod_scripting_lua/src/lib.rs index 9d4ccb121d..5ad3b18aea 100644 --- a/crates/languages/bevy_mod_scripting_lua/src/lib.rs +++ b/crates/languages/bevy_mod_scripting_lua/src/lib.rs @@ -53,7 +53,7 @@ impl Default for LuaScriptingPlugin { fn default() -> Self { LuaScriptingPlugin { scripting_plugin: ScriptingPlugin { - context_assigner: Default::default(), + context_assignment_strategy: Default::default(), runtime_settings: RuntimeSettings::default(), callback_handler: lua_handler, context_builder: ContextBuilder:: { @@ -187,7 +187,7 @@ pub fn lua_context_load( content: &[u8], initializers: &[ContextInitializer], pre_handling_initializers: &[ContextPreHandlingInitializer], - _: &mut (), + _: &(), ) -> Result { #[cfg(feature = "unsafe_lua_modules")] let mut context = unsafe { Lua::unsafe_new() }; @@ -212,7 +212,7 @@ pub fn lua_context_reload( old_ctxt: &mut Lua, initializers: &[ContextInitializer], pre_handling_initializers: &[ContextPreHandlingInitializer], - _: &mut (), + _: &(), ) -> Result<(), ScriptError> { load_lua_content_into_context( old_ctxt, @@ -234,7 +234,7 @@ pub fn lua_handler( callback_label: &CallbackLabel, context: &mut Lua, pre_handling_initializers: &[ContextPreHandlingInitializer], - _: &mut (), + _: &(), ) -> Result { pre_handling_initializers .iter() @@ -285,7 +285,7 @@ mod test { .as_bytes(), &initializers, &pre_handling_initializers, - &mut (), + &(), ) .unwrap(); @@ -298,7 +298,7 @@ mod test { &mut old_ctxt, &initializers, &pre_handling_initializers, - &mut (), + &(), ) .unwrap(); diff --git a/crates/languages/bevy_mod_scripting_rhai/Cargo.toml b/crates/languages/bevy_mod_scripting_rhai/Cargo.toml index bf154ae65f..89639cf273 100644 --- a/crates/languages/bevy_mod_scripting_rhai/Cargo.toml +++ b/crates/languages/bevy_mod_scripting_rhai/Cargo.toml @@ -20,6 +20,7 @@ bevy = { workspace = true, default-features = false } rhai = { version = "1.21" } bevy_mod_scripting_core = { workspace = true, features = ["rhai_impls"] } strum = { version = "0.26", features = ["derive"] } +parking_lot = "0.12.1" [dev-dependencies] script_integration_test_harness = { workspace = true } diff --git a/crates/languages/bevy_mod_scripting_rhai/src/lib.rs b/crates/languages/bevy_mod_scripting_rhai/src/lib.rs index d9bdf4a0d5..6ae0f69d52 100644 --- a/crates/languages/bevy_mod_scripting_rhai/src/lib.rs +++ b/crates/languages/bevy_mod_scripting_rhai/src/lib.rs @@ -23,6 +23,7 @@ use bindings::{ reference::{ReservedKeyword, RhaiReflectReference, RhaiStaticReflectReference}, script_value::{FromDynamic, IntoDynamic}, }; +use parking_lot::RwLock; use rhai::{CallFnOptions, Dynamic, Engine, EvalAltResult, Scope, AST}; pub use rhai; @@ -30,7 +31,7 @@ pub use rhai; pub mod bindings; /// The rhai runtime type. -pub type RhaiRuntime = Engine; +pub type RhaiRuntime = RwLock; /// The rhai context type. pub struct RhaiScriptContext { @@ -47,7 +48,7 @@ impl IntoScriptPluginParams for RhaiScriptingPlugin { const LANGUAGE: Language = Language::Rhai; fn build_runtime() -> Self::R { - RhaiRuntime::new() + Engine::new().into() } } @@ -67,12 +68,14 @@ impl Default for RhaiScriptingPlugin { fn default() -> Self { RhaiScriptingPlugin { scripting_plugin: ScriptingPlugin { - context_assigner: Default::default(), + context_assignment_strategy: Default::default(), runtime_settings: RuntimeSettings { - initializers: vec![|runtime: &mut Engine| { - runtime.build_type::(); - runtime.build_type::(); - runtime.register_iterator_result::(); + initializers: vec![|runtime: &RhaiRuntime| { + let mut engine = runtime.write(); + + engine.build_type::(); + engine.build_type::(); + engine.register_iterator_result::(); Ok(()) }], }, @@ -186,8 +189,10 @@ fn load_rhai_content_into_context( content: &[u8], initializers: &[ContextInitializer], pre_handling_initializers: &[ContextPreHandlingInitializer], - runtime: &mut RhaiRuntime, + runtime: &RhaiRuntime, ) -> Result<(), ScriptError> { + let runtime = runtime.read(); + context.ast = runtime.compile(std::str::from_utf8(content)?)?; context.ast.set_source(script.to_string()); @@ -209,7 +214,7 @@ pub fn rhai_context_load( content: &[u8], initializers: &[ContextInitializer], pre_handling_initializers: &[ContextPreHandlingInitializer], - runtime: &mut RhaiRuntime, + runtime: &RhaiRuntime, ) -> Result { let mut context = RhaiScriptContext { // Using an empty AST as a placeholder. @@ -234,7 +239,7 @@ pub fn rhai_context_reload( context: &mut RhaiScriptContext, initializers: &[ContextInitializer], pre_handling_initializers: &[ContextPreHandlingInitializer], - runtime: &mut RhaiRuntime, + runtime: &RhaiRuntime, ) -> Result<(), ScriptError> { load_rhai_content_into_context( context, @@ -255,7 +260,7 @@ pub fn rhai_callback_handler( callback: &CallbackLabel, context: &mut RhaiScriptContext, pre_handling_initializers: &[ContextPreHandlingInitializer], - runtime: &mut RhaiRuntime, + runtime: &RhaiRuntime, ) -> Result { pre_handling_initializers .iter() @@ -274,6 +279,8 @@ pub fn rhai_callback_handler( script_id, args ); + let runtime = runtime.read(); + match runtime.call_fn_with_options::( options, &mut context.scope, @@ -303,7 +310,7 @@ mod test { #[test] fn test_reload_doesnt_overwrite_old_context() { - let mut runtime = RhaiRuntime::new(); + let runtime = RhaiRuntime::new(Engine::new()); let script_id = ScriptId::from("asd.rhai"); let initializers: Vec> = vec![]; let pre_handling_initializers: Vec> = @@ -315,7 +322,7 @@ mod test { b"let hello = 2;", &initializers, &pre_handling_initializers, - &mut runtime, + &runtime, ) .unwrap(); @@ -326,7 +333,7 @@ mod test { &mut context, &initializers, &pre_handling_initializers, - &mut runtime, + &runtime, ) .unwrap(); diff --git a/crates/languages/bevy_mod_scripting_rhai/tests/rhai_tests.rs b/crates/languages/bevy_mod_scripting_rhai/tests/rhai_tests.rs index 32bc7b7a98..479f030a4e 100644 --- a/crates/languages/bevy_mod_scripting_rhai/tests/rhai_tests.rs +++ b/crates/languages/bevy_mod_scripting_rhai/tests/rhai_tests.rs @@ -34,6 +34,8 @@ impl Test { |app| { app.add_plugins(RhaiScriptingPlugin::default()); app.add_runtime_initializer::(|runtime| { + let mut runtime = runtime.write(); + runtime.register_fn("assert", |a: Dynamic, b: &str| { if !a.is::() { panic!("Expected a boolean value, but got {:?}", a); 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 02ad8ba56a..1a171e3097 100644 --- a/crates/testing_crates/script_integration_test_harness/src/lib.rs +++ b/crates/testing_crates/script_integration_test_harness/src/lib.rs @@ -167,7 +167,7 @@ fn run_test_callback( } let res = handler_ctxt.call::( - script_id.to_string().into(), + &script_id.to_string().into(), Entity::from_raw(0), vec![], guard.clone(), diff --git a/crates/testing_crates/test_utils/src/test_plugin.rs b/crates/testing_crates/test_utils/src/test_plugin.rs index 6acd16e248..f5d63c00a5 100644 --- a/crates/testing_crates/test_utils/src/test_plugin.rs +++ b/crates/testing_crates/test_utils/src/test_plugin.rs @@ -26,16 +26,17 @@ macro_rules! make_test_plugin { fn build_runtime() -> Self::R { TestRuntime { - invocations: vec![], + invocations: vec![].into(), } } } #[derive(Default)] struct TestRuntime { - pub invocations: Vec<(Entity, ScriptId)>, + pub invocations: parking_lot::Mutex>, } + #[derive(Default)] struct TestContext { pub invocations: Vec<$ident::bindings::script_value::ScriptValue>, }