search

WeaselGames / godot_luaAPI

Godot LuaAPI
https://luaapi.weaselgames.info
Other
381 stars 29 forks source link

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

Open Astrrra opened 7 months ago

Astrrra commented 7 months ago

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):

Additional context I feel like this might have been caused by https://github.com/WeaselGames/godot_luaAPI/pull/178, but I'm not sure. I've also tried to compile LuaAPI from source locally (as GDExtension) and that didn't help.

Astrrra commented 7 months ago

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

Trey2k commented 7 months ago

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 commented 7 months ago

Bumping into this as well!

Trey2k commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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...

On Fri, Feb 23, 2024 at 2:44 PM Trey Moller @.***> wrote:

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

— Reply to this email directly, view it on GitHub https://github.com/WeaselGames/godot_luaAPI/issues/193#issuecomment-1962097240, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG3PK6NHLD6GKUUU57FROB3YVELVRAVCNFSM6AAAAABDDBYF7OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRSGA4TOMRUGA . You are receiving this because you commented.Message ID: @.***>

-- Kevin Bruner / President & CEO / Bruner House, LLC e-mail: @.*** phone: 415-419-6620 / [image: Twitter] https://twitter.com/kevbru// [image: LinkedIn] https://www.linkedin.com/in/kevin-bruner-59a329/ web: www.brunerhouse.com

Trey2k commented 7 months ago

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 commented 7 months ago

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.

https://github.com/WeaselGames/godot_luaAPI/blob/edf4afe7665a03978e8a4a3a15b727a66c4dc03d/.github/workflows/gdextension.yml#L428

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 commented 7 months ago

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

On Fri, Feb 23, 2024 at 5:49 PM Trey Moller @.***> wrote:

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.

https://github.com/WeaselGames/godot_luaAPI/blob/edf4afe7665a03978e8a4a3a15b727a66c4dc03d/.github/workflows/gdextension.yml#L428

luaappi_luaver -> luaapi_luaver

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

@KevBruner https://github.com/KevBruner Can you confirm if you are on MacOS as well?

— Reply to this email directly, view it on GitHub https://github.com/WeaselGames/godot_luaAPI/issues/193#issuecomment-1962210242, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG3PK6MGB5NJB6CR6RFFFRDYVFBMJAVCNFSM6AAAAABDDBYF7OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRSGIYTAMRUGI . You are receiving this because you were mentioned.Message ID: @.***>

-- Kevin Bruner / President & CEO / Bruner House, LLC e-mail: @.*** phone: 415-419-6620 / [image: Twitter] https://twitter.com/kevbru// [image: LinkedIn] https://www.linkedin.com/in/kevin-bruner-59a329/ web: www.brunerhouse.com

Trey2k commented 7 months ago

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 commented 7 months ago

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?Sent from my iPhoneOn Feb 23, 2024, at 6:07 PM, Trey Moller @.***> wrote:

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.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

Trey2k commented 7 months ago

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. https://github.com/WeaselGames/godot_luaAPI/blob/edf4afe7665a03978e8a4a3a15b727a66c4dc03d/src/luaState.cpp#L49-L59

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. https://github.com/WeaselGames/godot_luaAPI/blob/edf4afe7665a03978e8a4a3a15b727a66c4dc03d/lua_libraries/codegen.py#L1-L95 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 commented 7 months ago

Hello,

I've had a little time to looking this, and here is what I'm seeing:

The library name passed to "bool loadLuaLibrary(lua_State *L, String libraryName)" is an empty string.

This is called from the lua binding "LuaAPI::bindLibraries" which is passed "TypedArray libs" which (on my machine) is an Array of empty strings.

And THAT is defined from "ClassDB::bind_method(D_METHOD("bind_libraries", "Array"), &LuaAPI::bindLibraries);" which is defining an "Array", but not specifically an "Array of Strings", so I suspect something is getting lost in translation there.

I did notice that this appears to be the only LuaPI bound method which uses an "Array" so it makes sense that this bug hasn't come up before. I'm not familiar enough with the insides of Godot to pursue further at the moment, but all the library loading code looks fine to me, It's just not getting the Array of libraries to load properly. (in my case). Sorry about the crappy formating!

-Kevin

