makspll / bevy_mod_scripting

Bevy Scripting Plugin
Apache License 2.0
398 stars 31 forks source link

Crash when trying to loop through empty Vec #86

Closed zwazel closed 11 months ago

zwazel commented 11 months ago

I got following struct:

#[derive(Resource, Reflect, Default, Clone)]
#[reflect(Resource)]
pub struct LobbyListResource {
    pub lobbies: Vec<u64>,
}

I register all the needed things to correctly access the vec and loop through it via scripts:

struct HandleLobbyApiProvider;

impl APIProvider for HandleLobbyApiProvider {
    type APITarget = Engine;
    type ScriptContext = RhaiContext;
    type DocTarget = RhaiDocFragment;

    fn register_with_app(&self, app: &mut App) {
        app.register_foreign_rhai_type::<Vec<u64>>();
    }

    fn attach_api(
        &mut self,
        api: &mut Self::APITarget,
    ) -> Result<(), bevy_mod_scripting::prelude::ScriptError> {
        api.register_vec_functions::<u64>();
        Ok(())
    }
}

With this I can then loop through the lobbies in Rhai like this:

let lobby_list_type = world.get_type_by_name(blablabla::LobbyListResource");
if world.has_resource(lobby_list_type) {
    let lobbies = world.get_resource(lobby_list_type).lobbies;
    for lobby in lobbies {
        print(`lobby: ${lobby}`);
    }
}

And this works... as long as the vec is not empty! When it's empty i get following error and it crashes:

thread 'main' panicked at 'attempt to subtract with overflow', C:\...\.cargo\git\checkouts\bevy_mod_scripting-ff78cea4271e6409\8879577\bevy_script_api\src\common\std.rs:136:26
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Encountered a panic in exclusive system `bevy_mod_scripting_core::systems::script_event_handler<bevy_mod_scripting_rhai::RhaiScriptHost<pgc::modding::ModdedScriptRuntimeArguments>, 0, 10>`!
Encountered a panic in system `bevy_app::main_schedule::Main::run_main`!

As I'm only using Rhai, I don't know if this error exists in Lua too.

The current workaround is to just check if the vec is empty before trying to loop through it:

let lobby_list_type = world.get_type_by_name(blablabla::LobbyListResource");
if world.has_resource(lobby_list_type) {
    let lobbies = world.get_resource(lobby_list_type).lobbies;
    if !lobbies.is_empty() {
        for lobby in lobbies {
            print(`lobby: ${lobby}`);
        }
    }
}
zwazel commented 11 months ago

I looked into this, and it seems the error comes from this line: https://github.com/makspll/bevy_mod_scripting/blob/eb691a26d316aba6fbd5f58322be7850db4833b0/bevy_script_api/src/common/std.rs#L136 as when the length is 0, it tries to subtract one. it being a usize, results in a attempt to subtract with overflow.

So just to test, i changed it to

fn into_iter(self) -> Self::IntoIter {
        ScriptVecIterator {
            current: 0,
            end: self.len().map(|v| v - 1).unwrap_or(0),
            base: self,
        }
    }

Which kind of solves it, as it doesnt crash anymore. ~But interestingly the variable, in my case lobbies, can't be found anymore:~ (not true, thats just me writing my variables wrong)

Now that i wrote the variables correctly, it still crashes, obviously because i'm still subtracting from usize... my bad, brb.

zwazel commented 11 months ago

Alright, changing it to

fn into_iter(self) -> Self::IntoIter {
    ScriptVecIterator {
        current: 0,
        // 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
        // if len > 0, subtract 1, otherwise set to 0
        end: self
            .len()
            .and_then(|len| Ok(if len > 0 { len - 1 } else { 0 }))
            .expect("Failed to get length of ScriptVec"),
        base: self,
    }
}

Makes it not crash. But I now get following error printed out:

Runtime error in script `lobby_list_container.rhai` Runtime error: Invalid reflection path: `[0]`. No such element
in call to function 'lobby_list_updated'
makspll commented 11 months ago

Is the no such element error happening on empty vecs? That might be because of the issue I mentioned with the iterator where it's still trying to index into it on empty vecs