Tomasu / LuaGlue

C++11 Lua 5.2 Binding Library
zlib License
79 stars 22 forks source link

C++ callbacks with Lua function #25

Closed JMLX42 closed 10 years ago

JMLX42 commented 10 years ago

Hello,

I'm using the following Signal class:

namespace minko
{
    template <typename... A>
    class Signal :
        public std::enable_shared_from_this<Signal<A...>>
    {
    private:
        typedef std::function<void(A...)>                   Callback;
        typedef typename std::list<Callback>::iterator      CallbackIterator;
        typedef typename std::list<unsigned int>::iterator  SlotIterator;

        template <typename... B>
        class SignalSlot;

    public:
        typedef std::shared_ptr<Signal<A...>>           Ptr;
        typedef std::shared_ptr<SignalSlot<A...>>       Slot;

    private:
        std::list<Callback>                                     _callbacks;
        std::list<unsigned int>                                 _slotIds;
        unsigned int                                            _nextSlotId;

        bool                                                    _locked;
        std::list<std::pair<Callback, uint>>                    _toAdd;
        std::list<std::pair<CallbackIterator, SlotIterator>>    _toRemove;

    private:
        Signal() :
            std::enable_shared_from_this<Signal<A...>>(),
            _nextSlotId(0),
            _locked(false)
        {
        }

        void
        removeConnectionById(const unsigned int connectionId)
        {
            auto connectionIdIt = _slotIds.begin();
            auto callbackIt     = _callbacks.begin();
            auto i              = 0;

            while (*connectionIdIt != connectionId)
            {
                connectionIdIt++;
                callbackIt++;
            }

            removeConnectionByIterator(connectionIdIt, callbackIt);
        }

        void
        removeConnectionByIterator(SlotIterator      connectionIdIt,
                                   CallbackIterator  callbackIt)
        {
            if (_locked)
            {
                auto addIt = std::find_if(_toAdd.begin(), _toAdd.end(), [&](std::pair<Callback, unsigned int>& add)
                {
                    return add.second == *connectionIdIt;
                });

                if (addIt != _toAdd.end())
                    _toAdd.erase(addIt);
                else
                    _toRemove.push_back(std::pair<CallbackIterator, SlotIterator>(callbackIt, connectionIdIt));
            }
            else
            {
                _callbacks.erase(callbackIt);
                _slotIds.erase(connectionIdIt);
            }
        }

    public:
        static
        Ptr
        create()
        {
            return std::shared_ptr<Signal<A...>>(new Signal<A...>());
        }

        inline
        uint
        numCallbacks() const
        {
            return _callbacks.size();
        }

        Slot
        connect(Callback callback)
        {
            auto connection = SignalSlot<A...>::create(Signal<A...>::shared_from_this(), _nextSlotId++);

            if (_locked)
                _toAdd.push_back(std::pair<Callback, unsigned int>(callback, connection->_id));
            else
            {
                _callbacks.push_back(callback);
                _slotIds.push_back(connection->_id);
            }

            return connection;
        }

        void
        execute(A... arguments)
        {
            _locked = true;
            for (auto& callback : _callbacks)
                callback(arguments...);
            _locked = false;

            for (auto& callbackAndConnectionId : _toAdd)
            {
                _callbacks.push_back(callbackAndConnectionId.first);
                _slotIds.push_back(callbackAndConnectionId.second);
            }

            for (auto& callbackIt : _toRemove)
            {
                _callbacks.erase(callbackIt.first);
                _slotIds.erase(callbackIt.second);
            }

            _toAdd.clear();
            _toRemove.clear();
        }

    private:
        template <typename... T>
        class SignalSlot :
            public std::enable_shared_from_this<SignalSlot<T...>>
        {
            friend class Signal<T...>;

        public:
            typedef std::shared_ptr<SignalSlot<T...>>   Ptr;

        public:
            std::shared_ptr<Signal<T...>>
            signal()
            {
                return _signal;
            }

            ~SignalSlot()
            {
                if (_signal != nullptr)
                {
                    _signal->removeConnectionById(_id);
                    _signal = nullptr;
                }
            }

        private:
            std::shared_ptr<Signal<T...>>   _signal;
            const unsigned int              _id;

        private:
            inline static
            Ptr
            create(std::shared_ptr<Signal<T...>> signal, const unsigned int id)
            {
                return std::shared_ptr<SignalSlot<T...>>(new SignalSlot(signal, id));
            }

            SignalSlot(std::shared_ptr<Signal<T...>> signal, const unsigned int id) :
                _signal(signal),
                _id(id)
            {
            }
        };

    };
}