On Fri, Feb 23, 2024 at 6:37 PM Trey Moller @.***> wrote:

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 https://luaapi.weaselgames.info/v2.1/contributing/development_environment/ .

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. https://github.com/WeaselGames/godot_luaAPI/blob/edf4afe7665a03978e8a4a3a15b727a66c4dc03d/src/luaState.cpp#L49-L59

This uses the loadLuaLibrary which is a bit over complicated since #178 https://github.com/WeaselGames/godot_luaAPI/pull/178. It is a generated method, this was to allow easy static inclusion of some 3rd party lua libraries. https://github.com/WeaselGames/godot_luaAPI/blob/edf4afe7665a03978e8a4a3a15b727a66c4dc03d/lua_libraries/codegen.py#L1-L95 The file it ends up generating looks like this

include "lua_libraries.h"

include

include

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.

— Reply to this email directly, view it on GitHub https://github.com/WeaselGames/godot_luaAPI/issues/193#issuecomment-1962223376, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG3PK6IV5OF5RWJC22VVI6LYVFG5BAVCNFSM6AAAAABDDBYF7OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRSGIZDGMZXGY . You are receiving this because you were mentioned.Message ID: @.***>

-- Kevin Bruner / President & CEO / Bruner House, LLC e-mail: @.*** phone: 415-419-6620 / [image: Twitter] https://twitter.com/kevbru// [image: LinkedIn] https://www.linkedin.com/in/kevin-bruner-59a329/ web: www.brunerhouse.com

Trey2k commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

Given that I suspect it might the sting to ascii conversion.  I’ll poke around with that. Sent from my iPhoneOn Feb 24, 2024, at 8:06 PM, Trey Moller @.***> wrote: 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.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

Trey2k commented 7 months ago

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 commented 7 months ago

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

KevBruner commented 7 months ago

That didn't fix it. Something appears to be up with strings. When I hack this into the top on "bindLibaries" std::cout << "Testing String conversion\n"; String libNameBase = "base"; const char lib_c_str = libNameBase.ascii().get_data(); std::cout << "\n"; std::cout << lib_c_str << "\n"; std::cout << "*\n";

I get this output: Testing String conversion *

*

So assigning literal "base" to a Godot String then attempting to pull it back to a char* is losing something....

Still poking around

On Sat, Feb 24, 2024 at 8:50 PM Trey Moller @.***> wrote:

@KevBruner https://github.com/KevBruner if possible I have a PR with that change #194 https://github.com/WeaselGames/godot_luaAPI/pull/194. Can you confirm if you see the issue still with it?

— Reply to this email directly, view it on GitHub https://github.com/WeaselGames/godot_luaAPI/issues/193#issuecomment-1962812323, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG3PK6NE4NNK5TUNC2WTLTDYVK7J5AVCNFSM6AAAAABDDBYF7OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRSHAYTEMZSGM . You are receiving this because you were mentioned.Message ID: @.***>

-- Kevin Bruner / President & CEO / Bruner House, LLC e-mail: @.*** phone: 415-419-6620 / [image: Twitter] https://twitter.com/kevbru// [image: LinkedIn] https://www.linkedin.com/in/kevin-bruner-59a329/ web: www.brunerhouse.com

KevBruner commented 7 months ago

This keeps getting weirder... I feel like I just missing something more obvious so I'll step away for a bit, but now I'm seeing this code:

std::cout << "Testing String conversion\n"; String libNameBase = "base";

std::cout << "Length is " << libNameBase.length() << "\n";

CharString charString = libNameBase.ascii(); std::cout << "CharString is " << charString.size() << "\n";

const char *lib_c_str = libNameBase.ascii().get_data(); for( int i = 0; i < charString.size(); i++ ) std::cout << (int)(lib_c_str[i]) << "\n";

std::cout << "\n"; std::cout << lib_c_str << "\n"; std::cout << "\n";

Produce this output:

Testing String conversion Length is 4 CharString is 5 0 0 0 0 0 *

*

Which makes it seem like the string is full of zeros.... ????

On Sat, Feb 24, 2024 at 9:11 PM Kevin Bruner @.***> wrote:

That didn't fix it. Something appears to be up with strings. When I hack this into the top on "bindLibaries" std::cout << "Testing String conversion\n"; String libNameBase = "base"; const char lib_c_str = libNameBase.ascii().get_data(); std::cout << "\n"; std::cout << lib_c_str << "\n"; std::cout << "*\n";

I get this output: Testing String conversion *

*

So assigning literal "base" to a Godot String then attempting to pull it back to a char* is losing something....

