Merge pull request godotengine#107005 from TokageItLab/fix-anim-node-param-type

akien-mga · akien-mga · commit e5c8fb8f1dbc · 2025-06-05T13:12:43.000+02:00
Fix ambiguous AnimationNode's parameter type in default value and make `validate_type_match()` static function
diff --git a/scene/animation/animation_blend_space_1d.cpp b/scene/animation/animation_blend_space_1d.cpp
@@ -45,9 +45,9 @@ Variant AnimationNodeBlendSpace1D::get_parameter_default_value(const StringName
 	}
 
 	if (p_parameter == closest) {
-		return -1;
+		return (int)-1;
 	} else {
-		return 0;
+		return 0.0;
 	}
 }
 
diff --git a/scene/animation/animation_blend_space_2d.cpp b/scene/animation/animation_blend_space_2d.cpp
@@ -46,7 +46,7 @@ Variant AnimationNodeBlendSpace2D::get_parameter_default_value(const StringName
 	}
 
 	if (p_parameter == closest) {
-		return -1;
+		return (int)-1;
 	} else {
 		return Vector2();
 	}
diff --git a/scene/animation/animation_blend_tree.cpp b/scene/animation/animation_blend_tree.cpp
@@ -55,7 +55,7 @@ Variant AnimationNodeAnimation::get_parameter_default_value(const StringName &p_
 	if (p_parameter == backward) {
 		return false;
 	}
-	return 0;
+	return 0.0;
 }
 
 AnimationNode::NodeTimeInfo AnimationNodeAnimation::get_node_time_info() const {
@@ -438,10 +438,10 @@ Variant AnimationNodeOneShot::get_parameter_default_value(const StringName &p_pa
 	} else if (p_parameter == active || p_parameter == internal_active) {
 		return false;
 	} else if (p_parameter == time_to_restart) {
-		return -1;
-	} else {
-		return 0.0;
+		return -1.0;
 	}
+
+	return 0.0;
 }
 
 bool AnimationNodeOneShot::is_parameter_read_only(const StringName &p_parameter) const {
@@ -774,7 +774,7 @@ Variant AnimationNodeAdd2::get_parameter_default_value(const StringName &p_param
 		return ret;
 	}
 
-	return 0;
+	return 0.0;
 }
 
 String AnimationNodeAdd2::get_caption() const {
@@ -815,7 +815,7 @@ Variant AnimationNodeAdd3::get_parameter_default_value(const StringName &p_param
 		return ret;
 	}
 
-	return 0;
+	return 0.0;
 }
 
 String AnimationNodeAdd3::get_caption() const {
@@ -859,7 +859,7 @@ Variant AnimationNodeBlend2::get_parameter_default_value(const StringName &p_par
 		return ret;
 	}
 
-	return 0; // For blend amount.
+	return 0.0; // For blend amount.
 }
 
 String AnimationNodeBlend2::get_caption() const {
@@ -900,7 +900,7 @@ Variant AnimationNodeBlend3::get_parameter_default_value(const StringName &p_par
 		return ret;
 	}
 
-	return 0; // For blend amount.
+	return 0.0; // For blend amount.
 }
 
 String AnimationNodeBlend3::get_caption() const {
@@ -940,7 +940,7 @@ Variant AnimationNodeSub2::get_parameter_default_value(const StringName &p_param
 		return ret;
 	}
 
-	return 0;
+	return 0.0;
 }
 
 String AnimationNodeSub2::get_caption() const {
@@ -1146,7 +1146,7 @@ Variant AnimationNodeTransition::get_parameter_default_value(const StringName &p
 	if (p_parameter == prev_xfading) {
 		return 0.0;
 	} else if (p_parameter == prev_index || p_parameter == current_index) {
-		return -1;
+		return (int)-1;
 	} else {
 		return String();
 	}
diff --git a/scene/animation/animation_tree.cpp b/scene/animation/animation_tree.cpp
@@ -77,7 +77,11 @@ void AnimationNode::set_parameter(const StringName &p_name, const Variant &p_val
 
 	const AHashMap<StringName, int>::Iterator it = property_cache.find(p_name);
 	if (it) {
-		process_state->tree->property_map.get_by_index(it->value).value.first = p_value;
+		Pair<Variant, bool> &prop = process_state->tree->property_map.get_by_index(it->value).value;
+		Variant value = p_value;
+		if (Animation::validate_type_match(prop.first, value)) {
+			prop.first = value;
+		}
 		return;
 	}
 
@@ -919,7 +923,11 @@ bool AnimationTree::_set(const StringName &p_name, const Variant &p_value) {
 		if (is_inside_tree() && property_map[p_name].second) {
 			return false; // Prevent to set property by user.
 		}
-		property_map[p_name].first = p_value;
+		Pair<Variant, bool> &prop = property_map[p_name];
+		Variant value = p_value;
+		if (Animation::validate_type_match(prop.first, value)) {
+			prop.first = value;
+		}
 		return true;
 	}
 
diff --git a/scene/animation/tween.cpp b/scene/animation/tween.cpp
@@ -75,20 +75,6 @@ void Tweener::_bind_methods() {
 	ADD_SIGNAL(MethodInfo("finished"));
 }
 
-bool Tween::_validate_type_match(const Variant &p_from, Variant &r_to) {
-	if (p_from.get_type() != r_to.get_type()) {
-		// Cast r_to between double and int to avoid minor annoyances.
-		if (p_from.get_type() == Variant::FLOAT && r_to.get_type() == Variant::INT) {
-			r_to = double(r_to);
-		} else if (p_from.get_type() == Variant::INT && r_to.get_type() == Variant::FLOAT) {
-			r_to = int(r_to);
-		} else {
-			ERR_FAIL_V_MSG(false, "Type mismatch between initial and final value: " + Variant::get_type_name(p_from.get_type()) + " and " + Variant::get_type_name(r_to.get_type()));
-		}
-	}
-	return true;
-}
-
 void Tween::_start_tweeners() {
 	if (tweeners.is_empty()) {
 		dead = true;
@@ -122,7 +108,7 @@ Ref<PropertyTweener> Tween::tween_property(const Object *p_target, const NodePat
 	const Variant &prop_value = p_target->get_indexed(property_subnames);
 #endif
 
-	if (!_validate_type_match(prop_value, p_to)) {
+	if (!Animation::validate_type_match(prop_value, p_to)) {
 		return nullptr;
 	}
 
@@ -153,7 +139,7 @@ Ref<CallbackTweener> Tween::tween_callback(const Callable &p_callback) {
 Ref<MethodTweener> Tween::tween_method(const Callable &p_callback, const Variant p_from, Variant p_to, double p_duration) {
 	CHECK_VALID();
 
-	if (!_validate_type_match(p_from, p_to)) {
+	if (!Animation::validate_type_match(p_from, p_to)) {
 		return nullptr;
 	}
 
@@ -562,7 +548,7 @@ Ref<PropertyTweener> PropertyTweener::from(const Variant &p_value) {
 	ERR_FAIL_COND_V(tween.is_null(), nullptr);
 
 	Variant from_value = p_value;
-	if (!tween->_validate_type_match(final_val, from_value)) {
+	if (!Animation::validate_type_match(final_val, from_value)) {
 		return nullptr;
 	}
 
diff --git a/scene/animation/tween.h b/scene/animation/tween.h
@@ -135,7 +135,6 @@ class Tween : public RefCounted {
 
 	void _start_tweeners();
 	void _stop_internal(bool p_reset);
-	bool _validate_type_match(const Variant &p_from, Variant &r_to);
 
 protected:
 	static void _bind_methods();
diff --git a/scene/resources/animation.cpp b/scene/resources/animation.cpp
@@ -5677,6 +5677,20 @@ bool Animation::is_variant_interpolatable(const Variant p_value) {
 	return (type >= Variant::BOOL && type <= Variant::STRING_NAME) || type == Variant::ARRAY || type >= Variant::PACKED_INT32_ARRAY; // PackedByteArray is unsigned, so it would be better to ignore since blending uses float.
 }
 
+bool Animation::validate_type_match(const Variant &p_from, Variant &r_to) {
+	if (p_from.get_type() != r_to.get_type()) {
+		// Cast r_to between double and int to avoid minor annoyances.
+		if (p_from.get_type() == Variant::FLOAT && r_to.get_type() == Variant::INT) {
+			r_to = double(r_to);
+		} else if (p_from.get_type() == Variant::INT && r_to.get_type() == Variant::FLOAT) {
+			r_to = int(r_to);
+		} else {
+			ERR_FAIL_V_MSG(false, "Type mismatch between initial and final value: " + Variant::get_type_name(p_from.get_type()) + " and " + Variant::get_type_name(r_to.get_type()));
+		}
+	}
+	return true;
+}
+
 Variant Animation::cast_to_blendwise(const Variant p_value) {
 	switch (p_value.get_type()) {
 		case Variant::BOOL:
diff --git a/scene/resources/animation.h b/scene/resources/animation.h
@@ -542,6 +542,7 @@ class Animation : public Resource {
 
 	// Helper functions for Variant.
 	static bool is_variant_interpolatable(const Variant p_value);
+	static bool validate_type_match(const Variant &p_from, Variant &r_to);
 
 	static Variant cast_to_blendwise(const Variant p_value);
 	static Variant cast_from_blendwise(const Variant p_value, const Variant::Type p_type);

Original file line number	Diff line number	Diff line change
`@@ -45,9 +45,9 @@ Variant AnimationNodeBlendSpace1D::get_parameter_default_value(const StringName`
`45`	`45`	`}`
`46`	`46`
`47`	`47`	`if (p_parameter == closest) {`
`48`		`- return -1;`
	`48`	`+ return (int)-1;`
`49`	`49`	`} else {`
`50`		`- return 0;`
	`50`	`+ return 0.0;`
`51`	`51`	`}`
`52`	`52`	`}`
`53`	`53`
Original file line number	Diff line number	Diff line change
`@@ -46,7 +46,7 @@ Variant AnimationNodeBlendSpace2D::get_parameter_default_value(const StringName`
`46`	`46`	`}`
`47`	`47`
`48`	`48`	`if (p_parameter == closest) {`
`49`		`- return -1;`
	`49`	`+ return (int)-1;`
`50`	`50`	`} else {`
`51`	`51`	`return Vector2();`
`52`	`52`	`}`
Original file line number	Diff line number	Diff line change
`@@ -55,7 +55,7 @@ Variant AnimationNodeAnimation::get_parameter_default_value(const StringName &p_`
`55`	`55`	`if (p_parameter == backward) {`
`56`	`56`	`return false;`
`57`	`57`	`}`
`58`		`- return 0;`
	`58`	`+ return 0.0;`
`59`	`59`	`}`
`60`	`60`
`61`	`61`	`AnimationNode::NodeTimeInfo AnimationNodeAnimation::get_node_time_info() const {`
`@@ -438,10 +438,10 @@ Variant AnimationNodeOneShot::get_parameter_default_value(const StringName &p_pa`
`438`	`438`	`} else if (p_parameter == active \|\| p_parameter == internal_active) {`
`439`	`439`	`return false;`
`440`	`440`	`} else if (p_parameter == time_to_restart) {`
`441`		`- return -1;`
`442`		`- } else {`
`443`		`- return 0.0;`
	`441`	`+ return -1.0;`
`444`	`442`	`}`
	`443`	`+`
	`444`	`+ return 0.0;`
`445`	`445`	`}`
`446`	`446`
`447`	`447`	`bool AnimationNodeOneShot::is_parameter_read_only(const StringName &p_parameter) const {`
`@@ -774,7 +774,7 @@ Variant AnimationNodeAdd2::get_parameter_default_value(const StringName &p_param`
`774`	`774`	`return ret;`
`775`	`775`	`}`
`776`	`776`
`777`		`- return 0;`
	`777`	`+ return 0.0;`
`778`	`778`	`}`
`779`	`779`
`780`	`780`	`String AnimationNodeAdd2::get_caption() const {`
`@@ -815,7 +815,7 @@ Variant AnimationNodeAdd3::get_parameter_default_value(const StringName &p_param`
`815`	`815`	`return ret;`
`816`	`816`	`}`
`817`	`817`
`818`		`- return 0;`
	`818`	`+ return 0.0;`
`819`	`819`	`}`
`820`	`820`
`821`	`821`	`String AnimationNodeAdd3::get_caption() const {`
`@@ -859,7 +859,7 @@ Variant AnimationNodeBlend2::get_parameter_default_value(const StringName &p_par`
`859`	`859`	`return ret;`
`860`	`860`	`}`
`861`	`861`
`862`		`- return 0; // For blend amount.`
	`862`	`+ return 0.0; // For blend amount.`
`863`	`863`	`}`
`864`	`864`
`865`	`865`	`String AnimationNodeBlend2::get_caption() const {`
`@@ -900,7 +900,7 @@ Variant AnimationNodeBlend3::get_parameter_default_value(const StringName &p_par`
`900`	`900`	`return ret;`
`901`	`901`	`}`
`902`	`902`
`903`		`- return 0; // For blend amount.`
	`903`	`+ return 0.0; // For blend amount.`
`904`	`904`	`}`
`905`	`905`
`906`	`906`	`String AnimationNodeBlend3::get_caption() const {`
`@@ -940,7 +940,7 @@ Variant AnimationNodeSub2::get_parameter_default_value(const StringName &p_param`
`940`	`940`	`return ret;`
`941`	`941`	`}`
`942`	`942`
`943`		`- return 0;`
	`943`	`+ return 0.0;`
`944`	`944`	`}`
`945`	`945`
`946`	`946`	`String AnimationNodeSub2::get_caption() const {`
`@@ -1146,7 +1146,7 @@ Variant AnimationNodeTransition::get_parameter_default_value(const StringName &p`
`1146`	`1146`	`if (p_parameter == prev_xfading) {`
`1147`	`1147`	`return 0.0;`
`1148`	`1148`	`} else if (p_parameter == prev_index \|\| p_parameter == current_index) {`
`1149`		`- return -1;`
	`1149`	`+ return (int)-1;`
`1150`	`1150`	`} else {`
`1151`	`1151`	`return String();`
`1152`	`1152`	`}`