I defined the following macro to be able to bind everything related to a specific Signal according to its variadic list of arguments:

#define BIND_SIGNAL(...) \
    {                                                                                               \
        _state.Class<std::function<void (__VA_ARGS__)>>("std::function<void(" #__VA_ARGS__ ")>");   \
        auto& s = _state.Class<Signal<__VA_ARGS__>>("Signal<" #__VA_ARGS__ ">");                    \
        _state.Class<Signal<__VA_ARGS__>::Slot>("Signal<" #__VA_ARGS__ ">::Slot");                  \
        s.method("connect", &Signal<__VA_ARGS__>::connect);                                         \
    }

I will then bind a mouse signal this way:

    auto& input_mouse = _state.Class<input::Mouse>("Mouse")
        .property("x",                  &input::Mouse::x)
        .property("y",                  &input::Mouse::y)
        .property("leftButtonIsDown",   &input::Mouse::leftButtonIsDown)
        .property("rightButtonIsDown",  &input::Mouse::rightButtonIsDown);
    BIND_SIGNAL(input::Mouse::Ptr);
    input_mouse.property("leftButtonDown", &input::Mouse::leftButtonDown);

It compiles fine. I also applied the fix described in #24 . But it fails at runtime:

glue.Class("Mouse")
glue.Class("std::function")

Program received signal SIGSEGV, Segmentation fault.
0x00000000005a41cb in LuaGlueObject)> >::LuaGlueObject(LuaGlueObject)> > const&) (this=0x7fffffffd070, rhs=...) at lib/LuaGlue/include/LuaGlue/LuaGlueObject.h:169
169         LuaGlueObject(const LuaGlueObject &rhs) : LuaGlueObjectBase(false), p(rhs.p)
(gdb) bt
#0  0x00000000005a41cb in LuaGlueObject)> >::LuaGlueObject(LuaGlueObject)> > const&) (this=0x7fffffffd070, rhs=...) at lib/LuaGlue/include/LuaGlue/LuaGlueObject.h:169
#1  0x00000000005a0a3e in stack)> >::get(LuaGlueBase*, lua_State*, int) (
    g=0x90b500 , s=0x1db2c20, idx=-1) at lib/LuaGlue/include/LuaGlue/LuaGlueApplyTuple.h:49
#2  0x000000000059b065 in apply_glueobj_sptr_func<1>::applyTuple >, std::shared_ptr >::SignalSlot > >, std::function)>, std::function)>>(LuaGlueBase*, lua_State*, LuaGlueObject > > >, std::shared_ptr >::SignalSlot > > (minko::Signal >::*)(std::function)>), std::tuple)> > const&) (g=0x90b500 , 
    s=0x1db2c20, pObj=..., f=
    (std::shared_ptr >::SignalSlot > > (minko::Signal >::*)(minko::Signal > * const, std::function)>)) 0x446120  >::connect(std::function)>)>, t=...) at lib/LuaGlue/include/LuaGlue/LuaGlueApplyTuple.h:571
#3  0x0000000000593e76 in applyTuple >, std::shared_ptr >::SignalSlot > >, std::function)>, std::function)> >(LuaGlueBase*, lua_State*, LuaGlueObject > > >, std::shared_ptr >::SignalSlot > > (minko::Signal >::*)(std::function)>), std::tuple)> > const&) (g=0x90b500 , s=0x1db2c20, pObj=..., f=
    (std::shared_ptr >::SignalSlot > > (minko::Signal >::*)(minko::Signal > * const, std::function)>)) 0x446120  >::connect(std::function)>)>, t=...) at lib/LuaGlue/include/LuaGlue/LuaGlueApplyTuple.h:611
#4  0x0000000000584728 in LuaGlueMethod >::SignalSlot > >, minko::Signal >)> > >::invoke(lua_State*) (this=0xac5520, state=0x1db2c20)
    at lib/LuaGlue/include/LuaGlue/LuaGlueMethod.h:64
