ashtonmeuser / godot-wasm

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

Parameterized Functions Always Error #63

Closed corysabol closed 8 months ago

corysabol commented 8 months ago

I am working on a Rust based WASM project. I'm running some simple tests to familiarize myself with you awesome plugin. However, whenever I try to invoke functions which take parameters an error is thrown by the plugin.

Here is my sample rust code

#[no_mangle]
pub extern "C" fn add(left: i32, right: i32) -> i32 {
    left + right
}

#[no_mangle]
pub extern "C" fn foo() -> u32 {
    1
}

#[no_mangle]
pub extern "C" fn bar() -> u32 {
    42
}

This is built using cargo build --release --target wasm32-wasi.

When importing the WASM into godot and executing the function add like so

print_debug(wasm.function("add", [1, 2]))

The error in question is then thrown from this line here - https://github.com/ashtonmeuser/godot-wasm/blob/23cff0715f2e2a9fd3e00fbc7c5c9acb95825828/src/godot-wasm.cpp#L424

For clarity sake here is the GDScript I am using to test things out.

extends Node

# Called when the node enters the scene tree for the first time.
func _ready():
    var wasm: Wasm = Wasm.new()
    var file = FileAccess.open("res://wasi_test.wasm", FileAccess.READ)
    var buffer = file.get_buffer(file.get_length())
    #var imports = { "functions": { "index.callback": [self, "callback"] } } # Set imports according to Wasm module
    #wasm.load(buffer, {})

    var err = wasm.compile(buffer)
    print_debug(err)
    err = wasm.instantiate({})
    print_debug(err) 
    print_debug(wasm.inspect())

    # invoke some functions
        # works returns 1
    print_debug(wasm.function("foo", []))
    # returns null for some reason
    print_debug(wasm.function("add", [1, 2]))
    # works returns 42
    print_debug(wasm.function("bar", []))

    file.close()

The functions which do not take any arguments and only return a value do work correctly. I'm not entirely sure what the issue could be. I suspect perhaps something to do with the way the arguments are being collected? When I invoke the add function from the cli using wasmer it works as expected.

 wasmer run --entrypoint add target/wasm32-wasi/release/wasi_test.wasm 1 2

Below is an inspection of the wasm.

Type: wasm
Size: 1.6 MB
Imports:
  Functions:
  Memories:
  Tables:
  Globals:
Exports:
  Functions:
    "add": [I32, I32] -> [I32]
    "foo": [] -> [I32]
    "bar": [] -> [I32]
  Memories:
    "memory": not shared (16 pages..)
  Tables:
  Globals:

I'm not sure if I'm just missing something in my Rust code that is needed or in my build process. Or if there is a bug in the godot-wasm addon. Any insight would be greatly appreciated.

Thanks!

ashtonmeuser commented 8 months ago

Quick fix: Try i64/f64 parameter types.

@corysabol Thank you for your compliments and for your very thorough bug report. I scratched my head for a while on this one but have a theory as to what's going wrong.

Internally, GDScript uses 64-bit integers and floats (docs). Calling your exported Wasm function add(i32, i32) -> i32 with GDScript integers will supply incorrect argument types by default.

I'll leave this issue up as a reminder to myself to come up with a better solution. A stopgap solution may be a clearer error message or even a documentation page re: Wasm function parameter data sizes. A more longterm fix would likely include recording (im/ex)ported Wasm function signatures and automatically converting arguments. I'd be more than happy to hear your feedback on this or even welcome a PR.

corysabol commented 8 months ago

Ah okay that makes sense then. I will try experimenting with using 64 bit data types and see how that fares. I would be happy to see if I can take stab at a PR if you can point me in the right direction / let me know a bit more about what you think needs to happen to improve things in this area.

I thinks clearer error messages is a great stop gap and some fleshed out documentation as well. As for a more permanent solution I wonder if having some additional type casting logic would make sense, or if that would create more headaches as opposed to just documenting that end users need to use a certain types to align with the API.

Cheers!

ashtonmeuser commented 8 months ago

@corysabol I went ahead and implemented the more-ideal fix with #64. Values being passed host to guest i.e. Godot to Wasm are cast into expected data types. Values passed host to guest are strictly 1) export function parameter types and 2) import function return types. Take a look at the PR for future reference if you're interested.

Once again, thanks for bringing this to attention! Let me know if I can help with using the project. I'm sure there are gaps in documentation and would love to hear if you find an example of such a gap.

ashtonmeuser commented 8 months ago

For posterity: closed for Godot 4.2 in https://github.com/ashtonmeuser/godot-wasm/commit/7ad22dd84ff05c99deac2b9620c8bfa60aaa2fc1 and Godot 3.x in https://github.com/ashtonmeuser/godot-wasm/commit/3cc750644760f186d50aab9d8e93dfd5021b34fe.