Skip to content

Commit

Permalink
C#: Generate and use compat methods
Browse files Browse the repository at this point in the history
- Implements `ClassDB::get_method_list_with_compatibility` to retrieve all methods from a class including compat methods.
- C# bindings generator now also generates compat methods.
- All generated C# methods now use `ClassDB::get_method_with_compatibility`.
  • Loading branch information
raulsntos committed Sep 19, 2023
1 parent 4714e95 commit 5f6082a
Show file tree
Hide file tree
Showing 7 changed files with 196 additions and 11 deletions.
62 changes: 59 additions & 3 deletions core/object/class_db.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,6 @@ void ClassDB::get_method_list(const StringName &p_class, List<MethodInfo> *p_met
}

#ifdef DEBUG_METHODS_ENABLED

for (const MethodInfo &E : type->virtual_methods) {
p_methods->push_back(E);
}
Expand All @@ -478,17 +477,74 @@ void ClassDB::get_method_list(const StringName &p_class, List<MethodInfo> *p_met

p_methods->push_back(minfo);
}

#else

for (KeyValue<StringName, MethodBind *> &E : type->method_map) {
MethodBind *m = E.value;
MethodInfo minfo = info_from_bind(m);
p_methods->push_back(minfo);
}
#endif

if (p_no_inheritance) {
break;
}

type = type->inherits_ptr;
}
}