#5  0x0000000000580f5e in LuaGlueMethod >::SignalSlot > >, minko::Signal >)> > >::lua_call_func(lua_State*) (state=0x1db2c20)
    at lib/LuaGlue/include/LuaGlue/LuaGlueMethod.h:82
...

Are std::function supposed to work? It might be worth doing a separate example.

Thx anyway!

Tomasu commented 10 years ago

I have not looked into std::function's at all. I'll have to play around with your code. It's relatively simple, but if you could make an example that is simpler that still has this problem, I'd really appreciate it.

JMLX42 commented 10 years ago

Here you go:

function.cpp

#include <LuaGlue/LuaGlue.h>

template <typename... A>
class Signal
{
private:
    std::function<void(A...)> _callback;

public:
    Signal() { printf("Signal()\n"); }
    void setCallback(std::function<void(A...)> c) { _callback = c; }
    void execute(A... args) { _callback(args...); }
};

int main(int, char **)
{
    LuaGlue state;

    state.Class<Signal<int>>("Signal_int")
        .ctor("new")
        .method("setCallback",  &Signal<int>::setCallback)
        .method("execute",      &Signal<int>::execute)
        .end();

    state.open().glue();

    printf("running lua script!\n");
    if(!state.doFile("function.lua"))
    {
        printf("failed to dofile: function.lua\n");
        printf("err: %s\n", state.lastError().c_str());
    }

    return 0;
}

function.lua

local s = Signal_int.new()

s:setCallback(test_callback_int)

function test_callback_int(i)
    print("test_callback_int: " .. i)
end
Starting program: /home/promethe/Projects/LuaGlue/examples/./function 
running lua script!
Program received signal SIGSEGV, Segmentation fault.
0x0000000000409fdd in LuaGlueObject >::LuaGlueObject(LuaGlueObject > const&) (this=0x7fffffffd780, rhs=...)
    at include/LuaGlue/LuaGlueObject.h:169
169         LuaGlueObject(const LuaGlueObject &rhs) : LuaGlueObjectBase(false), p(rhs.p)
(gdb) bt
#0  0x0000000000409fdd in LuaGlueObject >::LuaGlueObject(LuaGlueObject > const&) (this=0x7fffffffd780, rhs=...)
    at include/LuaGlue/LuaGlueObject.h:169
#1  0x0000000000409caf in stack >::get(LuaGlueBase*, lua_State*, int) (g=0x7fffffffe0b0, s=0x647340, idx=-1)
    at include/LuaGlue/LuaGlueApplyTuple.h:49
#2  0x0000000000409895 in apply_glueobj_func<1>::applyTuple, void, std::function, std::function>(LuaGlueBase*, lua_State*, LuaGlueObject >, void (Signal::*)(std::function), std::tuple > const&) (g=0x7fffffffe0b0, s=0x647340, pObj=..., f=
    (void (Signal::*)(Signal * const, std::function)) 0x403ff8 ::setCallback(std::function)>, t=...)
    at include/LuaGlue/LuaGlueApplyTuple.h:504
#3  0x0000000000409434 in applyTuple, void, std::function, std::function >(LuaGlueBase*, lua_State*, LuaGlueObject >, void (Signal::*)(std::function), std::tuple > const&) (g=0x7fffffffe0b0, s=0x647340, pObj=..., f=
    (void (Signal::*)(Signal * const, std::function)) 0x403ff8 ::setCallback(std::function)>, t=...)
    at include/LuaGlue/LuaGlueApplyTuple.h:544
#4  0x0000000000408932 in LuaGlueMethod, std::function >::invoke(lua_State*) (this=0x647230, state=0x647340)
    at include/LuaGlue/LuaGlueMethod.h:133
#5  0x00000000004084fe in LuaGlueMethod, std::function >::lua_call_func(lua_State*) (state=0x647340) at include/LuaGlue/LuaGlueMethod.h:144
#6  0x000000000041dbdc in luaD_precall (L=0x647340, func=0x64e8f0, nresults=0) at ../lua-5.2.2/src/ldo.c:318
#7  0x0000000000431fde in luaV_execute (L=0x647340) at ../lua-5.2.2/src/lvm.c:709
#8  0x000000000041e02c in luaD_call (L=0x647340, func=0x647630, nResults=-1, allowyield=0) at ../lua-5.2.2/src/ldo.c:395
#9  0x0000000000420ea3 in f_call (L=0x647340, ud=0x7fffffffdfd0) at ../lua-5.2.2/src/lapi.c:923
#10 0x000000000041d1dd in luaD_rawrunprotected (L=0x647340, f=0x420e6c , ud=0x7fffffffdfd0) at ../lua-5.2.2/src/ldo.c:131
#11 0x000000000041e782 in luaD_pcall (L=0x647340, func=0x420e6c , u=0x7fffffffdfd0, old_top=32, ef=0) at ../lua-5.2.2/src/ldo.c:595
#12 0x0000000000420f6e in lua_pcallk (L=0x647340, nargs=0, nresults=-1, errfunc=0, ctx=0, k=0x0) at ../lua-5.2.2/src/lapi.c:949
#13 0x0000000000403543 in LuaGlue::doFile (this=0x7fffffffe0b0, path=...) at include/LuaGlue/LuaGlue.h:103
#14 0x0000000000402b42 in main () at examples/function.cpp:28
JMLX42 commented 10 years ago

Doesn't it indicate we should define specific overrides for std::function in the stack?

Tomasu commented 10 years ago

I'm not sure how that is supposed to work at all to be honest. you're passing a lua function to the C++ method as an argument but I'm pretty sure luaglue has no idea how to handle that. lua functions are passed around in lua as a LUA_TFUNCTION on the stack. there is no C/C++ representation of them directly and would have to be handled specially.

Did you add a stack<std::function<void (int)> > class? If not, its using the default static stack<class T> one, which will just attempt to deal with it as a pointer/light-user-data internally, and I'm assuming lua and C++ won't like us trying to convert a lua function to a pointer/light-user-data.

I think we probably do want to add a std::function override to tell LuaGlue that we're expecting a function as an argument. I can't really think of a better way to do it.

JMLX42 commented 10 years ago

I can override setCallback() to handle C function pointers too:

void
setCallback(void(*fn)(A...))
{
  _callback = static_cast<std::function<void(A...)>>(fn);
}

It compiles and runs fine as is. Then I tried to glue it (the code is pretty much the same as wiht std::function). But I still got a segfault:

Program received signal SIGSEGV, Segmentation fault.
0x000000000040a0f3 in LuaGlueObject::LuaGlueObject(LuaGlueObject const&) (this=0x7fffffffd7b0, 
    rhs=...) at include/LuaGlue/LuaGlueObject.h:169
169         LuaGlueObject(const LuaGlueObject &rhs) : LuaGlueObjectBase(false), p(rhs.p)
(gdb) bt
#0  0x000000000040a0f3 in LuaGlueObject::LuaGlueObject(LuaGlueObject const&) (
    this=0x7fffffffd7b0, rhs=...) at include/LuaGlue/LuaGlueObject.h:169
#1  0x0000000000409ec4 in stack::get (g=0x7fffffffe0b0, s=0x647330, idx=-1)
    at include/LuaGlue/LuaGlueApplyTuple.h:306
#2  0x0000000000409af3 in apply_glueobj_func<1>::applyTuple, void, void (*)(int), void (*)(int)> (
    g=0x7fffffffe0b0, s=0x647330, pObj=..., f=
    (void (Signal::*)(Signal * const, void (*)(int))) 0x4040b0 ::setCallback(void (*)(int))>, 
    t=...) at include/LuaGlue/LuaGlueApplyTuple.h:504
#3  0x00000000004096f6 in applyTuple, void, void (*)(int), void (*)(int)> (g=0x7fffffffe0b0, s=0x647330, 
    pObj=..., f=
    (void (Signal::*)(Signal * const, void (*)(int))) 0x4040b0 ::setCallback(void (*)(int))>, 
    t=...) at include/LuaGlue/LuaGlueApplyTuple.h:544
#4  0x0000000000408bf4 in LuaGlueMethod, void (*)(int)>::invoke (this=0x647230, state=0x647330)
    at include/LuaGlue/LuaGlueMethod.h:133
