refactor: remove StaticScripts resource (#473)

Author: makspll

Cargo.toml

@@ -211,6 +211,7 @@ bevy = { workspace = true, features = [
     "bevy_asset",
     "bevy_core_pipeline",
     "bevy_sprite",
+    "x11",
 ] }
 bevy_platform = { workspace = true }
 clap = { workspace = true, features = ["derive"] }

crates/bevy_mod_scripting_core/src/asset.rs

@@ -12,7 +12,7 @@ use ::{
     // },
     bevy_reflect::Reflect,
 };
-use bevy_asset::AssetServer;
+use bevy_asset::{AssetServer, Handle};
 use bevy_ecs::{
     entity::Entity,
     event::{EventReader, EventWriter},
@@ -23,11 +23,11 @@ use bevy_ecs::{
 use serde::{Deserialize, Serialize};
 
 use crate::{
-    IntoScriptPluginParams, LanguageExtensions, ScriptComponent, ScriptingSystemSet, StaticScripts,
+    IntoScriptPluginParams, LanguageExtensions, ScriptComponent, ScriptingSystemSet,
     commands::{CreateOrUpdateScript, DeleteScript},
     error::ScriptError,
     event::ScriptEvent,
-    script::{ContextKey, DisplayProxy, ScriptAttachment},
+    script::{ContextKey, DisplayProxy, ScriptAttachment, ScriptContext},
 };
 
 /// Represents a scripting language. Languages which compile into another language should use the target language as their language.
@@ -215,10 +215,10 @@ fn sync_assets(
 fn handle_script_events<P: IntoScriptPluginParams>(
     mut events: EventReader<ScriptEvent>,
     script_assets: Res<Assets<ScriptAsset>>,
-    static_scripts: Res<StaticScripts>,
     scripts: Query<(Entity, &ScriptComponent)>,
     asset_server: Res<AssetServer>,
     mut script_queue: Local<ScriptQueue>,
+    script_contexts: Res<ScriptContext<P>>,
     mut commands: Commands,
     world_id: WorldId,
 ) {
@@ -233,6 +233,7 @@ fn handle_script_events<P: IntoScriptPluginParams>(
                     // We need to reload the script for any context it's
                     // associated with. That could be static scripts, script
                     // components.
+
                     for (entity, script_component) in &scripts {
                         if let Some(handle) =
                             script_component.0.iter().find(|handle| handle.id() == *id)
@@ -247,13 +248,17 @@ fn handle_script_events<P: IntoScriptPluginParams>(
                         }
                     }
 
-                    if let Some(handle) = static_scripts.scripts.iter().find(|s| s.id() == *id) {
-                        commands.queue(
-                            CreateOrUpdateScript::<P>::new(ScriptAttachment::StaticScript(
-                                handle.clone(),
-                            ))
-                            .with_responses(P::readonly_configuration(world_id).emit_responses),
-                        );
+                    let handle = Handle::Weak(*id);
+                    let attachment = ScriptAttachment::StaticScript(handle.clone());
+                    for (resident, _) in script_contexts
+                        .residents(&attachment)
+                        .filter(|(r, _)| r.script() == handle && r.is_static())
+                    {
+                        // if the script does not have any associated entity it's static.
+                        commands
+                            .queue(CreateOrUpdateScript::<P>::new(resident).with_responses(
+                                P::readonly_configuration(world_id).emit_responses,
+                            ));
                     }
                 }
             }

crates/bevy_mod_scripting_core/src/commands.rs

@@ -14,7 +14,7 @@ use crate::{
     },
     extractors::{HandlerContext, with_handler_system_state},
     handler::{handle_script_errors, send_callback_response},
-    script::{DisplayProxy, ScriptAttachment, StaticScripts},
+    script::{DisplayProxy, ScriptAttachment},
 };
 use bevy_ecs::{system::Command, world::World};
 use bevy_log::{error, info, trace};
@@ -56,7 +56,20 @@ impl<P: IntoScriptPluginParams> Command for DeleteScript<P> {
         // we demote to weak from here on out, so as not to hold the asset hostage
         self.context_key = self.context_key.into_weak();
 
-        // first apply unload callback
+        // 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::<ScriptContext<P>>();
+            if !script_contexts.contains(&self.context_key) {
+                debug!(
+                    "{}: No context found for {}, not deleting.",
+                    P::LANGUAGE,
+                    self.context_key
+                );
+                return;
+            }
+        }
+
+        // apply unload callback
         Command::apply(
             RunScriptCallback::<P>::new(
                 self.context_key.clone(),
@@ -66,23 +79,6 @@ impl<P: IntoScriptPluginParams> Command for DeleteScript<P> {
             ),
             world,
         );
-        match &self.context_key {
-            ScriptAttachment::EntityScript(_, _) => {
-                // nothing special needs to be done, just the context removal
-            }
-            ScriptAttachment::StaticScript(script) => {
-                // remove the static script
-                let mut scripts = world.get_resource_or_init::<StaticScripts>();
-                if scripts.remove(script.id()) {
-                    debug!("Deleted static script {}", script.display());
-                } else {
-                    warn!(
-                        "Attempted to delete static script {}, but it was not found",
-                        script.display()
-                    );
-                }
-            }
-        }
 
         let mut script_contexts = world.get_resource_or_init::<ScriptContext<P>>();
         let residents_count = script_contexts.residents_len(&self.context_key);
@@ -225,10 +221,6 @@ impl<P: IntoScriptPluginParams> CreateOrUpdateScript<P> {
     ) -> Result<(), ScriptError> {
         // we demote to weak from here on out, so as not to hold the asset hostage
         let attachment = attachment.clone().into_weak();
-        if let ScriptAttachment::StaticScript(id) = &attachment {
-            // add to static scripts
-            handler_ctxt.static_scripts.insert(id.clone());
-        }
 
         let script_id = attachment.script();
 

crates/bevy_mod_scripting_core/src/config.rs

@@ -46,7 +46,7 @@ pub trait GetPluginThreadConfig<P: IntoScriptPluginParams + ?Sized> {
     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>);
+    fn set_world_local_config(world: WorldId, config: ScriptingPluginConfiguration<P>);
 }
 
 #[macro_export]
@@ -76,7 +76,7 @@ macro_rules! make_plugin_config_static {
                     )
             }
 
-            fn set_thread_config(
+            fn set_world_local_config(
                 world: bevy_ecs::world::WorldId,
                 config: ScriptingPluginConfiguration<$ty>,
             ) {

crates/bevy_mod_scripting_core/src/extractors.rs

@@ -24,7 +24,7 @@ use crate::{
     error::{InteropError, ScriptError},
     event::{CallbackLabel, IntoCallbackLabel},
     handler::ScriptingHandler,
-    script::{ScriptAttachment, ScriptContext, StaticScripts},
+    script::{ScriptAttachment, ScriptContext},
 };
 
 /// 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.
@@ -47,8 +47,6 @@ pub fn with_handler_system_state<
 
 /// Context for systems which handle events for scripts
 pub struct HandlerContext<P: IntoScriptPluginParams> {
-    /// List of static scripts
-    pub(crate) static_scripts: StaticScripts,
     /// Script context
     pub(crate) script_context: ScriptContext<P>,
 }
@@ -58,7 +56,6 @@ impl<P: IntoScriptPluginParams> HandlerContext<P> {
     /// Every call to this function must be paired with a call to [`Self::release`].
     pub fn yoink(world: &mut World) -> Self {
         Self {
-            static_scripts: world.remove_resource().unwrap_or_default(),
             script_context: world.remove_resource().unwrap_or_default(),
         }
     }
@@ -67,15 +64,9 @@ impl<P: IntoScriptPluginParams> HandlerContext<P> {
     /// 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.static_scripts);
         world.insert_resource(self.script_context);
     }
 
-    /// Get the static scripts
-    pub fn static_scripts(&mut self) -> &mut StaticScripts {
-        &mut self.static_scripts
-    }
-
     /// Get the static scripts
     pub fn script_context(&mut self) -> &mut ScriptContext<P> {
         &mut self.script_context

crates/bevy_mod_scripting_core/src/lib.rs

@@ -33,7 +33,7 @@ use context::{Context, ContextInitializer, ContextPreHandlingInitializer};
 use event::{ScriptCallbackEvent, ScriptCallbackResponseEvent, ScriptEvent};
 use handler::HandlerFn;
 use runtime::{Runtime, RuntimeInitializer};
-use script::{ContextPolicy, ScriptComponent, ScriptContext, StaticScripts};
+use script::{ContextPolicy, ScriptComponent, ScriptContext};
 use std::ops::{Deref, DerefMut};
 
 pub mod asset;
@@ -165,7 +165,7 @@ impl<P: IntoScriptPluginParams> Plugin for ScriptingPlugin<P> {
             runtime: Box::leak(Box::new(runtime)),
         };
 
-        P::set_thread_config(app.world().id(), config);
+        P::set_world_local_config(app.world().id(), config);
 
         app.insert_resource(ScriptContext::<P>::new(self.context_policy.clone()));
 
@@ -294,7 +294,6 @@ impl Plugin for BMSScriptingInfrastructurePlugin {
             .add_event::<ScriptCallbackEvent>()
             .add_event::<ScriptCallbackResponseEvent>()
             .init_resource::<AppReflectAllocator>()
-            .init_resource::<StaticScripts>()
             .init_asset::<ScriptAsset>()
             .init_resource::<AppScriptFunctionRegistry>()
             .insert_resource(AppScheduleRegistry::new());

crates/bevy_mod_scripting_core/src/script/context_key.rs

@@ -60,6 +60,16 @@ impl ScriptAttachment {
             }
         }
     }
+
+    /// Returns true if the attachment is a static script.
+    pub fn is_static(&self) -> bool {
+        matches!(self, ScriptAttachment::StaticScript(_))
+    }
+
+    /// Returns true if the attachment is an entity script.
+    pub fn is_entity_script(&self) -> bool {
+        matches!(self, ScriptAttachment::EntityScript(_, _))
+    }
 }
 
 impl From<ScriptAttachment> for ContextKey {

crates/bevy_mod_scripting_core/src/script/mod.rs

@@ -125,88 +125,12 @@ impl ScriptComponent {
     }
 }
 
-/// A collection of scripts, not associated with any entity.
-///
-/// Useful for `global` or `static` scripts which operate over a larger scope than a single entity.
-#[derive(Default, Resource)]
-pub struct StaticScripts {
-    pub(crate) scripts: HashSet<Handle<ScriptAsset>>,
-}
-
-#[profiling::all_functions]
-impl StaticScripts {
-    /// Inserts a static script into the collection
-    pub fn insert<S: Into<Handle<ScriptAsset>>>(&mut self, script: S) {
-        self.scripts.insert(script.into());
-    }
-
-    /// Removes a static script from the collection, returning `true` if the script was in the collection, `false` otherwise
-    pub fn remove(&mut self, script_id: impl Into<ScriptId>) -> bool {
-        let script_id = script_id.into();
-        self.scripts
-            .extract_if(|handle| handle.id() == script_id)
-            .next()
-            .is_some()
-    }
-
-    /// Checks if a static script is in the collection
-    /// Returns `true` if the script is in the collection, `false` otherwise
-    pub fn contains(&self, script_id: impl Into<ScriptId>) -> bool {
-        let script_id = script_id.into();
-        self.scripts.iter().any(|handle| handle.id() == script_id)
-    }
-
-    /// Returns an iterator over the static scripts
-    pub fn values(&self) -> impl Iterator<Item = &Handle<ScriptAsset>> {
-        self.scripts.iter()
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use bevy_ecs::{event::Events, world::World};
 
     use super::*;
 
-    #[test]
-    fn static_scripts_insert() {
-        let mut static_scripts = StaticScripts::default();
-        let script1 = Handle::default();
-        static_scripts.insert(script1.clone());
-        assert_eq!(static_scripts.scripts.len(), 1);
-        assert!(static_scripts.scripts.contains(&script1));
-    }
-
-    #[test]
-    fn static_scripts_remove() {
-        let mut static_scripts = StaticScripts::default();
-        let script1 = Handle::default();
-        static_scripts.insert(script1.clone());
-        assert_eq!(static_scripts.scripts.len(), 1);
-        assert!(static_scripts.scripts.contains(&script1));
-        assert!(static_scripts.remove(&script1));
-        assert_eq!(static_scripts.scripts.len(), 0);
-        assert!(!static_scripts.scripts.contains(&script1));
-    }
-
-    fn scriptid_from_u128(uuid: u128) -> ScriptId {
-        ScriptId::from(uuid::Builder::from_random_bytes(uuid.to_le_bytes()).into_uuid())
-    }
-
-    fn handle_from_u128(uuid: u128) -> Handle<ScriptAsset> {
-        Handle::Weak(scriptid_from_u128(uuid))
-    }
-
-    #[test]
-    fn static_scripts_contains() {
-        let mut static_scripts = StaticScripts::default();
-        let script1 = handle_from_u128(0);
-        let script2 = handle_from_u128(1);
-        static_scripts.insert(script1.clone());
-        assert!(static_scripts.contains(&script1));
-        assert!(!static_scripts.contains(&script2));
-    }
-
     #[test]
     fn test_component_add() {
         let mut world = World::new();

crates/languages/bevy_mod_scripting_lua/src/lib.rs

@@ -279,14 +279,23 @@ mod test {
         let mut old_ctxt = 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();
+        LuaScriptingPlugin::set_world_local_config(
+            world_id,
+            ScriptingPluginConfiguration {
+                pre_handling_callbacks: &[],
+                context_initialization_callbacks: &[],
+                emit_responses: false,
+                runtime: &(),
+            },
+        );
         lua_context_load(
             &context_key,
             "function hello_world_from_first_load()
 
             end"
             .as_bytes(),
-            WorldId::new().unwrap(),
+            world_id,
         )
         .unwrap();
 
@@ -297,7 +306,7 @@ mod test {
             end"
             .as_bytes(),
             &mut old_ctxt,
-            WorldId::new().unwrap(),
+            world_id,
         )
         .unwrap();