rhai re-implementation WIP

Author: makspll

.vscode/settings.json

@@ -10,7 +10,7 @@
     },
     "rust-analyzer.rustc.source": "discover",
     "rust-analyzer.linkedProjects": [
-        "./crates/bevy_api_gen/Cargo.toml",
+        // "./crates/bevy_api_gen/Cargo.toml",
         "Cargo.toml",
     ],
     "rust-analyzer.check.invocationStrategy": "per_workspace",
@@ -28,5 +28,8 @@
     "rust-analyzer.runnables.extraArgs": [
         "--profile=release-with-debug",
     ],
+    "rust-analyzer.cargo.features": [
+        "bevy_mod_scripting_functions/test_functions"
+    ]
     // "rust-analyzer.semanticHighlighting.operator.enable": false
 }

assets/scripts/game_of_life.rhai

@@ -1,13 +1,14 @@
-fn init() {
-    let LifeState = world.get_type_by_name("LifeState");
-    let life_state = world.get_component(entity,LifeState);
-    let cells = life_state.cells;
+fn on_script_loaded() {
+    world.info("Game of Life script loaded");
+    // let LifeState = world.get_type_by_name("LifeState");
+    // let life_state = world.get_component(entity,LifeState);
+    // let cells = life_state.cells;
 
-    // set some cells alive
-    for x in 1..10000 {
-        let index = rand(0..cells.len());
-        cells[index] = 255;
-    }
+    // // set some cells alive
+    // for x in 1..10000 {
+    //     let index = rand(0..cells.len());
+    //     cells[index] = 255;
+    // }
 }
 
 fn on_update() {

check.sh

@@ -6,5 +6,6 @@ CURRENT_DIR=$(basename "$PWD")
 if [[ "$CURRENT_DIR" == "bevy_api_gen" ]]; then
     cargo +nightly-2024-11-05 clippy --all-targets --message-format=json 
 else
-    cargo clippy --workspace --all-targets --message-format=json --features="lua54 rhai rune bevy/file_watcher bevy/multi_threaded"
+    cargo xtask check --ide-mode
+    # cargo clippy --workspace --all-targets --message-format=json --features="lua54 rhai rune bevy/file_watcher bevy/multi_threaded "
 fi

crates/bevy_mod_scripting_core/src/asset.rs

@@ -17,8 +17,6 @@ pub enum Language {
     Lua,
     Rune,
     External(Cow<'static, str>),
-    /// Initial setting before being processed by the script synchronization systems
-    Unset,
     /// Set if none of the asset path to language mappers match
     Unknown,
 }
@@ -30,7 +28,6 @@ impl std::fmt::Display for Language {
             Language::Lua => "Lua".fmt(f),
             Language::Rune => "Rune".fmt(f),
             Language::External(cow) => cow.fmt(f),
-            Language::Unset => "Unset".fmt(f),
             Language::Unknown => "Unknown".fmt(f),
         }
     }
@@ -42,9 +39,9 @@ pub struct ScriptAsset {
     pub content: Box<[u8]>,
     /// The virtual filesystem path of the asset, used to map to the script Id for asset backed scripts
     pub asset_path: PathBuf,
-    pub language: Language,
 }
 
+#[derive(Default)]
 pub struct ScriptAssetLoader {
     /// The file extensions this loader should handle
     pub extensions: &'static [&'static str],
@@ -76,7 +73,6 @@ impl AssetLoader for ScriptAssetLoader {
         let asset = ScriptAsset {
             content: content.into_boxed_slice(),
             asset_path: load_context.path().to_owned(),
-            language: Language::Unset,
         };
         Ok(asset)
     }
@@ -97,7 +93,7 @@ impl ScriptAssetSettings {
         for mapper in &self.script_language_mappers {
             let language = (mapper.map)(path);
             match language {
-                Language::Unset | Language::Unknown => continue,
+                Language::Unknown => continue,
                 _ => return language,
             }
         }
@@ -112,9 +108,7 @@ impl Default for ScriptAssetSettings {
             script_id_mapper: AssetPathToScriptIdMapper {
                 map: (|path: &Path| path.to_string_lossy().into_owned().into()),
             },
-            script_language_mappers: vec![AssetPathToLanguageMapper {
-                map: (|_: &Path| Language::Unset),
-            }],
+            script_language_mappers: vec![],
         }
     }
 }
@@ -144,6 +138,7 @@ pub struct ScriptMetadata {
 
 impl ScriptMetadataStore {
     pub fn insert(&mut self, id: AssetId<ScriptAsset>, meta: ScriptMetadata) {
+        // TODO: new generations of assets are not going to have the same ID as the old one
         self.map.insert(id, meta);
     }
 

crates/bevy_mod_scripting_core/src/bindings/allocator.rs

@@ -195,7 +195,6 @@ impl ReflectAllocator {
     /// Runs a garbage collection pass on the allocations, removing any allocations which have no more strong references
     /// Needs to be run periodically to prevent memory leaks
     pub fn clean_garbage_allocations(&mut self) {
-        bevy::log::trace!("Cleaning garbage allocations");
         self.allocations.retain(|k, _| Arc::strong_count(&k.0) > 1);
     }
 

crates/bevy_mod_scripting_core/src/bindings/function/from.rs

@@ -11,6 +11,8 @@ use std::{
     path::PathBuf,
 };
 
+use super::script_function::DynamicScriptFunctionMut;
+
 /// Describes the procedure for constructing a value of type `T` from a [`ScriptValue`].
 ///
 /// The [`FromScript::This`] associated type is used to allow for the implementation of this trait to return
@@ -387,3 +389,20 @@ where
         }
     }
 }
+
+impl FromScript for DynamicScriptFunctionMut {
+    type This<'w> = Self;
+
+    fn from_script(value: ScriptValue, _: WorldGuard<'_>) -> Result<Self::This<'_>, InteropError>
+    where
+        Self: Sized,
+    {
+        match value {
+            ScriptValue::Function(f) => Ok(f),
+            _ => Err(InteropError::value_mismatch(
+                std::any::TypeId::of::<Self>(),
+                value,
+            )),
+        }
+    }
+}

crates/bevy_mod_scripting_core/src/bindings/function/mod.rs

@@ -8,9 +8,7 @@ use script_function::{CallerContext, DynamicScriptFunction, DynamicScriptFunctio
 
 use crate::error::InteropError;
 
-use super::{
-    pretty_print::DisplayWithWorld, script_value::ScriptValue, WorldCallbackAccess, WorldGuard,
-};
+use super::{script_value::ScriptValue, WorldCallbackAccess, WorldGuard};
 
 /// Can be implemented for callables which require dynamic access to the world to be called.
 ///
@@ -33,11 +31,6 @@ impl CallScriptFunction for DynamicScriptFunction {
     ) -> Result<ScriptValue, InteropError> {
         let args = args.into_iter().collect::<Vec<_>>();
         let world_callback_access = WorldCallbackAccess::from_guard(world.clone());
-        bevy::log::debug!(
-            "Calling function {} with args {:?}",
-            self.name(),
-            args.display_with_world(world.clone())
-        );
         // should we be inlining call errors into the return value?
         let return_val = self.call(context, world_callback_access, args);
         match return_val {

crates/bevy_mod_scripting_core/src/bindings/function/script_function.rs

@@ -116,7 +116,7 @@ pub trait GetFunctionTypeDependencies<Marker> {
 
 /// The caller context when calling a script function.
 /// Functions can choose to react to caller preferences such as converting 1-indexed numbers to 0-indexed numbers
-#[derive(Clone, Copy, Debug, Reflect)]
+#[derive(Clone, Copy, Debug, Reflect, Default)]
 #[reflect(opaque)]
 pub struct CallerContext {
     pub convert_to_0_indexed: bool,

crates/bevy_mod_scripting_core/src/commands.rs

@@ -120,10 +120,6 @@ impl<P: IntoScriptPluginParams> CreateOrUpdateScript<P> {
 
 impl<P: IntoScriptPluginParams> Command for CreateOrUpdateScript<P> {
     fn apply(self, world: &mut bevy::prelude::World) {
-        debug!(
-            "CreateOrUpdateScript command applying to script_id: {}",
-            self.id
-        );
         let settings = world
             .get_resource::<ContextLoadingSettings<P>>()
             .unwrap()
@@ -148,24 +144,28 @@ impl<P: IntoScriptPluginParams> Command for CreateOrUpdateScript<P> {
             let mut script = scripts.scripts.get_mut(&self.id);
             let previous_context_id = script.as_ref().map(|s| s.context_id);
             debug!(
-                "CreateOrUpdateScript command applying with to (script_id: {}, previous_context_id: {:?})",
+                "{}: CreateOrUpdateScript command applying (script_id: {}, previous_context_id: {:?})",
+                P::LANGUAGE,
                 self.id, previous_context_id
             );
 
             // If None assign new context ID, otherwise assign the old one
             // If re-loading and different from the previous one, the old one will be removed
             let current_context_id = (assigner.assign)(script.as_deref(), &self.id, &self.content, &mut contexts);
-            debug!("Context assigned: {:?}", current_context_id);
+            
+            debug!("{}: New context assigned?: {:?}", P::LANGUAGE, current_context_id.is_none() || current_context_id != previous_context_id);
 
             let current_context_id = if let Some(id) = current_context_id {
                 // reload existing context
                 id
             } else {
+                let log_context = format!("{}: Loading script: {}", P::LANGUAGE, self.id);
+                bevy::log::info!("{}", log_context);
                 let ctxt = (builder.load)(&self.id, &self.content, &settings.context_initializers, &settings.context_pre_handling_initializers, world, &mut runtime.runtime);
                 match ctxt {
                     Ok(ctxt) => contexts.insert(ctxt),
                     Err(e) => {
-                        handle_script_errors(world, [e.with_context(format!("Loading script with id: {}. Language: {}", self.id, P::LANGUAGE))].into_iter());
+                        handle_script_errors(world, [e.with_context(log_context)].into_iter());
                         return;
                     }
                 }
@@ -174,22 +174,21 @@ impl<P: IntoScriptPluginParams> Command for CreateOrUpdateScript<P> {
 
             if let Some(previous) = previous_context_id {
                 if let Some(previous_context_id) = contexts.get_mut(previous) {
+                    let log_context = format!("{}: Reloading script: {}.", P::LANGUAGE, self.id);
+                    bevy::log::info!("{}", log_context);
                     match (builder.reload)(&self.id, &self.content, previous_context_id, &settings.context_initializers, &settings.context_pre_handling_initializers, world, &mut runtime.runtime) {
                         Ok(_) => {},
                         Err(e) => {
-                            handle_script_errors(world, [e.with_context(format!("Reloading script with id: {}. Language: {}", self.id, P::LANGUAGE))].into_iter());
+                            handle_script_errors(world, [e.with_context(log_context)].into_iter());
                             return;
                         }
                     };
                 } else {
-                    bevy::log::error!("Could not find previous context with id: {}. Could not reload script id: {}", previous, self.id);
+                    bevy::log::error!("{}: Could not find previous context with id: {}. Could not reload script: {}", P::LANGUAGE, previous, self.id);
                 }
 
                 if previous != current_context_id {
-                    debug!(
-                        "Script is being moved to a new context with id: {}, removing up old context.",
-                        current_context_id
-                    );
+                    bevy::log::info!("{}: Unloading script with id: {}. As it was assigned to a new context", P::LANGUAGE, self.id);
                     script.as_deref_mut().unwrap().context_id = current_context_id;
                     (assigner.remove)(previous, script.unwrap(), &mut contexts);
                 }
@@ -199,24 +198,22 @@ impl<P: IntoScriptPluginParams> Command for CreateOrUpdateScript<P> {
                 match (runner)(vec![], bevy::ecs::entity::Entity::from_raw(0), &self.id, &OnScriptLoaded::into_callback_label(), context, &settings.context_pre_handling_initializers, &mut runtime.runtime, world) {
                     Ok(_) => {},
                     Err(e) => {
-                        handle_script_errors(world, [e.with_context(format!("Running initialization hook for script with id: {}. Language: {}", self.id, P::LANGUAGE))].into_iter());
+                        handle_script_errors(world, [e.with_context(format!("{}: Running initialization hook for script with id: {}", P::LANGUAGE, self.id))].into_iter());
                     },
                 }
+
+                // we only want to insert the script if a context is present, otherwise something went wrong
+                scripts.scripts.insert(
+                    self.id.clone(),
+                    Script {
+                        id: self.id,
+                        asset: self.asset,
+                        context_id: current_context_id,
+                    },
+                );
             } else {
-                bevy::log::error!("Could not find context with id: {}. Could not run initialization hook for script id: {}. This may be because the script failed to load.", current_context_id, self.id);
+                bevy::log::error!("{}: Context loading failed for script: {}. Did not run on_script_loaded hook",P::LANGUAGE ,self.id);
             }
-
-            // now we can insert the actual script
-            scripts.scripts.insert(
-                self.id.clone(),
-                Script {
-                    id: self.id,
-                    asset: self.asset,
-                    context_id: current_context_id,
-                },
-            );
-
-            // finally we trigger on_script_loaded
         });
         world.insert_resource(settings);
         world.insert_non_send_resource(runtime);

crates/bevy_mod_scripting_core/src/handler.rs

@@ -16,7 +16,7 @@ pub type HandlerFn<P> = fn(
     pre_handling_initializers: &[ContextPreHandlingInitializer<P>],
     runtime: &mut <P as IntoScriptPluginParams>::R,
     world: &mut World,
-) -> Result<(), ScriptError>;
+) -> Result<ScriptValue, ScriptError>;
 
 /// A resource that holds the settings for the callback handler for a specific combination of type parameters
 #[derive(Resource)]