#5  0x00000000004087c0 in LuaGlueMethod, void (*)(int)>::lua_call_func (state=0x647330)
    at include/LuaGlue/LuaGlueMethod.h:144
#6  0x000000000041dd18 in luaD_precall (L=0x647330, func=0x64e8e0, nresults=0) at ../lua-5.2.2/src/ldo.c:318
#7  0x000000000043211a in luaV_execute (L=0x647330) at ../lua-5.2.2/src/lvm.c:709
#8  0x000000000041e168 in luaD_call (L=0x647330, func=0x647620, nResults=-1, allowyield=0)
    at ../lua-5.2.2/src/ldo.c:395
#9  0x0000000000420fdf in f_call (L=0x647330, ud=0x7fffffffdfd0) at ../lua-5.2.2/src/lapi.c:923
#10 0x000000000041d319 in luaD_rawrunprotected (L=0x647330, f=0x420fa8 , ud=0x7fffffffdfd0)
    at ../lua-5.2.2/src/ldo.c:131
#11 0x000000000041e8be in luaD_pcall (L=0x647330, func=0x420fa8 , u=0x7fffffffdfd0, old_top=32, ef=0)
    at ../lua-5.2.2/src/ldo.c:595
#12 0x00000000004210aa in lua_pcallk (L=0x647330, nargs=0, nresults=-1, errfunc=0, ctx=0, k=0x0)
    at ../lua-5.2.2/src/lapi.c:949
#13 0x00000000004035e1 in LuaGlue::doFile (this=0x7fffffffe0b0, path=...) at include/LuaGlue/LuaGlue.h:103
#14 0x0000000000402bc3 in main () at examples/function.cpp:42

But we still got that stack specialization issue...

As I understand it, Lua makes it easy to pass a function pointer from C to Lua, but not the other way around (which is understandable). And it's the root of all my problems here. Correct?

Tomasu commented 10 years ago

Correct. It'll take a bit of work, probably not a lot.

JMLX42 commented 10 years ago

OK good. I should be able to use the code above to override Signal::connect() to work with C function pointers. The question is now how I could properly wrap things to work with a stateless C function...

One thing I would need to make things work would be to be able to override some of my methods when I bind them using LuaGlue. For now I can bind:

Free functions are great, but they are stateless.

I would like to override/decorate my methods. I would need a third type: free functions that will require a first mandatory argument that will be handled by Lua glue by passing the C++ value corresponding to the Lua "self" that made the method call. The other arguments would be those of the actual method call (it would be like C++/Lua implicit handling of this/self, but explicit in this case).

In C++:

// this method will decorate Signal::connect() to be able to handle Lua function names
// as Signal callbacks using LuaGlue::invokeVoidMethod()
// I have no idea how I could have an actual Lua function "pointer" so I'm using a string
template<typename... A>
void connectDecorator(std::shared_ptr<Signal<A...>> self, cont std::string& callbackName)
{
  _slots.push_back(self->connect(std::bind(&LuaGlue::invokeVoidMethod, _state, A...)));
}

//...

_state.Class<Signal<int>>("Signal_int")
  .method("connect", &connectDecorator<int>);

In Lua:

signal:connect(test_callback)

This would allow us to "decorate" our original methods using free functions that have a C++-side reference to the Lua self. Does it make sense?

As you can see in my sample code I don't know how Lua could pass actual function pointers. Maybe we could define a special type that would be expected by those decorator functions and make LuaGlue do the work of providing the correct corresponding pointer to LuaGlue::invokeVoidMethod?

I think it would make a great mechanism to wrap/decorate C++ methods with Lua-specific behaviors. And I think it should solve my issue in this case: I would not bind Signal::connect() directly but instead decorate it to handle Lua's "function pointers" properly.

JMLX42 commented 10 years ago

I've made the following sample:

#include <LuaGlue/LuaGlue.h>
#include <lua.hpp>

void test_callback(int a)
{
    printf("test_callback %i\n", a);
}

int cb = 0;

int main(int, char **)
{
    LuaGlue state;

    state.open().glue();

    lua_pushcfunction(state.state(), [](lua_State* s) -> int
    {
        int n = lua_gettop(s);

        printf("setCallback: %i\n", n);

        if (lua_isfunction(s, 1))
        {
            cb = luaL_ref(s, LUA_REGISTRYINDEX);
            printf("function ref: %i\n", cb);
        }

        return 0;
    });
    lua_setglobal(state.state(), "setCallback");

    printf("running lua script!\n");
    if(!state.doFile("function.lua"))
    {
        printf("failed to dofile: function.lua\n");
        printf("err: %s\n", state.lastError().c_str());
    }

    printf("trying to call the callback...\n");
    lua_rawgeti(state.state(), LUA_REGISTRYINDEX, cb);
    lua_pushinteger(state.state(), 42);
    lua_pcall(state.state(), 1, 0, 0);

    return 0;
}
function test_callback_int(i)
    print("test_callback_int: " .. i)
end

setCallback(test_callback_int)

Thanks to lua_isfunction and luaL_ref I can successfully get a "Lua function pointer" passed from my Lua code. We could have a special LuaGlueFunctionPointer type to be able to hold the return of luaL_ref. Objects of this class could also be callable. This type would be handled by the stack.

This with the decoration system described above should make it easy to mix static variadic templates approches and dynamic Lua function calls.

What do you think?

JMLX42 commented 10 years ago

Hello,

I've made a "live" test that wil use the method described above with the original Signal class provided in the first pass.

void
initializeBindings()
{
    // ...

    auto& file_assetlibrary = _state->Class<file::AssetLibrary>("AssetLibrary");

    _state->Class<Signal<file::AssetLibrary::Ptr>>("Signal_AssetLibrary").glue(_state);
    _state->Class<Signal<file::AssetLibrary::Ptr>::Slot>("Signal_AssetLibrary_Slot").glue(_state);

    auto s = _state->state();
    lua_getglobal(s, "Signal_AssetLibrary");
    lua_pushcfunction(s, &LuaScriptManager::wrapSignalConnect);
    lua_setfield(s, 1, "connect");

    file_assetlibrary.property("complete", &file::AssetLibrary::complete);

    // ...
}

int
LuaScriptManager::wrapSignalConnect(lua_State* s)
{
    int n = lua_gettop(s);
    Signal<file::AssetLibrary::Ptr>::Ptr sig;
    Signal<file::AssetLibrary::Ptr>::Slot slot;

    if (lua_isuserdata(s, 1))
    {
        sig = **(LuaGlueObject<Signal<file::AssetLibrary::Ptr>::Ptr>*)lua_touserdata(s, 1);
    }
    else
        throw std::invalid_argument("self");

    if (lua_isfunction(s, 2))
    {
        auto cb = luaL_ref(s, LUA_REGISTRYINDEX);
        lua_pop(s, 1);

        slot = sig->connect([=](file::AssetLibrary::Ptr assets) -> void
        {
            lua_rawgeti(s, LUA_REGISTRYINDEX, cb);

            // push instance
            file::AssetLibrary::Ptr* ptr_ptr = new file::AssetLibrary::Ptr(assets);
            LuaGlueObject<file::AssetLibrary::Ptr>* udata = (LuaGlueObject<file::AssetLibrary::Ptr> *)lua_newuserdata(s, sizeof(LuaGlueObject<file::AssetLibrary::Ptr>));
            new (udata) LuaGlueObject<file::AssetLibrary::Ptr>(ptr_ptr, nullptr, true); // placement new to initialize object

            luaL_getmetatable(s, "AssetLibrary");
            lua_setmetatable(s, -2);
            // ! push instance

            lua_pcall(s, 1, 0, 0);
        });
    }
    else
        throw std::invalid_argument("callback");

    // push instance
    Signal<file::AssetLibrary::Ptr>::Slot* ptr_ptr = new Signal<file::AssetLibrary::Ptr>::Slot(slot);
    LuaGlueObject<Signal<file::AssetLibrary::Ptr>::Slot>* udata = (LuaGlueObject<Signal<file::AssetLibrary::Ptr>::Slot>*)lua_newuserdata(s, sizeof(LuaGlueObject<Signal<file::AssetLibrary::Ptr>::Slot>));
    new (udata) LuaGlueObject<Signal<file::AssetLibrary::Ptr>::Slot>(ptr_ptr, nullptr, true); // placement new to initialize object

    luaL_getmetatable(s, "Signal_AssetLibrary_Slot");
    lua_setmetatable(s, -2);
    // ! push instance

    return 1;
}

then in my Lua code:

self.slot = assets.complete:connect(function(assets)
  -- do some stuff with the loaded assets...
end)

As you can see I'm mixing the Lua C API and LuaGlue in LuaScriptManager::wrapSignalConnect() in order to get access to:

But it's very hard to generalize (here it works for Signalfile::AssetLibrary::Ptr only). It would be much better I think if it was properly wrapped in LuaGlue, especially with a custom function type.

What do you think?

Tomasu commented 10 years ago

The first thing that came to mind was using std::functions with a LuaGlueFunctionRef functor object, so when the std::function is "called", it then calls: LuaGlueFunctionRef::operator()(ref) and inside that object we actually do the magic stuff of calling lua functions. I did not think of storing the lua functions in the C++ side.. that is something to consider, though it could just be stored by name, or by the registry index if we can depend on that not changing at runtime. User code would be able to just store the LuaGlueFunctionRef (if thats what it ends up being called), and call it like a regular function, or pass it to lua, and allow luaglue to call it when needed.

In the stack:: code, and applyTuple code we can detect that we've been passed a lua function, and wrap it. In the end I would like it to be as automagical as is possible. So the user (ie: you) don't have to even think about it, and it "just works".

Would that handle your use case?

JMLX42 commented 10 years ago

I guess it would yes. I only need the Lua function ref to be wrapped properly - implying std::function like templating. Then if we can "decorate" original methods like I did here by fetching the "self" reference manually, things should be much easier to deal with.

Tomasu commented 10 years ago

I'm not sure what this decoration is for exactly?

JMLX42 commented 10 years ago

I need the decoration thing because I need access to this/self but for some very good reasons I cannot add new methods to the Signal class. I guess it will be the case for most people who are binding code they can't change.

With decoration I could write external functions that would not be stateless since they would have access to the C++ equivalent of "self" like I did it in my experiment code. A decorator function would have the following prototype:

ReturnType (*decorator)(SelfType, Args...)

The SelfType argument value would be handled automatically by LuaGlue just like I did in my wrapSignalConnect() method.

With this and a new type to describe & call Lua functions, we would get a lot more flexibility and that Signal thing would be actually doable. I actually wonder if using the LuaGlueClass::method(const std::string &name, _Ret (*fn)(_Args...)) wouldn't have this very behavior when called using the ":" operator in Lua? Maybe the "self" doesn't get passed along... I'll check that.

What do you think?

JMLX42 commented 10 years ago

It looks like this is doing the trick to handle "decorator" methods:

// in LuaGlueClass
template<typename _Ret, typename... _Args>
        LuaGlueClass<_Class> &method(const std::string &name, _Ret (*fn)(_Class*, _Args...))
        {
            //printf("decorator method(%s)\n", name.c_str());
            auto impl = new LuaGlueStaticMethod<_Ret, _Class, _Class*, _Args...>(this, name, std::forward<decltype(fn)>(fn));
            methods.addSymbol(name.c_str(), impl);

            return *this;
        }

        template<typename _Ret, typename... _Args>
        LuaGlueClass<_Class> &method(const std::string &name, _Ret (*fn)(std::shared_ptr<_Class>, _Args...))
        {
            //printf("decorator method(%s)\n", name.c_str());
            auto impl = new LuaGlueStaticMethod<_Ret, _Class, std::shared_ptr<_Class>, _Args...>(this, name, std::forward<decltype(fn)>(fn));
            methods.addSymbol(name.c_str(), impl);

            return *this;
        }

usage:

int
LuaScriptManager::stubSelfTest(Signal<file::AssetLibrary::Ptr>::Ptr s)
{
    return s->numCallbacks();
}

// ...

_state->Class<Signal<file::AssetLibrary::Ptr>>("Signal_AssetLibraryPtr")
        .method("stubSelfTest", &LuaScriptManager::stubSelfTest)
        .glue(_state);

Now if we could just have this Lua function pointer type it would be perfect I guess! What do you think?

JMLX42 commented 10 years ago

Here is how to add support for Lua function pointer with automated un-referencing:

class LuaFunctionPointer
{
public:
    typedef std::shared_ptr<LuaFunctionPointer> Ptr;

private:    
    int         _ref;
    lua_State*  _state;

private:
    LuaFunctionPointer(lua_State* s, int ref) :
        _state(s),
        _ref(ref)
    {}

public:
    static inline
    Ptr
    create(lua_State* s, int ref)
    {
        return std::shared_ptr<LuaFunctionPointer>(new LuaFunctionPointer(s, ref));
    }

    inline
    int
    ref()
    {
        return _ref;
    }

    ~LuaFunctionPointer()
    {
        luaL_unref(_state, LUA_REGISTRYINDEX, _ref);
    }
};

template<>
struct stack<LuaFunctionPointer::Ptr> {
    static LuaFunctionPointer::Ptr get(LuaGlueBase *, lua_State *s, int idx)
    {
        //lua_pushvalue(s, idx);
        auto ptr = LuaFunctionPointer::create(s, luaL_ref(s, LUA_REGISTRYINDEX));
        lua_rawgeti(s, LUA_REGISTRYINDEX, ptr->ref());

        return ptr;
    }

    static void put(LuaGlueBase *, lua_State *s, LuaFunctionPointer::Ptr v)
    {
        lua_rawgeti(s, LUA_REGISTRYINDEX, v->ref());
    }
};

Now the last question: how can we make this LuaFunctionPointer callable from C?

JMLX42 commented 10 years ago

OK got it! To make LuaFunctionPointer invokable:

class LuaFunctionPointer
{
public:
    typedef std::shared_ptr<LuaFunctionPointer> Ptr;

private:    
    int             _ref;
    LuaGlueBase*    _state;

private:
    LuaFunctionPointer(LuaGlueBase* s, int ref) :
        _state(s),
        _ref(ref)
    {}

public:
    static inline
    Ptr
    create(LuaGlueBase* s, int ref)
    {
        return std::shared_ptr<LuaFunctionPointer>(new LuaFunctionPointer(s, ref));
    }

    inline
    int
    ref()
    {
        return _ref;
    }

    template<typename _Ret, typename... _Args>
    _Ret invoke(_Args... args)
    {
        const unsigned int Arg_Count_ = sizeof...(_Args);

        lua_rawgeti(_state->state(), LUA_REGISTRYINDEX, _ref);
        applyTuple(_state, _state->state(), args...);
        lua_call(_state->state(), Arg_Count_, 1);

        return stack<_Ret>::get(_state, _state->state(), -1);
    }

    template<typename... _Args>
    void invokeVoid(_Args... args)
    {
        const unsigned int Arg_Count_ = sizeof...(_Args);

        lua_rawgeti(_state->state(), LUA_REGISTRYINDEX, _ref);
        applyTuple(_state, _state->state(), args...);
        lua_call(_state->state(), Arg_Count_, 0);
    }

    ~LuaFunctionPointer()
    {
        luaL_unref(_state->state(), LUA_REGISTRYINDEX, _ref);
    }
};

I think I've got all the pieces I needed... I'll try to to my Signal stuff with those and let you know. Of course I'll submit a proper pull request ASAP.

JMLX42 commented 10 years ago

Ok I confirm the method works fine! To fix the gc issue with the Signal disconnect, I implement a Singal<...>::Slot::disconnect method and it works fine.

I'll try to do the pull requests in the next few days...

JMLX42 commented 10 years ago

Fixed by pull request #28 and #29 Please merge and close to confirm.

Tomasu commented 10 years ago

I may implement this slightly differently. I don't think that'll properly handle passing lua functions around into C++ methods as transparently as possible. I think what'll try is using std::function, and add a operator()() to that LuaGlueFunctionPtr, then you can just call the std::function.

Tomasu commented 10 years ago

I just pushed up an alternate implementation that will pass std::functions to the C++ side. it should be fairly compatible with standard C++ code that way. Also, it should properly support passing around the same std::function back and forth between lua and C++. It may need some work, as if the function is passed back and forth too many times, the main call will get nested in a bunch of anonymous functions... not a problem for now, but something to keep note of.