Skip to content
This repository was archived by the owner on Dec 29, 2024. It is now read-only.

Bug Report: Binding libraries broken on MacOS Sonoma ARM64 #193

Closed
Astrrra opened this issue Feb 10, 2024 · 26 comments
Closed

Bug Report: Binding libraries broken on MacOS Sonoma ARM64 #193

Astrrra opened this issue Feb 10, 2024 · 26 comments
Labels
bug Something isn't working closed-for-archiving

Comments

@Astrrra
Copy link

Astrrra commented Feb 10, 2024 •
edited
Loading

Describe the bug
Project showing the error: Libraries_broken.zip

Binding libraries seems to be broken right now. I've noticed this while trying to use the os library, but from further testing it turns out that all the libraries I've tried are broken. I get Library "base" does not exist. (or an equivalent with a different library) when trying to use lua.bind_libraries(). Unsurprisingly, trying to actually use the libraries in my lua scripts doesn't work either.

To Reproduce

  1. Create a new project
  2. Install LuaAPI from the AssetLib
  3. Create a node with the demo script from the README
  4. Add error checking to the bind_libraries() call
  5. Run your project and observe the logs
    OR
    Just download the ZIP attached above, extract, and run the project

Expected behavior
I expect the libraries to be available for use in my scripts and the bind_libraries() call to not return any errors

Screenshots
CleanShot 2024-02-11 at 03 20 38@2x

Enviromint (please complete the following information):

  • OS: MacOS Sonoma 14.2.1, running on an M1 device (ARM64), lua (and luajit, same error applies to that, including for ffi) installed via brew
  • Godot version: v4.2.1.stable.official [b09f793f5] (tested in the mono version as well, same behavior, as seen on the screenshot above)
  • Module version: v2.1-beta11

Additional context
I feel like this might have been caused by #178, but I'm not sure. I've also tried to compile LuaAPI from source locally (as GDExtension) and that didn't help.
@Astrrra Astrrra added the bug Something isn't working label Feb 10, 2024
@Astrrra
Copy link
Author

Astrrra commented Feb 10, 2024

Yep, done more tests, #178 definitely broke it, as it works on the commit before that and doesn't work after.

@Trey2k
Copy link
Member

Trey2k commented Feb 10, 2024

Odd, not sure how this was not seen sooner.

I'm hoping to do some maintenance on luaAPI tomorrow. Will look into this then as well. Thanks for the report!

@KevBruner
Copy link

Bumping into this as well!

@Trey2k
Copy link
Member

Trey2k commented Feb 23, 2024

Bumping into this as well!

Very sorry about this. Been hard to find time recently but I will try to work on this some point this weekend.

From what I can tell I think this is limited to the luaJIT builds. So if it's a blocker you might be able to develop with the vanilla lua builds until I can get a fix out.

@Trey2k
Copy link
Member

Trey2k commented Feb 23, 2024

From what I can tell I think this is limited to the luaJIT builds.

This might not be the case. If not though then it's a false error of some kind I would think. I need to dig into it more

@KevBruner
Copy link

KevBruner commented Feb 23, 2024 via email

@Trey2k
Copy link
Member

Trey2k commented Feb 24, 2024

Sorry to report that I am using the PUC Lua addon, not the JIT. I'm compiling locally to see if I can find anything...

I appreciate the info, if you find anything let me know. I have some time this evening so will be looking into it some myself.

@Trey2k
Copy link
Member

Trey2k commented Feb 24, 2024 •
edited
Loading

Well I did find one related issue. Linux extension builds are not luaJIT. This was kind of a red hearing for me for a bit. I thought i was replicating the issue not being able to load the jit library.

godot_luaAPI/.github/workflows/gdextension.yml

Line 428 in edf4afe

PATH=/opt/buildroot/bin:$PATH scons target='${{ matrix.opts.target }}' platform='${{ matrix.opts.platform }}' arch='${{ matrix.opts.arch }}' luaappi_luaver=jit -j2

luaappi_luaver -> luaapi_luaver

