Skip to content
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

GDExtension: Fix setting base class properties on a runtime class #94089

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 44 additions & 19 deletions core/object/class_db.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,34 +76,46 @@ class PlaceholderExtensionInstance {
StringName class_name;
HashMap<StringName, Variant> properties;

// Checks if a property is from a runtime class, and not a non-runtime base class.
bool is_runtime_property(const StringName &p_property_name) {
StringName current_class_name = class_name;

while (ClassDB::is_class_runtime(current_class_name)) {
if (ClassDB::has_property(current_class_name, p_property_name, true)) {
return true;
}

current_class_name = ClassDB::get_parent_class(current_class_name);
}

return false;
}

public:
PlaceholderExtensionInstance(const StringName &p_class_name) {
class_name = p_class_name;
}

~PlaceholderExtensionInstance() {}

void set(const StringName &p_name, const Variant &p_value) {
bool is_default_valid = false;
Variant default_value = ClassDB::class_get_default_property_value(class_name, p_name, &is_default_valid);

// If there's a default value, then we know it's a valid property.
if (is_default_valid) {
void set(const StringName &p_name, const Variant &p_value, bool &r_valid) {
r_valid = is_runtime_property(p_name);
if (r_valid) {
properties[p_name] = p_value;
}
}

Variant get(const StringName &p_name) {
Variant get(const StringName &p_name, bool &r_valid) {
const Variant *value = properties.getptr(p_name);
Variant ret;

if (value) {
ret = *value;
r_valid = true;
} else {
bool is_default_valid = false;
Variant default_value = ClassDB::class_get_default_property_value(class_name, p_name, &is_default_valid);
if (is_default_valid) {
ret = default_value;
r_valid = is_runtime_property(p_name);
if (r_valid) {
ret = ClassDB::class_get_default_property_value(class_name, p_name);
}
}

Expand All @@ -115,21 +127,21 @@ class PlaceholderExtensionInstance {
const StringName &name = *(StringName *)p_name;
const Variant &value = *(const Variant *)p_value;

self->set(name, value);
bool valid = false;
self->set(name, value, valid);

// We have to return true so Godot doesn't try to call the real setter function.
return true;
return valid;
}

static GDExtensionBool placeholder_instance_get(GDExtensionClassInstancePtr p_instance, GDExtensionConstStringNamePtr p_name, GDExtensionVariantPtr r_ret) {
PlaceholderExtensionInstance *self = (PlaceholderExtensionInstance *)p_instance;
const StringName &name = *(StringName *)p_name;
Variant *value = (Variant *)r_ret;

*value = self->get(name);
bool valid = false;
*value = self->get(name, valid);

// We have to return true so Godot doesn't try to call the real getter function.
return true;
return valid;
}

static const GDExtensionPropertyInfo *placeholder_instance_get_property_list(GDExtensionClassInstancePtr p_instance, uint32_t *r_count) {
Expand Down Expand Up @@ -172,9 +184,9 @@ class PlaceholderExtensionInstance {
static GDExtensionObjectPtr placeholder_class_create_instance(void *p_class_userdata) {
ClassDB::ClassInfo *ti = (ClassDB::ClassInfo *)p_class_userdata;

// Find the closest native parent.
// Find the closest native parent, that isn't a runtime class.
ClassDB::ClassInfo *native_parent = ti->inherits_ptr;
while (native_parent->gdextension) {
while (native_parent->gdextension || native_parent->is_runtime) {
native_parent = native_parent->inherits_ptr;
}
ERR_FAIL_NULL_V(native_parent->creation_func, nullptr);
Expand Down Expand Up @@ -1952,6 +1964,14 @@ bool ClassDB::is_class_reloadable(const StringName &p_class) {
return ti->reloadable;
}

bool ClassDB::is_class_runtime(const StringName &p_class) {
OBJTYPE_RLOCK;

ClassInfo *ti = classes.getptr(p_class);
ERR_FAIL_NULL_V_MSG(ti, false, "Cannot get class '" + String(p_class) + "'.");
return ti->is_runtime;
}

void ClassDB::add_resource_base_extension(const StringName &p_extension, const StringName &p_class) {
if (resource_base_extensions.has(p_extension)) {
return;
Expand Down Expand Up @@ -2063,6 +2083,11 @@ void ClassDB::register_extension_class(ObjectGDExtension *p_extension) {

ClassInfo *parent = classes.getptr(p_extension->parent_class_name);

#ifdef TOOLS_ENABLED
// @todo This is a limitation of the current implementation, but it should be possible to remove.
ERR_FAIL_COND_MSG(p_extension->is_runtime && parent->gdextension && !parent->is_runtime, "Extension runtime class " + String(p_extension->class_name) + " cannot descend from " + parent->name + " which isn't also a runtime class");
#endif

ClassInfo c;
c.api = p_extension->editor_class ? API_EDITOR_EXTENSION : API_EXTENSION;
c.gdextension = p_extension;
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 @@ -460,6 +460,7 @@ class ClassDB {

static bool is_class_exposed(const StringName &p_class);
static bool is_class_reloadable(const StringName &p_class);
static bool is_class_runtime(const StringName &p_class);

static void add_resource_base_extension(const StringName &p_extension, const StringName &p_class);
static void get_resource_base_extensions(List<String> *p_extensions);
Expand Down
Loading