GDExtension: Fix setting base class properties on a runtime class

chryan · Aug 6, 2024 · 94f978d · 94f978d
1 parent d3fc7cb
commit 94f978d
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 19 deletions.
diff --git a/core/object/class_db.cpp b/core/object/class_db.cpp
@@ -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);
 			}
 		}
 
@@ -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) {
@@ -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);
@@ -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;
@@ -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;

diff --git a/core/object/class_db.h b/core/object/class_db.h
@@ -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);