makspll / bevy_mod_scripting

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

Apply LUA api providers before loading script. #98

Closed ConnorBP closed 9 months ago

ConnorBP commented 9 months ago

Fixes issue where api is not available during script initialization. https://github.com/makspll/bevy_mod_scripting/issues/97

PR is not yet complete. There is still a callback error caused by: error converting Lua nil to userdata to investigate which happens when the api function is called.

ConnorBP commented 9 months ago

Found what is causing the NIL error. It happened upon fetching the bevy "world" from globals. It is not set on script load execution, only after event execution is fired. This makes it so that any API function which accesses world state cannot be used during script load. To fix this requires a refactor involving changing the script_add_synchronizer into an exclusive system with a SystemState

ConnorBP commented 9 months ago

Update: the other issues have their own concerns that should be in another PR. But the api registration issue is solved by this PR and should be considered Complete.

I will open a different PR for potential api refactoring for connecting callback functions with event signals.

ConnorBP commented 9 months ago

Looks good, just one small nit

`providers.attach_all()' requires a mutex as it's argument.

Also get_mut is describes as follows: ` Returns a mutable reference to the underlying data.

Since this call borrows the Mutex mutably, no actual locking needs to take place -- the mutable borrow statically guarantees no locks exist. ` So it should not be an issue

makspll commented 9 months ago

Sounds good, let's just fix CI and happy to merge (rustfmt not aarch64, that should fail)

ConnorBP commented 9 months ago

It worked it looks like... but then failed to cache?