Correct RichTextLabel.custom_effects property type metadata

[touilleMan]

Current entry in api.json:

{
	"name": "custom_effects",
	"type": "17/17:RichTextEffect",
	"getter": "get_effects",
	"setter": "set_effects",
	"index": -1
},

I couldn't figure out the meaning of 17/17:RichTextEffect and didn't see any other places where this is used.

The correction mimic what is done in RDPipelineColorBlendState::attachments:

godot/servers/rendering/rendering_device_binds.h

Line 625 in e387278

ADD_PROPERTY(PropertyInfo(Variant::ARRAY, "attachments", PROPERTY_HINT_ARRAY_TYPE, "RDPipelineColorBlendStateAttachment"), "set_attachments", "get_attachments");

[akien-mga]

Thanks!

[akien-mga]

Cherry-picked for 3.2.3. Edit: Reverted.

[akien-mga]

Undid the cherry-pick, there are no typed arrays in 3.2.

[EricEzaM]

Unfortunately this broke things. I agree the syntax in the ADD_PROPERTY() is bad with the slash and the colon, but this is not the way to solve it.

Anyway, this broke customising of effects in RichTextLabel. This is the only place an Array of Resource type is used, until I needed to do the same thing, which led me down the rabbit hole to find this bug.

17/17 corresponds to PROPERTY_HINT_RESOURCE_TYPE and Variant::OBJECT (which were both enum value 17 at the time). This was used here:

godot/editor/editor_properties_array_dict.cpp

Lines 550 to 567 in 0638ca0

	void EditorPropertyArray::setup(Variant::Type p_array_type, const String &p_hint_string) {
	array_type = p_array_type;
	if (array_type == Variant::ARRAY && !p_hint_string.is_empty()) {
	int hint_subtype_separator = p_hint_string.find(":");
	if (hint_subtype_separator >= 0) {
	String subtype_string = p_hint_string.substr(0, hint_subtype_separator);
	int slash_pos = subtype_string.find("/");
	if (slash_pos >= 0) {
	subtype_hint = PropertyHint(subtype_string.substr(slash_pos + 1, subtype_string.size() - slash_pos - 1).to_int());
	subtype_string = subtype_string.substr(0, slash_pos);
	}
	subtype_hint_string = p_hint_string.substr(hint_subtype_separator + 1, p_hint_string.size() - hint_subtype_separator - 1);
	subtype = Variant::Type(subtype_string.to_int());
	}
	}
	}

See all those slices looking for : and /... yeah... that is where it was used.

The PROPERTY_HINT corresponds to the sub-type, meaning RichTextEffect. The type of the property itself is already defined as Variant::ARRAY. So saying PROPERTY_HINT_ARRAY is wrong. It was correct in being PROPERTY_HINT_RESOURCE_TYPE, although ironically that value was not used, instead the "17"'s in the string were used...

Previously when you opened the menu for custom effects, you would only see this:

After this change, you would see the whole list because the editor did not know what to do.

This begs the question... did you test the changes before submitting the PR? If you built, ran and tested before submitting you would have seen that the custom_effects property was broken after your change, and known that something went wrong. Doesn't take long to test, and would have saved me a truckload of time 1 year down the line, finding the issue. Especially considering that the syntax of the hint string was so abstract and strange, I think it should have raised red flags that it was like that for a reason.