From 1ec8f3b43d107d42e2117c6ad338cdfe06fa61e1 Mon Sep 17 00:00:00 2001 From: Ashton Meuser Date: Thu, 13 Jun 2024 08:49:56 -0700 Subject: [PATCH] Import function target IDs (#70) * Prefer library wasm.h file * Rename godot-wasm.h to wasm.h to reflect class * Prefer Godot library * Use import function target object ID Closes #69 * Tests for freed target * Fix Godot test comparable check * Use defines for object ID functions --- .../.godot/global_script_class_cache.cfg | 6 +++ examples/wasm-test/TestImports.gd | 37 +++++++++++++++++++ examples/wasm-test/utils/ImportTarget.gd | 11 ++++++ examples/wasm-test/utils/Utils.gd | 2 +- register_types.cpp | 2 +- src/defs.h | 27 ++++++++------ src/store.h | 2 +- src/wasi-shim.cpp | 2 +- src/wasi-shim.h | 2 +- src/wasm-memory.cpp | 2 +- src/{godot-wasm.cpp => wasm.cpp} | 12 +++--- src/{godot-wasm.h => wasm.h} | 2 +- 12 files changed, 84 insertions(+), 23 deletions(-) create mode 100644 examples/wasm-test/utils/ImportTarget.gd rename src/{godot-wasm.cpp => wasm.cpp} (97%) rename src/{godot-wasm.h => wasm.h} (98%) diff --git a/examples/wasm-test/.godot/global_script_class_cache.cfg b/examples/wasm-test/.godot/global_script_class_cache.cfg index edce3f8..1985767 100644 --- a/examples/wasm-test/.godot/global_script_class_cache.cfg +++ b/examples/wasm-test/.godot/global_script_class_cache.cfg @@ -5,6 +5,12 @@ list=Array[Dictionary]([{ "language": &"GDScript", "path": "res://utils/GodotWasmTestSuite.gd" }, { +"base": &"Object", +"class": &"ImportTarget", +"icon": "", +"language": &"GDScript", +"path": "res://utils/ImportTarget.gd" +}, { "base": &"Node", "class": &"TestRunner", "icon": "", diff --git a/examples/wasm-test/TestImports.gd b/examples/wasm-test/TestImports.gd index dd730e5..fa88f7c 100644 --- a/examples/wasm-test/TestImports.gd +++ b/examples/wasm-test/TestImports.gd @@ -43,6 +43,43 @@ func test_invalid_imports(): expect_eq(error, ERR_CANT_CREATE) expect_error("Invalid import method import.import_int") +func test_instantiation_freed_target(): + var target = ImportTarget.new() + var imports = { "functions": { + "import.import_int": target.dummy_import(), + "import.import_float": target.dummy_import(), + } } + var wasm = Wasm.new() + var buffer = read_file("import") + var error = wasm.compile(buffer) + expect_eq(error, OK) + target.free() # Free target before instantiation + error = wasm.instantiate(imports) + expect_eq(error, ERR_CANT_CREATE) + expect_error("Invalid import target import.import_(int|float)") + +func test_invocation_freed_target(): + var target = ImportTarget.new() + var imports = { "functions": { + "import.import_int": target.dummy_import(), + "import.import_float": target.dummy_import(), + } } + var wasm = load_wasm("import", imports) + target.free() # Free target before invocation + wasm.function("callback", []) + expect_error("Failed to retrieve import function target") + expect_error("Failed calling function callback") + +func test_import_method_missing(): + var imports = { "functions": { + "import.import_int": [self, "invalid_method_0"], + "import.import_float": [self, "invalid_method_1"], + } } + var wasm = load_wasm("import", imports) + wasm.function("callback", []) + expect_error(".*::invalid_method_0': Method not found.*") + expect_error(".*::invalid_method_1': Method not found.*") + func test_callback_function(): var imports = { "functions": { "import.import_int": dummy_import(), diff --git a/examples/wasm-test/utils/ImportTarget.gd b/examples/wasm-test/utils/ImportTarget.gd new file mode 100644 index 0000000..67f72a4 --- /dev/null +++ b/examples/wasm-test/utils/ImportTarget.gd @@ -0,0 +1,11 @@ +extends Object +class_name ImportTarget + +# Dummy import to supply to Wasm modules +static func _dummy_import(a = "", b = "", c = "", d = ""): + var message = "Dummy import %s %s %s %s" % [a, b, c, d] + print(message.strip_edges()) + return a + +func dummy_import() -> Array: + return [self, "_dummy_import"] diff --git a/examples/wasm-test/utils/Utils.gd b/examples/wasm-test/utils/Utils.gd index 85ed242..6cea536 100644 --- a/examples/wasm-test/utils/Utils.gd +++ b/examples/wasm-test/utils/Utils.gd @@ -2,7 +2,7 @@ extends Object class_name Utils static func comparable(o): - return o.hash() if (o is Dictionary or o is Object) else o + return o.hash() if ((o is Dictionary or o is Object) and o != null) else o static func make_regex(pattern: String) -> RegEx: var regex = RegEx.new() diff --git a/register_types.cpp b/register_types.cpp index 8bd0135..acd017c 100644 --- a/register_types.cpp +++ b/register_types.cpp @@ -1,5 +1,5 @@ #include "register_types.h" -#include "src/godot-wasm.h" +#include "src/wasm.h" #include "src/wasm-memory.h" using namespace godot; diff --git a/src/defs.h b/src/defs.h index 0a3ece6..c9ddbc6 100644 --- a/src/defs.h +++ b/src/defs.h @@ -7,29 +7,34 @@ Useful for minimizing changes to implementation files between targets e.g. GDExt */ #ifdef GODOT_MODULE // Godot includes when building module - #include "core/os/os.h" - #include "core/os/time.h" - #include "core/crypto/crypto.h" - #include "core/io/stream_peer.h" + #include + #include + #include + #include + #include #else // Godot addon includes - #include "godot_cpp/classes/ref_counted.hpp" - #include "godot_cpp/classes/os.hpp" - #include "godot_cpp/classes/time.hpp" - #include "godot_cpp/classes/crypto.hpp" - #include "godot_cpp/classes/stream_peer_extension.hpp" - #include "godot_cpp/variant/utility_functions.hpp" + #include + #include + #include + #include + #include + #include #endif #ifdef GODOT_MODULE - #define godot_error Error #define PRINT(message) print_line(String(message)) #define PRINT_ERROR(message) print_error("Godot Wasm: " + String(message)) + #define godot_error Error + #define INSTANCE_FROM_ID(id) ObjectDB::get_instance(id) + #define INSTANCE_VALIDATE(id) VariantUtilityFunctions::is_instance_valid(id) #define REGISTRATION_METHOD _bind_methods #define RANDOM_BYTES(n) Crypto::create()->generate_random_bytes(n) #else #define PRINT(message) UtilityFunctions::print(String(message)) #define PRINT_ERROR(message) _err_print_error(__FUNCTION__, __FILE__, __LINE__, "Godot Wasm: " + String(message)) #define godot_error Error + #define INSTANCE_FROM_ID(id) ObjectDB::get_instance(id) + #define INSTANCE_VALIDATE(id) UtilityFunctions::is_instance_valid(id) #define REGISTRATION_METHOD _bind_methods #define RANDOM_BYTES(n) [n]()->PackedByteArray{Ref c;c.instantiate();return c->generate_random_bytes(n);}() #endif diff --git a/src/store.h b/src/store.h index a0e232d..f6fcff5 100644 --- a/src/store.h +++ b/src/store.h @@ -6,7 +6,7 @@ Singleton Wasm C API store The same store is used between all compiled Wasm modules */ -#include "wasm.h" +#include #define STORE ::godot_wasm::Store::instance().store diff --git a/src/wasi-shim.cpp b/src/wasi-shim.cpp index 08fca22..4ee87ea 100644 --- a/src/wasi-shim.cpp +++ b/src/wasi-shim.cpp @@ -2,7 +2,7 @@ #include #include #include "wasi-shim.h" -#include "godot-wasm.h" +#include "wasm.h" #include "defer.h" #include "string-container.h" diff --git a/src/wasi-shim.h b/src/wasi-shim.h index 54a1952..20783cd 100644 --- a/src/wasi-shim.h +++ b/src/wasi-shim.h @@ -2,7 +2,7 @@ #define WASI_SHIM_H #include -#include "wasm.h" +#include #include "defs.h" namespace godot { diff --git a/src/wasm-memory.cpp b/src/wasm-memory.cpp index 65ea368..1737606 100644 --- a/src/wasm-memory.cpp +++ b/src/wasm-memory.cpp @@ -1,4 +1,4 @@ -#include "wasm.h" +#include #include "wasm-memory.h" #include "store.h" diff --git a/src/godot-wasm.cpp b/src/wasm.cpp similarity index 97% rename from src/godot-wasm.cpp rename to src/wasm.cpp index afa610b..70507fc 100644 --- a/src/godot-wasm.cpp +++ b/src/wasm.cpp @@ -1,6 +1,6 @@ #include #include -#include "godot-wasm.h" +#include "wasm.h" #include "wasi-shim.h" #include "defer.h" #include "store.h" @@ -13,7 +13,7 @@ namespace godot { }; struct ContextFuncImport: public ContextExtern { - Object* target; // The object from which to invoke callback method + ObjectID target; // The ID of the object on which to invoke callback method String method; // External name; doesn't necessarily match import name std::vector results; // Return types ContextFuncImport(uint16_t i, const wasm_functype_t* func_type): ContextExtern(i) { @@ -204,8 +204,9 @@ namespace godot { Array params = Array(); // TODO: Check if args and results match expected sizes for (uint16_t i = 0; i < args->size; i++) params.push_back(decode_variant(args->data[i])); - // TODO: Ensure target is valid and has method - Variant variant = context->target->callv(context->method, params); + Object* target = INSTANCE_FROM_ID(context->target); + FAIL_IF(target == nullptr, "Failed to retrieve import function target", trap("Failed to retrieve import function target\0")); + Variant variant = target->callv(context->method, params); godot_error error = extract_results(variant, context, results); if (error) FAIL("Extracting import function results failed", trap("Extracting import function results failed\0")); return NULL; @@ -334,9 +335,10 @@ namespace godot { const Array& import = dict_safe_get(functions, it.first, Array()); FAIL_IF(import.size() != 2, "Invalid import function " + it.first, ERR_CANT_CREATE); FAIL_IF(import[0].get_type() != Variant::OBJECT, "Invalid import target " + it.first, ERR_CANT_CREATE); + FAIL_IF(!INSTANCE_VALIDATE(import[0]), "Invalid import target " + it.first, ERR_CANT_CREATE); FAIL_IF(import[1].get_type() != Variant::STRING, "Invalid import method " + it.first, ERR_CANT_CREATE); godot_wasm::ContextFuncImport* context = (godot_wasm::ContextFuncImport*)&it.second; - context->target = import[0]; + context->target = import[0].operator Object*()->get_instance_id(); context->method = import[1]; extern_map[it.second.index] = wasm_func_as_extern(create_callback(context)); } diff --git a/src/godot-wasm.h b/src/wasm.h similarity index 98% rename from src/godot-wasm.h rename to src/wasm.h index bab775d..8e644c1 100644 --- a/src/godot-wasm.h +++ b/src/wasm.h @@ -2,7 +2,7 @@ #define GODOT_WASM_H #include -#include "wasm.h" +#include #include "defs.h" #include "wasm-memory.h"