Merge pull request godotengine#90575 from TokageItLab/boneattachment-performance

Remove `bone_pose_updated` signal and replace it with the `skeleton_updated` signal

Author: akien-mga

doc/classes/BoneAttachment3D.xml

@@ -21,11 +21,10 @@
 				Returns whether the BoneAttachment3D node is using an external [Skeleton3D] rather than attempting to use its parent node as the [Skeleton3D].
 			</description>
 		</method>
-		<method name="on_bone_pose_update">
+		<method name="on_skeleton_update">
 			<return type="void" />
-			<param index="0" name="bone_index" type="int" />
 			<description>
-				A function that is called automatically when the [Skeleton3D] the BoneAttachment3D node is using has a bone that has changed its pose. This function is where the BoneAttachment3D node updates its position so it is correctly bound when it is [i]not[/i] set to override the bone pose.
+				A function that is called automatically when the [Skeleton3D] is updated. This function is where the [BoneAttachment3D] node updates its position so it is correctly bound when it is [i]not[/i] set to override the bone pose.
 			</description>
 		</method>
 		<method name="set_external_skeleton">

doc/classes/Skeleton3D.xml

@@ -252,7 +252,7 @@
 			<param index="1" name="pose" type="Transform3D" />
 			<description>
 				Sets the global pose transform, [param pose], for the bone at [param bone_idx].
-				[b]Note:[/b] If other bone poses have been changed, this method executes an update process and will cause performance to deteriorate. If you know that multiple global poses will be applied, consider using [method set_bone_pose] with precalculation.
+				[b]Note:[/b] If other bone poses have been changed, this method executes a dirty poses recalculation and will cause performance to deteriorate. If you know that multiple global poses will be applied, consider using [method set_bone_pose] with precalculation.
 			</description>
 		</method>
 		<method name="set_bone_global_pose_override" deprecated="">
@@ -355,27 +355,27 @@
 			<description>
 			</description>
 		</signal>
-		<signal name="bone_pose_changed">
-			<param index="0" name="bone_idx" type="int" />
-			<description>
-				Emitted when the bone at [param bone_idx] changes its transform/pose. This can be used to update other nodes that rely on bone positions.
-			</description>
-		</signal>
 		<signal name="pose_updated">
 			<description>
-				Emitted when the pose is updated, after [constant NOTIFICATION_UPDATE_SKELETON] is received.
+				Emitted when the pose is updated.
+				[b]Note:[/b] During the update process, this signal is not fired, so modification by [SkeletonModifier3D] is not detected.
 			</description>
 		</signal>
 		<signal name="show_rest_only_changed">
 			<description>
 				Emitted when the value of [member show_rest_only] changes.
 			</description>
 		</signal>
+		<signal name="skeleton_updated">
+			<description>
+				Emitted when the final pose has been calculated will be applied to the skin in the update process.
+				This means that all [SkeletonModifier3D] processing is complete. In order to detect the completion of the processing of each [SkeletonModifier3D], use [signal SkeletonModifier3D.modification_processed].
+			</description>
+		</signal>
 	</signals>
 	<constants>
 		<constant name="NOTIFICATION_UPDATE_SKELETON" value="50">
-			Notification received when this skeleton's pose needs to be updated.
-			This notification is received [i]before[/i] the related [signal pose_updated] signal.
+			Notification received when this skeleton's pose needs to be updated. In that case, this is called only once per frame in a deferred process.
 		</constant>
 		<constant name="MODIFIER_CALLBACK_MODE_PROCESS_PHYSICS" value="0" enum="ModifierCallbackModeProcess">
 			Set a flag to process modification during physics frames (see [constant Node.NOTIFICATION_INTERNAL_PHYSICS_PROCESS]).

misc/extension_api_validation/4.2-stable.expected

@@ -272,3 +272,11 @@ Validate extension JSON: API was removed: classes/SkeletonIK3D/methods/set_inter
 Validate extension JSON: API was removed: classes/SkeletonIK3D/properties/interpolation
 
 These base class is changed to SkeletonModifier3D which is processed by Skeleton3D with the assumption that it is Skeleton3D's child.
+
+
+GH-90575
+--------
+Validate extension JSON: API was removed: classes/BoneAttachment3D/methods/on_bone_pose_update
+Validate extension JSON: API was removed: classes/Skeleton3D/signals/bone_pose_changed
+
+They have been replaced by a safer API due to performance concerns. Compatibility method registered.

scene/3d/bone_attachment_3d.compat.inc

@@ -0,0 +1,43 @@
+/**************************************************************************/
+/*  bone_attachment_3d.compat.inc                                         */
+/**************************************************************************/
+/*                         This file is part of:                          */
+/*                             GODOT ENGINE                               */
+/*                        https://godotengine.org                         */
+/**************************************************************************/
+/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */
+/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur.                  */
+/*                                                                        */
+/* Permission is hereby granted, free of charge, to any person obtaining  */
+/* a copy of this software and associated documentation files (the        */
+/* "Software"), to deal in the Software without restriction, including    */
+/* without limitation the rights to use, copy, modify, merge, publish,    */
+/* distribute, sublicense, and/or sell copies of the Software, and to     */
+/* permit persons to whom the Software is furnished to do so, subject to  */
+/* the following conditions:                                              */
+/*                                                                        */
+/* The above copyright notice and this permission notice shall be         */
+/* included in all copies or substantial portions of the Software.        */
+/*                                                                        */
+/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,        */
+/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF     */
+/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */
+/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY   */
+/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,   */
+/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE      */
+/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.                 */
+/**************************************************************************/
+
+#ifndef DISABLE_DEPRECATED
+
+#include "bone_attachment_3d.h"
+
+void BoneAttachment3D::_on_bone_pose_update_bind_compat_90575(int p_bone_index) {
+	return on_skeleton_update();
+}
+
+void BoneAttachment3D::_bind_compatibility_methods() {
+	ClassDB::bind_compatibility_method(D_METHOD("on_bone_pose_update", "bone_index"), &BoneAttachment3D::_on_bone_pose_update_bind_compat_90575);
+}
+
+#endif // DISABLE_DEPRECATED

scene/3d/bone_attachment_3d.cpp

@@ -29,6 +29,7 @@
 /**************************************************************************/
 
 #include "bone_attachment_3d.h"
+#include "bone_attachment_3d.compat.inc"
 
 void BoneAttachment3D::_validate_property(PropertyInfo &p_property) const {
 	if (p_property.name == "bone_name") {
@@ -148,9 +149,9 @@ void BoneAttachment3D::_check_bind() {
 			bone_idx = sk->find_bone(bone_name);
 		}
 		if (bone_idx != -1) {
-			sk->connect(SNAME("bone_pose_changed"), callable_mp(this, &BoneAttachment3D::on_bone_pose_update));
+			sk->connect(SNAME("skeleton_updated"), callable_mp(this, &BoneAttachment3D::on_skeleton_update));
 			bound = true;
-			callable_mp(this, &BoneAttachment3D::on_bone_pose_update).call_deferred(bone_idx);
+			callable_mp(this, &BoneAttachment3D::on_skeleton_update);
 		}
 	}
 }
@@ -176,7 +177,7 @@ void BoneAttachment3D::_check_unbind() {
 		Skeleton3D *sk = _get_skeleton3d();
 
 		if (sk) {
-			sk->disconnect(SNAME("bone_pose_changed"), callable_mp(this, &BoneAttachment3D::on_bone_pose_update));
+			sk->disconnect(SNAME("skeleton_updated"), callable_mp(this, &BoneAttachment3D::on_skeleton_update));
 		}
 		bound = false;
 	}
@@ -308,12 +309,12 @@ void BoneAttachment3D::_notification(int p_what) {
 	}
 }
 
