jeremyong/Selene

not work when more than one userdef data in one lua function

lucklove opened this issue · 11 comments

For example. this not work:

struct X
{
    int x;
};

struct Y
{
    int y;
};

bool test_param_order(sel::State &state) {
    state["X"].SetClass<X>("x", &X::x);
    state["Y"].SetClass<Y>("y", &Y::y);
    X x{1};
    Y y{2};
    state(R"(
        function exchange_value(x, y)
            local tmp = x:x()
            x.set_x(y:y())
            y.set_y(tmp)
        end
    )");
    state["exchange_value"](x, y); 
    return x.x == 2 && y.y == 1;
}

Hello, I have been working on this for a week, and finaly create a repo nua. It's basically a copy of Selene. It seems work when I use full userdata instead of light userdata to represent userdef type. the test is here(see "set_multi_class_with_member")

I encountered the same problem with the use of metatables for lightuserdata. Tests to show that are prepared and the causing source locations are marked with FIXME comments.

@lucklove I read through your repo to understand the solution using full userdata. You use std::reference_wrapper<T> and T to distinguish the cases where Selene currently uses lightuserdata and userdata. There are then separate metatables for std::reference_wrapper<T> and T. So a C++ object published to Lua using std::reference_wrapper<T> is incompatible with a later use as T&, since that would expect a Lua object associated with the metatable for T. Am I right?

I hope we find a solution where C++ objects of type T can freely be used as T, T* or T& without the need to know whether C++ or Lua controls their lifetime.

@AnonymousProgrammerLandau You are right, I use std::reference_wrapper and T to distinguish reference and value(just like the use of std::bind). It's hard to guess whether user want to use reference(T&), const reference(const T&) or just a value(T) when a lvalue reference of type T was given, so I force user to choice one of them by reference wrapper. As a result, there are separate metatables, however, upsetly, I don't understand that a C++ object published to Lua using std::reference_wrapper<T> is incompatible with a later use as T&, since that would expect a Lua object associated with the metatable for T, could you show me lines of code? or would you explain that further?

@lucklove Suppose you have registered some C++ functions:

  • void useVal(T)
  • void useRef(T&)
  • T make()
  • T& global()

As Selene uses the same metatable for both T and T&, one can write all combinations in Lua code like this:

  • useVal(make())
  • useVal(global())
  • useRef(make())
  • useRef(global())

If I understood your approach, you would register:

  • void useVal(T)
  • useRef(reference_wrapper<T>)
  • T make()
  • reference_wrapper<T> global()

Lua code then needs to distinguish two valid combinations:

  • useVal(make())
  • useRef(global())

from two incompatible ones:

  • useVal(global())
  • useRef(make())

Viewed from Lua this distinction seems artificial to me. In essence both return values from make and global refer to a T and useVal as well as useRef need a T.

@AnonymousProgrammerLandau Thanks for your patient.
If while user need a value of type T, we only check if T is at the top of lua stack, and while user need a reference of type T, we only check if reference_wrapper is at the top of lua stack, then we get into the problem you said. However, if user need a value of type T, we can check if one of T and reference_wrapper is at the top of lua stack, this is transparent for user, so there is no need for user to distinct such a artificial thing.
I post a test for the case you give, it's here.
Note that useRef(make()) is illegal because I think it can guard correctness(for example, user write T& global() as T global() by mistake, without this, it may be hard to debug). Although, you can still make useRef(make()) work if you think it's useful.

Sorry to drop in like this, but is it the same bug that trigger that:

Given a lua function:

function test(cpp_obj)
    cpp_obj:print()
    local cpp_obj2 = cpp_obj:obj2()
    cpp_obj:print()
    cpp_obj2:print()
end

A C++ code:

        struct Obj2
        { void print() { GLOG(error) << "obj2"; } };
        struct Obj
        {
            Obj2& get_obj2()
            {return obj2; }
            void print()
            { GLOG(error) << "obj1"; }
            Obj2 obj2;
        };

        st["Obj"].SetClass<Obj>("obj2", &Obj::get_obj2, "print", &Obj::print);
        st["Obj2"].SetClass<Obj2>("print", &Obj2::print);
        st.Load("test.lua");
        Obj o;
        st["test"](o);

I get the output:

obj1
obj2
obj2

Instead of:

obj1
obj1
obj2

Any advices on how to workaround the problem ?

Cheers,

@daedric Hello, I test your code in both selene and nua, the result is that it print out

obj1
obj2
obj2

in selene, and print out

obj1
obj1
obj2

in nua.
So I think you encountered the same problem as us, and I guess the reason is also the same(the lua light userdata). So I think this issue is also useful for you.

@deadric Possible workaround: If you do not need reference semantics, you might get around the bug by copying. Change Obj2& Obj::get_obj2() to Obj2 Obj::get_obj2(), so that Selene copies the returned object into a full userdata.
You also need to work around C++ code like Obj o; st["test"](o);, because Selene uses the light userdata path for the parameter o. One thing that comes to mind: Create instances of Obj in Lua.

I see, this is why you were talking about wrapping a reference.
I'll see how it goes. I'll also evaluate others wrapper.

@lucklove too bad you did not write your readme in english, nua could have been interesting for me to evaluate as well :)

To find code that can cause the bug, disable push for references and pointers in primitives.h.

@daedric The use of nua is basically the same with selene, to make you clear, I post a english version readme.
Note that I cut out some of the features of selene which I don't need(to give a minimal support for my project, PM-Game-Server), so I am not sure if it's also useful for you, but It can be an advice on how to workaround your problem.