makspll / bevy_mod_scripting

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

lua globals are not registered until callback function. #97

Open ConnorBP opened 7 months ago

ConnorBP commented 7 months ago

Globals are not being defined on my lua script until partway into it's execution (when a callback is called). Is this how it is supposed to go? and can it be changed? or is this a bug?

For example, in this modified version of console_integration.lua the print_to_console function is not defined in the beginning of the script. It does work however in the on_update function. This makes it hard to reliably override the print function since it only sometimes applies.

local a = 0

print("testing print")
print_to_console("testing print2")

function on_update()

    if a % 100 == 0 then
        -- print() is defined in lua_system.rs
        -- by the api provider
        print_to_console(string.format("%d, entity index:%d", a, entity:index()))
    end

    a = a + 1
end
ConnorBP commented 7 months ago

Another thing:

pub fn forward_script_err_to_console(
    mut r: EventReader<ScriptErrorEvent>,
    mut w: EventWriter<PrintConsoleLine>,
) {
    for e in r.iter() {
        w.send(PrintConsoleLine {
            line: format!("ERROR:{}", e.error).into(),
        });
    }
}

is not forwarding errors when a script fails to load:

2024-01-18T21:55:17.170874Z  WARN bevy_mod_scripting_core::hosts: Error in loading script console_example.lua:
Failed to load script asset for `console_example.lua` runtime error: [string "console_example.lua"]:4: attempt to call global 'print_test' (a nil value)
stack traceback:
        [C]: in function 'print_test'
        [string "console_example.lua"]:4: in main chunk
ConnorBP commented 7 months ago

pretty sure it is related to this line being called after script load: https://github.com/makspll/bevy_mod_scripting/blob/48d80128be3089bbf5fc8772492638e618cecdcd/languages/bevy_mod_scripting_lua/src/lib.rs#L126

ConnorBP commented 7 months ago

re-ordered the suspicious function and now i get this error instead:

Failed to load script asset for `console_example.lua` callback error
stack traceback:
        [C]: in function 'print_test'
        [string "console_example.lua"]:5: in main chunk
caused by: error converting Lua nil to userdata
ConnorBP commented 7 months ago

re-ordered the suspicious function and now i get this error instead:

Failed to load script asset for `console_example.lua` callback error
stack traceback:
        [C]: in function 'print_test'
        [string "console_example.lua"]:5: in main chunk
caused by: error converting Lua nil to userdata

This crashes on the line: let world = ctx.get_world()?;

in

ctx.globals()
            .set(
                "print_test",
                ctx.create_function(|ctx, msg: String| {
                    println!("RUNNING PRINT COMMAND");
                    // retrieve the world pointer
                    let world = ctx.get_world()?;
                    println!("GOT WORLD");
                    let mut world = world.write();
                    println!("GOT WRITE ACCESS TO WORLD");

                    let mut events: Mut<Events<PrintConsoleLine>> =
                        world.get_resource_mut().unwrap();
                    println!("GOT EVENT WRITER");
                    events.send(PrintConsoleLine { line: msg.into() });
                    println!("SENT EVENT");

                    // return something
                    Ok(())
                })
                .map_err(|e| {println!("print was called and errored. {e}");ScriptError::new_other(e)})?,
            )
            .map_err(|e| {println!("failed to attach print command. {e}");ScriptError::new_other(e)})?;
ATTACHED API
testing print
RUNNING PRINT COMMAND
2024-01-19T03:55:01.515956Z  WARN bevy_mod_scripting_core::hosts: Error in loading script console_example.lua:
Failed to load script asset for `console_example.lua` callback error
stack traceback:
        [C]: in function 'print_test'
        [string "console_example.lua"]:7: in function 'test'
        [string "console_example.lua"]:10: in main chunk
caused by: error converting Lua nil to userdata
makspll commented 7 months ago

Hi, the API being available on script load is not something I thought about, out of curiosity what do you intend to call from the scripts on load?

makspll commented 7 months ago

strange the new error suggests the world global is not set, but it looks like it should be

ConnorBP commented 7 months ago

strange the new error suggests the world global is not set, but it looks like it should be

