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

Incorrect property types in extension_api.json #63066

Closed
VladoCC opened this issue Jul 16, 2022 · 2 comments · Fixed by #64428
Closed

Incorrect property types in extension_api.json #63066

VladoCC opened this issue Jul 16, 2022 · 2 comments · Fixed by #64428

Comments

@VladoCC
Copy link

VladoCC commented Jul 16, 2022

Godot version

4.0.alpha12

System information

Windows 10

Issue description

Information about properties of classes in extension_api.json appears to be incorrect in some cases.

There are cases where int turns into list of two elements:

  • SkeletonModificationStack2D and SkeletonModificationStack3D
    • Property: modification_count
    • Expected value (as in docs): int
    • Actual value (as in json): Modifications,modifications/
  • Curve, Curve2D and Curve3D
    • Property: point_count
    • Expected value (as in docs): int
    • Actual value (as in json): Points,point_
  • ItemList and PopupMenu
    • Property: item_count
    • Expected value (as in docs): int
    • Actual value (as in json): Items,item_
  • MenuButton and OptionButton
    • Property: item_count
    • Expected value (as in docs): int
    • Actual value (as in json): Items,popup/item_
  • SkeletonProfile
    • Property: group_size
    • Expected value (as in docs): int
    • Actual value (as in json): Groups,group/
    • Property: bone_size
    • Expected value (as in docs): int
    • Actual value (as in json): Bones,bone/
  • TabBar
    • Property: tab_count
    • Expected value (as in docs): int
    • Actual value (as in json): Tabs,tab_

And there are also a few of a different type. This ones are not documented, but can be accessed through GDScript.
Getters and setters are also empty strings for them.

  • AudioStreamRandomizer
    • Property: streams
    • Expected value (as in docs): not documented
    • Actual value (as in json): stream_
  • TileMap
    • Property: layers
    • Expected value (as in docs): not documented
    • Actual value (as in json): layer_
  • TileSet
    • Property: occlusion_layers
    • Expected value (as in docs): not documented
    • Actual value (as in json): occlusion_layer_
    • Property: physics_layers
    • Expected value (as in docs): not documented
    • Actual value (as in json): physics_layer_
    • Property: terrain_sets
    • Expected value (as in docs): not documented
    • Actual value (as in json): terrain_set_
    • Property: navigation_layers
    • Expected value (as in docs): not documented
    • Actual value (as in json): navigation_layer_
    • Property: custom_data_layers
    • Expected value (as in docs): not documented
    • Actual value (as in json): custom_data_layer_

Steps to reproduce

  1. Get extension_api.json
  2. Check for yourself

Minimal reproduction project

No response

@VladoCC
Copy link
Author

VladoCC commented Jul 16, 2022

While auto-completion for GDScript shows options described in part 2, this code:

TileSet.new().occlusion_layers

results in error Invalid get index 'occlusion_layers' (on base: 'TileSet').

@raulsntos
Copy link
Member

Those properties seem to all be using the ADD_ARRAY or ADD_ARRAY_COUNT macros. The macros add the property to ClassDB with this code:

void ClassDB::add_property_array_count(const StringName &p_class, const String &p_label, const StringName &p_count_property, const StringName &p_count_setter, const StringName &p_count_getter, const String &p_array_element_prefix, uint32_t p_count_usage) {
add_property(p_class, PropertyInfo(Variant::INT, p_count_property, PROPERTY_HINT_NONE, "", p_count_usage | PROPERTY_USAGE_ARRAY, vformat("%s,%s", p_label, p_array_element_prefix)), p_count_setter, p_count_getter);
}
void ClassDB::add_property_array(const StringName &p_class, const StringName &p_path, const String &p_array_element_prefix) {
OBJTYPE_WLOCK;
ClassInfo *type = classes.getptr(p_class);
ERR_FAIL_COND(!type);
type->property_list.push_back(PropertyInfo(Variant::NIL, p_path, PROPERTY_HINT_NONE, "", PROPERTY_USAGE_EDITOR | PROPERTY_USAGE_ARRAY, p_array_element_prefix));
}

Those methods create a PropertyInfo with the class_name set to {label},{prefix} (e.g.: Points,point_) or {prefix} (e.g.: stream_).

Then, extension_api_dump gets the type from the PropertyInfo with this code:

static String get_type_name(const PropertyInfo &p_info) {
if (p_info.type == Variant::INT && (p_info.hint == PROPERTY_HINT_INT_IS_POINTER)) {
if (p_info.hint_string.is_empty()) {
return "void*";
} else {
return p_info.hint_string + "*";
}
}
if (p_info.type == Variant::INT && (p_info.usage & (PROPERTY_USAGE_CLASS_IS_ENUM))) {
return String("enum::") + String(p_info.class_name);
}
if (p_info.type == Variant::INT && (p_info.usage & (PROPERTY_USAGE_CLASS_IS_BITFIELD))) {
return String("bitfield::") + String(p_info.class_name);
}
if (p_info.class_name != StringName()) {
return p_info.class_name;
}
if (p_info.hint == PROPERTY_HINT_RESOURCE_TYPE) {
return p_info.hint_string;
}
if (p_info.type == Variant::NIL && (p_info.usage & PROPERTY_USAGE_NIL_IS_VARIANT)) {
return "Variant";
}
if (p_info.type == Variant::NIL) {
return "void";
}
return Variant::get_type_name(p_info.type);
}

And ends up returning when class_name is not empty and uses it as the type which is incorrect in this case.

When the property usage is set to PROPERTY_USAGE_ARRAY the class_name is used by the EditorInspector to create a dedicated array editor for the array.

I guess we need to add a check for PROPERTY_USAGE_ARRAY in extension_api_dump::get_type_name like this:

diff --git a/core/extension/extension_api_dump.cpp b/core/extension/extension_api_dump.cpp
index d5c49b01e9..fb07103a4c 100644
--- a/core/extension/extension_api_dump.cpp
+++ b/core/extension/extension_api_dump.cpp
@@ -49,6 +49,9 @@ static String get_type_name(const PropertyInfo &p_info) {
 	if (p_info.type == Variant::INT && (p_info.usage & (PROPERTY_USAGE_CLASS_IS_ENUM | PROPERTY_USAGE_CLASS_IS_BITFIELD))) {
 		return String("enum::") + String(p_info.class_name);
 	}
+	if (p_info.type == Variant::INT && (p_info.usage & PROPERTY_USAGE_ARRAY)) {
+		return "int";
+	}
 	if (p_info.class_name != StringName()) {
 		return p_info.class_name;
 	}

But I don't know if this is entirely correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants