Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove bone_pose_updated signal and replace it with the skeleton_updated signal #90575

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions doc/classes/BoneAttachment3D.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down
20 changes: 10 additions & 10 deletions doc/classes/Skeleton3D.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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="">
Expand Down Expand Up @@ -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]).
Expand Down
8 changes: 8 additions & 0 deletions misc/extension_api_validation/4.2-stable.expected
Original file line number Diff line number Diff line change
Expand Up @@ -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.
43 changes: 43 additions & 0 deletions scene/3d/bone_attachment_3d.compat.inc
Original file line number Diff line number Diff line change
@@ -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
13 changes: 7 additions & 6 deletions scene/3d/bone_attachment_3d.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down Expand Up @@ -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);
}
}
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 5 additions & 1 deletion scene/3d/bone_attachment_3d.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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();
Expand Down
6 changes: 3 additions & 3 deletions scene/3d/skeleton_3d.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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"));
Expand Down
2 changes: 1 addition & 1 deletion scene/scene_string_names.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down
2 changes: 1 addition & 1 deletion scene/scene_string_names.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Loading