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

Proposals #3

Closed
Trey2k opened this issue Apr 3, 2023 · 10 comments
Closed

Proposals #3

Trey2k opened this issue Apr 3, 2023 · 10 comments

Comments

@Trey2k
Copy link
Contributor

Trey2k commented Apr 3, 2023

So I was planning on creating a PR for gdExtension, but before I get started, I would like to propose a couple of things for consideration.

Integrate build system more tightly:

Currently it is expected that wasmer be downloaded and extracted manually before compilation. This is fine, but it is also possible to semi intergrade it into the scons build system so that its built from source via a git submodule. This wont be as simple as just having scons compile it, as its written in rust. But for an example you can see what I have done for luaJIT here which has similar issues. I think this approach could work here.

Potentially support engine module builds:

gdExtension is still in a very early stage. But one of the upsides is that code written for it is 98% the same as module code. So if support is to come for gdExtension I think there should be a compile time option as well to build as a engine module.

Would like to hear any input you have on regarding this.

@Trey2k Trey2k changed the title Proposal Proposals for gdExtension support Apr 3, 2023
@ashtonmeuser
Copy link
Owner

Integrate build system more tightly

Good suggestion. Although I'd push back on compiling from source and instead opt for downloading the binaries (and universalizing in the case of macOS) in the Scons workflow. Might be wise to download if either A) wasmer directory does not exist implying we're building for the first time, or B) a download-wasmer (or similar) option is set e.g. scons platform=... download-wasmer=yes. The Wasmer folks seem to be pretty good with builds and releases. Wasmer version could even be a Scons option. Do you see any advantage to building Wasmer from source?

Potentially support engine module builds

If this is easy to do, I'm all for it. That said, it's not going to be my priority as the addon approach will suffice for anyone looking to play around with Godot Wasm and there's lots more to do before a 1.x is reasonable.

@Trey2k
Copy link
Contributor Author

Trey2k commented Apr 4, 2023

Do you see any advantage to building Wasmer from source?

Looking into it more, there would not be much of an advantage other then convince. I think a scons option to download and unpack could achieve the same thing. I saw the download page did not include static libraries and had hoped if we compiled from source, static would be an option. But looking at the docs it seems it may only build as a shared library.

EDIT: Looking into it more, static libs actually are available and included with the downloads. So that being the case, my question is now would it not make sense to favor static over dynamic that way there is only one bin that needs to be shipped.

If this is easy to do, I'm all for it. That said, it's not going to be my priority as the addon approach will suffice for anyone looking to play around with Godot Wasm and there's lots more to do before a 1.x is reasonable.

I think at least for a potential godot 4 version this would be a good idea. as I have ran into lots of random bugs and stability issues with gdextension. And assuming the changes are made for gdextension, the extra support would take minimal effort.

@Trey2k
Copy link
Contributor Author

Trey2k commented Apr 5, 2023

Can a godot 4 branch be made to target PRs at?

@ashtonmeuser ashtonmeuser changed the title Proposals for gdExtension support Proposals Apr 5, 2023
@ashtonmeuser
Copy link
Owner

ashtonmeuser commented Apr 5, 2023

@Trey2k Thanks for opening this issue and kicking off discussion. Keep the fantastic suggestions coming! Update re: some of the points raised above:

  • Created a branch for Godot 4 PRs.
  • Using static libs as of d0ff431.
  • Wasmer is now downloaded in the SConstruct build process if directory is missing or download_wasmer=yes option provided.
  • Created and labelled a bunch of new issues for improvements mentioned above.

@fire
Copy link

fire commented May 3, 2023

I did minor patching for janked version of downloading the wasm libraries here https://github.com/V-Sekai/godot/tree/wasm

My design of always downloading the library is bad, but it's also simple to verify

Todo: Disable all platforms that are not building.

  • Android
  • Web

@ashtonmeuser
Copy link
Owner

ashtonmeuser commented May 3, 2023

@fire Can you expand on the janked Wasmer download? It seems to work for Linux, macOS, and Windows in CI build but if there's an issue, I'll address it.

Todo: Disable all platforms that are not building.

Done. Only supporting Linux, macOS, and Windows.

@fire
Copy link

fire commented May 3, 2023

I'll see if find some time tomorrow night to reply.

@ashtonmeuser
Copy link
Owner

No rush. I think I see the issue you're describing and tried to capture it in #22.

@fire
Copy link

fire commented May 7, 2023

godot-wasm engine module builds are completed. There's a msvc builds can't link, but that's a bug. Tracked here: #29

Wish to close?

@ashtonmeuser
Copy link
Owner

Yep gonna close. New proposals can come in as their own issues. Provided we get MSVC builds working, I'd be happy with the the existing functionality as a v1.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants