jeremyong / Selene

Simple C++11 friendly header-only bindings to Lua
zlib License
813 stars 117 forks source link

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

Open lucklove opened 8 years ago

lucklove commented 8 years ago

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;
}
lucklove commented 8 years ago

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")

AnonymousProgrammerLandau commented 8 years ago

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.

lucklove commented 8 years ago

@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?

AnonymousProgrammerLandau commented 8 years ago

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

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

If I understood your approach, you would register:

Lua code then needs to distinguish two valid combinations:

from two incompatible ones:

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.

lucklove commented 8 years ago

@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.

daedric commented 8 years ago

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,

lucklove commented 8 years ago

@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.

AnonymousProgrammerLandau commented 8 years ago

@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.

daedric commented 8 years ago

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 :)

AnonymousProgrammerLandau commented 8 years ago

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

lucklove commented 8 years ago

@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.