refactor: add world-local static plugin config, remove ContextLoadingSettings resource (#470)

Author: makspll

crates/bevy_mod_scripting_core/src/asset.rs

@@ -18,13 +18,13 @@ use bevy_ecs::{
     event::{EventReader, EventWriter},
     schedule::IntoScheduleConfigs,
     system::{Commands, Local, Query, Res},
+    world::WorldId,
 };
 use serde::{Deserialize, Serialize};
 
 use crate::{
     IntoScriptPluginParams, LanguageExtensions, ScriptComponent, ScriptingSystemSet, StaticScripts,
     commands::{CreateOrUpdateScript, DeleteScript},
-    context::ContextLoadingSettings,
     error::ScriptError,
     event::ScriptEvent,
     script::{ContextKey, DisplayProxy, ScriptAttachment},
@@ -220,7 +220,7 @@ fn handle_script_events<P: IntoScriptPluginParams>(
     asset_server: Res<AssetServer>,
     mut script_queue: Local<ScriptQueue>,
     mut commands: Commands,
-    context_loading_settings: Res<ContextLoadingSettings<P>>,
+    world_id: WorldId,
 ) {
     for event in events.read() {
         trace!("{}: Received script event: {:?}", P::LANGUAGE, event);
@@ -242,7 +242,7 @@ fn handle_script_events<P: IntoScriptPluginParams>(
                                     entity,
                                     handle.clone(),
                                 ))
-                                .with_responses(context_loading_settings.emit_responses),
+                                .with_responses(P::readonly_configuration(world_id).emit_responses),
                             );
                         }
                     }
