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 require not loading from assets dir #69

Closed
LaserWitch opened this issue Aug 27, 2023 · 5 comments
Closed

lua require not loading from assets dir #69

LaserWitch opened this issue Aug 27, 2023 · 5 comments

Comments

@LaserWitch
Copy link
Contributor

In testing, using require "common" with the require feature gets me the output

2023-08-27T22:50:30.267464Z  WARN bevy_mod_scripting_core::hosts: Error in loading script AssetPath {
    path: "scripts/1.lua",
    label: None,
}:
Failed to load script asset for `AssetPath {
    path: "scripts/1.lua",
    label: None,
}` runtime error: [string "AssetPath {..."]:1: module 'common' not found:
        no field package.preload['common']
        no file 'D:\Code\luademo\target\debug\lua\common.lua'
        no file 'D:\Code\luademo\target\debug\lua\common\init.lua'
        no file 'D:\Code\luademo\target\debug\common.lua'
        no file 'D:\Code\luademo\target\debug\common\init.lua'
        no file 'D:\Code\luademo\target\debug\..\share\lua\5.4\common.lua'
        no file 'D:\Code\luademo\target\debug\..\share\lua\5.4\common\init.lua'
        no file '.\common.lua'
        no file '.\common\init.lua'
        no file 'D:\Code\luademo\target\debug\common.dll'
        no file 'D:\Code\luademo\target\debug\..\lib\lua\5.4\common.dll'
        no file 'D:\Code\luademo\target\debug\loadall.dll'
        no file '.\common.dll'
stack traceback:
        [C]: in ?
        [C]: in function 'require'
        [string "AssetPath {..."]:1: in main chunk

common.lua is in this case sitting in my project's assets\scripts directory right next to my other lua files, it's actually being loaded by bevy's asset server even. To get it to load I must use require "assets/scripts/common" (running via cargo from the project root dir)

Ultimately, for my ends I think I want require to load from bevy's asset server somehow as part of a sandboxing effort. That feels potentially out of scope as a core feature, but it seems like this crate should probably at the least add the assets directory to the searcher paths.

@makspll
Copy link
Owner

makspll commented Aug 29, 2023

I think this should achievable with a custom APIProvider setup_scripts which modifies the package.path or alternatively injects a custom loader into the default searcher sequence in: package.searchers as described in: https://www.lua.org/manual/5.3/manual.html#6.3

As for core functionality we could potentially include scripts directly in the assets folder or something akin to that within the LuaBevyAPIProvider

@LaserWitch
Copy link
Contributor Author

I think a custom provider can probably be made which inserts a searcher over loaded LuaFile assets which would be idea. I've a bit of experience with custom searchers before so I'll give it a shot.

The biggest question mark for loading from assets is in hot reloading change detection. I think for the standard luahost that searcher would need to update a map of all the script contexts that required a given asset and reload them too. If I get my own ambitions to retain lua state between reloads working then that'll require some extra bespoke tinkering because if I recall correctly require caches what it loads inside lua, so that cache needs cleaning. If I get global state working as, then I believe the way lua works means that cache would need cleaning but the 'downstream' scripts wouldn't need to be reloaded anymore... unless of course they errored on the require previously, hm. I think if they do that they end up constantly retrying anyway so it might be fine.

@LaserWitch
Copy link
Contributor Author

Ok, I've got this close to how I want it.

I've ended up doing a bit of an end-run around searchers and loaders. Instead of all that, in my custom host's load_file I'm not executing the chunk immediately, instead I'm treating it as a function and then running some Lua to insert that function into the global package.preload. Then if there's already anything in package.loaded for that name I am doing some juggling to let modules preserve some internal state but replace or add functions, bypassing the normal Lua behavior of not executing the same require's code more than once.

This has a few wrinkles left. One is hook functions, the current handle events only looks for singular global hooks, so multiple scripts loaded into global space this way can stomp on each other which sucks. I expect I'll need to make a central hook dispatch in lua that modules can register with in an on-load hook-like function. To that end it'd be nice to keep scripts from touching gobals during load(outside that on-load function) to avoid stepping on those dispatch functions, I'm not sure how to do that in lua 5.4 in a way that meshes well with what I'm already doing.

The other wrinkle is load order. As is now I'm just running the load function, effectively requiring the script, as part of the load file process. I'm going to have to shift that to a step that's run after all the scripts load, at which point the requires should all run fine. I may also want to draw a distinction between library files that don't get automatically required by the engine and active scripts that are. Unsure if that's needed though.

I don't think this needs any further upstream changes. As a potential future crate inclusion, this all feels like it'd need to be a new host along with the shared global enviroment changes, or some pretty deep configuration switches on the current lua host. Since it kind of puts different demand on game world architecture in how you set up script collections, it feels like a separate host might be appropriate.

If I were to try to implement this for the isolated lua instances of the main hosts, I think I might do it as an APIProvider. That could load chunks from a resource and insert wrapped versions into each context's package.preload table.

Either approach seems like it could allow for a safe version of require to be allowed by the crate, if remove all the standard filesystems searchers and only allow loading from the loaders which have to come from the bevy asset system.

I hope none of this comes off as either me commiting to implement all this for the crate or demanding that you do either :) I'm just prone to going off on speculative tangents about how things could work.

@makspll
Copy link
Owner

makspll commented Sep 6, 2023

I hope none of this comes off as either me commiting to implement all this for the crate or demanding that you do either :) I'm just prone to going off on speculative tangents about how things could work.

Your tangents are much appreciated! And I am sure these will provide a fantastic foundation for future PR's! And of course I do not expect that you in particular implement these.

Thanks for all the help!

@LaserWitch
Copy link
Contributor Author

I've accomplished all my goals in this and put my implementation up in issue #73

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