From d8078d3f4ce338d39ee591641e44020fb98cca2a Mon Sep 17 00:00:00 2001 From: Juan Linietsky Date: Tue, 25 Apr 2023 20:53:07 +0200 Subject: [PATCH] Add a backwards-compatibility system for GDExtension method This adds a way to ensure that methods that were modified in the Godot API will continue working in older builds of GDExtension even if the new signature is different. ```C++ // New version (changed) ClassDB::bind_method(D_METHOD("add_sphere","radius","position"),&MyShapes::add_sphere); // Compatibility version (still available to extensions). ClassDB::bind_compatibility_method(D_METHOD("add_sphere","radius"),&MyShapes::_compat_add_sphere); ``` **Q**: If I add an extra argument and provide a default value (hence can still be called the same), do I still have to provide the compatibility version? **A**: Yes, you must still provide a compatibility method. Most language bindings use the raw method pointer to do the call and process the default parameters in the binding language, hence if the actual method signature changes it will no longer work. **Q**: If I removed a method, can I still bind a compatibility version even though the main method no longer exists? **A**: Yes, for methods that were removed or renamed, compatibility versions can still be provided. **Q**: Would it be possible to automate checking that methods were removed by mistake? **A**: Yes, as part of a future PR, the idea is to add a a command line option to Godot that can be run like : `$ godot --test-api-compatibility older_api_dump.json`, which will also be integrated to the CI runs. --- core/extension/extension_api_dump.cpp | 280 +++++++++++++++++++++++ core/extension/extension_api_dump.h | 1 + core/extension/gdextension_interface.cpp | 7 +- core/object/class_db.cpp | 130 ++++++++++- core/object/class_db.h | 76 ++++-- main/main.cpp | 28 +++ 6 files changed, 490 insertions(+), 32 deletions(-) diff --git a/core/extension/extension_api_dump.cpp b/core/extension/extension_api_dump.cpp index 79b0ebc6414e..0bcc8a44e8a0 100644 --- a/core/extension/extension_api_dump.cpp +++ b/core/extension/extension_api_dump.cpp @@ -880,6 +880,15 @@ Dictionary GDExtensionAPIDump::generate_extension_api() { d2["is_virtual"] = false; d2["hash"] = method->get_hash(); + Vector compat_hashes = ClassDB::get_method_compatibility_hashes(class_name, method_name); + if (compat_hashes.size()) { + Array compatibility; + for (int i = 0; i < compat_hashes.size(); i++) { + compatibility.push_back(compat_hashes[i]); + } + d2["hash_compatibility"] = compatibility; + } + Vector default_args = method->get_default_arguments(); Array arguments; @@ -1056,4 +1065,275 @@ void GDExtensionAPIDump::generate_extension_json_file(const String &p_path) { fa->store_string(text); } +static bool compare_dict_array(const Dictionary &p_old_api, const Dictionary &p_new_api, const String &p_base_array, const String &p_name_field, const Vector &p_fields_to_compare, bool p_compare_hashes, const String &p_outer_class = String(), bool p_compare_operators = false) { + String base_array = p_outer_class + p_base_array; + if (!p_old_api.has(p_base_array)) { + return true; // May just not have this array and its still good. Probably added recently. + } + bool failed = false; + ERR_FAIL_COND_V_MSG(!p_new_api.has(p_base_array), false, "New API lacks base array: " + p_base_array); + Array new_api = p_new_api[p_base_array]; + HashMap new_api_assoc; + + for (int i = 0; i < new_api.size(); i++) { + Dictionary elem = new_api[i]; + ERR_FAIL_COND_V_MSG(!elem.has(p_name_field), false, "Validate extension JSON: Element of base_array '" + base_array + "' is missing field '" + p_name_field + "'. This is a bug."); + String name = elem[p_name_field]; + if (p_compare_operators && elem.has("right_type")) { + name += " " + String(elem["right_type"]); + } + new_api_assoc.insert(name, elem); + } + + Array old_api = p_old_api[p_base_array]; + for (int i = 0; i < old_api.size(); i++) { + Dictionary old_elem = old_api[i]; + if (!old_elem.has(p_name_field)) { + failed = true; + print_error("Validate extension JSON: JSON file: element of base array '" + base_array + "' is missing the field: '" + p_name_field + "'."); + continue; + } + String name = old_elem[p_name_field]; + if (p_compare_operators && old_elem.has("right_type")) { + name += " " + String(old_elem["right_type"]); + } + if (!new_api_assoc.has(name)) { + failed = true; + print_error("Validate extension JSON: API was removed: " + base_array + "/" + name); + continue; + } + + Dictionary new_elem = new_api_assoc[name]; + + for (int j = 0; j < p_fields_to_compare.size(); j++) { + String field = p_fields_to_compare[j]; + bool optional = field.begins_with("*"); + if (optional) { + // This is an optional field, but if exists it has to exist in both. + field = field.substr(1, field.length()); + } + + bool added = field.begins_with("+"); + if (added) { + // Meaning this field must either exist or contents may not exist. + field = field.substr(1, field.length()); + } + + Variant old_value; + + if (!old_elem.has(field)) { + if (optional) { + if (new_elem.has(field)) { + failed = true; + print_error("Validate extension JSON: JSON file: Field was added in a way that breaks compatibility '" + base_array + "/" + name + "': " + field); + } + } else if (added && new_elem.has(field)) { + // Should be ok, field now exists, should not be verified in prior versions where it does not. + } else { + failed = true; + print_error("Validate extension JSON: JSON file: Missing filed in '" + base_array + "/" + name + "': " + field); + } + continue; + } else { + old_value = old_elem[field]; + } + + if (!new_elem.has(field)) { + failed = true; + ERR_PRINT("Validate extension JSON: Missing filed in current API '" + base_array + "/" + name + "': " + field + ". This is a bug."); + continue; + } + + Variant new_value = new_elem[field]; + + bool equal = Variant::evaluate(Variant::OP_EQUAL, old_value, new_value); + if (!equal) { + failed = true; + print_error("Validate extension JSON: Error: Field '" + base_array + "/" + name + "': " + field + " changed value in new API, from " + old_value.get_construct_string() + " to " + new_value.get_construct_string() + "."); + continue; + } + } + + if (p_compare_hashes) { + if (!old_elem.has("hash")) { + if (old_elem.has("is_virtual") && bool(old_elem["is_virtual"]) && !new_elem.has("hash")) { + continue; // No hash for virtual methods, go on. + } + + failed = true; + print_error("Validate extension JSON: JSON file: element of base array '" + base_array + "' is missing the field: 'hash'."); + continue; + } + + uint64_t old_hash = old_elem["hash"]; + + if (!new_elem.has("hash")) { + failed = true; + print_error("Validate extension JSON: Error: Field '" + base_array + "' is missing the field: 'hash'."); + continue; + } + + uint64_t new_hash = new_elem["hash"]; + bool hash_found = false; + if (old_hash == new_hash) { + hash_found = true; + } else if (new_elem.has("hash_compatibility")) { + Array compatibility = new_elem["hash_compatibility"]; + for (int j = 0; j < compatibility.size(); j++) { + new_hash = compatibility[j]; + if (new_hash == old_hash) { + hash_found = true; + break; + } + } + } + + if (!hash_found) { + failed = true; + print_error("Validate extension JSON: Error: Hash mismatch for '" + base_array + "/" + name + "'. This means that the function has changed and no compatibility function was provided."); + continue; + } + } + } + + return !failed; +} + +static bool compare_sub_dict_array(HashSet &r_removed_classes_registered, const String &p_outer, const String &p_outer_name, const Dictionary &p_old_api, const Dictionary &p_new_api, const String &p_base_array, const String &p_name_field, const Vector &p_fields_to_compare, bool p_compare_hashes, bool p_compare_operators = false) { + if (!p_old_api.has(p_outer)) { + return true; // May just not have this array and its still good. Probably added recently or optional. + } + bool failed = false; + ERR_FAIL_COND_V_MSG(!p_new_api.has(p_outer), false, "New API lacks base array: " + p_outer); + Array new_api = p_new_api[p_outer]; + HashMap new_api_assoc; + + for (int i = 0; i < new_api.size(); i++) { + Dictionary elem = new_api[i]; + ERR_FAIL_COND_V_MSG(!elem.has(p_outer_name), false, "Validate extension JSON: Element of base_array '" + p_outer + "' is missing field '" + p_outer_name + "'. This is a bug."); + new_api_assoc.insert(elem[p_outer_name], elem); + } + + Array old_api = p_old_api[p_outer]; + + for (int i = 0; i < old_api.size(); i++) { + Dictionary old_elem = old_api[i]; + if (!old_elem.has(p_outer_name)) { + failed = true; + print_error("Validate extension JSON: JSON file: element of base array '" + p_outer + "' is missing the field: '" + p_outer_name + "'."); + continue; + } + String name = old_elem[p_outer_name]; + if (!new_api_assoc.has(name)) { + failed = true; + if (!r_removed_classes_registered.has(name)) { + print_error("Validate extension JSON: API was removed: " + p_outer + "/" + name); + r_removed_classes_registered.insert(name); + } + continue; + } + + Dictionary new_elem = new_api_assoc[name]; + + if (!compare_dict_array(old_elem, new_elem, p_base_array, p_name_field, p_fields_to_compare, p_compare_hashes, p_outer + "/" + name + "/", p_compare_operators)) { + failed = true; + } + } + + return !failed; +} + +Error GDExtensionAPIDump::validate_extension_json_file(const String &p_path) { + Error error; + String text = FileAccess::get_file_as_string(p_path, &error); + if (error != OK) { + ERR_PRINT("Validate extension JSON: Could not open file '" + p_path + "'."); + return error; + } + + Ref json; + json.instantiate(); + error = json->parse(text); + if (error != OK) { + ERR_PRINT("Validate extension JSON: Error parsing '" + p_path + "' at line " + itos(json->get_error_line()) + ": " + json->get_error_message()); + return error; + } + + Dictionary old_api = json->get_data(); + Dictionary new_api = generate_extension_api(); + + { // Validate header: + Dictionary header = old_api["header"]; + ERR_FAIL_COND_V(!header.has("version_major"), ERR_INVALID_DATA); + ERR_FAIL_COND_V(!header.has("version_minor"), ERR_INVALID_DATA); + int major = header["version_major"]; + int minor = header["version_minor"]; + + ERR_FAIL_COND_V_MSG(major != VERSION_MAJOR, ERR_INVALID_DATA, "JSON API dump is for a different engine version (" + itos(major) + ") than this one (" + itos(major) + ")"); + ERR_FAIL_COND_V_MSG(minor > VERSION_MINOR, ERR_INVALID_DATA, "JSON API dump is for a newer version of the engine: " + itos(major) + "." + itos(minor)); + } + + bool failed = false; + + HashSet removed_classes_registered; + + if (!compare_dict_array(old_api, new_api, "constants", "name", Vector({ "value", "is_bitfield" }), false)) { + failed = true; + } + + if (!compare_dict_array(old_api, new_api, "utility_functions", "name", Vector({ "category" }), true)) { + failed = true; + } + + if (!compare_sub_dict_array(removed_classes_registered, "builtin_classes", "name", old_api, new_api, "members", "name", { "type" }, false)) { + failed = true; + } + + if (!compare_sub_dict_array(removed_classes_registered, "builtin_classes", "name", old_api, new_api, "constants", "name", { "type", "value" }, false)) { + failed = true; + } + + if (!compare_sub_dict_array(removed_classes_registered, "builtin_classes", "name", old_api, new_api, "operators", "name", { "return_type" }, false, true)) { + failed = true; + } + + if (!compare_sub_dict_array(removed_classes_registered, "builtin_classes", "name", old_api, new_api, "methods", "name", {}, true)) { + failed = true; + } + + if (!compare_sub_dict_array(removed_classes_registered, "builtin_classes", "name", old_api, new_api, "constructors", "index", { "*arguments" }, false)) { + failed = true; + } + + if (!compare_sub_dict_array(removed_classes_registered, "classes", "name", old_api, new_api, "constants", "name", { "value" }, false)) { + failed = true; + } + + if (!compare_sub_dict_array(removed_classes_registered, "classes", "name", old_api, new_api, "methods", "name", { "is_virtual", "is_vararg", "is_static" }, true)) { // is_const sometimes can change because they are added if someone forgot, but should not be a problem for the extensions. + failed = true; + } + + if (!compare_sub_dict_array(removed_classes_registered, "classes", "name", old_api, new_api, "signals", "name", { "*arguments" }, false)) { + failed = true; + } + + if (!compare_sub_dict_array(removed_classes_registered, "classes", "name", old_api, new_api, "properties", "name", { "type", "*setter", "*getter", "*index" }, false)) { + failed = true; + } + + if (!compare_dict_array(old_api, new_api, "singletons", "name", Vector({ "type" }), false)) { + failed = true; + } + + if (!compare_dict_array(old_api, new_api, "native_structures", "name", Vector({ "format" }), false)) { + failed = true; + } + + if (failed) { + return ERR_INVALID_DATA; + } else { + return OK; + } +} + #endif // TOOLS_ENABLED diff --git a/core/extension/extension_api_dump.h b/core/extension/extension_api_dump.h index 7e588c944635..11ea2cf92300 100644 --- a/core/extension/extension_api_dump.h +++ b/core/extension/extension_api_dump.h @@ -39,6 +39,7 @@ class GDExtensionAPIDump { public: static Dictionary generate_extension_api(); static void generate_extension_json_file(const String &p_path); + static Error validate_extension_json_file(const String &p_path); }; #endif diff --git a/core/extension/gdextension_interface.cpp b/core/extension/gdextension_interface.cpp index 2bedb623e4f7..c892fa3a686b 100644 --- a/core/extension/gdextension_interface.cpp +++ b/core/extension/gdextension_interface.cpp @@ -984,7 +984,12 @@ static GDExtensionScriptInstancePtr gdextension_script_instance_create(const GDE static GDExtensionMethodBindPtr gdextension_classdb_get_method_bind(GDExtensionConstStringNamePtr p_classname, GDExtensionConstStringNamePtr p_methodname, GDExtensionInt p_hash) { const StringName classname = *reinterpret_cast(p_classname); const StringName methodname = *reinterpret_cast(p_methodname); - MethodBind *mb = ClassDB::get_method(classname, methodname); + bool exists = false; + MethodBind *mb = ClassDB::get_method_with_compatibility(classname, methodname, p_hash, &exists); + if (!mb && exists) { + ERR_PRINT("Method '" + classname + "." + methodname + "' has changed and no compatibility fallback has been provided. Please open an issue."); + return nullptr; + } ERR_FAIL_COND_V(!mb, nullptr); if (mb->get_hash() != p_hash) { ERR_PRINT("Hash mismatch for method '" + classname + "." + methodname + "'."); diff --git a/core/object/class_db.cpp b/core/object/class_db.cpp index d920ae6ca05b..b9cababc9084 100644 --- a/core/object/class_db.cpp +++ b/core/object/class_db.cpp @@ -556,6 +556,60 @@ MethodBind *ClassDB::get_method(const StringName &p_class, const StringName &p_n return nullptr; } +Vector ClassDB::get_method_compatibility_hashes(const StringName &p_class, const StringName &p_name) { + OBJTYPE_RLOCK; + + ClassInfo *type = classes.getptr(p_class); + + while (type) { + if (type->method_map_compatibility.has(p_name)) { + LocalVector *c = type->method_map_compatibility.getptr(p_name); + Vector ret; + for (uint32_t i = 0; i < c->size(); i++) { + ret.push_back((*c)[i]->get_hash()); + } + return ret; + } + type = type->inherits_ptr; + } + return Vector(); +} + +MethodBind *ClassDB::get_method_with_compatibility(const StringName &p_class, const StringName &p_name, uint64_t p_hash, bool *r_method_exists, bool *r_is_deprecated) { + OBJTYPE_RLOCK; + + ClassInfo *type = classes.getptr(p_class); + + while (type) { + MethodBind **method = type->method_map.getptr(p_name); + if (method && *method) { + if (r_method_exists) { + *r_method_exists = true; + } + if ((*method)->get_hash() == p_hash) { + return *method; + } + } + + LocalVector *compat = type->method_map_compatibility.getptr(p_name); + if (compat) { + if (r_method_exists) { + *r_method_exists = true; + } + for (uint32_t i = 0; i < compat->size(); i++) { + if ((*compat)[i]->get_hash() == p_hash) { + if (r_is_deprecated) { + *r_is_deprecated = true; + } + return (*compat)[i]; + } + } + } + type = type->inherits_ptr; + } + return nullptr; +} + void ClassDB::bind_integer_constant(const StringName &p_class, const StringName &p_enum, const StringName &p_name, int64_t p_constant, bool p_is_bitfield) { OBJTYPE_WLOCK; @@ -1268,11 +1322,30 @@ bool ClassDB::has_method(const StringName &p_class, const StringName &p_method, } void ClassDB::bind_method_custom(const StringName &p_class, MethodBind *p_method) { + _bind_method_custom(p_class, p_method, false); +} +void ClassDB::bind_compatibility_method_custom(const StringName &p_class, MethodBind *p_method) { + _bind_method_custom(p_class, p_method, true); +} + +void ClassDB::_bind_compatibility(ClassInfo *type, MethodBind *p_method) { + if (!type->method_map_compatibility.has(p_method->get_name())) { + type->method_map_compatibility.insert(p_method->get_name(), LocalVector()); + } + type->method_map_compatibility[p_method->get_name()].push_back(p_method); +} + +void ClassDB::_bind_method_custom(const StringName &p_class, MethodBind *p_method, bool p_compatibility) { ClassInfo *type = classes.getptr(p_class); if (!type) { ERR_FAIL_MSG("Couldn't bind custom method '" + p_method->get_name() + "' for instance '" + p_class + "'."); } + if (p_compatibility) { + _bind_compatibility(type, p_method); + return; + } + if (type->method_map.has(p_method->get_name())) { // overloading not supported ERR_FAIL_MSG("Method already bound '" + p_class + "::" + p_method->get_name() + "'."); @@ -1285,11 +1358,44 @@ void ClassDB::bind_method_custom(const StringName &p_class, MethodBind *p_method type->method_map[p_method->get_name()] = p_method; } +MethodBind *ClassDB::_bind_vararg_method(MethodBind *p_bind, const StringName &p_name, const Vector &p_default_args, bool p_compatibility) { + MethodBind *bind = p_bind; + bind->set_name(p_name); + bind->set_default_arguments(p_default_args); + + String instance_type = bind->get_instance_class(); + + ClassInfo *type = classes.getptr(instance_type); + if (!type) { + memdelete(bind); + ERR_FAIL_COND_V(!type, nullptr); + } + + if (p_compatibility) { + _bind_compatibility(type, bind); + return bind; + } + + if (type->method_map.has(p_name)) { + memdelete(bind); + // Overloading not supported + ERR_FAIL_V_MSG(nullptr, "Method already bound: " + instance_type + "::" + p_name + "."); + } + type->method_map[p_name] = bind; #ifdef DEBUG_METHODS_ENABLED -MethodBind *ClassDB::bind_methodfi(uint32_t p_flags, MethodBind *p_bind, const MethodDefinition &method_name, const Variant **p_defs, int p_defcount) { + // FIXME: set_return_type is no longer in MethodBind, so I guess it should be moved to vararg method bind + //bind->set_return_type("Variant"); + type->method_order.push_back(p_name); +#endif + + return bind; +} + +#ifdef DEBUG_METHODS_ENABLED +MethodBind *ClassDB::bind_methodfi(uint32_t p_flags, MethodBind *p_bind, bool p_compatibility, const MethodDefinition &method_name, const Variant **p_defs, int p_defcount) { StringName mdname = method_name.name; #else -MethodBind *ClassDB::bind_methodfi(uint32_t p_flags, MethodBind *p_bind, const char *method_name, const Variant **p_defs, int p_defcount) { +MethodBind *ClassDB::bind_methodfi(uint32_t p_flags, MethodBind *p_bind, bool p_compatibility, const char *method_name, const Variant **p_defs, int p_defcount) { StringName mdname = StaticCString::create(method_name); #endif @@ -1301,7 +1407,7 @@ MethodBind *ClassDB::bind_methodfi(uint32_t p_flags, MethodBind *p_bind, const c #ifdef DEBUG_ENABLED - ERR_FAIL_COND_V_MSG(has_method(instance_type, mdname), nullptr, "Class " + String(instance_type) + " already has a method " + String(mdname) + "."); + ERR_FAIL_COND_V_MSG(!p_compatibility && has_method(instance_type, mdname), nullptr, "Class " + String(instance_type) + " already has a method " + String(mdname) + "."); #endif ClassInfo *type = classes.getptr(instance_type); @@ -1310,7 +1416,7 @@ MethodBind *ClassDB::bind_methodfi(uint32_t p_flags, MethodBind *p_bind, const c ERR_FAIL_V_MSG(nullptr, "Couldn't bind method '" + mdname + "' for instance '" + instance_type + "'."); } - if (type->method_map.has(mdname)) { + if (!p_compatibility && type->method_map.has(mdname)) { memdelete(p_bind); // overloading not supported ERR_FAIL_V_MSG(nullptr, "Method already bound '" + instance_type + "::" + mdname + "'."); @@ -1325,10 +1431,16 @@ MethodBind *ClassDB::bind_methodfi(uint32_t p_flags, MethodBind *p_bind, const c p_bind->set_argument_names(method_name.args); - type->method_order.push_back(mdname); + if (!p_compatibility) { + type->method_order.push_back(mdname); + } #endif - type->method_map[mdname] = p_bind; + if (p_compatibility) { + _bind_compatibility(type, p_bind); + } else { + type->method_map[mdname] = p_bind; + } Vector defvals; @@ -1602,7 +1714,13 @@ void ClassDB::cleanup() { for (KeyValue &F : ti.method_map) { memdelete(F.value); } + for (KeyValue> &F : ti.method_map_compatibility) { + for (uint32_t i = 0; i < F.value.size(); i++) { + memdelete(F.value[i]); + } + } } + classes.clear(); resource_base_extensions.clear(); compat_classes.clear(); diff --git a/core/object/class_db.h b/core/object/class_db.h index 1a5e9235cf19..ce64336a45c1 100644 --- a/core/object/class_db.h +++ b/core/object/class_db.h @@ -105,6 +105,7 @@ public: ObjectGDExtension *gdextension = nullptr; HashMap method_map; + HashMap> method_map_compatibility; HashMap constant_map; struct EnumInfo { List constants; @@ -148,9 +149,9 @@ public: static HashMap compat_classes; #ifdef DEBUG_METHODS_ENABLED - static MethodBind *bind_methodfi(uint32_t p_flags, MethodBind *p_bind, const MethodDefinition &method_name, const Variant **p_defs, int p_defcount); + static MethodBind *bind_methodfi(uint32_t p_flags, MethodBind *p_bind, bool p_compatibility, const MethodDefinition &method_name, const Variant **p_defs, int p_defcount); #else - static MethodBind *bind_methodfi(uint32_t p_flags, MethodBind *p_bind, const char *method_name, const Variant **p_defs, int p_defcount); + static MethodBind *bind_methodfi(uint32_t p_flags, MethodBind *p_bind, bool p_compatibility, const char *method_name, const Variant **p_defs, int p_defcount); #endif static APIType current_api; @@ -172,6 +173,9 @@ private: // Non-locking variants of get_parent_class and is_parent_class. static StringName _get_parent_class(const StringName &p_class); static bool _is_parent_class(const StringName &p_class, const StringName &p_inherits); + static void _bind_compatibility(ClassInfo *type, MethodBind *p_method); + static MethodBind *_bind_vararg_method(MethodBind *p_bind, const StringName &p_name, const Vector &p_default_args, bool p_compatibility); + static void _bind_method_custom(const StringName &p_class, MethodBind *p_method, bool p_compatibility); public: // DO NOT USE THIS!!!!!! NEEDS TO BE PUBLIC BUT DO NOT USE NO MATTER WHAT!!! @@ -273,7 +277,7 @@ public: if constexpr (std::is_same::return_type, Object *>::value) { bind->set_return_type_is_raw_object_ptr(true); } - return bind_methodfi(METHOD_FLAGS_DEFAULT, bind, p_method_name, sizeof...(p_args) == 0 ? nullptr : (const Variant **)argptrs, sizeof...(p_args)); + return bind_methodfi(METHOD_FLAGS_DEFAULT, bind, false, p_method_name, sizeof...(p_args) == 0 ? nullptr : (const Variant **)argptrs, sizeof...(p_args)); } template @@ -288,7 +292,36 @@ public: if constexpr (std::is_same::return_type, Object *>::value) { bind->set_return_type_is_raw_object_ptr(true); } - return bind_methodfi(METHOD_FLAGS_DEFAULT, bind, p_method_name, sizeof...(p_args) == 0 ? nullptr : (const Variant **)argptrs, sizeof...(p_args)); + return bind_methodfi(METHOD_FLAGS_DEFAULT, bind, false, p_method_name, sizeof...(p_args) == 0 ? nullptr : (const Variant **)argptrs, sizeof...(p_args)); + } + + template + static MethodBind *bind_compatibility_method(N p_method_name, M p_method, VarArgs... p_args) { + Variant args[sizeof...(p_args) + 1] = { p_args..., Variant() }; // +1 makes sure zero sized arrays are also supported. + const Variant *argptrs[sizeof...(p_args) + 1]; + for (uint32_t i = 0; i < sizeof...(p_args); i++) { + argptrs[i] = &args[i]; + } + MethodBind *bind = create_method_bind(p_method); + if constexpr (std::is_same::return_type, Object *>::value) { + bind->set_return_type_is_raw_object_ptr(true); + } + return bind_methodfi(METHOD_FLAGS_DEFAULT, bind, true, p_method_name, sizeof...(p_args) == 0 ? nullptr : (const Variant **)argptrs, sizeof...(p_args)); + } + + template + static MethodBind *bind_compatibility_static_method(const StringName &p_class, N p_method_name, M p_method, VarArgs... p_args) { + Variant args[sizeof...(p_args) + 1] = { p_args..., Variant() }; // +1 makes sure zero sized arrays are also supported. + const Variant *argptrs[sizeof...(p_args) + 1]; + for (uint32_t i = 0; i < sizeof...(p_args); i++) { + argptrs[i] = &args[i]; + } + MethodBind *bind = create_static_method_bind(p_method); + bind->set_instance_class(p_class); + if constexpr (std::is_same::return_type, Object *>::value) { + bind->set_return_type_is_raw_object_ptr(true); + } + return bind_methodfi(METHOD_FLAGS_DEFAULT, bind, true, p_method_name, sizeof...(p_args) == 0 ? nullptr : (const Variant **)argptrs, sizeof...(p_args)); } template @@ -298,36 +331,27 @@ public: MethodBind *bind = create_vararg_method_bind(p_method, p_info, p_return_nil_is_variant); ERR_FAIL_COND_V(!bind, nullptr); - bind->set_name(p_name); - bind->set_default_arguments(p_default_args); if constexpr (std::is_same::return_type, Object *>::value) { bind->set_return_type_is_raw_object_ptr(true); } + return _bind_vararg_method(bind, p_name, p_default_args, false); + } - String instance_type = bind->get_instance_class(); + template + static MethodBind *bind_compatibility_vararg_method(uint32_t p_flags, const StringName &p_name, M p_method, const MethodInfo &p_info = MethodInfo(), const Vector &p_default_args = Vector(), bool p_return_nil_is_variant = true) { + GLOBAL_LOCK_FUNCTION; - ClassInfo *type = classes.getptr(instance_type); - if (!type) { - memdelete(bind); - ERR_FAIL_COND_V(!type, nullptr); + MethodBind *bind = create_vararg_method_bind(p_method, p_info, p_return_nil_is_variant); + ERR_FAIL_COND_V(!bind, nullptr); + + if constexpr (std::is_same::return_type, Object *>::value) { + bind->set_return_type_is_raw_object_ptr(true); } - - if (type->method_map.has(p_name)) { - memdelete(bind); - // Overloading not supported - ERR_FAIL_V_MSG(nullptr, "Method already bound: " + instance_type + "::" + p_name + "."); - } - type->method_map[p_name] = bind; -#ifdef DEBUG_METHODS_ENABLED - // FIXME: set_return_type is no longer in MethodBind, so I guess it should be moved to vararg method bind - //bind->set_return_type("Variant"); - type->method_order.push_back(p_name); -#endif - - return bind; + return _bind_vararg_method(bind, p_name, p_default_args, true); } static void bind_method_custom(const StringName &p_class, MethodBind *p_method); + static void bind_compatibility_method_custom(const StringName &p_class, MethodBind *p_method); static void add_signal(const StringName &p_class, const MethodInfo &p_signal); static bool has_signal(const StringName &p_class, const StringName &p_signal, bool p_no_inheritance = false); @@ -358,6 +382,8 @@ public: static void get_method_list(const StringName &p_class, List *p_methods, bool p_no_inheritance = false, bool p_exclude_from_properties = false); static bool get_method_info(const StringName &p_class, const StringName &p_method, MethodInfo *r_info, bool p_no_inheritance = false, bool p_exclude_from_properties = false); static MethodBind *get_method(const StringName &p_class, const StringName &p_name); + static MethodBind *get_method_with_compatibility(const StringName &p_class, const StringName &p_name, uint64_t p_hash, bool *r_method_exists = nullptr, bool *r_is_deprecated = nullptr); + static Vector get_method_compatibility_hashes(const StringName &p_class, const StringName &p_name); static void add_virtual_method(const StringName &p_class, const MethodInfo &p_method, bool p_virtual = true, const Vector &p_arg_names = Vector(), bool p_object_core = false); static void get_virtual_methods(const StringName &p_class, List *p_methods, bool p_no_inheritance = false); diff --git a/main/main.cpp b/main/main.cpp index 6b89d961476e..5ba1eac1f3a0 100644 --- a/main/main.cpp +++ b/main/main.cpp @@ -217,6 +217,8 @@ static bool print_fps = false; #ifdef TOOLS_ENABLED static bool dump_gdextension_interface = false; static bool dump_extension_api = false; +static bool validate_extension_api = false; +static String validate_extension_api_file; #endif bool profile_gpu = false; @@ -458,6 +460,7 @@ void Main::print_help(const char *p_binary) { OS::get_singleton()->print(" --build-solutions Build the scripting solutions (e.g. for C# projects). Implies --editor and requires a valid project to edit.\n"); OS::get_singleton()->print(" --dump-gdextension-interface Generate GDExtension header file 'gdextension_interface.h' in the current folder. This file is the base file required to implement a GDExtension.\n"); OS::get_singleton()->print(" --dump-extension-api Generate JSON dump of the Godot API for GDExtension bindings named 'extension_api.json' in the current folder.\n"); + OS::get_singleton()->print(" --validate-extension-api Validate an extension API file dumped (with the option above) from a previous version of the engine to ensure API compatibility. If incompatibilities or errors are detected, the return code will be non zero.\n"); OS::get_singleton()->print(" --startup-benchmark Benchmark the startup time and print it to console.\n"); OS::get_singleton()->print(" --startup-benchmark-file Benchmark the startup time and save it to a given file in JSON format.\n"); #ifdef TESTS_ENABLED @@ -1133,6 +1136,25 @@ Error Main::setup(const char *execpath, int argc, char *argv[], bool p_second_ph // run the project instead of a cmdline tool. // Needs full refactoring to fix properly. main_args.push_back(I->get()); + } else if (I->get() == "--validate-extension-api") { + // Register as an editor instance to use low-end fallback if relevant. + editor = true; + cmdline_tool = true; + validate_extension_api = true; + // Hack. Not needed but otherwise we end up detecting that this should + // run the project instead of a cmdline tool. + // Needs full refactoring to fix properly. + main_args.push_back(I->get()); + + if (I->next()) { + validate_extension_api_file = I->next()->get(); + + N = I->next()->next(); + } else { + OS::get_singleton()->print("Missing file to load argument after --validate-extension-api, aborting."); + goto error; + } + } else if (I->get() == "--export-release" || I->get() == "--export-debug" || I->get() == "--export-pack") { // Export project // Actually handling is done in start(). @@ -2650,6 +2672,12 @@ bool Main::start() { return false; } + if (validate_extension_api) { + bool valid = GDExtensionAPIDump::validate_extension_json_file(validate_extension_api_file) == OK; + OS::get_singleton()->set_exit_code(valid ? EXIT_SUCCESS : EXIT_FAILURE); + return false; + } + #ifndef DISABLE_DEPRECATED if (converting_project) { int ret = ProjectConverter3To4(converter_max_kb_file, converter_max_line_length).convert();