@@ -252,15 +252,15 @@ fn handle_script_events<P: IntoScriptPluginParams>(
                             CreateOrUpdateScript::<P>::new(ScriptAttachment::StaticScript(
                                 handle.clone(),
                             ))
-                            .with_responses(context_loading_settings.emit_responses),
+                            .with_responses(P::readonly_configuration(world_id).emit_responses),
                         );
                     }
                 }
             }
             ScriptEvent::Detached { key } => {
                 commands.queue(
                     DeleteScript::<P>::new(key.clone())
-                        .with_responses(context_loading_settings.emit_responses),
+                        .with_responses(P::readonly_configuration(world_id).emit_responses),
                 );
             }
             ScriptEvent::Attached { key } => {
@@ -322,7 +322,7 @@ fn handle_script_events<P: IntoScriptPluginParams>(
             if language == P::LANGUAGE {
                 commands.queue(
                     CreateOrUpdateScript::<P>::new(context_key)
-                        .with_responses(context_loading_settings.emit_responses),
+                        .with_responses(P::readonly_configuration(world_id).emit_responses),
                 );
             }
         }

crates/bevy_mod_scripting_core/src/bindings/schedule.rs

@@ -192,14 +192,17 @@ impl WorldAccessGuard<'_> {
     reason = "tests are there but not working currently"
 )]
 mod tests {
+    use crate::config::{GetPluginThreadConfig, ScriptingPluginConfiguration};
     use ::{
         bevy_app::{App, Plugin, Update},
         bevy_ecs::{
             entity::Entity,
             schedule::{NodeId, Schedules},
             system::IntoSystem,
         },
+        std::{cell::OnceCell, rc::Rc},
     };
+
     use test_utils::make_test_plugin;
 
     use super::*;

crates/bevy_mod_scripting_core/src/bindings/script_system.rs

@@ -12,7 +12,6 @@ use super::{
 use crate::{
     IntoScriptPluginParams,
     bindings::pretty_print::DisplayWithWorld,
-    context::ContextLoadingSettings,
     error::{InteropError, ScriptError},
     event::CallbackLabel,
     extractors::get_all_access_ids,
@@ -161,12 +160,6 @@ impl ScriptSystemBuilder {
             let after_systems = self.after.clone();
             let system_name = self.name.to_string();
 
-            // let system: DynamicScriptSystem<P> =
-            //     IntoSystem::<(), (), (IsDynamicScriptSystem<P>, ())>::into_system(self);
-
-            // dummy node id for now
-            // let mut reflect_system = ReflectSystem::from_system(&system, NodeId::System(0));
-
             // this is quite important, by default systems are placed in a set defined by their TYPE, i.e. in this case
             // all script systems would be the same
 
@@ -208,7 +201,6 @@ impl ScriptSystemBuilder {
 
 struct DynamicHandlerContext<'w, P: IntoScriptPluginParams> {
     script_context: &'w ScriptContext<P>,
-    context_loading_settings: &'w ContextLoadingSettings<P>,
     runtime_container: &'w RuntimeContainer<P>,
 }
 
@@ -220,17 +212,17 @@ impl<'w, P: IntoScriptPluginParams> DynamicHandlerContext<'w, P> {
     )]
     pub fn init_param(world: &mut World, system: &mut FilteredAccessSet<ComponentId>) {
         let mut access = FilteredAccess::<ComponentId>::matches_nothing();
-        // let scripts_res_id = world
-        //     .query::<&Script<P>>();
-        let context_loading_settings_res_id = world
-            .resource_id::<ContextLoadingSettings<P>>()
-            .expect("ContextLoadingSettings resource not found");
+
         let runtime_container_res_id = world
             .resource_id::<RuntimeContainer<P>>()
             .expect("RuntimeContainer resource not found");
 
-        access.add_resource_read(context_loading_settings_res_id);
+        let script_context_res_id = world
+            .resource_id::<ScriptContext<P>>()
+            .expect("Scripts resource not found");
+
         access.add_resource_read(runtime_container_res_id);
+        access.add_resource_read(script_context_res_id);
 
         system.add(access);
     }
@@ -243,9 +235,6 @@ impl<'w, P: IntoScriptPluginParams> DynamicHandlerContext<'w, P> {
         unsafe {
             Self {
                 script_context: system.get_resource().expect("Scripts resource not found"),
-                context_loading_settings: system
-                    .get_resource()
-                    .expect("ContextLoadingSettings resource not found"),
                 runtime_container: system
                     .get_resource()
                     .expect("RuntimeContainer resource not found"),
@@ -268,22 +257,11 @@ impl<'w, P: IntoScriptPluginParams> DynamicHandlerContext<'w, P> {
         };
 
         // call the script
-        let pre_handling_initializers = &self
-            .context_loading_settings
-            .context_pre_handling_initializers;
         let runtime = &self.runtime_container.runtime;
 
         let mut context = context.lock();
 
-        P::handle(
-            payload,
-            context_key,
-            label,
-            &mut context,
-            pre_handling_initializers,
-            runtime,
-            guard,
-        )
+        P::handle(payload, context_key, label, &mut context, runtime, guard)
     }
 }
 
@@ -684,7 +662,10 @@ mod test {
     };
     use test_utils::make_test_plugin;
 
-    use crate::BMSScriptingInfrastructurePlugin;
+    use crate::{
+        BMSScriptingInfrastructurePlugin,
+        config::{GetPluginThreadConfig, ScriptingPluginConfiguration},
+    };
 
     use super::*;
 

crates/bevy_mod_scripting_core/src/bindings/world.rs

@@ -53,6 +53,7 @@ use bevy_ecs::{
     component::Mutable,
     hierarchy::{ChildOf, Children},
     system::Command,
+    world::WorldId,
 };
 use bevy_platform::collections::HashMap;
 use bevy_reflect::{TypeInfo, VariantInfo};
@@ -80,6 +81,13 @@ pub struct WorldAccessGuard<'w> {
     /// stored separate from the contents of the guard
     invalid: Rc<AtomicBool>,
 }
+impl WorldAccessGuard<'_> {
+    /// Returns the id of the world this guard provides access to
+    pub fn id(&self) -> WorldId {
+        self.inner.cell.id()
+    }
+}
+
 /// Used to decrease the stack size of [`WorldAccessGuard`]
 pub(crate) struct WorldAccessGuardInner<'w> {
     /// Safety: cannot be used unless the scope depth is less than the max valid scope

crates/bevy_mod_scripting_core/src/commands.rs

@@ -16,13 +16,13 @@ use crate::{
     handler::{handle_script_errors, send_callback_response},
     script::{DisplayProxy, ScriptAttachment, StaticScripts},
 };
-use ::{
+use bevy_ecs::{system::Command, world::World};
+use bevy_log::{error, info, trace};
+use {
     bevy_asset::{Assets, Handle},
     bevy_ecs::event::Events,
     bevy_log::{debug, warn},
 };
-use bevy_ecs::{system::Command, world::World};
-use bevy_log::{error, info, trace};
 
 /// Detaches a script, invoking the `on_script_unloaded` callback if it exists, and removes the script from the static scripts collection.
 pub struct DeleteScript<P: IntoScriptPluginParams> {
@@ -157,10 +157,6 @@ impl<P: IntoScriptPluginParams> CreateOrUpdateScript<P> {
             attachment,
             content,
             context,
-            &handler_ctxt.context_loading_settings.context_initializers,
-            &handler_ctxt
-                .context_loading_settings
-                .context_pre_handling_initializers,
             guard.clone(),
             &handler_ctxt.runtime_container.runtime,
         )
@@ -176,10 +172,6 @@ impl<P: IntoScriptPluginParams> CreateOrUpdateScript<P> {
         let context = P::load(
             attachment,
             content,
-            &handler_ctxt.context_loading_settings.context_initializers,
-            &handler_ctxt
-                .context_loading_settings
-                .context_pre_handling_initializers,
             guard.clone(),
             &handler_ctxt.runtime_container.runtime,
         )?;

crates/bevy_mod_scripting_core/src/config.rs

@@ -0,0 +1,90 @@
+//! Init only configuration and relevant types.
+
+use bevy_ecs::world::WorldId;
+
+use crate::{
+    IntoScriptPluginParams,
+    context::{ContextInitializer, ContextPreHandlingInitializer},
+};
+
+/// A set of global* configs keyed by the plugin params type.
+///
+/// Configuration is immutable after initialization.
+///
+/// Configs contained here should be
+///
+/// *global meaning stored in thread-locals, i.e. not annoyingly global, but pretty global.
+#[derive(Debug, Default)]
+pub struct ScriptingPluginConfiguration<P: IntoScriptPluginParams + ?Sized> {
+    /// callbacks executed before a handler callback is executed every time
+    pub pre_handling_callbacks: &'static [ContextPreHandlingInitializer<P>],
+    /// callbacks executed once after creating a context but before executing it for the first time
+    pub context_initialization_callbacks: &'static [ContextInitializer<P>],
+    /// Whether to emit responses from the core callbacks like `on_script_loaded`.
+    pub emit_responses: bool,
+}
+
+impl<P: IntoScriptPluginParams + ?Sized> Clone for ScriptingPluginConfiguration<P> {
+    fn clone(&self) -> Self {
+        *self
+    }
+}
+
+impl<P: IntoScriptPluginParams + ?Sized> Copy for ScriptingPluginConfiguration<P> {}
+
+/// A utility trait for accessing the readonly configuration for types that provide some.
+///
+/// This is typically implemented using the `make_plugin_config_static!` macro.
+///
+/// The default implementation will allow you to statically retrieve the configuration for a given world id.
+///
+/// I.e. this config is not quite thread-local but world-local, meaning it should play nice with tests.
+pub trait GetPluginThreadConfig<P: IntoScriptPluginParams + ?Sized> {
+    /// Get a reference to the readonly configuration.
+    fn readonly_configuration(world: WorldId) -> ScriptingPluginConfiguration<P>;
+
+    /// Set the configuration or overwrites it if already set.
+    fn set_thread_config(world: WorldId, config: ScriptingPluginConfiguration<P>);
+}
+
+#[macro_export]
+/// A macro to implement `WithReadonlyConfiguration` for a given plugin type using thread-local storage.
+macro_rules! make_plugin_config_static {
+    ($ty:ty) => {
+        static CONFIG: std::sync::RwLock<
+            bevy_platform::prelude::Vec<
+                Option<ScriptingPluginConfiguration<$ty>>,
+            >,
+        > = std::sync::RwLock::new(bevy_platform::prelude::Vec::new());
+        impl GetPluginThreadConfig<$ty> for $ty {
+            fn readonly_configuration(
+                world: bevy_ecs::world::WorldId,
+            ) -> ScriptingPluginConfiguration<$ty> {
+                CONFIG
+                    .read()
+                    .unwrap()
+                    .get(<bevy_ecs::world::WorldId as bevy_ecs::storage::SparseSetIndex>::sparse_set_index(&world))
+                    .and_then(|c| *c)
+                    .unwrap_or_else(||
+                        panic!(
+                            "Configuration for plugin {} not set for world {:?}. Did you add the plugin to the app?",
+                            stringify!($ty),
+                            world
+                        ),
+                    )
+            }
+
+            fn set_thread_config(
+                world: bevy_ecs::world::WorldId,
+                config: ScriptingPluginConfiguration<$ty>,
+            ) {
+                let mut guard = CONFIG.write().unwrap();
+                let index = <bevy_ecs::world::WorldId as bevy_ecs::storage::SparseSetIndex>::sparse_set_index(&world) as usize;
+                if index >= guard.len() {
+                    guard.resize_with(index + 1, || None);
+                }
+                guard[index] = Some(config);
+            }
+        }
+    };
+}