I do not see the issue at all at least on Linux builds for gdExtension or module. JIT vs PUC makes no difference.

@KevBruner Can you confirm if you are on MacOS as well?

@KevBruner
Copy link

KevBruner commented Feb 24, 2024 via email

@Trey2k
Copy link
Member

Trey2k commented Feb 24, 2024

I can confirm on on MacOS as Well. MacOS Sonoma Version 14.1.2 (23B2091). MacBookPro M3.

Thanks, sadly since this is seeming to be MacOS specific it will be challenging for me to debug.

If anyone can figure out any other details it would be appreciated. Currently I see no reason that this should be happening. Its a bit confusing honestly.

@KevBruner
Copy link

KevBruner commented Feb 24, 2024 via email

@Trey2k
Copy link
Member

Trey2k commented Feb 24, 2024

I’m happy look. I’m very familiar with Lua and it’s c-api but new to Godot. Maybe point me in a direction to get a start?

That would be awesome, lucky the issue seems to be limited to libraries which does not have much specific to godot.

FYI we have some info to help get started with a development enviorment for this project on our wiki.

The areas of note in the code base are probably the bindLibraries method its self. This is the method that gets called on the gdScript with bind_libraries.

godot_luaAPI/src/luaState.cpp

Lines 49 to 59 in edf4afe

Ref<LuaError> LuaState::bindLibraries(TypedArray<String> libs) {
for (int i = 0; i < libs.size(); i++) {
if (!loadLuaLibrary(L, libs[i])) {
return LuaError::newError(vformat("Library \"%s\" does not exist.", libs[i]), LuaError::ERR_RUNTIME);
}
if (libs[i] == "base") {
lua_register(L, "print", luaPrint);
}
}
return nullptr;
}

This uses the loadLuaLibrary which is a bit over complicated since #178. It is a generated method, this was to allow easy static inclusion of some 3rd party lua libraries.

godot_luaAPI/lua_libraries/codegen.py

Lines 1 to 95 in edf4afe

import os
def code_gen(luaJIT=False):
lua_libraries = [
"base",
"coroutine",
"debug",
"io",
"math",
"os",
"package",
"string",
"table",
"utf8"
]
luajit_libraries = [
"base",
"bit",
"debug",
"ffi",
"io",
"jit",
"math",
"os",
"package",
"string",
"string_buffer",
"table"
]
libraries = lua_libraries
lib_source_files = []
if luaJIT:
libraries = luajit_libraries
for library in os.listdir("./"):
if not os.path.isdir(library) or library == "__pycache__" or library == "bin":
continue
libraries.append(library)
for source_file in os.listdir("./%s" % library):
if source_file.endswith(".cpp") or source_file.endswith(".c"):
lib_source_files.append(os.path.join(library, source_file))
luaLibraries_gen_cpp = "#include \"lua_libraries.h\"\n#include <map>\n#include <string>\n\n"
if len(lib_source_files) > 0:
for source_file in lib_source_files:
luaLibraries_gen_cpp += "#include \"%s\"\n" % source_file
luaLibraries_gen_cpp += "\n"
luaLibraries_gen_cpp += "std::map<std::string, lua_CFunction> luaLibraries = {\n"
for library in libraries:
luaLibraries_gen_cpp += "\t{ \"%s\", luaopen_%s },\n" % (library, library)
luaLibraries_gen_cpp += "};\n"
if luaJIT:
luaLibraries_gen_cpp += """
bool loadLuaLibrary(lua_State *L, String libraryName) {
const char *lib_c_str = libraryName.ascii().get_data();
if (luaLibraries[lib_c_str] == nullptr) {
return false;
}
lua_pushcfunction(L, luaLibraries[lib_c_str]);
if (libraryName == "base") {
lua_pushstring(L, "");
} else {
lua_pushstring(L, lib_c_str);
}
lua_call(L, 1, 0);
return true;
}
"""
else:
luaLibraries_gen_cpp += """
bool loadLuaLibrary(lua_State *L, String libraryName) {
const char *lib_c_str = libraryName.ascii().get_data();
if (luaLibraries[lib_c_str] == nullptr) {
return false;
}
luaL_requiref(L, lib_c_str, luaLibraries[lib_c_str], 1);
lua_pop(L, 1);
return true;
}
"""
gen_file = open("lua_libraries.gen.cpp", "w")
gen_file.write(luaLibraries_gen_cpp)
gen_file.close()

