Skip to content

How to provide temporary access to data from Lua function calls? #171

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

Closed
ilyvion opened this issue Apr 20, 2020 · 13 comments
Closed

How to provide temporary access to data from Lua function calls? #171

ilyvion opened this issue Apr 20, 2020 · 13 comments

Comments

@ilyvion
Copy link

ilyvion commented Apr 20, 2020

I've been looking through existing issues like #14, #118 and #162 and I'm not coming away with anything too useful, so I thought I'd ask for my problem specifically.

I'm trying to write a game in which all the actual work takes place in Rust, but where the control of what happens and when takes place in Lua. In other words, I'm not trying to store any Rust data in Lua or the other way around (at least not yet). However, I'm struggling to pass data from one place to another when there's a Lua call in-between.

Here's the code so far where I'm running up against the wall. I've marked the significant spots with XXX:

pub struct Scripts {
    lua: Lua,
}

impl Scripts {
    pub fn new() -> Result<Self> {
        let lua = Lua::new_with(
            StdLib::BASE
                | StdLib::COROUTINE
                | StdLib::TABLE
                | StdLib::STRING
                | StdLib::UTF8
                | StdLib::MATH
                | StdLib::PACKAGE,
        );

        lua.context(|lua_ctx| {
            let globals = lua_ctx.globals();

            // XXX: Define a `load_asset` function that provides the path to an asset
            let load_asset = lua_ctx.create_function(|_, string: String| {
                println!("Loading {}", string);

                // XXX: I need access to the `Mod` whose script was responsible for
                // this specific `load_asset` call. Probably also a bunch of other
                // things eventually, here or in other functions.
                let c: &mut Mod = lua_ctx.globals().get("module")?;

                Ok(())
            })?;
            globals.set("load_asset", load_asset)?;

            Ok(())
        })
        .context(Initialize)?;

        Ok(Self { lua })
    }

    pub fn run_script(&self, module: &mut Mod, script: &str) -> Result<()> {
        let script_path = module.scripts.join(script);
        if script_path.exists() {
            let script_contents = read_to_string(&script_path).context(LoadScript)?;
            let script_name = format!(
                "{}::{}",
                module.name,
                script_path
                    .file_name()
                    .map(std::ffi::OsStr::to_str)
                    .flatten()
                    .expect("Could not get file name of script")
            );
            self.lua
                .context(|lua_ctx| {
                    // XXX: Set up a scope to provide temporary access to non-'static stuff
                    lua_ctx.scope(|scope| {
                        // XXX: Pass our `&mut Mod` in via a global
                        lua_ctx
                            .globals()
                            .set("module", scope.create_nonstatic_userdata(module)?);

                        lua_ctx
                            .load(&script_contents)
                            .set_name(&script_name)?
                            .exec()
                    })?;

                    Ok(())
                })
                .context(RunScript)?;
        }

        Ok(())
    }
}

I previously also tried using set_named_registry_value and named_registry_value, but ran into the same problem. First:

error[E0277]: the trait bound `&mut mods::Mod: rlua::userdata::UserData` is not satisfied
  --> src\scripts.rs:61:76
   |
61 | ...                   .set("module", scope.create_nonstatic_userdata(module)?);
   |                                                                      ^^^^^^ the trait `rlua::userdata::UserData` is not implemented for `&mut mods::Mod`

and then, after

impl rlua::UserData for &mut Mod {}

I get:

error[E0277]: the trait bound `&mut mods::Mod: std::clone::Clone` is not satisfied
  --> src\scripts.rs:30:53
   |
30 |                 let c: &mut Mod = lua_ctx.globals().get("module")?;
   |                                                     ^^^ the trait `std::clone::Clone` is not implemented for `&mut mods::Mod`
   |
   = note: `std::clone::Clone` is implemented for `&mods::Mod`, but not for `&mut mods::Mod`
   = note: required because of the requirements on the impl of `rlua::value::FromLua<'_>` for `&mut mods::Mod`

I'm just... not sure where to go from here. I ran into the same problem with rlua::userdata::UserData not being implemented when using something like Cell or Rc, except I'm not allowed to impl UserData for Cell<Mod>, so... what is the "right" way to do something like this?

@kyren
Copy link
Contributor

kyren commented Apr 20, 2020

