WeaselGames / godot_luaAPI

Godot LuaAPI
https://luaapi.weaselgames.info
Other
348 stars 27 forks source link

(wip) Add the ability to push a global module to lua, close #197 #198

Open Fran6nd opened 4 months ago

Fran6nd commented 4 months ago

Pull request of a possible implementation to fix #197.

It allows to push a global as follow: push_module('my_module_name', [[function_name_1, function_1], [function_name_2, function_2]]) then we can use it in lua as follow: my_module_name. function_name_1() my_module_name. function_name_2()

It is not registered as a library as since the latest lua release it is a bit tricky. This PR is WIP and intended for discussion. Yet no unit test and my understanding of Godot_luaAPI is still limited so things might be not ready to merge!

Commits will be squashed of course before any merge.

Thank you for Godot_luaAPI which is an awesome tool!

Trey2k commented 4 months ago

So my only concern with the current implementation is functionally this would behave the same as pushing an object or dictionary as a global with all the module methods.

Ideally we would implment it as an actual library that can be obtained via require if we are going to separate it. My only concern then us in unsure of a use case where you would not want the module already made available for the user buy they have access to require.

Fran6nd commented 4 months ago

I do agree that making it available from require would be the best. I am working on it but for now I am unsuccessful.

My main issue is that we have to use luaL_requiref(L, module_name, push_fodule_function, true);. However push_module_function needs to shaped as follow: static int LuaState:: push_module_function(lua_State* L) So we can't make it as a member of luaState. So we need to like register all instances of luaState, then for each instance we do need to register all libraries by name and then having a function called for any instance of luaState able to load the right library into the right instance of luaState?

So libraries could look as follow: static std::vector<std::tuple<LuaState * state, lua_State * L, std::vector<std::pair<String, Array>>>> gdLibraries; And so static int LuaState:: push_module_function(lua_State* L) could do the job once called?

I did a promising prototyping tonight but gonna try further tomorrow.

But of course I am wondering if there would be any way to perform it in a less tricky way? As this would add really a lot of complexity. My initial version is much simpler though.

Anyway thank you for your answer.

Fran6nd commented 4 months ago

It is pretty dirty but here is what I came up with. You can only require Godot made libraries as the default require is disabled. I did not understand how. So I pushed my own which have the benefit of being sandboxed. And I also think that: LuaAPI::~LuaAPI() { lua_close(lState); } could be moved into the destructor of luaState? What do you think?

EDIT: I do not prevent yet the dev to register multiple libraries with the same name. So only the first one will be loaded.

Trey2k commented 4 months ago

It is pretty dirty but here is what I came up with. You can only require Godot made libraries as the default require is disabled. I did not understand how.

It's provided via the package library in lua

Fran6nd commented 4 months ago

Thank you,

It is pretty dirty but here is what I came up with. You can only require Godot made libraries as the default require is disabled. I did not understand how.

It's provided via the package library in lua

Even by doing lua.bind_libraries(["base", "table", "string", "package"]) I still get this error using require:

ERROR 2: [LUA_ERRRUN - runtime error ]
[string "..."]:2: attempt to call a nil value (global 'require')
stack traceback:

So it remains mysterious to me!

Trey2k commented 4 months ago

Even by doing lua.bind_libraries(["base", "table", "string", "package"]) I still get this error using require:

ERROR 2: [LUA_ERRRUN - runtime error ]
[string "..."]:2: attempt to call a nil value (global 'require')
stack traceback:

So it remains mysterious to me!

I am curious as to what version of the addon you are using. I just tested on my end and this code runs without error.

extends Node

var lua: LuaAPI = LuaAPI.new()

func _ready():
    lua.bind_libraries(["base", "package"])

    var err: LuaError = lua.do_string("""
    require "base"
    print("test")
    """)
    if err is LuaError:
        print("ERROR %d: %s" % [err.type, err.message])
        return
Fran6nd commented 4 months ago

I ran your code with the latest gdextension available on the GitHub page, and still have the issue.

Same on a brand new git clone, then submodule init, update from the repo. Running on a Mac M1 (ARM).

⚠️Just tried on my windows laptop where it works fine. So something during compilation? Gonna do further investigations...

extends Node

var lua: LuaAPI = LuaAPI.new()

func _ready():
    var err: LuaError = lua.bind_libraries(["base", "package"," sqfdq", "dsfds"])
    if err is LuaError:
        print("ERROR %d: %s" % [err.type, err.message])
        return  
    err = lua.do_string("""
    --require "base"
    print("test")
    """)
    print(err)
    if err is LuaError:
        print("ERROR %d: %s" % [err.type, err.message])
        return

This code give me that:

Godot Engine v4.3.dev4.official.df78c0636 - https://godotengine.org
OpenGL API 4.1 Metal - 88 - Compatibility - Using Device: Apple - Apple M1

ERROR 2: Library "base" does not exist.
--- Debugging process stopped ---

So it is like the problem is about all modules.... The failing points:

bool loadLuaLibrary(lua_State *L, String libraryName)

from:

lua_libraries.gen.cpp
Trey2k commented 4 months ago

Same on a brand new git clone, then submodule init, update from the repo. Running on a Mac M1 (ARM).

this is probably because of #193, we have #194 we should fix it but it was causing some other issues.

Fran6nd commented 4 months ago

Hello sir! As I have been trying to use the #194 patch without success. Compilation ok but unable to load the project, I have done my own workaround Fix loadLuaLibrary for MAc OS which seems to compile fine and I have been able to test. It is really simple and kinda ugly... But seems to do the job? Furthermore, now require ' ' from package module can now load a gd made module. However to keep being able to sandbox it I am thinking about some kind of a luaAPI.alllow_only_gd_packages()?

SumianVoice commented 2 months ago

Not sure if I'm misunderstanding something but this seems to be the same use case as push_variant as used on a table of functions or a node?

extends Node
var lua: LuaAPI = LuaAPI.new()
func message():
    print("hello world")
func _ready():
    lua.push_variant("MyNode", self)

    var err: LuaError = lua.do_string("""
    MyNode.message()
    """)

As described in the OP, it seems this does what you want? You could obviously obscure some functions by packing them into an array like:

    lua.push_variant("MyModule", [function_1, function_2])

And this would work the same way. Why are you specifically wanting to use require? I thought that for files you want to only selectively run.

For the stated use case, push_variant works as intended for it.

Fran6nd commented 2 months ago

Hello Sir, sorry for the delay. My point is to allow pushing like "toolboxes" to lua to allow the user to use what they want. I also want to be able to push a module accessed like:

robot = require 'robot'
robot.head.tilt_right()
robot.motor.stop()

So I want to be able to have nested named tables of functions. Not sure if that's clear? Using 'require' is to do not push useless stuff to lua to avoid memory issues (In my case I am having to push a lot of tools). And it makes code more readable imo. Anyway, maybe my use case is too a corner case? Regarding the fix for Mac OS, my own (in the commit list) is working really well.

EDIT: Currently it works as follow:

GDSCRIPT
var lib = [["head", ["tilt_right", rb_tilt_r]], ["motor", ["stop", rb_stop]]]
lua.bind_gd_library("robot", lib)
RadiantUwU commented 1 month ago

This looks completely unnecessary to my point of view, as all you need to do is

api.push_variant("module_name",{
   "function_name_1":function_1,
   "function_name_2":function_2
})

and it is wayyy straight forward.