Skip to content

Commit

Permalink
Merge pull request #855 from Zylann/fix_issue854_virtual_methods
Browse files Browse the repository at this point in the history 
Fix deriving a custom class with virtual methods
  • Loading branch information
@akien-mga
akien-mga committed Oct 5, 2022
2 parents 4e5d0ee + 5de2c05 commit 91afc08
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 14 deletions.
11 changes: 8 additions & 3 deletions binding_generator.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1177,17 +1177,22 @@ def generate_engine_class_header(class_api, used_classes, fully_used_classes, us
result.append(method_signature + ";")

result.append("protected:")
result.append("\ttemplate <class T>")
# T is the custom class we want to register (from which the call initiates, going up the inheritance chain),
# B is its base class (can be a custom class too, that's why we pass it).
result.append("\ttemplate <class T, class B>")
result.append("\tstatic void register_virtuals() {")
if class_name != "Object":
result.append(f"\t\t{inherits}::register_virtuals<T>();")
result.append(f"\t\t{inherits}::register_virtuals<T, B>();")
if "methods" in class_api:
for method in class_api["methods"]:
if not method["is_virtual"]:
continue
method_name = escape_identifier(method["name"])
result.append(
f"\t\tif constexpr (!std::is_same_v<decltype(&{class_name}::{method_name}),decltype(&T::{method_name})>) {{"
# If the method is different from the base class, it means T overrides it, so it needs to be bound.
# Note that with an `if constexpr`, the code inside the `if` will not even be compiled if the
# condition returns false (in such cases it can't compile due to ambiguity).
f"\t\tif constexpr (!std::is_same_v<decltype(&B::{method_name}),decltype(&T::{method_name})>) {{"
)
result.append(f"\t\t\tBIND_VIRTUAL_METHOD(T, {method_name});")
result.append("\t\t}")
Expand Down
6 changes: 3 additions & 3 deletions include/godot_cpp/classes/wrapped.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,9 @@ protected:
return (::godot::String(::godot::Wrapped::*)()) & m_class::_to_string; \
} \
\
template <class T> \
template <class T, class B> \
static void register_virtuals() { \
m_inherits::register_virtuals<T>(); \
m_inherits::register_virtuals<T, B>(); \
} \
\
public: \
Expand All @@ -159,7 +159,7 @@ public:
m_inherits::initialize_class(); \
if (m_class::_get_bind_methods() != m_inherits::_get_bind_methods()) { \
_bind_methods(); \
m_inherits::register_virtuals<m_class>(); \
m_inherits::register_virtuals<m_class, m_inherits>(); \
} \
initialized = true; \
} \
Expand Down
10 changes: 7 additions & 3 deletions include/godot_cpp/core/class_db.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,12 @@ class ClassDB {
std::unordered_map<std::string, GDNativeExtensionClassCallVirtual> virtual_methods;
std::set<std::string> property_names;
std::set<std::string> constant_names;
// Pointer to the parent custom class, if any. Will be null if the parent class is a Godot class.
ClassInfo *parent_ptr = nullptr;
};

private:
// This may only contain custom classes, not Godot classes
static std::unordered_map<std::string, ClassInfo> classes;

static MethodBind *bind_methodfi(uint32_t p_flags, MethodBind *p_bind, const MethodDefinition &method_name, const void **p_defs, int p_defcount);
Expand Down Expand Up @@ -158,10 +160,12 @@ void ClassDB::register_class() {
cl.name = T::get_class_static();
cl.parent_name = T::get_parent_class_static();
cl.level = current_level;
classes[cl.name] = cl;
if (classes.find(cl.parent_name) != classes.end()) {
cl.parent_ptr = &classes[cl.parent_name];
std::unordered_map<std::string, ClassInfo>::iterator parent_it = classes.find(cl.parent_name);
if (parent_it != classes.end()) {
// Assign parent if it is also a custom class
cl.parent_ptr = &parent_it->second;
}
classes[cl.name] = cl;

// Register this class with Godot
GDNativeExtensionClassCreationInfo class_info = {
Expand Down
19 changes: 14 additions & 5 deletions src/core/class_db.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,20 +277,29 @@ void ClassDB::bind_integer_constant(const char *p_class_name, const char *p_enum
}

GDNativeExtensionClassCallVirtual ClassDB::get_virtual_func(void *p_userdata, const char *p_name) {
// This is called by Godot the first time it calls a virtual function, and it caches the result, per object instance.
// Because of this, it can happen from different threads at once.
// It should be ok not using any mutex as long as we only READ data.

const char *class_name = (const char *)p_userdata;

std::unordered_map<std::string, ClassInfo>::iterator type_it = classes.find(class_name);
ERR_FAIL_COND_V_MSG(type_it == classes.end(), nullptr, "Class doesn't exist.");

ClassInfo &type = type_it->second;
const ClassInfo *type = &type_it->second;

// Find method in current class, or any of its parent classes (Godot classes not included)
while (type != nullptr) {
std::unordered_map<std::string, GDNativeExtensionClassCallVirtual>::const_iterator method_it = type->virtual_methods.find(p_name);

std::unordered_map<std::string, GDNativeExtensionClassCallVirtual>::iterator method_it = type.virtual_methods.find(p_name);
if (method_it != type->virtual_methods.end()) {
return method_it->second;
}

if (method_it == type.virtual_methods.end()) {
return nullptr;
type = type->parent_ptr;
}

return method_it->second;
return nullptr;
}

void ClassDB::bind_virtual_method(const char *p_class, const char *p_method, GDNativeExtensionClassCallVirtual p_call) {
Expand Down

0 comments on commit 91afc08

Please sign in to comment.