Merge pull request #90575 from TokageItLab/boneattachment-performance

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

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;