Expose `ClassDB::get_api_type()` `ClassDB::get_property_getter()` `ClassDB::get_property_setter()` method

[ZerxZ]

Expose ClassDB::get_api_type() ClassDB::get_property_getter() ClassDB::get_property_setter() for editor-plugins and other languages wrapper generator. https://github.com/godotengine/godot-proposals/issues/9526

[ZerxZ]

CI Tests Godot Cpp test fails cause. It is because godot-cpp repository binding_generator.py script replaces extension_api.json, ClassDB with ClassDBSingleton. But it does not change return_type or arguments_type the enumeration ClassDB::APIType to ClassDBSingleton::APIType. @dsnopek @AThousandShips

[dsnopek]

I just posted PR https://github.com/godotengine/godot-cpp/pull/1445 which will fix godot-cpp with these changes

[ZerxZ]

I just posted PR godotengine/godot-cpp#1445 which will fix godot-cpp with these changes

Thanks @dsnopek @AThousandShips ! I've already rebase the branch in single comment and run in local you PR success build and work.

[ZerxZ]

https://github.com/godotengine/godot/blob/048d9d328cc748d4f8c67a13345d22ac24dd13f0/core/core_bind.cpp#L1565-L1586

The first_argument_is_class variable in this code is becoming increasingly lengthy and less maintainable. Would it be more suitable to change it to a HashSet<String> field to store these names, and then use the HashSet<String> field to check for existence?

 void ClassDB::get_argument_options(const StringName &p_function, int p_idx, List<String> *r_options) const { 
   const String pf = p_function; 
- 	bool first_argument_is_class = false; 
- 	if (p_idx == 0) { 
- 		first_argument_is_class = (pf == "get_inheriters_from_class" || pf == "get_parent_class" || 
- 				pf == "class_exists" || pf == "can_instantiate" || pf == "instantiate" || 
- 				pf == "class_has_signal" || pf == "class_get_signal" || pf == "class_get_signal_list" || 
- 				pf == "class_get_property_list" || pf == "class_get_property" || pf == "class_set_property" || 
- 				pf == "class_has_method" || pf == "class_get_method_list" || 
- 				pf == "class_get_integer_constant_list" || pf == "class_has_integer_constant" || pf == "class_get_integer_constant" || 
- 				pf == "class_has_enum" || pf == "class_get_enum_list" || pf == "class_get_enum_constants" || pf == "class_get_integer_constant_enum" || 
- 				pf == "is_class_enabled" || pf == "is_class_enum_bitfield" || pf == "class_get_api_type" || 
-				pf == "class_get_property_getter" || pf == "class_get_property_setter"); 
- 	} 
+ 	bool first_argument_is_class = p_idx == 0 && has_first_argument_class_methods(pf);
 	if (first_argument_is_class || pf == "is_parent_class") { 
 		for (const String &E : get_class_list()) { 
 			r_options->push_back(E.quote()); 
 		} 
 	} 
  
 	Object::get_argument_options(p_function, p_idx, r_options); 
 } 

+ HashSet<String> first_argument_class_methods;
+ bool has_first_argument_class_methods(const String p_function) const { 
+	if (!first_argument_class_methods.is_empty()) {
+		return first_argument_class_methods.has(p_function);
+	}
+	auto class_methods = {
+		"get_inheriters_from_class",
+		"get_parent_class",
+		"class_exists",
+		"is_parent_class",
+		"can_instantiate",
+		"instantiate",
+		"class_has_signal",
+		"class_get_signal",
+		"class_get_signal_list",
+		"class_get_property_list",
+		"class_get_property",
+		"class_set_property",
+		"class_has_method",
+		"class_get_method_list",
+		"class_get_integer_constant_list",
+		"class_has_integer_constant",
+		"class_get_integer_constant",
+		"class_has_enum",
+		"class_get_enum_list",
+		"class_get_enum_constants",
+		"class_get_integer_constant_enum",
+		"is_class_enabled",
+		"is_class_enum_bitfield",
+		"class_get_api_type",
+		"class_get_property_getter",
+		"class_get_property_setter"
+	};
+	bool result = false;
+	for (const String &E : class_methods) {
+		first_argument_class_methods.insert(E);
+		if (E == p_function) {
+			result = true;
+		}
+	}
+	return result;
+}

[dsnopek]

The first_argument_is_class variable in this code is becoming increasingly lengthy and less maintainable. Would it be more suitable to change it to a HashSet<String> field to store these names, and then use the HashSet<String> field to check for existence?

I would just leave this as it is for now.

Overall, this PR is looking good to me!

CI is complaining about an extra line in the docs XML:

diff --git a/doc/classes/ClassDB.xml b/doc/classes/ClassDB.xml
index ed670fa..6a58536 100644
--- a/doc/classes/ClassDB.xml
+++ b/doc/classes/ClassDB.xml
@@ -106,7 +106,6 @@
 				Returns the default value of [param property] of [param class] or its ancestor classes.
 			</description>
 		</method>
-
 		<method name="class_get_property_getter" qualifiers="const">
 			<return type="StringName" />
 			<param index="0" name="class" type="StringName" />

However, aside from that, the main thing blocking this is that we need to get godot-cpp PR https://github.com/godotengine/godot-cpp/pull/1445 merged and cherry-picked so that the rest of the tests can pass here.

[ZerxZ]

I would just leave this as it is for now.

You're right. I was just leaving a comment to ask for opinions, and I didn't intend to modify that section of the code. It's not within the scope of this PR.

CI is complaining about an extra line in the docs XML:

I'll fix that section of the code right away. It was my oversight, I didn't notice the extra line breaks.

However, aside from that, the main thing blocking this is that we need to get godot-cpp PR https://github.com/godotengine/godot-cpp/pull/1445 merged and cherry-picked so that the rest of the tests can pass here.

I can only wait for your PR to be merged to pass the tests smoothly. Thanks for taking the time to look at this PR over the past few days. I thought not many people would have the free time to review it. Also, thanks for your hard work, @AThousandShips @dsnopek .

[dsnopek]

Once PR https://github.com/godotengine/godot-cpp/pull/1465 is merged (which cherry-picks the necessary changes to binding_generator.py), then the tests here should finally start passing

[ZerxZ]

Once PR godotengine/godot-cpp#1465 is merged (which cherry-picks the necessary changes to binding_generator.py), then the tests here should finally start passing

Thanks!

[ZerxZ]

cc @AThousandShips @dsnopek

[Delsin-Yu]

Any chance we can have this in 4.3? As the PR has approved.

[AThousandShips]

We're in feature freeze so I don't expect so, but we'll see what the production team says, I suspect 4.4

[dalexeev]

• See #94826.

I'm not sure about class_get_api_type().

[ZerxZ]

• See Expose ClassDB methods get_property_setter / get_property_getter #94826.

I'm not sure about class_get_api_type().

I think the class_get_api_type part is still necessary. It is mainly used to distinguish between Native Class Type and GDExtension Class Type. Seeing that others PR have merged get_property_getter and set_property_getter. so I just need to keep the class_get_api_type one? @AThousandShips @dalexeev @dsnopek @Naros

[dalexeev]

so I just need to keep the class_get_api_type one?

I think yes, since the other two methods are already exposed.

[ZerxZ]

so I just need to keep the class_get_api_type one?

I think yes, since the other two methods are already exposed.

I've removed that part and just kept the class_get_api_type part. Can you see if it's working? @dalexeev @akien-mga