-void BoneAttachment3D::on_bone_pose_update(int p_bone_index) {
+void BoneAttachment3D::on_skeleton_update() {
 	if (updating) {
 		return;
 	}
 	updating = true;
-	if (bone_idx == p_bone_index) {
+	if (bone_idx >= 0) {
 		Skeleton3D *sk = _get_skeleton3d();
 		if (sk) {
 			if (!override_pose) {
@@ -371,7 +372,7 @@ void BoneAttachment3D::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("set_bone_idx", "bone_idx"), &BoneAttachment3D::set_bone_idx);
 	ClassDB::bind_method(D_METHOD("get_bone_idx"), &BoneAttachment3D::get_bone_idx);
 
-	ClassDB::bind_method(D_METHOD("on_bone_pose_update", "bone_index"), &BoneAttachment3D::on_bone_pose_update);
+	ClassDB::bind_method(D_METHOD("on_skeleton_update"), &BoneAttachment3D::on_skeleton_update);
 
 	ClassDB::bind_method(D_METHOD("set_override_pose", "override_pose"), &BoneAttachment3D::set_override_pose);
 	ClassDB::bind_method(D_METHOD("get_override_pose"), &BoneAttachment3D::get_override_pose);

scene/3d/bone_attachment_3d.h

@@ -67,6 +67,10 @@ class BoneAttachment3D : public Node3D {
 	void _notification(int p_what);
 
 	static void _bind_methods();
+#ifndef DISABLE_DEPRECATED
+	virtual void _on_bone_pose_update_bind_compat_90575(int p_bone_index);
+	static void _bind_compatibility_methods();
+#endif
 
 public:
 #ifdef TOOLS_ENABLED
@@ -89,7 +93,7 @@ class BoneAttachment3D : public Node3D {
 	void set_external_skeleton(NodePath p_skeleton);
 	NodePath get_external_skeleton() const;
 
-	virtual void on_bone_pose_update(int p_bone_index);
+	virtual void on_skeleton_update();
 
 #ifdef TOOLS_ENABLED
 	virtual void notify_rebind_required();

scene/3d/skeleton_3d.cpp

@@ -327,6 +327,8 @@ void Skeleton3D::_notification(int p_what) {
 				_process_modifiers();
 			}
 
+			emit_signal(SceneStringNames::get_singleton()->skeleton_updated);
+
 			// Update skins.
 			RenderingServer *rs = RenderingServer::get_singleton();
 			for (SkinReference *E : skin_bindings) {
@@ -921,8 +923,6 @@ void Skeleton3D::force_update_bone_children_transforms(int p_bone_idx) {
 		for (int i = 0; i < child_bone_size; i++) {
 			bones_to_process.push_back(b.child_bones[i]);
 		}
-
-		emit_signal(SceneStringNames::get_singleton()->bone_pose_changed, current_bone_idx);
 	}
 }
 
@@ -1059,7 +1059,7 @@ void Skeleton3D::_bind_methods() {
 	ADD_PROPERTY(PropertyInfo(Variant::INT, "modifier_callback_mode_process", PROPERTY_HINT_ENUM, "Physics,Idle"), "set_modifier_callback_mode_process", "get_modifier_callback_mode_process");
 
 	ADD_SIGNAL(MethodInfo("pose_updated"));
-	ADD_SIGNAL(MethodInfo("bone_pose_changed", PropertyInfo(Variant::INT, "bone_idx")));
+	ADD_SIGNAL(MethodInfo("skeleton_updated"));
 	ADD_SIGNAL(MethodInfo("bone_enabled_changed", PropertyInfo(Variant::INT, "bone_idx")));
 	ADD_SIGNAL(MethodInfo("bone_list_changed"));
 	ADD_SIGNAL(MethodInfo("show_rest_only_changed"));

scene/scene_string_names.cpp

@@ -63,7 +63,7 @@ SceneStringNames::SceneStringNames() {
 	RESET = StaticCString::create("RESET");
 
 	pose_updated = StaticCString::create("pose_updated");
-	bone_pose_changed = StaticCString::create("bone_pose_changed");
+	skeleton_updated = StaticCString::create("skeleton_updated");
 	bone_enabled_changed = StaticCString::create("bone_enabled_changed");
 	show_rest_only_changed = StaticCString::create("show_rest_only_changed");
 

scene/scene_string_names.h

@@ -99,7 +99,7 @@ class SceneStringNames {
 	StringName RESET;
 
 	StringName pose_updated;
-	StringName bone_pose_changed;
+	StringName skeleton_updated;
 	StringName bone_enabled_changed;
 	StringName show_rest_only_changed;