Merge pull request #1081 from Spartan322/4.4-cherry-pick/animation-bugs

Logan-ReXDev · web-flow · commit 0e1487132fef · 2025-08-18T05:24:03.000-04:00
[4.4] Cherry-picks for the 4.4 branch - 1st animation bugs batch
diff --git a/doc/classes/SkeletonModifier3D.xml b/doc/classes/SkeletonModifier3D.xml
@@ -19,6 +19,20 @@
 				[method _process_modification] must not apply [member influence] to bone poses because the [Skeleton3D] automatically applies influence to all bone poses set by the modifier.
 			</description>
 		</method>
+		<method name="_skeleton_changed" qualifiers="virtual">
+			<return type="void" />
+			<param index="0" name="old_skeleton" type="Skeleton3D" />
+			<param index="1" name="new_skeleton" type="Skeleton3D" />
+			<description>
+				Called when the skeleton is changed.
+			</description>
+		</method>
+		<method name="_validate_bone_names" qualifiers="virtual">
+			<return type="void" />
+			<description>
+				Called when bone name and index need to be validated such as the timing of the entering tree or changing skeleton.
+			</description>
+		</method>
 		<method name="get_skeleton" qualifiers="const">
 			<return type="Skeleton3D" />
 			<description>
diff --git a/editor/animation_track_editor.cpp b/editor/animation_track_editor.cpp
@@ -6454,7 +6454,6 @@ void AnimationTrackEditor::goto_next_step(bool p_from_mouse_event, bool p_timeli
 }
 
 void AnimationTrackEditor::_edit_menu_pressed(int p_option) {
-	last_menu_track_opt = p_option;
 	switch (p_option) {
 		case EDIT_COPY_TRACKS: {
 			track_copy_select->clear();
@@ -6614,11 +6613,15 @@ void AnimationTrackEditor::_edit_menu_pressed(int p_option) {
 
 			undo_redo->commit_action();
 		} break;
-
-		case EDIT_SCALE_SELECTION:
+		case EDIT_SCALE_SELECTION: {
+			scale_dialog->popup_centered(Size2(200, 100) * EDSCALE);
+			scale->get_line_edit()->grab_focus();
+			scale_from_cursor = false;
+		} break;
 		case EDIT_SCALE_FROM_CURSOR: {
 			scale_dialog->popup_centered(Size2(200, 100) * EDSCALE);
 			scale->get_line_edit()->grab_focus();
+			scale_from_cursor = true;
 		} break;
 		case EDIT_SCALE_CONFIRM: {
 			if (selection.is_empty()) {
@@ -6641,9 +6644,8 @@ void AnimationTrackEditor::_edit_menu_pressed(int p_option) {
 			}
 
 			len = to_t - from_t;
-			if (last_menu_track_opt == EDIT_SCALE_FROM_CURSOR) {
+			if (scale_from_cursor) {
 				pivot = timeline->get_play_position();
-
 			} else {
 				pivot = from_t;
 			}
@@ -6685,7 +6687,7 @@ void AnimationTrackEditor::_edit_menu_pressed(int p_option) {
 				to_restore.push_back(amr);
 			}
 
-#define NEW_POS(m_ofs) (((s > 0) ? m_ofs : from_t + (len - (m_ofs - from_t))) - pivot) * ABS(s) + from_t
+#define NEW_POS(m_ofs) (((s > 0) ? m_ofs : from_t + (len - (m_ofs - from_t))) - pivot) * ABS(s) + pivot
 			// 3 - Move the keys (re insert them).
 			for (RBMap<SelectedKey, KeyInfo>::Element *E = selection.back(); E; E = E->prev()) {
 				float newpos = NEW_POS(E->get().pos);
diff --git a/editor/animation_track_editor.h b/editor/animation_track_editor.h
@@ -793,7 +793,7 @@ class AnimationTrackEditor : public VBoxContainer {
 
 	void _edit_menu_about_to_popup();
 	void _edit_menu_pressed(int p_option);
-	int last_menu_track_opt = 0;
+	bool scale_from_cursor = false;
 
 	void _cleanup_animation(Ref<Animation> p_animation);
 
diff --git a/editor/gui/editor_spin_slider.cpp b/editor/gui/editor_spin_slider.cpp
@@ -437,10 +437,8 @@ void EditorSpinSlider::_draw_spin_slider() {
 					grabber->set_texture(grabber_tex);
 				}
 
-				Vector2 scale = get_global_transform_with_canvas().get_scale();
-				grabber->set_scale(scale);
 				grabber->reset_size();
-				grabber->set_position((grabber_rect.get_center() - grabber->get_size() * 0.5) * scale);
+				grabber->set_position(grabber_rect.get_center() - grabber->get_size() * 0.5);
 
 				if (mousewheel_over_grabber) {
 					Input::get_singleton()->warp_mouse(grabber->get_position() + grabber_rect.size);
diff --git a/editor/import/3d/post_import_plugin_skeleton_rest_fixer.cpp b/editor/import/3d/post_import_plugin_skeleton_rest_fixer.cpp
@@ -369,31 +369,15 @@ void PostImportPluginSkeletonRestFixer::internal_process(InternalImportCategory
 
 				Vector3 prof_dir = prof_tail - prof_head;
 				Vector3 src_dir = src_tail - src_head;
+				if (Math::is_zero_approx(prof_dir.length_squared()) || Math::is_zero_approx(src_dir.length_squared())) {
+					continue;
+				}
+				prof_dir.normalize();
+				src_dir.normalize();
 
 				// Rotate rest.
 				if (Math::abs(Math::rad_to_deg(src_dir.angle_to(prof_dir))) > float(p_options["retarget/rest_fixer/fix_silhouette/threshold"])) {
-					// Get rotation difference.
-					Vector3 up_vec; // Need to rotate other than roll axis.
-					switch (Vector3(abs(src_dir.x), abs(src_dir.y), abs(src_dir.z)).min_axis_index()) {
-						case Vector3::AXIS_X: {
-							up_vec = Vector3(1, 0, 0);
-						} break;
-						case Vector3::AXIS_Y: {
-							up_vec = Vector3(0, 1, 0);
-						} break;
-						case Vector3::AXIS_Z: {
-							up_vec = Vector3(0, 0, 1);
-						} break;
-					}
-					Basis src_b;
-					src_b = src_b.looking_at(src_dir, up_vec);
-					Basis prof_b;
-					prof_b = src_b.looking_at(prof_dir, up_vec);
-					if (prof_b.is_equal_approx(Basis())) {
-						continue; // May not need to rotate.
-					}
-					Basis diff_b = prof_b * src_b.inverse();
-
+					Basis diff_b = Basis(Quaternion(src_dir, prof_dir));
 					// Apply rotation difference as global transform to skeleton.
 					Basis src_pg;
 					int src_parent = src_skeleton->get_bone_parent(src_idx);
diff --git a/editor/import/3d/resource_importer_scene.cpp b/editor/import/3d/resource_importer_scene.cpp
@@ -1958,6 +1958,7 @@ void ResourceImporterScene::_create_slices(AnimationPlayer *ap, Ref<Animation> a
 			}
 		}
 
+		new_anim->set_name(name);
 		new_anim->set_loop_mode(loop_mode);
 		new_anim->set_length(to - from);
 		new_anim->set_step(anim->get_step());
diff --git a/editor/plugins/skeleton_3d_editor_plugin.cpp b/editor/plugins/skeleton_3d_editor_plugin.cpp
@@ -158,7 +158,7 @@ void BonePropertiesEditor::_meta_changed(const String &p_property, const Variant
 	}
 
 	String key = p_property.get_slicec('/', 3);
-	if (!skeleton->has_bone_meta(1, key)) {
+	if (!skeleton->has_bone_meta(bone, key)) {
 		return;
 	}
 
@@ -182,7 +182,7 @@ void BonePropertiesEditor::_meta_deleted(const String &p_property) {
 	}
 
 	String key = p_property.get_slicec('/', 3);
-	if (!skeleton->has_bone_meta(1, key)) {
+	if (!skeleton->has_bone_meta(bone, key)) {
 		return;
 	}
 
diff --git a/scene/3d/look_at_modifier_3d.cpp b/scene/3d/look_at_modifier_3d.cpp
@@ -77,6 +77,20 @@ PackedStringArray LookAtModifier3D::get_configuration_warnings() const {
 	return warnings;
 }
 
+void LookAtModifier3D::_validate_bone_names() {
+	// Prior bone name.
+	if (!bone_name.is_empty()) {
+		set_bone_name(bone_name);
+	} else if (bone != -1) {
+		set_bone(bone);
+	}
+	if (!origin_bone_name.is_empty()) {
+		set_origin_bone_name(origin_bone_name);
+	} else if (origin_bone != -1) {
+		set_origin_bone(origin_bone);
+	}
+}
+
 void LookAtModifier3D::set_bone_name(const String &p_bone_name) {
 	bone_name = p_bone_name;
 	Skeleton3D *sk = get_skeleton();
@@ -531,7 +545,7 @@ void LookAtModifier3D::_process_modification() {
 		} else {
 			origin_tr = bone_rest_space;
 		}
-		forward_vector = bone_rest_space.orthonormalized().basis.xform_inv((target->get_global_position() - origin_tr.translated_local(origin_offset).origin));
+		forward_vector = bone_rest_space.orthonormalized().basis.xform_inv(target->get_global_position() - origin_tr.translated_local(origin_offset).origin);
 		forward_vector_nrm = forward_vector.normalized();
 		if (forward_vector_nrm.abs().is_equal_approx(get_vector_from_axis(primary_rotation_axis))) {
 			destination = skeleton->get_bone_pose_rotation(bone);
diff --git a/scene/3d/look_at_modifier_3d.h b/scene/3d/look_at_modifier_3d.h
@@ -108,6 +108,8 @@ class LookAtModifier3D : public SkeletonModifier3D {
 	virtual PackedStringArray get_configuration_warnings() const override;
 	void _validate_property(PropertyInfo &p_property) const;
 
+	virtual void _validate_bone_names() override;
+
 	static void _bind_methods();
 
 	virtual void _process_modification() override;
diff --git a/scene/3d/skeleton_3d.cpp b/scene/3d/skeleton_3d.cpp
@@ -1106,6 +1106,12 @@ void Skeleton3D::_force_update_bone_children_transforms(int p_bone_idx) const {
 
 	// Loop through nested set.
 	for (int offset = 0; offset < bone_size; offset++) {
+		if (rest_dirty) {
+			int current_bone_idx = nested_set_offset_to_bone_index[offset];
+			Bone &b = bonesptr[current_bone_idx];
+			b.global_rest = b.parent >= 0 ? bonesptr[b.parent].global_rest * b.rest : b.rest; // Rest needs update apert from pose.
+		}
+
 		if (!bone_global_pose_dirty[offset]) {
 			continue;
 		}
@@ -1130,9 +1136,6 @@ void Skeleton3D::_force_update_bone_children_transforms(int p_bone_idx) const {
 				b.global_pose = b.rest;
 			}
 		}
-		if (rest_dirty) {
-			b.global_rest = b.parent >= 0 ? bonesptr[b.parent].global_rest * b.rest : b.rest;
-		}
 
 #ifndef DISABLE_DEPRECATED
 		if (bone_enabled) {
diff --git a/scene/3d/skeleton_modifier_3d.cpp b/scene/3d/skeleton_modifier_3d.cpp
@@ -70,11 +70,18 @@ void SkeletonModifier3D::_update_skeleton() {
 	if (old_sk != new_sk) {
 		_skeleton_changed(old_sk, new_sk);
 	}
+	if (new_sk) {
+		_validate_bone_names();
+	}
 	update_configuration_warnings();
 }
 
 void SkeletonModifier3D::_skeleton_changed(Skeleton3D *p_old, Skeleton3D *p_new) {
-	//
+	GDVIRTUAL_CALL(_skeleton_changed, p_old, p_new);
+}
+
+void SkeletonModifier3D::_validate_bone_names() {
+	GDVIRTUAL_CALL(_validate_bone_names);
 }
 
 void SkeletonModifier3D::_force_update_skeleton_skin() {
@@ -155,6 +162,9 @@ void SkeletonModifier3D::_bind_methods() {
 	ADD_SIGNAL(MethodInfo("modification_processed"));
 	GDVIRTUAL_BIND(_process_modification);
 
+	GDVIRTUAL_BIND(_skeleton_changed, "old_skeleton", "new_skeleton");
+	GDVIRTUAL_BIND(_validate_bone_names);
+
 	BIND_ENUM_CONSTANT(BONE_AXIS_PLUS_X);
 	BIND_ENUM_CONSTANT(BONE_AXIS_MINUS_X);
 	BIND_ENUM_CONSTANT(BONE_AXIS_PLUS_Y);
diff --git a/scene/3d/skeleton_modifier_3d.h b/scene/3d/skeleton_modifier_3d.h
@@ -64,6 +64,9 @@ class SkeletonModifier3D : public Node3D {
 	void _force_update_skeleton_skin();
 
 	virtual void _skeleton_changed(Skeleton3D *p_old, Skeleton3D *p_new);
+	virtual void _validate_bone_names();
+	GDVIRTUAL2(_skeleton_changed, Skeleton3D *, Skeleton3D *);
+	GDVIRTUAL0(_validate_bone_names);
 
 	void _validate_property(PropertyInfo &p_property) const;
 	void _notification(int p_what);
diff --git a/scene/3d/spring_bone_collision_3d.cpp b/scene/3d/spring_bone_collision_3d.cpp
@@ -60,6 +60,15 @@ void SpringBoneCollision3D::_validate_property(PropertyInfo &p_property) const {
 	}
 }
 
+void SpringBoneCollision3D::_validate_bone_name() {
+	// Prior bone name.
+	if (!bone_name.is_empty()) {
+		set_bone_name(bone_name);
+	} else if (bone != -1) {
+		set_bone(bone);
+	}
+}
+
 Skeleton3D *SpringBoneCollision3D::get_skeleton() const {
 	SpringBoneSimulator3D *parent = Object::cast_to<SpringBoneSimulator3D>(get_parent());
 	if (!parent) {
@@ -175,6 +184,15 @@ void SpringBoneCollision3D::_bind_methods() {
 	ADD_PROPERTY(PropertyInfo(Variant::QUATERNION, "rotation_offset"), "set_rotation_offset", "get_rotation_offset");
 }
 
+void SpringBoneCollision3D::_notification(int p_what) {
+	switch (p_what) {
+		case NOTIFICATION_ENTER_TREE:
+		case NOTIFICATION_PARENTED: {
+			_validate_bone_name();
+		} break;
+	}
+}
+
 Vector3 SpringBoneCollision3D::collide(const Transform3D &p_center, float p_bone_radius, float p_bone_length, const Vector3 &p_current) const {
 	return _collide(p_center, p_bone_radius, p_bone_length, p_current);
 }
diff --git a/scene/3d/spring_bone_collision_3d.h b/scene/3d/spring_bone_collision_3d.h
@@ -44,10 +44,13 @@ class SpringBoneCollision3D : public Node3D {
 	Vector3 position_offset;
 	Quaternion rotation_offset;
 
+	void _validate_bone_name();
+
 protected:
 	PackedStringArray get_configuration_warnings() const override;
 
 	void _validate_property(PropertyInfo &p_property) const;
+	void _notification(int p_what);
 	static void _bind_methods();
 
 	virtual Vector3 _collide(const Transform3D &p_center, float p_bone_radius, float p_bone_length, const Vector3 &p_current) const;
diff --git a/scene/3d/spring_bone_simulator_3d.cpp b/scene/3d/spring_bone_simulator_3d.cpp
@@ -1215,6 +1215,23 @@ void SpringBoneSimulator3D::_bind_methods() {
 	BIND_ENUM_CONSTANT(ROTATION_AXIS_ALL);
 }
 
+void SpringBoneSimulator3D::_validate_bone_names() {
+	for (int i = 0; i < settings.size(); i++) {
+		// Prior bone name.
+		if (!settings[i]->root_bone_name.is_empty()) {
+			set_root_bone_name(i, settings[i]->root_bone_name);
+		} else if (settings[i]->root_bone != -1) {
+			set_root_bone(i, settings[i]->root_bone);
+		}
+		// Prior bone name.
+		if (!settings[i]->end_bone_name.is_empty()) {
+			set_end_bone_name(i, settings[i]->end_bone_name);
+		} else if (settings[i]->end_bone != -1) {
+			set_end_bone(i, settings[i]->end_bone);
+		}
+	}
+}
+
 void SpringBoneSimulator3D::_make_joints_dirty(int p_index) {
 	ERR_FAIL_INDEX(p_index, settings.size());
 	settings[p_index]->joints_dirty = true;
@@ -1346,6 +1363,9 @@ void SpringBoneSimulator3D::_find_collisions() {
 }
 
 void SpringBoneSimulator3D::_process_collisions() {
+	if (!is_inside_tree()) {
+		return;
+	}
 	for (const ObjectID &oid : collisions) {
 		Object *t_obj = ObjectDB::get_instance(oid);
 		if (!t_obj) {
@@ -1469,6 +1489,10 @@ void SpringBoneSimulator3D::_set_active(bool p_active) {
 }
 
 void SpringBoneSimulator3D::_process_modification() {
+	if (!is_inside_tree()) {
+		return;
+	}
+
 	Skeleton3D *skeleton = get_skeleton();
 	if (!skeleton) {
 		return;
@@ -1490,6 +1514,9 @@ void SpringBoneSimulator3D::_process_modification() {
 }
 
 void SpringBoneSimulator3D::reset() {
+	if (!is_inside_tree()) {
+		return;
+	}
 	Skeleton3D *skeleton = get_skeleton();
 	if (!skeleton) {
 		return;
diff --git a/scene/3d/spring_bone_simulator_3d.h b/scene/3d/spring_bone_simulator_3d.h
@@ -152,6 +152,8 @@ class SpringBoneSimulator3D : public SkeletonModifier3D {
 
 	void _notification(int p_what);
 
+	virtual void _validate_bone_names() override;
+
 	static void _bind_methods();
 
 	virtual void _set_active(bool p_active) override;
diff --git a/scene/resources/animation.cpp b/scene/resources/animation.cpp
@@ -466,7 +466,9 @@ bool Animation::_get(const StringName &p_name, Variant &r_ret) const {
 	String prop_name = p_name;
 
 	if (p_name == SNAME("_compression")) {
-		ERR_FAIL_COND_V(!compression.enabled, false);
+		if (!compression.enabled) {
+			return false;
+		}
 		Dictionary comp;
 		comp["fps"] = compression.fps;
 		Array bounds;
@@ -2441,7 +2443,7 @@ int Animation::_find(const Vector<K> &p_keys, double p_time, bool p_backward, bo
 		}
 	}
 
-	if (p_limit) {
+	if (p_limit && middle > -1 && middle < len) {
 		double diff = length - keys[middle].time;
 		if ((signbit(keys[middle].time) && !Math::is_zero_approx(keys[middle].time)) || (signbit(diff) && !Math::is_zero_approx(diff))) {
 			ERR_PRINT_ONCE_ED("Found the key outside the animation range. Consider using the clean-up option in AnimationTrackEditor to fix it.");

Original file line number	Diff line number	Diff line change
`@@ -1958,6 +1958,7 @@ void ResourceImporterScene::_create_slices(AnimationPlayer *ap, Ref<Animation> a`
`1958`	`1958`	`}`
`1959`	`1959`	`}`
`1960`	`1960`
	`1961`	`+ new_anim->set_name(name);`
`1961`	`1962`	`new_anim->set_loop_mode(loop_mode);`
`1962`	`1963`	`new_anim->set_length(to - from);`
`1963`	`1964`	`new_anim->set_step(anim->get_step());`
Original file line number	Diff line number	Diff line change
`@@ -158,7 +158,7 @@ void BonePropertiesEditor::_meta_changed(const String &p_property, const Variant`
`158`	`158`	`}`
`159`	`159`
`160`	`160`	`String key = p_property.get_slicec('/', 3);`
`161`		`- if (!skeleton->has_bone_meta(1, key)) {`
	`161`	`+ if (!skeleton->has_bone_meta(bone, key)) {`
`162`	`162`	`return;`
`163`	`163`	`}`
`164`	`164`
`@@ -182,7 +182,7 @@ void BonePropertiesEditor::_meta_deleted(const String &p_property) {`
`182`	`182`	`}`
`183`	`183`
`184`	`184`	`String key = p_property.get_slicec('/', 3);`
`185`		`- if (!skeleton->has_bone_meta(1, key)) {`
	`185`	`+ if (!skeleton->has_bone_meta(bone, key)) {`
`186`	`186`	`return;`
`187`	`187`	`}`
`188`	`188`