[WIP] Add C module support

[anzz1]

So as LUA natively supports extending it with C modules, with minor changes it was possible to add this functionality to Eluna too. I think this would be a great way to extend Eluna functionality with things like http requests, luasocket, discord bot apis etc. with cross-compability between cores and platforms so it would sit well with the goal and spirit of Eluna. Only minimal core changes are required (basically only check-fix CMake builds) and no existing functionality will be broken.

Pros:

• Can add new functionality without bloating the lean & clean code base of Eluna. People who don't need/want it can easily opt-out (everything is behind ELUNA_MODULES build flag in CMake)
• Modules can be created by different people in their own repos instead of pushing the changes to Eluna itself, thus making maintainability easier, codebase smaller, and let the module-makers worry about keeping their modules updated if need to be.
• C code can be used without directly patching the core itself.
• Cross-compatibility between cores, use modules in any core.
• Not suspectible to changes in cores and avoid merge conflicts
• Modularity

Core changes and testing:

• [x] TrinityCore https://github.com/ElunaLuaEngine/ElunaTrinityWotlk/pull/15
• [ ] AzerothCore
• [ ] MaNGOS
• [ ] CMaNGOS
  • [x] CMaNGOS-One [Classic] https://github.com/Niam5/mangos-classic/commit/91375e414b0588a85ed04f4a6641ebd947a74127 (Thanks @Niam5)
  • [ ] CMaNGOS-Two [TBC]
  • [ ] CMaNGOS-Three [WotLK]

Any thoughts? All feedback is welcome and appreciated and also testing for Windows/Linux builds and making the required changes if needed in cores would be helpful. Anyone who are familiar with the cores I'm not (MaNGOS, CMaNGOS) if could test this with those respective cores would be greatly appreciated.

[anzz1]

Also an example usage of this, the HTTPManager @r-o-b-o-t-o suggested but implemented as a module. https://github.com/anzz1/eluna-module-httpmanager

As it creates an extra thread and uses extra resources (even though miniscule), that's one reason why I support adding "non-core" functionality like this as load-on-demand modules instead of the core itself. This way the module will be loaded and the HTTPWorker thread created only if a lua script requires it first.

I envision that many people could want a Discord bot for example, and there are lot of ready-made C/C++ sources for such a thing found online, and it would be very simple to wrap something like that in a module. But also I dislike the idea of adding a discord bot to the core codebase. This way can have it both ways.

[anzz1]

One improvement for the current implementation that I can think of would be adding a magic function like _Unload which is currently called for all loaded modules on Eluna's class uninitialization, but for passing a C-style function pointer to the ELUNA_LOG_ERROR directive on load, so the modules could use that instead of implementing their own logging in cases where execution is outside the lua_state scope (i.e. threads like what can be seen in the HTTPManager example).

[Niam5]

I can look at the Cmangos core for you since I'm the unofficial maintainer for eluna integration.

[anzz1]

Thanks @Niam5 , that will be very helpful !

To point you into right direction, basically only small changes in the cmake files should be enough. You can use this commit as a reference https://github.com/anzz1/ElunaTrinityWotlk/commit/dd7df96d37492ba398993e98704b23aed78c006f

CMake build environment needs these things for it to work:

• Need to have a CMake option for building Eluna with module support which adds
  • Flags -DELUNA_MODULES and -DLUA_COMPAT_MODULE
  • Windows only also: -DLUA_BUILD_AS_DLL
• If ELUNA_MODULES is defined, then build dep/lualib as SHARED instead of the regular STATIC (this builds the liblua as liblua.dll/liblua.so to the server binary folder)
• Make sure that the CMakeLists.txt in the Eluna (LuaEngine) folder is parsed

Everything else should be handled by the CMakeLists.txt file in the Eluna folder which adds all directories under the modules directory (which have *.c, *.cpp, *.h, *.hpp files) to the solution as their own projects, buildable independently of the core. I even added a safeguard script to the CMakeLists which should make sure that the module sources will not get added to any other build targets than their own projects. This also removes them as build targets completely if ELUNA_MODULES is not defined.

That was actually needed with the TrinityCore CMake build env, as their file-parser is a bit overzealous in wanting to add every file it finds anywhere in the folders as build target sources for the core.

If the core's CMake build script does not auto-add everything (including the module sources to the core build target, which shouldn't happen), then the CMakeLists.txt does not need to be parsed if ELUNA_MODULES is not defined, but it doesn't hurt. Having it always parsed irregardless of whether ELUNA_MODULES is defined or not should work in every scenario.

[Niam5]

This should allow CMangos to build this PR. https://github.com/Niam5/mangos-classic/commit/91375e414b0588a85ed04f4a6641ebd947a74127

I will port to other cores at a later date.

[Foereaper]

This is definitely very interesting, and something that would be very beneficial for Eluna as a whole.

We're currently discussing internally what direction we want to take Eluna, due to the difficulty of keeping it maintained between the different emulators. We have a few ideas on how we can make it a lot easier to maintain compatibility and open up for a lot more pull requests and expansion, but we want to take some time to do this properly.

Please be aware that it might take us some time to review PR's until this has been sorted out, but we'll come back to this as soon as we're ready :)

[anzz1]

Sure thing , no rush. Always better to take your time and think things through instead of rushing :)

My thoughts were exactly on the problem on cross-compatibility and maintainability between cores when I had this idea. The core concept is to keep "optional" functionality which people might not need/want in modules instead of the core or Eluna itself. And as modules being completely core-agnostic and pretty much their own C/C++ applications, they wouldn't be affected of core changes and also vice-versa no additional changes should be needed to support new modules.

Maintained modules could be kept in a list as links to other repos, with their own issue trackers so Eluna's core maintainers wouldn't have to deal with it. Or maybe add them as subrepos under ElunaLuaEngine but giving maintainer rights to the module makers for only that repo. Or just don't worry about it at all and let people share them outside Eluna's realm.

On a sidenote: While the current docgen is amazing (and one of the reasons I like to work with Eluna, because of the great documentation), an addition of core-filter would be great where it could parse #ifdefs or add some flag in the hooks/funcs that which cores they support. That way as some cores outpace others in development and usage, new functions could be added first to some and others could implement them later. If that is the direction you are thinking of taking Eluna.

[Foereaper]

@anzz1 Do you happen to have Discord? If so, could you add me? Foe#5438

[Rochet2]

A possible alternative to the _Unload scheme in the core/eluna could be that the DLL will register a function to be run when the ELUNA_EVENT_ON_LUA_STATE_CLOSE event fires.

MODULEAPI luaopen_example_module(lua_State *L) {
    luaL_register(L, "example_module", functions);
    if (luaL_loadstring(L, "RegisterServerEvent(16, example_module._Unload)") != 0)
        return lua_error(L);
    lua_call(L, 0, 0);
    return 1;
}

[Foereaper]

I am continuing this work in the following branch:

https://github.com/ElunaLuaEngine/Eluna/tree/modules

[Foereaper]

Posted a new updated PR with the functionality, https://github.com/ElunaLuaEngine/Eluna/pull/460

I could probably have updated this PR but eh.. I decided to not add static/dynamic checks, as cores compiled as static will just not load dynamic libraries regardless, and we default to dynamic now anyway.

[Foereaper]

Added as of https://github.com/ElunaLuaEngine/Eluna/commit/9e32296c747197f86b5d4f5512838f75ff234d94