ashtonmeuser / godot-wasm

Interact with WebAssembly modules from Godot
https://github.com/ashtonmeuser/godot-wasm/wiki
MIT License
191 stars 11 forks source link

Auto download Wasmer libs for module use #22

Closed ashtonmeuser closed 1 year ago

ashtonmeuser commented 1 year ago

Should download Wasmer libs (if required) when Godot Wasm used as a module i.e. in SCsub. Spawned from this comment. This may be a good opportunity to abstract the Wasmer lib download functionality already present in SConstruct.

fire commented 1 year ago

You got the description of downloading Wasmer libraries correct.

Also note that windows supports both mingw and mvsc which corresponds to .a and .lib.

fire commented 1 year ago

Fails here:

https://github.com/V-Sekai/godot/actions/runs/4867601731/jobs/8680337146

image

Trying to get more screenshots

ashtonmeuser commented 1 year ago

Pushed a fix in https://github.com/ashtonmeuser/godot-wasm/commit/aa4b49edc2bceafe229b559e23ca71ceac9ae7eb (Godot 3.x) and https://github.com/ashtonmeuser/godot-wasm/commit/e6bb0ee8ee2f0583295d73d23c4afa74c076a3f8 (Godot 4.x). The issue was that the Wasmer library wasn't being downloaded for the linuxbsd platform specified by Godot modules. The library download and default version are also now kept in a common file (utils.py) which should simplify maintaining the two branches.

Also addresses using a .a library suffix for MinGW. Note that the MVSC suffix is actually .dll.lib which seems to be specific for Wasmer.

@fire Lemme know if this will suffice and I'll close this issue.

Edit: For posterity's sake, here's the workflow being discussed.

ashtonmeuser commented 1 year ago

This made me realize that the paths for Linux in the actual GDExtension file will probably be incorrect (should be .../linuxbsd/... instead of .../linux/...). Should compile but won't run. Away from my computer and can't fix right now. Just a heads up.

Ignore this. GDNative and GDExtension use linux platform and no library is created for Godot modules.

fire commented 1 year ago

Doing a test now.

Made a pr for https://github.com/ashtonmeuser/godot-wasm/pull/23

Waiting for https://github.com/V-Sekai/godot/actions/runs/4880026588/jobs/8707264367

fire commented 1 year ago

Some errors. I added a fix for the lib download but failing elsewhere.

image

fire commented 1 year ago

Modified:

#ifndef WASM_API_EXTERN
#if defined(_WIN32) && !defined(__MINGW32__)
#define WASM_API_EXTERN __declspec(dllimport)
#else
#define WASM_API_EXTERN
#endif
#endif

In wasm.h to have a check for mingw.

fire commented 1 year ago
> git diff
diff --git a/modules/wasm/SCsub b/modules/wasm/SCsub
index 0f8a3342402..918495c0a99 100644
--- a/modules/wasm/SCsub
+++ b/modules/wasm/SCsub
@@ -17,6 +17,7 @@ elif env["platform"] in ["osx", "macos"]:
     env.Append(LINKFLAGS=["-framework", "Security"])
 elif env["platform"] == "windows":
     module_env["LIBWASMERSUFFIX"] = ".a" if env.get("use_mingw") else ".dll.lib"
+    env.Append(LIBS=["userenv"])

 # Explicit static libraries
 wasmer_library = env.File(
diff --git a/modules/wasm/utils.py b/modules/wasm/utils.py
index 725da531c20..dcc2d089011 100644
--- a/modules/wasm/utils.py
+++ b/modules/wasm/utils.py
@@ -47,4 +47,7 @@ def download_wasmer(env, force=False, version=VERSION_DEFAULT):
     elif env["platform"] in ["linux", "linuxbsd"]:
         download_tarfile(BASE_URL.format(version, "linux-amd64"), "wasmer")
     elif env["platform"] == "windows":
-        download_tarfile(BASE_URL.format(version, "windows-amd64"), "wasmer")
+        if env.get("use_mingw"):
+            download_tarfile(BASE_URL.format(version, "windows-gnu64"), "wasmer")
+        else:
+            download_tarfile(BASE_URL.format(version, "windows-amd64"), "wasmer")

I was able to launch on windows with the wasm.h changes.

ashtonmeuser commented 1 year ago

Created #26 to capture failing MinGW build.

ashtonmeuser commented 1 year ago

@fire Huge thanks for all of the feedback you've provided! I think I've addressed everything you mentioned. All outstanding tasks have been rolled into their own issues seeing as we've gone on a bit of a tangent here. Let me know if anything's been missed.

fire commented 1 year ago

I'm a bit tired, so I'll let you know when I do an update.

I believe you can use a hot patch on the wasmer includes. Like literally apply a diff.

fire commented 1 year ago

We can close this one though.

ashtonmeuser commented 1 year ago

No rush at all! Yeah I'm sure there are plenty of workarounds but figured I'd open a PR in case others faced the same issue. Closing this.