-
-
Notifications
You must be signed in to change notification settings - Fork 21.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
C#: Generate and use compat methods #80527
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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" | ||
|
@@ -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"); | ||
|
||
|
@@ -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" | ||
|
@@ -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 { | ||
|
@@ -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"; | ||
} | ||
|
||
|
@@ -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 "); | ||
|
||
|
@@ -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(); | ||
|
||
|
@@ -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()) { | ||
|
@@ -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; | ||
|
@@ -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(); | ||
|
||
|
@@ -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) { | ||
|
@@ -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; | ||
|
@@ -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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comparing the Unfortunately, we don't have access to the |
||
// 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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,6 +42,9 @@ private partial struct UnmanagedCallbacks | |
public static partial IntPtr godotsharp_method_bind_get_method(in godot_string_name p_classname, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think |
||
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); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This warning is noisy and there's nothing we can do about the warning anyway, so maybe it's better to remove it, or at least only print it with
--verbose
. I just wanted to make sure we were aware of ignored methods, but maybe it's not that important.