Small cleaning up + Fix for crash when looping through empty Vec (#87)

* dwadwa * debug prints * we ignore this ever happened * mmmmmmmmmmm i'm still a bit dumb lol * does this fix the crash? * that maybe fixes it * unncessecary get_children + fixed get_parent * rustfmt * clippy * moved wrappers example into lua and renamed it * WIP new example for rhai wrapper * WIP implementing fix mentioned in review #87 (comment) * basic one shot works * accessing variables work * accessing variables not work * Revert "accessing variables not work" This reverts commit c282ada. * empty array * go back to <= * commented out errors, setting values seem to work * expanded multiple_events example for parent testing * returning UNIT instead of int if parent couldn't be found * Update bevy_script_api/src/common/std.rs Co-authored-by: Maksymilian Mozolewski <[email protected]> * Update bevy_script_api/src/common/std.rs Co-authored-by: Maksymilian Mozolewski <[email protected]> * no longer problem at looping through empty vec * if/else to check if parent exists * removed rhai wrapper example, moved normal wrapper example back and renamed * rust fmt --------- Co-authored-by: Maksymilian Mozolewski <[email protected]>
makspll · Nov 18, 2023 · 9c830b1 · 9c830b1
1 parent eb691a2
commit 9c830b1
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 34 deletions.
diff --git a/assets/scripts/multiple_events_rhai.rhai b/assets/scripts/multiple_events_rhai.rhai
@@ -1,5 +1,12 @@
 fn on_init(name) {
     print(`Hello World! From "${name}" in Init`);
+
+    let parent = world.get_parent(entity);
+    if parent == () {
+        print(`Parent doesn't exist`);
+    } else {
+        print(`Parent exists`);
+    }
 }
 
 fn on_update(name, delta) {

diff --git a/bevy_mod_scripting_core/src/hosts.rs b/bevy_mod_scripting_core/src/hosts.rs
@@ -360,22 +360,25 @@ impl<T: Asset> Script<T> {
         event_writer: &mut EventWriter<ScriptLoaded>,
     ) {
         debug!("reloading script {}", script.id);
-        // retrieve owning entity
-        let entity = contexts.script_owner(script.id()).unwrap();
-
-        // remove old context
-        contexts.remove_context(script.id());
 
-        // insert new re-loaded context
-        Self::insert_new_script_context::<H>(
-            host,
-            script,
-            entity,
-            script_assets,
-            providers,
-            contexts,
-            event_writer,
-        );
+        // retrieve owning entity
+        if let Some(entity) = contexts.script_owner(script.id()) {
+            // remove old context
+            contexts.remove_context(script.id());
+            // insert new re-loaded context
+            Self::insert_new_script_context::<H>(
+                host,
+                script,
+                entity,
+                script_assets,
+                providers,
+                contexts,
+                event_writer,
+            );
+        } else {
+            // remove old context
+            contexts.remove_context(script.id());
+        }
     }
 
     /// checks if a script has loaded, and if so loads (`ScriptHost::load_script`),

diff --git a/bevy_script_api/src/common/std.rs b/bevy_script_api/src/common/std.rs
@@ -102,15 +102,15 @@ impl<T> From<ScriptVec<T>> for ScriptRef {
 
 pub struct ScriptVecIterator<T> {
     current: usize,
-    end: usize,
+    len: usize,
     base: ScriptVec<T>,
 }
 
 impl<T: FromReflect> Iterator for ScriptVecIterator<T> {
     type Item = ScriptRef;
 
     fn next(&mut self) -> Option<Self::Item> {
-        let nxt = (self.current <= self.end).then(|| self.base.index(self.current));
+        let nxt = (self.current < self.len).then(|| self.base.index(self.current));
         self.current += 1;
         nxt
     }
@@ -131,10 +131,8 @@ impl<T: FromReflect + TypePath> IntoIterator for ScriptVec<T> {
             // TODO?: end used to be an Option, and this check moved into the next method but
             // I am not sure if this will ever realistically fail, so if you do get this exception happening
             // hit me with an issue
-            end: self
-                .len()
-                .map(|v| v - 1)
-                .expect("Could not access length when creating iterator"),
+            // if len > 0, subtract 1, otherwise set to 0
+            len: self.len().expect("Failed to get length of ScriptVec"),
             base: self,
         }
     }

diff --git a/bevy_script_api/src/rhai/bevy/mod.rs b/bevy_script_api/src/rhai/bevy/mod.rs
@@ -40,13 +40,6 @@ impl CustomType for ScriptWorld {
                     .map(Dynamic::from)
                     .unwrap_or_default()
             })
-            .with_fn("get_children", |self_: ScriptWorld, parent: Entity| {
-                self_
-                    .get_children(parent)
-                    .into_iter()
-                    .map(Dynamic::from)
-                    .collect::<Vec<_>>()
-            })
             .with_fn(
                 "add_default_component",
                 |self_: ScriptWorld, entity: Entity, type_registration: ScriptTypeRegistration| {
@@ -138,15 +131,19 @@ impl CustomType for ScriptWorld {
                     })
                 },
             )
-            .with_fn("get_children", |self_: &ScriptWorld, parent: Entity| {
+            .with_fn("get_parent", |self_: ScriptWorld, entity: Entity| {
+                if let Some(parent) = self_.get_parent(entity) {
+                    Dynamic::from(parent)
+                } else {
+                    Dynamic::UNIT
+                }
+            })
+            .with_fn("get_children", |self_: ScriptWorld, parent: Entity| {
                 self_
                     .get_children(parent)
                     .into_iter()
                     .map(Dynamic::from)
-                    .collect::<Vec<Dynamic>>()
-            })
-            .with_fn("get_parent", |self_: &ScriptWorld, entity: Entity| {
-                self_.get_parent(entity)
+                    .collect::<Vec<_>>()
             })
             .with_fn(
                 "push_child",

diff --git a/examples/rhai/multiple_events_rhai.rs b/examples/rhai/multiple_events_rhai.rs
@@ -60,7 +60,20 @@ impl FuncArgs for ScriptArgs {
 fn setup_entities(mut commands: Commands, asset_server: Res<AssetServer>) {
     let script_path = "scripts/multiple_events_rhai.rhai";
 
-    for i in 0..10 {
+    commands.spawn_empty().with_children(|parent| {
+        parent.spawn((
+            NewlyAddedEntityCallInit,
+            Name::from("Test Entity 0"),
+            ScriptCollection::<RhaiFile> {
+                scripts: vec![Script::new(
+                    script_path.to_owned(),
+                    asset_server.load(script_path),
+                )],
+            },
+        ));
+    });
+
+    for i in 1..10 {
         let entity_name = format!("Test Entity {}", i);
         commands.spawn((
             NewlyAddedEntityCallInit,