The file it ends up generating looks like this

#include "lua_libraries.h"
#include <map>
#include <string>

std::map<std::string, lua_CFunction> luaLibraries = {
	{ "base", luaopen_base },
	{ "coroutine", luaopen_coroutine },
	{ "debug", luaopen_debug },
	{ "io", luaopen_io },
	{ "math", luaopen_math },
	{ "os", luaopen_os },
	{ "package", luaopen_package },
	{ "string", luaopen_string },
	{ "table", luaopen_table },
	{ "utf8", luaopen_utf8 },
};

bool loadLuaLibrary(lua_State *L, String libraryName) {
	const char *lib_c_str = libraryName.ascii().get_data();
	if (luaLibraries[lib_c_str] == nullptr) {
		return false;
	}

	luaL_requiref(L, lib_c_str, luaLibraries[lib_c_str], 1);
	lua_pop(L, 1);
	return true;
}

To be perfectly honest im not too sure where we would even start looking. Maybe stepping through it with a debugger would help.

If you have any other questions be sure to let me know, thanks again.

@KevBruner
Copy link

KevBruner commented Feb 25, 2024 via email

@Trey2k
Copy link
Member

Trey2k commented Feb 25, 2024

but all the library loading code looks fine to me, It's just not getting
the Array of libraries to load properly.

This is strange as if the issue did indeed start after #178 I believe it was still an Array argument then too.

Sadly on the Godot side there is no way to type hint Arrays without using packed variants. If that does fix the issue we could use a packed string array instead. But I feel something else must be going on.

Are you able to confirm the Variants in the array are definitely a String type and empty. If so then I believe it has go be a bug on the Godot side. But if not I suspect the the way we started decoding the string after #178 might be to blame here.

@Trey2k
Copy link
Member

Trey2k commented Feb 25, 2024

By the way, thanks a ton for what you have done already. I have no way to debug on mac as of current sadly so it is very appreciated.

@KevBruner
Copy link

KevBruner commented Feb 25, 2024 via email

@Trey2k
Copy link
Member

Trey2k commented Feb 25, 2024

Actually I have just noticed. We are using a TypedArray on the c++ side. Forgot that actually was implemented. Maybe just reverting back to an Array would resolve this.

@Trey2k Trey2k mentioned this issue Feb 25, 2024
Closed
@Trey2k
Copy link
Member

Trey2k commented Feb 25, 2024

@KevBruner if possible I have a PR with that change #194. Can you confirm if you see the issue still with it?

@KevBruner
Copy link

KevBruner commented Feb 25, 2024 via email

@KevBruner
Copy link

KevBruner commented Feb 25, 2024 via email

@Trey2k
Copy link
Member

Trey2k commented Feb 25, 2024 •
edited
Loading

This is very strange.... I wonder if using utf8 over ascii here makes a difference? This makes it seem as if strings aren't working at all on macOS which wouldn't make sense as clearly we are still able to load lua code into the state and execute it. The only difference I can think of is most or all other strings would use utf8.

@KevBruner
Copy link

KevBruner commented Feb 26, 2024 via email

@Trey2k
Copy link
Member

Trey2k commented Feb 26, 2024

I was able to make LoadLibraries work by passing a Godot String instead of a const char* and comparing that to a map of Godot Strings, etc. and just modifying "codegen.py" to avoid any of the const char* to String comparisons.

If this works I am okay with this change. To be honest its probably more proper anyways to avoid converting from a godot string if its not needed.

but something else is actually going on. Do Godot modules have their own build parameters? I wonder if the compiled code has some string compatibility setting that might be different from the rest of the plug-is and engine?

Modules and extensions can have custom build params. When building as a module we inherit the common build params from the engine though

This is for module builds

godot_luaAPI/SCsub

Lines 1 to 33 in edf4afe

#!/usr/bin/env python
Import("env")
Import('env_modules')
env_lua = env_modules.Clone()
if not env.msvc:
CXXFLAGS=['-std=c++17']
else:
CXXFLAGS=['/std:c++17']
env_lua.Append(CXXFLAGS=CXXFLAGS)
if env["luaapi_luaver"] == "jit" and env['platform']=='javascript':
print("WARNING: LuaJIT can not build for a web target. Falling back to lua 5.1")
env["luaapi_luaver"] = '5.1'
Export('env_lua')
SConscript('external/SCsub')
SConscript('lua_libraries/SCsub')
if env["luaapi_luaver"] == 'jit':
env_lua.Append(CPPDEFINES=['LAPI_LUAJIT'])
elif env["luaapi_luaver"] == '5.1':
env_lua.Append(CPPDEFINES=['LAPI_51'])
env_lua.Append(CPPPATH=[Dir('src').abspath])
env_lua.Append(CPPPATH=[Dir('external').abspath])
env_lua.add_source_files(env.modules_sources,'*.cpp')
env_lua.add_source_files(env.modules_sources,'src/*.cpp')
env_lua.add_source_files(env.modules_sources,'src/classes/*.cpp')

This is for gdExtension builds

godot_luaAPI/SConstruct

Lines 1 to 46 in edf4afe

#!/usr/bin/env python
import os
import sys
import fnmatch
from config import configure
Export('configure')
env = SConscript("external/SConscript")
env.Tool('compilation_db')
if os.name == 'posix':
if os.environ.get('CXX', None) != None and env['CXX'] != os.environ['CXX']:
print("Using system CXX: {}".format(os.environ['CXX']))
env['CXX'] = os.environ['CXX']
if os.environ.get('CC', None) != None and env['CC'] != os.environ['CC']:
print("Using system CC: {}".format(os.environ['CC']))
env['CC'] = os.environ['CC']
env.Append(CPPDEFINES = ['LAPI_GDEXTENSION'])
env.Append(CPPPATH = [Dir('src').abspath, Dir('external').abspath])
env['sources'] = []
Export('env')
SConscript("lua_libraries/SConscript")
sources = Glob('*.cpp')
sources.append(Glob('src/*.cpp'))
sources.append(Glob('src/classes/*.cpp'))
sources.append(env['sources'])
if env["luaapi_luaver"] == 'jit':
env.Append(CPPDEFINES=['LAPI_LUAJIT'])
elif env["luaapi_luaver"] == '5.1':
env.Append(CPPDEFINES=['LAPI_51'])
cdb = env.CompilationDatabase('compile_commands.json')
Alias('cdb', cdb)
library = env.SharedLibrary(
"project/addons/luaAPI/bin/libluaapi{}{}".format(env["suffix"], env["SHLIBSUFFIX"]),
source=sources,
)
env.Default(library)

Currently we do not do a whole lot of custom build params at all.

@KevBruner
Copy link

KevBruner commented Feb 26, 2024 via email

@Trey2k
Copy link
Member

Trey2k commented Feb 26, 2024

I've created #196 which I hope gets us going again. I'm no GitHub wizard so I hope I've done that correctly! I can continue to investigate the root cause of the problem as well. Thanks, Kevin

Thanks for all that you have done so far. To be honest this smells of a Godot bug to me. If you are interested in tracking down the root cause I would probably try to make a minimal reproduction gdExtension and report the issue upstream. Definitely is a weird one.

@Trey2k Trey2k mentioned this issue Mar 14, 2024
Closed
@jbromberg
Copy link

Unrelated but @Astrrra how did you get the error messages to print like that? On my end the .message property on LuaError contains a stack trace and other info

@Trey2k Trey2k closed this as completed Dec 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug Something isn't working closed-for-archiving
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Potential fix for #193 WeaselGames/godot_luaAPI
4 participants
@Trey2k @KevBruner @jbromberg @Astrrra