godotengine/godot
1
0
Fork 0
mirror of https://github.com/godotengine/godot.git synced 2025-12-05 17:15:09 +00:00
Code Releases Wiki Activity

Add a backwards-compatibility system for GDExtension method

Browse Source 
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.
This commit is contained in:
Juan Linietsky
2023-04-25 20:53:07 +02:00
parent 45cd5dcad3
commit d8078d3f4c
6 changed files with 490 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

130
core/object/class_db.cpp
View File

@@ -556,6 +556,60 @@ MethodBind *ClassDB::get_method(const StringName &p_class, const StringName &p_n
return nullptr;
}
Vector<uint32_t> 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<MethodBind *> *c = type->method_map_compatibility.getptr(p_name);
Vector<uint32_t> 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<uint32_t>();
}
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<MethodBind *> *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<MethodBind *>());
}
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<Variant> &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: <reduz> 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<Variant> defvals;
@@ -1602,7 +1714,13 @@ void ClassDB::cleanup() {
for (KeyValue<StringName, MethodBind *> &F : ti.method_map) {
memdelete(F.value);
}
for (KeyValue<StringName, LocalVector<MethodBind *>> &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();