What you want to do is definitely possible and exactly what the "scoped" non-static userdata is designed for.

The reason you're running into the Clone bound is because there's a "helpful" automatic FromLua impl for T: UserData + Clone if you try to convert an rlua::Value into a type T. This is for conversion from a Value to an owned T by cloning, it's arguably more confusing than helpful.

Instead, you should grab an AnyUserData handle and then call AnyUserData::borrow_mut on it. In your case, it would be something like:

let c: AnyUserData = lua_ctx.globals().get("module")?;
let cref = c.borrow_mut()?;

That definitely works but it might be inconvenient, if you can stand to change your Lua-side API it might be easier to put the &mut Mod into a newtype wrapper and just use the normal userdata methods instead.

@ilyvion
Copy link
Author

ilyvion commented Apr 20, 2020

Thanks for the swift reply, @kyren! I tried your suggestion, but as soon as I added the line let c: AnyUserData = lua_ctx.globals().get("module")?; to the load_asset function, I get this error:

error[E0277]: `*mut std::marker::PhantomData<std::cell::UnsafeCell<()>>` cannot be shared between threads safely
  --> src\scripts.rs:28:38
   |
28 |             let load_asset = lua_ctx.create_function(|_, string: String| {
   |                                      ^^^^^^^^^^^^^^^ `*mut std::marker::PhantomData<std::cell::UnsafeCell<()>>` cannot be shared between threads safely
   |
   = help: within `rlua::context::Context<'_>`, the trait `std::marker::Sync` is not implemented for `*mut std::marker::PhantomData<std::cell::UnsafeCell<()>>`
   = note: required because it appears within the type `std::marker::PhantomData<*mut std::marker::PhantomData<std::cell::UnsafeCell<()>>>`
   = note: required because it appears within the type `rlua::context::Context<'_>`
   = note: required because of the requirements on the impl of `std::marker::Send` for `&rlua::context::Context<'_>`
   = note: required because it appears within the type `[closure@src\scripts.rs:28:54: 37:14 lua_ctx:&rlua::context::Context<'_>]`

I have no idea what this error even means.

I also tried this:

pub struct MutModRef<'a>(pub &'a mut Mod);
impl<'a> rlua::UserData for MutModRef<'a> {}

but it lends itself to the exact same Clone problem as trying to use &mut Mod directly.

@kyren
Copy link
Contributor

kyren commented Apr 20, 2020

Use the context that's the first parameter to the closure you give to create_function, not the context from the outer scope. (Just replace the _ with lua_ctx)

@ilyvion
Copy link
Author

ilyvion commented Apr 20, 2020

@kyren Aha. I feel like I'm so close now. Here's what I've got:

let c: rlua::AnyUserData = inner_ctx.globals().get("module")?;
let cref: RefMut<&mut Mod> = c.borrow_mut()?;

and here's what happens:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: ScriptError { source: RunScript { source: CallbackError { traceback: "stack traceback:\n\t[C]: in ?\n\t[C]: in function \'load_asset\'\n\t[string \"core::init.lua\"]:1: in main chunk", cause: UserDataTypeMismatch } } }', src\lib.rs:150:17

I tried doing this to see if it could give me something more useful, but it doesn't, really:

let c: rlua::AnyUserData = inner_ctx.globals().get("module")?;
let cref_result: LuaResult<RefMut<&mut Mod>> = c.borrow_mut();
let cref = match cref_result {
    Ok(cref) => cref,
    Err(e) => {
        dbg!(&e);
        return Err(e);
    }
};

The output of that dbg!(...) is just [src\scripts.rs:36] &e = UserDataTypeMismatch.

I'm guessing this happens because a &mut Mod isn't necessarily the same &mut Mod due to different lifetimes or something? Also, just for info, if I dbg!(&c) I get things like

[src\scripts.rs:37] &c = AnyUserData(
    Ref(3),
)

so I'm getting something, clearly.

I don't know if it's relevant, but there's this snippet from the docs of create_nonstatic_userdata:

The main limitation that comes from using non-'static userdata is that the produced userdata will no longer have a TypeId associated with it, becuase TypeId can only work for 'static types. This means that it is impossible, once the userdata is created, to get a reference to it back out of an AnyUserData handle.

It's particularly that bit about it "[being] impossible to get a reference to it back out" that I'm concerned about, since that's what I'm trying to do, isn't it?

@kyren
Copy link
Contributor

kyren commented Apr 20, 2020

Oh yeah it definitely is impossible to get a reference back out, yeah sorry I totally forgot about that. You'll have to use userdata methods instead of getting the reference out through AnyUserData. You can add any methods to the type that you want when you impl UserData for &mut Mod {}, here's an example: https://github.com/kyren/rlua/blob/master/examples/guided_tour.rs#L173

If that's not quite the API you want you may have to make API wrappers on the Lua side or something like that.

@ilyvion
Copy link
Author

ilyvion commented Apr 20, 2020

This is probably wildly unsafe if used incorrectly, but I'm hoping that since I'm using this in a controlled manner, it should probably be okay..? I made this type:

pub struct LuaPointer<P>(*mut P);
impl<P> Clone for LuaPointer<P> {
    fn clone(&self) -> Self {
        LuaPointer(self.0)
    }
}
impl<P> rlua::UserData for LuaPointer<P> {}
impl<P> Deref for LuaPointer<P> {
    type Target = P;

    #[allow(unsafe_code)]
    fn deref(&self) -> &Self::Target {
        unsafe { &*self.0 }
    }
}
impl<P> DerefMut for LuaPointer<P> {
    #[allow(unsafe_code)]
    fn deref_mut(&mut self) -> &mut Self::Target {
        unsafe { &mut *self.0 }
    }
}

and then I put this on my Mod:

pub fn get_pointer(&mut self) -> LuaPointer<Self> {
    LuaPointer(self as *mut Mod)
}

and now I can do this in the scope:

lua_ctx.set_named_registry_value(
    "module",
    scope.create_static_userdata(module.get_pointer())?,
)?;

and use it in functions by doing

let m: LuaPointer<Mod> = inner_ctx.named_registry_value("module")?;
dbg!(&m.name);

I'm sure you can easily tell me why this is a horrible idea, though. 😉

My reasoning why I think this should be okay is that 1) it's scope-bound, so the LuaPointer should only live as long as this call into my Lua script does, and 2) because everything is single-threaded, there should never be a place where more than one possible place that &mut Mod is ever dereferenced. Am I wrong?