yeah not quite sure what's going on there

ConnorBP commented 7 months ago

Hi, the API being available on script load is not something I thought about, out of curiosity what do you intend to call from the scripts on load?

Initialization stuff primarily and I want to be able to log out to my graphical console. For this I am overriding the print function to redirect it, but it is useless if it is not consistently overridden for the whole script.

ConnorBP commented 7 months ago

strange the new error suggests the world global is not set, but it looks like it should be

I suspect that perhaps the world pointer is accessed in one of the functions that is calling my print function perhaps and so is failing then in my sub-function when i get it to print to console.

ConnorBP commented 7 months ago
                    let world = match(ctx.get_world()) {
                        Ok(world) => {
                            world
                        },
                        Err(e) => {
                            println!("Get an error trying to print to console: {e}");
                            return Ok(());
                        }
                    };

changed the get world part to this. It is definitely ctx.get_world() that is failing.

ConnorBP commented 7 months ago

strange the new error suggests the world global is not set, but it looks like it should be

How is it set? If it is set with bevy ecs commands it doesn't get finalized until the next iteration unless you force finalize a set to compete. I'll try and find what line of code it is set with in a min

ConnorBP commented 7 months ago

Found the offending code. load_script does not call setup_runtime_all before loading the script, and thus any code that is run on initialization has no access to any api.

https://github.com/makspll/bevy_mod_scripting/blob/48d80128be3089bbf5fc8772492638e618cecdcd/languages/bevy_mod_scripting_lua/src/lib.rs#L116

https://github.com/makspll/bevy_mod_scripting/blob/85e66a569528b5c68531a81933d7300fef6e4d56/bevy_mod_scripting_core/src/hosts.rs#L158

so this:

fn setup_script_runtime(
        &mut self,
        world_ptr: bevy_mod_scripting_core::world::WorldPointer,
        _script_data: &ScriptData,
        ctx: &mut Self::ScriptContext,
    ) -> Result<(), ScriptError> {
        let ctx = ctx.get_mut().expect("Could not get context");
        let globals = ctx.globals();
        globals
            .set("world", crate::lua::bevy::LuaWorld::new(world_ptr))
            .map_err(ScriptError::new_other)
    }

never gets called before script load. And world is in fact not set in the globals context.

edit: It looks like adding setup_runtime would need a pretty decent refactor on the function and all the systems that call it to add a world pointer. Not sure if there is some way to do that without turning them all into exclusive Systems

makspll commented 7 months ago

Hi, ah yes I confused the two API hooks, yes in that case this is by design, and the easiest workaround is just to have an intialization callback:

function on_load()
 ...
end

You could probably listen to the bevy_mod_scripting_core::event::ScriptLoaded events and trigger your own which get handled first

ConnorBP commented 7 months ago

Hi, ah yes I confused the two API hooks, yes in that case this is by design, and the easiest workaround is just to have an intialization callback:

function on_load()
 ...
end

You could probably listen to the bevy_mod_scripting_core::event::ScriptLoaded events and trigger your own which get handled first

Would refactoring the script load function into an exclusive system be a bad idea do you think? Not having api overrides be consistent runs me the wrong way. I submitted a PR to fix the ordering of script load api overriding, I can add on a small system refactor as well if desired.

Or perhaps a better fix would be adding bevy event writers to the context that the callback functions receive. That way global world access is not required in those functions.

ConnorBP commented 7 months ago

Hi, ah yes I confused the two API hooks, yes in that case this is by design, and the easiest workaround is just to have an intialization callback:


function on_load()

 ...

end

You could probably listen to the bevy_mod_scripting_core::event::ScriptLoaded events and trigger your own which get handled first

Can you review my PR? https://github.com/makspll/bevy_mod_scripting/pull/98

makspll commented 7 months ago

Hmm good questions, making the script management systems exclusive will not have major repercussions I don't think, I am quite happy with the addition of a world pointer to the load_script callback, we'd just need to make it clear in the appropriate docs that this is what happens. As you say it would be quite a large re-factor on the systems side of things, but if you're happy with it I don't see any issues