mlua-rs / rlua

High level Lua bindings to Rust
Other
1.72k stars 115 forks source link

Possible bug with __newindex not called on numeric indexing #251

Closed makspll closed 2 years ago

makspll commented 2 years ago

I have a macro generating userdatas, one of them is for a vector LuaVector2 which i want to be indexable by either integers or axis names i.e. : vec.x or vec[0] (don't mind the 0-indexing for now it doesn't matter):

#[meta_mut] MetaMethod::NewIndex => |_,s : &mut [<Lua $vec_base>],(idx,val) : (Value,$vec_num)| { 
    match idx {
        Value::Integer(v) => Ok(s.val[v as usize] = val),
        Value::String(ref v) => {
            let idx = match v.to_str()? {
                "x" => 0,
                "y" => 1,
                "z" => 2,
                "w" => 3,
                _ => Err(Error::RuntimeError(format!("Cannot index {} with {:#?}",stringify!($vec_base),idx)))?
            };
            println!("Assigning to {:?}, {:?}",s.vref, val);
            // if our wrapper holds a reference it means it is an immediate indexing into
            // the original value, i.e. some_struct.our_vec[idx] = value
            Ok(match &mut s.vref {
                Some(r) => r.get_mut().downcast_mut::<$vec_base>().unwrap()[idx] = val,
                None => s.val[idx] = val,
            })

        },
        _ => Err(Error::RuntimeError(format!("Cannot index {} with {:#?}",stringify!($vec_base),idx)))
    }
};

I have the following lua code which does not work:

local comp

function on_update()
    if comp == nil then
        comp = world:get_component(entity,"MyComponent")

        print(string.format("%s",comp.vec2))
        comp.vec2[0] = 5.4
        print(string.format("%s",comp.vec2))

    end
end

and prints: (1,0) then (1,0)

whereas this one works:


local comp

function on_update()
    if comp == nil then
        comp = world:get_component(entity,"MyComponent")

        print(string.format("%s",comp.vec2))
        comp.vec2.x = 5.4
        print(string.format("%s",comp.vec2))

    end
end

and prints: (1,0) then (5.4,2.0)

For context the userdata is initialised as follows:

        MyComponent {
            ...
            vec2: Vec2::new(1.0,2.0),
        }

I know that in the second script newindex is called, whereas in the first one neither index nor __newindex are called.

Is this something to do with a Lua mechanic I am unaware of or a bug in rlua ?

makspll commented 2 years ago

And i think what makes me think this is a bug is that if I change the signature of the metamethod to force idx to be a String instead of a Value calling comp.vec[0] = 5.4 results in a runtime error.

jugglerchris commented 2 years ago

Hi, I agree this doesn't sound right. The only Lua behaviour that comes to mind is that if you have an ordinary table, then the __newindex or __index metamethods aren't called if the table already contains that key - but if it's a userdata that shouldn't be an issue.

Are you able to reproduce this in an ideally smaller rlua-only example (or at least something I can run and try out)?

makspll commented 2 years ago

Apologies for the delay, left for holiday very soon after posting this issue,

I've just gotten back and attempted to reproduce with a minimal example. I was unable to reproduce so this makes me think that this may very well be UB from my side (playing with bevy and pointers), I will close this for now but if i fix things on my side and still bump into this I will re-open and link to the full example.

Sorry!

For context this is what I tried and it works:

use rlua::{Value,UserData,MetaMethod,Function,prelude::*};

#[derive(Debug)]
pub struct MyUserData {
    v : [u32;3]
}

impl UserData for MyUserData {
    fn add_methods<'lua, T: LuaUserDataMethods<'lua, Self>>(methods: &mut T) {
        methods.add_meta_method(MetaMethod::ToString, |_,s,()| {
            Ok(format!("{:?}",s))
        });

        methods.add_meta_method_mut(MetaMethod::NewIndex, |_,s,(idx,val) : (Value,u32)|{
            let idx = match idx {
                Value::Integer(i) => i as usize,
                Value::String(s) => match s.to_str().unwrap() {
                    "x" => 0,
                    "y" => 1,
                    "z" => 2,
                    _ => panic!()
                },
                _ => panic!(),
            };

            Ok(s.v[idx] = val)
        })
    }
}

fn main() {
    let lua = Lua::new();
    lua.context(|lua_ctx| {
        lua_ctx
            .load("
            function on_update(my_user_data)
                print(my_user_data)
                my_user_data[0] = 69
                my_user_data.y = 42
                print(my_user_data)
            end
            ")
            .exec()
    }).unwrap();

    lua.context(|lua_ctx| {
        let g = lua_ctx.globals();
        let f : Function = g.get("on_update").unwrap();

        f.call::<MyUserData,()>(MyUserData{
            v: [0,1,2],
        }).unwrap();
    })

}
jugglerchris commented 2 years ago

Thanks for letting us know, and hope you find the issue!