@kyren
Copy link
Contributor

kyren commented Apr 20, 2020

It's not a sound interface, you can use it to do mutable aliasing or use after free easily afaict. That doesn't mean you are I'm just saying you can.

If you're willing to do go the unsafe route though, then why bother with a scope in the first place.

@kyren
Copy link
Contributor

kyren commented Apr 20, 2020

I want to explain some more about why this pattern is so unsafe, and why the scope system doesn't allow you to get a reference out of a non-'static userdata (I know I totally forgot about this at the beginning, my brain wasn't engaged yet). I know that the limitation of only getting callbacks or callback-like things in non-'static userdata is limiting, but it's not possible to make it safe otherwise.

What you're basically trying to do is this (which doesn't work, just an example):

let x = 2;
let r = &x;
let b: Box<Any> = Box::new(r);
let r2: &i32 = r.downcast();
drop(x);
// do anything with r2

Rust lifetime annotations are a compile time concern, and we cannot turn it somehow into a runtime property. There's not ever going to be a mechanism to do that safely, it would take whole new mechanisms in the Rust language to make that work somehow.

By throwing away the lifetime information, you can do all kinds of horrible things and suddenly the borrow checker has no idea what you're doing anymore, that's why the only way to do such a thing in real Rust is through a pointer and the use of unsafe.

You might think that there should be a way to give back a reference to a created userdata that is 'scope, since the scope system knows that any userdata it creates lives for that long, but that can't be made safe either because of variance, you could take an invariant or contravariant type and unsafely shorten the lifetime.

All of this is why any use of Any / TypeId requires 'static.

I know it sucks, but your life will be a lot easier if you can find an API that uses the safe interface provided.

@ilyvion
Copy link
Author

ilyvion commented Apr 21, 2020

Alright, after a night's sleep and after reading your explanation, I took a whole new approach.

I'm using ggez as my game engine, which isn't all that important, but I'm mentioning it because one of the things it does is make you implement an EventHandler trait that has update and draw methods, where you do your usual game engine things. This trait gets implemented on the struct that holds all your game state. In my case, I've unimaginatively named this struct GameState.

