Avoid unnecessary Dictionary conversions in GDScriptInstance::validate_property

[aaronp64]

Updated GDScriptInstance::validate_property to only convert PropertyInfo to Dictionary if _validate_property function is found.

This can make Node.duplicate around 4-5x faster for nodes that have gdscript attached, when the script doesn't override _validate_property. Compared with scripts below:

TestClass:

class_name TestClass extends Node2D

var a
var b
var c
var d

#func _validate_property(property: Dictionary) -> void:
#	pass

Main Scene:

extends Node

func _ready():
	var node := TestClass.new()
	var start := Time.get_ticks_msec()
	for i in 1000:
		add_child(node.duplicate())
	var end := Time.get_ticks_msec()
	print("%dms" % (end - start))

Old:

139ms
238ms (_validate_property uncommented)

New:

28ms
239ms (_validate_property uncommented)

[dalexeev]

• This probably fixes/improves Many properties in _get_property_list() slows down instantiate() by 2-8x. #97829.

[dalexeev]

Looks good to me. I haven't tested it, but it seems reasonable in any case.

The only issue is the return condition of the function, _validate_property() is not a multi-level call, unlike _get_property_list() (see #81139). But the function call error is some kind of undefined behavior, so we can deal with it later (I mean move the return on line 1850 out of the conditional block).

C# and GDExtension seem to be unaffected by the bug.

godot/modules/mono/csharp_script.cpp

Lines 1594 to 1610 in 4631a61

	void CSharpInstance::validate_property(PropertyInfo &p_property) const {
	ERR_FAIL_COND(!script.is_valid());
	Variant property_arg = (Dictionary)p_property;
	const Variant *args[1] = { &property_arg };
	Variant ret;
	Callable::CallError call_error;
	GDMonoCache::managed_callbacks.CSharpInstanceBridge_Call(
	gchandle.get_intptr(), &SNAME("_validate_property"), args, 1, &call_error, &ret);
	if (call_error.error != Callable::CallError::CALL_OK) {
	return;
	}
	p_property = PropertyInfo::from_dict(property_arg);
	}

godot/core/object/script_language_extension.h

Lines 757 to 778 in 4631a61

	virtual void validate_property(PropertyInfo &p_property) const override {
	if (native_info->validate_property_func) {
	// GDExtension uses a StringName rather than a String for property name.
	StringName prop_name = p_property.name;
	GDExtensionPropertyInfo gdext_prop = {
	(GDExtensionVariantType)p_property.type,
	&prop_name,
	&p_property.class_name,
	(uint32_t)p_property.hint,
	&p_property.hint_string,
	p_property.usage,
	};
	if (native_info->validate_property_func(instance, &gdext_prop)) {
	p_property.type = (Variant::Type)gdext_prop.type;
	p_property.name = *reinterpret_cast<StringName *>(gdext_prop.name);
	p_property.class_name = *reinterpret_cast<StringName *>(gdext_prop.class_name);
	p_property.hint = (PropertyHint)gdext_prop.hint;
	p_property.hint_string = *reinterpret_cast<String *>(gdext_prop.hint_string);
	p_property.usage = gdext_prop.usage;
	}
	}
	}

[clayjohn]

@dalexeev Is the issue you mention a blocking issue for merging this PR or something that should be handled separately?

[dalexeev]

@clayjohn It is not a blocking issue.

[Repiteo]

Thanks!