Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lua globals are not registered until callback function. #97

Open
ConnorBP opened this issue Jan 18, 2024 · 16 comments
Open

lua globals are not registered until callback function. #97

ConnorBP opened this issue Jan 18, 2024 · 16 comments

Comments

@ConnorBP
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

ConnorBP commented Jan 18, 2024

pretty sure it is related to this line being called after script load:

providers.attach_all(&mut lua)?;

@ConnorBP
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Owner

makspll commented Jan 19, 2024

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
Copy link
Owner

makspll commented Jan 19, 2024

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

@ConnorBP
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

                    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
Copy link
Contributor Author

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
Copy link
Contributor Author

ConnorBP commented Jan 21, 2024

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.

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
Copy link
Owner

makspll commented Jan 21, 2024

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
Copy link
Contributor Author

ConnorBP commented Jan 21, 2024

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
Copy link
Contributor Author

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? #98

@makspll
Copy link
Owner

makspll commented Jan 22, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants