Lua Scripting

[Jacob-Mango]

This is basic LUA scripting. Currently supported is only Audio playback. You can do everything the normal Sound class does. You can currently not add sounds to the SoundManager via the script.

Support for calling functions with arguments have been added. Use the ScriptingTest project. Pressing P plays the Cherno.ogg file. Pressing and holding 1 or 2 decrease and increases the sound.

View the AudioScript.h and AudioScript.cpp files to understand how the Sound class was made.

The script is located in "/scripts/test.lua".

[Jacob-Mango]

What are the conflicts? How would I be able to tell? So I can fix it.

[KevinW1998]

I would HIGHLY recommend using variadic template instead of template overloads for Scripting::Call.

[Jacob-Mango]

@KevinW1998 Ok. Will change that. The current Scripting::Call took me around ~1 hour to create so this shouldn't take long as well.

[Jacob-Mango]

Added Variadic Templates in 18 minutes. :)

[Jacob-Mango]

What is this Continous-Intergration error about?

[Jacob-Mango]

Also, what are the conflicts? So I can fix them.

[TheCherno]

Hey Jacob, thanks so much for putting this together. Since this is quite a large addition, I'll have to check it out and do some proper testing/code review, and this may result in some follow up revisions. I'll try and do this as soon as possible, and get back to you with any actions required on your part.

As for the CI failure, the reason it failed was because you have merge conflicts in your current revision. This is because master has changed since you last merged it into your branch. You'll need to resolve these conflicts before I can test/merge them. Check out something like this article if you're not sure how to do that. :)

[Gogsi]

Just to help you out, the conflict is in Sandbox.cpp In b21421a you change line 37 and 40, but these lines don't exist now. The easiest fix I can think of is downloading the current Sandbox.cpp and pushing it on your fork.

[Jacob-Mango]

Hey :)

After a fair amount of commits, I have successfully merged the code together. While also learning more about git and how annoying it can be at sometimes.

It should all work fine.

[Jacob-Mango]

All conflicts seemed to have been fixed. I am only going to change 2 files to use a different LUA wrapper maybe due to the current one may be hard to actually implement.

[KevinW1998]

Ahh, good ol' luabind. It is one of my favourite front-end lua binders. It definitely offers a much nicer syntax.

[Jacob-Mango]

@KevinW1998 yea. i was originally going to use it but didn't want to actually use boost. that was until i found a version that doesn't need boost.

[Jacob-Mango]

Really only thing that is needed to be added is the ability to add and load materials, layers, text, textures. Basically anything to do with graphics.

Since that isn't as important, I am wondering if this can start under the testing. There is currently no pull errors.

Tell me what might need to be fixed or changed and I will do it.

@TheCherno

E: Accidentally clicked closed, well it is reopened.

[KevinW1998]

Really only thing that is needed to be added is the ability to add and load materials, layers, text, textures.

And that's something we should have a wrapper generator for. (It's a shame that C++ doesn't support Reflection)

[Jacob-Mango]

@KevinW1998 I can try to make one (again). Just found this to be easier for now.

[KevinW1998]

Shouldn't we wait until cligen is finished and then extending it with luabind wrapper generation?

[Jacob-Mango]

@KevinW1998 that might be a great idea. ok. i will just implement the graphics then I will create a new pull request (or you if you wish) that adds a luabind wrapper gen.

[TheCherno]

I'm extremely busy (as always), so I'm just going to drop my thoughts here and we can discuss them. I haven't had the chance to look into this too much, so take everything I say with a grain of salt.

• Firstly, Lua should be written as "Lua" rather than "LUA". I have no idea where the latter comes from, but for some reason many people decide to write it incorrectly
• I would like to have Lua's source code in the actual Sparky solution, rather than just as a binary
• I want to be able to use the latest version of Lua
• The previous point means that we can't use LuaJIT. I'm okay with that, and in fact I'm not exactly a huge fan of LuaJIT anyway. It doesn't work properly on many platforms/archs (yes I know it claims otherwise), and it adds extra complexity which isn't worth the benefits in my opinion. It also means that we have to use an old version of Lua. So overall, it is not worth it
• As far as wrappers go, I'm okay with one wrapper/binding helper library, however let's not go overboard and use too many
• The biggest issue with the current solution we have is the binding. It's incredibly difficult to maintain and requires a separate method for each function, which is unacceptable. Furthermore, the Lua binding should be stored in the actual C++ class file of the class that is being bound, rather than in an additional file (eg. instead of AudioScript.h/.cpp, it should be in Audio.h/.cpp).

I briefly took some time to come up with a better solution for binding, using templates. Here's a rough version of what I came up with:

    template<typename R>
    struct Marshal
    {
        static int Dispatch(lua_State* L, const R& value) { SP_ASSERT(false); return 0; }
    };

    template<>
    struct Marshal<float>
    {
        static int Dispatch(lua_State* L, const float& value)
        {
            lua_pushnumber(L, value);
            return 1;
        }
    };

    template<typename R>
    struct Unmarshal
    {
        static R&& Dispatch(lua_State* L, int index) { SP_ASSERT(false); return 0; }
    };

    template<>
    struct Unmarshal<float>
    {
        static float&& Dispatch(lua_State* L, int index)
        {
            return lua_tonumber(L, index);
        }
    };

    template<>
    struct Unmarshal<String>
    {
        static String&& Dispatch(lua_State* L, int index)
        {
            return lua_tostring(L, index);
        }
    };

    template<typename T>
    struct FunctionWrapper
    {

    };

    template<typename T, typename R, typename... Args>
    struct FunctionWrapper<R(T::*)(Args...)>
    {
        enum { ArgCount = sizeof...(Args) };

        typedef R(T::* FunctionType)(Args...);

        template<FunctionType method, size_t... index>
        static lua_CFunction WrapMethod()
        {
            return [](lua_State *L)
            {
                T* obj = luaW_check<T>(L, 1);
                Marshal<std::remove_const<R>::type>::Dispatch(L, (obj->*method)(Unmarshal<Args>::Dispatch(L, index + 2)...));
                return 1;
            };
        }

        template<FunctionType method, size_t... index>
        static lua_CFunction WrapMethod(std::index_sequence<index...>)
        {
            return WrapMethod<method, index...>();
        }
    };

    template<typename T, typename R, typename... Args>
    struct FunctionWrapper<R(T::*)(Args...) const>
    {
        enum { ArgCount = sizeof...(Args) };

        typedef R(T::* FunctionType)(Args...) const;
        typedef R(T::* FunctionTypeNonConst)(Args...);
        template <FunctionType method, size_t... index> static lua_CFunction WrapMethod()
        {
            return FunctionWrapper<FunctionTypeNonConst>::WrapMethod<(FunctionTypeNonConst)method, index...>();
        }
        template <FunctionType method, size_t... index> static lua_CFunction WrapMethodInternal(std::index_sequence<index...>)
        {
            return FunctionWrapper<FunctionTypeNonConst>::WrapMethod<(FunctionTypeNonConst)method, index...>();
        }
    };

    template<typename T, typename... Args>
    struct FunctionWrapper<void(T::*)(Args...)>
    {
        enum { ArgCount = sizeof...(Args) };
        typedef void (T::* FunctionType)(Args...);

        template<FunctionType method, size_t... index>
        static lua_CFunction WrapMethod()
        {
            return [](lua_State *L)
            {
                T* obj = luaW_check<T>(L, 1);
                (obj->*method)(Unmarshal<Args>::Dispatch(L, index + 2)...);
                return 0;
            };
        }

        template<FunctionType method, size_t... index>
        static lua_CFunction WrapMethodInternal(std::index_sequence<index...>)
        {
            return WrapMethod<method, index...>();
        }
    };

    template<typename T, T method, std::size_t N, typename Indices = std::make_index_sequence<N>>
    lua_CFunction WrapMethod()
    {
        return FunctionWrapper<T>::WrapMethodInternal<method>(Indices());
    }

    template<typename T, T method>
    lua_CFunction WrapMethod()
    {
        return WrapMethod<T, method, FunctionWrapper<T>::ArgCount>();
    }

    #define SCRIPT_METHOD(S) WrapMethod<decltype(&S), &S>()

    Sound* L_Sound_New(lua_State *L)
    {
        String name = lua_tostring(L, 1);
        String filename = lua_tostring(L, 2);
        return spnew Sound(name, filename);
    }

    int L_Register_Audio_Classes(lua_State *L)
    {
        luaL_Reg L_Sound_Table[] =
        {
            { NULL, NULL }
        };

        luaL_Reg L_Sound_MetaTable[] =
        {
            { "Play", SCRIPT_METHOD(Sound::Play) },
            { "Loop", SCRIPT_METHOD(Sound::Loop) },
            { "Pause", SCRIPT_METHOD(Sound::Pause) },
            { "Stop", SCRIPT_METHOD(Sound::Stop) },
            { "Resume", SCRIPT_METHOD(Sound::Resume) },
            { "GetGain", SCRIPT_METHOD(Sound::GetGain) },
            { "SetGain", SCRIPT_METHOD(Sound::SetGain) },
            { NULL, NULL }
        };
        luaW_register<Sound>(L, "Sound", L_Sound_Table, L_Sound_MetaTable, L_Sound_New);
        return 1;
    }

Hopefully you get the idea of what a good solution would look like, however this obviously isn't complete yet (i.e. constructors aren't supported yet).

Anyway, leave your thoughts here; let's talk about this.

[KevinW1998]

• I want to be able to use the latest version of Lua
• The previous point means that we can't use LuaJIT. I'm okay with that, and in fact I'm not exactly a huge fan of LuaJIT anyway. It doesn't work properly on many platforms/archs (yes I know it claims otherwise), and it adds extra complexity which isn't worth the benefits in my opinion. It also means that we have to use an old version of Lua. So overall, it is not worth it

Well, I had never really issues with LuaJIT (But maybe because I only used it on Windows).

• The biggest issue with the current solution we have is the binding. It's incredibly difficult to maintain and requires a separate method for each function, which is unacceptable. Furthermore, the Lua binding should be stored in the actual C++ class file of the class that is being bound, rather than in an additional file (eg. instead of AudioScript.h/.cpp, it should be in Audio.h/.cpp).

I still suggest using a binding generator (like cligen, but for lua bindings).

[TheCherno]

I'm definitely all for a binding generator, but for that to work there still needs to be the base binding code, such as my example above.

[Jacob-Mango]

As far as wrappers go, I'm okay with one wrapper/binding helper library, however let's not go overboard and use too many

Well, that was the plan. The easiest binding to use thus far is LuaBind which is what the project is using. It is only one wrapper and no more wrappers are needed.

The biggest issue with the current solution we have is the binding. It's incredibly difficult to maintain and requires a separate method for each function, which is unacceptable.

Noticed you were using an outdated pull. I have changed wrappers to LuaBind. Currently using a version not using boost although there is a boost version which has (some little) additional features. it seems fairly easy to use and so far no problems.

Furthermore, the Lua binding should be stored in the actual C++ class file of the class that is being bound, rather than in an additional file (eg. instead of AudioScript.h/.cpp, it should be in Audio.h/.cpp).

We could do two things here. We can create a generator which reads every file and parses it into one giant .h/.cpp file or we can go through each file and create a module for each. Currently, as of latest commit, it is mostly in one file,non generated.

there still needs to be the base binding code, such as my example above.

LuaBind seems to be working well for that. It is readable and easy enough so if a generator was to be made, no additional binding code would be needed.

I will merge LuaBind into the project next commit.

Jacob

[Jacob-Mango]

Just wondering if you are thinking about letting us see the CI logs? Seeing it fail yet other pull requests not failing makes me wonder what is the problem.

[Gogsi]

You can actually see the error if you click on details and then either debug or release. In this case it's LINK : fatal error LNK1104: cannot open file 'luabind-deboostified.lib' [C:\projects\sparky\Sparky-core\Sparky-core.vcxproj]

[KevinW1998]

Yea, I think you have to set luabind as dependency so it is build first

[Jacob-Mango]

Git did something wierd... It deleted a file which wasnt there...

[Jacob-Mango]

Hello @TheCherno, I have been looking into implementing Lua scripting your way and so far, it has been well. I have decided to first implement in a different project (a new one) so I can easily figure if it may be bug with your code or mine easier and so I won't modify files I do not need to. This project can be found at;

https://github.com/Jacob-Mango/MangoLua (Couldn't think of a name)

The amount of work I will be doing will be decreased during these next few weeks due to exams. Sorry.

[Jacob-Mango]

Alright. Fixed the ScriptGen compile bug. Turns out you have to use LLVM 3.4.1 (I am using windows binaries).

[Jacob-Mango]

Problem. Function overloading not working.

[Jacob-Mango]

That was just for debugging. Forgot to remove that.

All that is left from that I see visible is to finish the generator and then it should all work.

Tell me if you see something missing.

[KevinW1998]

Well there are some recommendation I would give:

• script.h @ line 35 should call std::forward
  instead of:
  luabind::call_function<void>(m_State, func, args...);
  we can use perfect forwarding:
  luabind::call_function<void>(m_State, func, std::forward<Args>(args)...);
• script.h @ line 24: I personally recommend using std::vector< std::string > instead of std::vector< const char* >, because this will ensure that the string will be valid through the lifetime of the script object.
• script.cpp @ line 8: Why do you push an empty string?
• In addition I would recommend closing the lua state on destruction of the Script object for proper cleanup

[Jacob-Mango]

Thank you for the recomendations

• I will use forwarding. I dont know why I didn't considering I did use it last time.
• I had a problem with using a string vector. It was to do with the instance of the string being deleted as it was being pushed into the string. I changed to chars and the problem fixed itself. I will look into why strings didn't work.
• Was one of the things I was doing to try and fix the above problem. Forgot to remove that line.

[KevinW1998]

It was to do with the instance of the string being deleted as it was being pushed into the string.

This sounds very wrong. If you still have issues with it, then I'll try it.

[Jacob-Mango]

Sorry but I feel that I can not improve this. I will close this pull request now. Feel free to use what I have done.