Still poking around

On Sat, Feb 24, 2024 at 8:50 PM Trey Moller @.***> wrote:

@KevBruner https://github.com/KevBruner if possible I have a PR with that change #194 https://github.com/WeaselGames/godot_luaAPI/pull/194. Can you confirm if you see the issue still with it?

— Reply to this email directly, view it on GitHub https://github.com/WeaselGames/godot_luaAPI/issues/193#issuecomment-1962812323, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG3PK6NE4NNK5TUNC2WTLTDYVK7J5AVCNFSM6AAAAABDDBYF7OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRSHAYTEMZSGM . You are receiving this because you were mentioned.Message ID: @.***>

-- Kevin Bruner / President & CEO / Bruner House, LLC e-mail: @.*** phone: 415-419-6620 / [image: Twitter] https://twitter.com/kevbru// [image: LinkedIn] https://www.linkedin.com/in/kevin-bruner-59a329/ web: www.brunerhouse.com

-- Kevin Bruner / President & CEO / Bruner House, LLC e-mail: @.*** phone: 415-419-6620 / [image: Twitter] https://twitter.com/kevbru// [image: LinkedIn] https://www.linkedin.com/in/kevin-bruner-59a329/ web: www.brunerhouse.com

Trey2k commented 7 months ago

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 commented 7 months ago

Hello,

After doing some more investigation there is definitely something weird going on converting from const char* arrays to String and back on MacOS. It can fetch individual characters from a utf8 buffer (String.utf8) but calls to "get_data" or "get_prt" on ascii and utf8 buffers don't seem to point to valid strings. I'm not sure how to hook the XCode debugger to the process, so I'm just printf debugging like a caveman, so it's slow going.

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. That makes things work, 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? I'm just new to Godot (and Xcode, frankly, most of my career has been under DevStudio). But it can be made to work!

-Kevin

On Sun, Feb 25, 2024 at 5:42 AM Trey Moller @.***> wrote:

This is very strange.... I wonder if using utf8 over ascii here makes a difference? Thus 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.

— Reply to this email directly, view it on GitHub https://github.com/WeaselGames/godot_luaAPI/issues/193#issuecomment-1962944949, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG3PK6PYIQNJVDPOLTNYWIDYVM5THAVCNFSM6AAAAABDDBYF7OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRSHE2DIOJUHE . You are receiving this because you were mentioned.Message ID: @.***>

-- Kevin Bruner / President & CEO / Bruner House, LLC e-mail: @.*** phone: 415-419-6620 / [image: Twitter] https://twitter.com/kevbru// [image: LinkedIn] https://www.linkedin.com/in/kevin-bruner-59a329/ web: www.brunerhouse.com

Trey2k commented 7 months ago

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 https://github.com/WeaselGames/godot_luaAPI/blob/edf4afe7665a03978e8a4a3a15b727a66c4dc03d/SCsub#L1-L33

This is for gdExtension builds https://github.com/WeaselGames/godot_luaAPI/blob/edf4afe7665a03978e8a4a3a15b727a66c4dc03d/SConstruct#L1-L46

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

KevBruner commented 7 months ago

I've created https://github.com/WeaselGames/godot_luaAPI/pull/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

On Mon, Feb 26, 2024 at 11:39 AM Trey Moller @.***> wrote:

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

https://github.com/WeaselGames/godot_luaAPI/blob/edf4afe7665a03978e8a4a3a15b727a66c4dc03d/SCsub#L1-L33

This is for gdExtension builds

https://github.com/WeaselGames/godot_luaAPI/blob/edf4afe7665a03978e8a4a3a15b727a66c4dc03d/SConstruct#L1-L46

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

— Reply to this email directly, view it on GitHub https://github.com/WeaselGames/godot_luaAPI/issues/193#issuecomment-1965111113, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG3PK6MKKNY4HU7N525FADLYVTQIVAVCNFSM6AAAAABDDBYF7OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRVGEYTCMJRGM . You are receiving this because you were mentioned.Message ID: @.***>

-- Kevin Bruner / President & CEO / Bruner House, LLC e-mail: @.*** phone: 415-419-6620 / [image: Twitter] https://twitter.com/kevbru// [image: LinkedIn] https://www.linkedin.com/in/kevin-bruner-59a329/ web: www.brunerhouse.com

Trey2k commented 7 months ago

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.

jbromberg commented 6 months ago

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