void ClassDB::get_method_list_with_compatibility(const StringName &p_class, List<Pair<MethodInfo, uint32_t>> *p_methods, bool p_no_inheritance, bool p_exclude_from_properties) {
OBJTYPE_RLOCK;

ClassInfo *type = classes.getptr(p_class);

while (type) {
if (type->disabled) {
if (p_no_inheritance) {
break;
}

type = type->inherits_ptr;
continue;
}

#ifdef DEBUG_METHODS_ENABLED
for (const MethodInfo &E : type->virtual_methods) {
Pair<MethodInfo, uint32_t> pair(E, 0);
p_methods->push_back(pair);
}

for (const StringName &E : type->method_order) {
if (p_exclude_from_properties && type->methods_in_properties.has(E)) {
continue;
}

MethodBind *method = type->method_map.get(E);
MethodInfo minfo = info_from_bind(method);

Pair<MethodInfo, uint32_t> pair(minfo, method->get_hash());
p_methods->push_back(pair);
}
#else
for (KeyValue<StringName, MethodBind *> &E : type->method_map) {
MethodBind *method = E.value;
MethodInfo minfo = info_from_bind(method);

Pair<MethodInfo, uint32_t> pair(minfo, method->get_hash());
p_methods->push_back(pair);
}
#endif

for (const KeyValue<StringName, LocalVector<MethodBind *, unsigned int, false, false>> &E : type->method_map_compatibility) {
LocalVector<MethodBind *> compat = E.value;
for (MethodBind *method : compat) {
MethodInfo minfo = info_from_bind(method);

Pair<MethodInfo, uint32_t> pair(minfo, method->get_hash());
p_methods->push_back(pair);
}
}

if (p_no_inheritance) {
break;
}
Expand Down
1 change: 1 addition & 0 deletions core/object/class_db.h
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@ class ClassDB {
static void set_method_flags(const StringName &p_class, const StringName &p_method, int p_flags);

static void get_method_list(const StringName &p_class, List<MethodInfo> *p_methods, bool p_no_inheritance = false, bool p_exclude_from_properties = false);
static void get_method_list_with_compatibility(const StringName &p_class, List<Pair<MethodInfo, uint32_t>> *p_methods_with_hash, 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);
Expand Down
110 changes: 102 additions & 8 deletions modules/mono/editor/bindings_generator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ StringBuilder &operator<<(StringBuilder &r_sb, const char *p_cstring) {

#define ICALL_PREFIX "godot_icall_"
#define ICALL_CLASSDB_GET_METHOD "ClassDB_get_method"
#define ICALL_CLASSDB_GET_METHOD_WITH_COMPATIBILITY "ClassDB_get_method_with_compatibility"
#define ICALL_CLASSDB_GET_CONSTRUCTOR "ClassDB_get_constructor"

#define C_LOCAL_RET "ret"
Expand Down Expand Up @@ -1354,6 +1355,7 @@ Error BindingsGenerator::_generate_cs_type(const TypeInterface &itype, const Str
output.append("namespace " BINDINGS_NAMESPACE ";\n\n");

output.append("using System;\n"); // IntPtr
output.append("using System.ComponentModel;\n"); // EditorBrowsable
output.append("using System.Diagnostics;\n"); // DebuggerBrowsable
output.append("using Godot.NativeInterop;\n");

Expand Down Expand Up @@ -1827,7 +1829,13 @@ Error BindingsGenerator::_generate_cs_type(const TypeInterface &itype, const Str
}
output << "\n"
<< INDENT1 "{\n";
HashMap<String, StringName> method_names;
for (const MethodInterface &imethod : itype.methods) {
if (method_names.has(imethod.proxy_name)) {
ERR_FAIL_COND_V_MSG(method_names[imethod.proxy_name] != imethod.cname, ERR_BUG, "Method name '" + imethod.proxy_name + "' already exists with a different value.");
continue;
}
method_names[imethod.proxy_name] = imethod.cname;
output << INDENT2 "/// <summary>\n"
<< INDENT2 "/// Cached name for the '" << imethod.cname << "' method.\n"
<< INDENT2 "/// </summary>\n"
Expand Down Expand Up @@ -2080,7 +2088,7 @@ Error BindingsGenerator::_generate_cs_method(const BindingsGenerator::TypeInterf

arguments_sig += iarg.name;

if (iarg.default_argument.size()) {
if (!p_imethod.is_compat && iarg.default_argument.size()) {
if (iarg.def_param_mode != ArgumentInterface::CONSTANT) {
arguments_sig += " = null";
} else {
Expand Down Expand Up @@ -2168,8 +2176,8 @@ Error BindingsGenerator::_generate_cs_method(const BindingsGenerator::TypeInterf
p_output << "GodotObject.";
}

p_output << ICALL_CLASSDB_GET_METHOD "(" BINDINGS_NATIVE_NAME_FIELD ", MethodName."
<< p_imethod.proxy_name
p_output << ICALL_CLASSDB_GET_METHOD_WITH_COMPATIBILITY "(" BINDINGS_NATIVE_NAME_FIELD ", MethodName."
<< p_imethod.proxy_name << ", " << itos(p_imethod.hash) << "ul"
<< ");\n";
}

Expand Down Expand Up @@ -2208,6 +2216,10 @@ Error BindingsGenerator::_generate_cs_method(const BindingsGenerator::TypeInterf
p_output.append("\")]");
}

if (p_imethod.is_compat) {
p_output.append(MEMBER_BEGIN "[EditorBrowsable(EditorBrowsableState.Never)]");
}

p_output.append(MEMBER_BEGIN);
p_output.append(p_imethod.is_internal ? "internal " : "public ");

Expand Down Expand Up @@ -2924,6 +2936,12 @@ bool method_has_ptr_parameter(MethodInfo p_method_info) {
return false;
}

struct SortMethodWithHashes {
_FORCE_INLINE_ bool operator()(const Pair<MethodInfo, uint32_t> &p_a, const Pair<MethodInfo, uint32_t> &p_b) const {
return p_a.first < p_b.first;
}
};

bool BindingsGenerator::_populate_object_type_interfaces() {
obj_types.clear();

Expand Down Expand Up @@ -3051,11 +3069,15 @@ bool BindingsGenerator::_populate_object_type_interfaces() {
List<MethodInfo> virtual_method_list;
ClassDB::get_virtual_methods(type_cname, &virtual_method_list, true);

List<MethodInfo> method_list;
ClassDB::get_method_list(type_cname, &method_list, true);
method_list.sort();
List<Pair<MethodInfo, uint32_t>> method_list_with_hashes;
ClassDB::get_method_list_with_compatibility(type_cname, &method_list_with_hashes, true);
method_list_with_hashes.sort_custom_inplace<SortMethodWithHashes>();

List<MethodInterface> compat_methods;
for (const Pair<MethodInfo, uint32_t> &E : method_list_with_hashes) {
const MethodInfo &method_info = E.first;
const uint32_t hash = E.second;

for (const MethodInfo &method_info : method_list) {
int argc = method_info.arguments.size();

if (method_info.name.is_empty()) {
Expand All @@ -3077,6 +3099,7 @@ bool BindingsGenerator::_populate_object_type_interfaces() {
MethodInterface imethod;
imethod.name = method_info.name;
imethod.cname = cname;
imethod.hash = hash;

if (method_info.flags & METHOD_FLAG_STATIC) {
imethod.is_static = true;
Expand All @@ -3089,7 +3112,17 @@ bool BindingsGenerator::_populate_object_type_interfaces() {

PropertyInfo return_info = method_info.return_val;

MethodBind *m = imethod.is_virtual ? nullptr : ClassDB::get_method(type_cname, method_info.name);
MethodBind *m = nullptr;

if (!imethod.is_virtual) {
bool method_exists = false;
m = ClassDB::get_method_with_compatibility(type_cname, method_info.name, hash, &method_exists, &imethod.is_compat);

if (unlikely(!method_exists)) {
ERR_FAIL_COND_V_MSG(!virtual_method_list.find(method_info), false,
"Missing MethodBind for non-virtual method: '" + itype.name + "." + imethod.name + "'.");
}
}

imethod.is_vararg = m && m->is_vararg();

Expand Down Expand Up @@ -3210,6 +3243,14 @@ bool BindingsGenerator::_populate_object_type_interfaces() {
ERR_FAIL_COND_V_MSG(itype.find_property_by_name(imethod.cname), false,
"Method name conflicts with property: '" + itype.name + "." + imethod.name + "'.");

// Compat methods aren't added to the type yet, they need to be checked for conflicts
// after all the non-compat methods have been added. The compat methods are added in
// reverse so the most recently added ones take precedence over older compat methods.
if (imethod.is_compat) {
compat_methods.push_front(imethod);
continue;
}

// Methods starting with an underscore are ignored unless they're used as a property setter or getter
if (!imethod.is_virtual && imethod.name[0] == '_') {
for (const PropertyInterface &iprop : itype.properties) {
Expand All @@ -3224,6 +3265,15 @@ bool BindingsGenerator::_populate_object_type_interfaces() {
}
}

// Add compat methods that don't conflict with other methods in the type.
for (const MethodInterface &imethod : compat_methods) {
if (_method_has_conflicting_signature(imethod, itype)) {
WARN_PRINT("Method '" + imethod.name + "' conflicts with an already existing method in type '" + itype.name + "' and has been ignored.");
continue;
}
itype.methods.push_back(imethod);
}

// Populate signals

const HashMap<StringName, MethodInfo> &signal_map = class_info->signal_map;
Expand Down Expand Up @@ -4007,6 +4057,50 @@ void BindingsGenerator::_populate_global_constants() {
}
}

bool BindingsGenerator::_method_has_conflicting_signature(const MethodInterface &p_imethod, const TypeInterface &p_itype) {
// Compare p_imethod with all the methods already registered in p_itype.
for (const MethodInterface &method : p_itype.methods) {
if (method.proxy_name == p_imethod.proxy_name) {
if (_method_has_conflicting_signature(p_imethod, method)) {
return true;
}
}
}

return false;
}

bool BindingsGenerator::_method_has_conflicting_signature(const MethodInterface &p_imethod_left, const MethodInterface &p_imethod_right) {
// Check if a method already exists in p_itype with a method signature that would conflict with p_imethod.
// The return type is ignored because only changing the return type is not enough to avoid conflicts.
// The const keyword is also ignored since it doesn't generate different C# code.

if (p_imethod_left.arguments.size() != p_imethod_right.arguments.size()) {
// Different argument count, so no conflict.
return false;
}

for (int i = 0; i < p_imethod_left.arguments.size(); i++) {
const ArgumentInterface &iarg_left = p_imethod_left.arguments[i];
const ArgumentInterface &iarg_right = p_imethod_right.arguments[i];

if (iarg_left.type.cname != iarg_right.type.cname) {
// Different types for arguments in the same position, so no conflict.
return false;
}

if (iarg_left.def_param_mode != iarg_right.def_param_mode) {
// If the argument is a value type and nullable, it will be 'Nullable<T>' instead of 'T'
// and will not create a conflict.
if (iarg_left.def_param_mode == ArgumentInterface::NULLABLE_VAL || iarg_right.def_param_mode == ArgumentInterface::NULLABLE_VAL) {
return false;
}
}
}

return true;
}

void BindingsGenerator::_initialize_blacklisted_methods() {
blacklisted_methods["Object"].push_back("to_string"); // there is already ToString
blacklisted_methods["Object"].push_back("_to_string"); // override ToString instead
Expand Down
14 changes: 14 additions & 0 deletions modules/mono/editor/bindings_generator.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,11 @@ class BindingsGenerator {
*/
String proxy_name;

/**
* Hash of the ClassDB method
*/
uint64_t hash = 0;

/**
* [TypeInterface::name] of the return type
*/
Expand Down Expand Up @@ -165,6 +170,12 @@ class BindingsGenerator {
*/
bool is_internal = false;

/**
* Determines if the method is a compatibility method added to avoid breaking binary compatibility.
* These methods will be generated but hidden and are considered deprecated.
*/
bool is_compat = false;

List<ArgumentInterface> arguments;

const DocData::MethodDoc *method_doc = nullptr;
Expand Down Expand Up @@ -783,6 +794,9 @@ class BindingsGenerator {

void _populate_global_constants();

bool _method_has_conflicting_signature(const MethodInterface &p_imethod, const TypeInterface &p_itype);
bool _method_has_conflicting_signature(const MethodInterface &p_imethod_left, const MethodInterface &p_imethod_right);

Error _generate_cs_type(const TypeInterface &itype, const String &p_output_file);

Error _generate_cs_property(const TypeInterface &p_itype, const PropertyInterface &p_iprop, StringBuilder &p_output);
Expand Down
12 changes: 12 additions & 0 deletions modules/mono/glue/GodotSharp/GodotSharp/Core/GodotObject.base.cs
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,18 @@ internal static IntPtr ClassDB_get_method(StringName type, StringName method)
return methodBind;
}

internal static IntPtr ClassDB_get_method_with_compatibility(StringName type, StringName method, ulong hash)
{
var typeSelf = (godot_string_name)type.NativeValue;
var methodSelf = (godot_string_name)method.NativeValue;
IntPtr methodBind = NativeFuncs.godotsharp_method_bind_get_method_with_compatibility(typeSelf, methodSelf, hash);

if (methodBind == IntPtr.Zero)
throw new NativeMethodBindNotFoundException(type + "." + method);

return methodBind;
}

internal static unsafe delegate* unmanaged<IntPtr> ClassDB_get_constructor(StringName type)
{
// for some reason the '??' operator doesn't support 'delegate*'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ private partial struct UnmanagedCallbacks
public static partial IntPtr godotsharp_method_bind_get_method(in godot_string_name p_classname,
in godot_string_name p_methodname);

public static partial IntPtr godotsharp_method_bind_get_method_with_compatibility(
in godot_string_name p_classname, in godot_string_name p_methodname, ulong p_hash);

public static partial delegate* unmanaged<IntPtr> godotsharp_get_class_constructor(
in godot_string_name p_classname);

Expand Down
5 changes: 5 additions & 0 deletions modules/mono/glue/runtime_interop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ MethodBind *godotsharp_method_bind_get_method(const StringName *p_classname, con
return ClassDB::get_method(*p_classname, *p_methodname);
}

MethodBind *godotsharp_method_bind_get_method_with_compatibility(const StringName *p_classname, const StringName *p_methodname, uint64_t p_hash) {
return ClassDB::get_method_with_compatibility(*p_classname, *p_methodname, p_hash);
}

godotsharp_class_creation_func godotsharp_get_class_constructor(const StringName *p_classname) {
ClassDB::ClassInfo *class_info = ClassDB::classes.getptr(*p_classname);
if (class_info) {
Expand Down Expand Up @@ -1416,6 +1420,7 @@ void godotsharp_object_to_string(Object *p_ptr, godot_string *r_str) {
static const void *unmanaged_callbacks[]{
(void *)godotsharp_dotnet_module_is_initialized,
(void *)godotsharp_method_bind_get_method,
(void *)godotsharp_method_bind_get_method_with_compatibility,
(void *)godotsharp_get_class_constructor,
(void *)godotsharp_engine_get_singleton,
(void *)godotsharp_stack_info_vector_resize,
Expand Down

0 comments on commit 5f6082a

Please sign in to comment.