Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Godot Engine 4.0 module fails to compile #20

Closed
fire opened this issue May 2, 2023 · 15 comments
Closed

Godot Engine 4.0 module fails to compile #20

fire opened this issue May 2, 2023 · 15 comments
Labels
bug Something isn't working

Comments

@fire
Copy link

fire commented May 2, 2023

Can you confirm?

@ashtonmeuser
Copy link
Owner

@fire I'll take a look. Out of curiosity, are you able to use it as a Godot 3 module? What about as an addon with Godot 3 or 4?

@fire
Copy link
Author

fire commented May 2, 2023

I haven’t seriously worked in godot 3 for 3 years :/

@Trey2k
Copy link
Contributor

Trey2k commented May 2, 2023

I was taking a bit of a look at the issues with module builds. The define should probably be swapped to being defined in the case of extension builds. As with modules the define wont be present when register_types.h is used by the code gen.

Also these methods should be removed completely:

void register_wasm_types() {
ClassDB::register_class<godot::Wasm>();
ClassDB::register_class<godot::StreamPeerWasm>();
}
void unregister_wasm_types() { }

Last thing there are a lot of #ifdef GDNATIVE's in the code. Just seems a bit odd. Is it being used as a define for GDEXtension here?

After that is fixed there seems to be a lot of issues with the stream-peer class.

@ashtonmeuser
Copy link
Owner

ashtonmeuser commented May 2, 2023

Thanks for the input, all. Should be fixed.

The define should probably be swapped to being defined in the case of extension builds. As with modules the define wont be present when register_types.h is used by the code gen.

Done. Used GDEXTENSION C++ define in SCsub.

Also these methods should be removed completely.

Done.

Last thing there are a lot of #ifdef GDNATIVE's in the code. Just seems a bit odd. Is it being used as a define for GDEXtension here?

Kinda. The define check (#ifdef GDNATIVE) still exists but will never be asserted in the Godot 4 branch. This is used when Godot 3 modules, GDExtension, and Godot 4 modules all use the same API and GDNative is the only one that differs e.g. method registration. Could remove this vestige from the Godot 4 branch but keeping it in allows the two main branches to remain more similar and thus easier to resolve conflicts.

After that is fixed there seems to be a lot of issues with the stream-peer class.

Done. Few more odd defines to resolve the difference between StreamPeer and StreamPeerExtension.

@fire
Copy link
Author

fire commented May 2, 2023

I can check in in 12 hours, but if someone can confirm it works you can close the issue.

@Trey2k
Copy link
Contributor

Trey2k commented May 2, 2023

It now compiles, however it fails at the linking stage.

[ 91%] progress_finish(["progress_finish"], [])
[ 94%] Linking Static Library servers/libservers.linuxbsd.editor.x86_64.a ...
Ranlib Library servers/libservers.linuxbsd.editor.x86_64.a ...
[ 94%] Linking Static Library core/libcore.linuxbsd.editor.x86_64.a ...
Ranlib Library core/libcore.linuxbsd.editor.x86_64.a ...
[ 94%] scons: *** [bin/godot.linuxbsd.editor.x86_64] Implicit dependency `modules/wasm/wasmer/lib/libwasmer.linuxbsd.editor.x86_64.a' not found, needed by target `bin/godot.linuxbsd.editor.x86_64'.
scons: building terminated because of errors.
[Time elapsed: 00:04:33.480]

I think this is caused by:

wasmer_library = env.File('wasmer/lib/{}wasmer{}'.format(env['LIBPREFIX'], module_env.get('LIBWASMERSUFFIX', module_env['LIBSUFFIX'])))

as it should be looking for libwasmer.a not libwasmer.linuxbsd.editor.x86_64.a

@ashtonmeuser
Copy link
Owner

@Trey2k pushed a fix. Try changing line 6 in SCsub to if env['platform'] in ['linux', 'linuxbsd']:.

@Trey2k
Copy link
Contributor

Trey2k commented May 2, 2023

Try changing line 6 in SCsub to if env['platform'] in ['linux', 'linuxbsd']:

Yup! It builds with that change.

@ashtonmeuser
Copy link
Owner

Glad to hear! Gonna close this but feel free to reopen if this continues to be an issue.

@ashtonmeuser
Copy link
Owner

@fire Total tangent but is it expected that GDExtension takes a lot longer to compile than GDNative? Looking at the builds for this project, GDExtension seems to be an order of magnitude slower. Am I misusing something or perhaps not caching something properly?

@fire
Copy link
Author

fire commented May 2, 2023

I personally use modules because I prefer to ship a binary and a pck. So I'm biased towards modules. As far as I know GDExtension allows creating extensions of classes while gdnative is like a script on an existing class. Not sure that helps.

@ashtonmeuser
Copy link
Owner

@fire Do you also mind editing this comment to reflect fixed module support?

@fire
Copy link
Author

fire commented May 2, 2023

I will pending compiling the module.

@fire
Copy link
Author

fire commented May 3, 2023

@ashtonmeuser I was able to get the build to run locally, but had some problems fetching the wasmer libs. There's no way to fetch the was and bundling all cicd platforms libs is a bit ugly

https://github.com/V-Sekai/godot/tree/wasm

@ashtonmeuser
Copy link
Owner

@fire Awesome that's a good start! Loving all the feedback and bug reports! Does #22 sufficiently capture what you're saying?

@ashtonmeuser ashtonmeuser added the bug Something isn't working label May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants