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

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

Open
lucklove opened this issue Dec 14, 2015 · 11 comments
Open

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

lucklove opened this issue Dec 14, 2015 · 11 comments

Comments

@lucklove
Copy link

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

lucklove commented Feb 4, 2016

@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
Copy link
Contributor

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

@lucklove
Copy link
Author

@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
Copy link

daedric commented Feb 10, 2016

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
Copy link
Author

@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
Copy link
Contributor

@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
Copy link

daedric commented Feb 11, 2016

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
Copy link
Contributor

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

@lucklove
Copy link
Author

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

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

3 participants