What I had tried to do previous was to grab references to fields on said struct. It holds a Vec<Mod> field for instance, that knows about all the mods. Anyway, I figured it'd be better if I could instead just access the whole GameState struct in calls coming from Lua instead of having to pass around references to its individual fields. And it turned out to be possible! Here's what I did:

impl rlua::UserData for &mut GameState {
    fn add_methods<'lua, T: rlua::UserDataMethods<'lua, Self>>(methods: &mut T) {
        methods.add_method("load_asset", |_lua_ctx, game_state, asset_path: String| {
            let module = game_state.mods.get(game_state.current_mod.unwrap()).unwrap();

            dbg!(&module.name, asset_path);

            Ok(())
        });
    }
}

Very nice! I struggled a bit with passing &mut GameState into Lua though, particularly because GameState owns the Lua instance. I eventually got around this by putting the Lua inside a Rc<RefCell<Lua>>, and this is how the call site looks now:

let lua = Rc::clone(&self.lua);
let lua = lua.borrow();
lua.context(|lua_ctx| {
    lua_ctx.scope(|scope| {
        // XXX: Look ma, no static!
        lua_ctx
            .globals()
            .set("game", scope.create_nonstatic_userdata(self)?)?;

        lua_ctx
            .load(&script_contents)
            .set_name(&script_name)?
            .exec()
    })?;

    Ok(())
})
.context(RunScript)?;

By being able to Rc::clone and then RefCell::borrow that Lua, I can get away with passing self (which is a &mut GameState) directly into create_nonstatic_userdata. All is well!

I would never have gotten here without our back and forth, so thanks a lot for being willing to entertain my newbie questions and approaches while I made my way towards the sound and functional solution, @kyren. I really appreciate it.

@kyren
Copy link
Contributor

kyren commented Apr 21, 2020

I would never have gotten here without our back and forth, so thanks a lot for being willing to entertain my newbie questions and approaches while I made my way towards the sound and functional solution, @kyren. I really appreciate it.

You're very welcome!

@makspll
Copy link

makspll commented Apr 30, 2022

Sorry for resurrecting this thread @kyren, but I am quite new to Bevy and Rust, but trying to implement something similar I believe, and before I start fighting the compiler I wanted to clarify if this approach would work.

So I am building a bevy plugin to handle scripting, what I'd like to do ideally is this:

  1. Initiate a script only once, and provide a rust API to it then
  2. Run the script on certain events with the ability to call the rust API

Now the main dilemma comes from the fact that the rust API needs to operate on the current &mut World, what i was thinking is maybe there's a way to store this reference on the Lua side as a global, I believe this is identical to the use of game in @alexschrod's example. The alternative would be to re-register THE ENTIRE API on every call to the script, which would seem a bit inefficient to me, although i could maybe cut down on the number of re-registers if I parsed the script in the initialisation stage and stored which functions are being used by the script.

It appears to me that Alex's solution would work in my case however I think I see potential problems, namely:

create_nonstatic_userdata appears to transfer the world instance to the Lua world, now if it does what I think this would mean copying the entire bevy state into Lua every event for every script. Am I correct in thinking this would be horribly inefficient ?

Ideally I'd want to simply store a pointer to the world, and get it back out via the API but I am also not sure that's possible.

I would really appreciate guidance on this!

@jugglerchris
Copy link
Collaborator

Hi @makspll ,
I don't know Bevy so I'm guessing a little here...
I don't think you'll be able to put a &mut World into a Lua (non-scoped) global variable, as that would imply you can't use it elsewhere.

However, I think what @kyren wrote above is probably exactly what you need:

  • Note that UserData is implemented for &mut GameState not GameState - it's only storing a reference in the (scoped) Lua global. There's no copying, and after the scope exits the state reference will be safely inaccessible (any attempt to use it from Lua, e.g. if it saved it away, will result in an error).
  • You don't need to reload the script each time - instead of the load you can just execute a function:
   lua_ctx.globals()
              .get("script_main")?
              .call::<_, ()>(some_args)?;

I hope this helps!

@makspll
Copy link

makspll commented Apr 30, 2022

Amazing! Thank you so much for the quick response. I think this plugs into bevy perfectly in this case!

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

4 participants