Show a selected Bone Gizmo when Clicking on it keyframe #99696
Conversation
|
Hello! This is my first PR review so please let me know if I've got something wrong. I compiled the changes and can confirm it does 2 things:
Code LGTM 👍 There are spelling errors in the comments as pointed out by Github Actions If you need any help just @me and I'll gladly help! |
|
You have a lot of commented out code, did you mean to mark this as a draft? |
Hey sorry I am new to making changes in Godot! I have since removed the commented out code. |
|
I'll bring it to the godot animation team meeting for review |
| Skeleton3DEditor *se = Skeleton3DEditor::get_singleton(); | ||
|
|
||
| // Only show selected gizmos if we have a skeleton3DEditor instance and it is in editor mode | ||
| if (track_type == Animation::TYPE_ROTATION_3D && se && se->is_edit_mode()) { |
There was a problem hiding this comment.
Shouldn't TYPE_POSITION_3D and TYPE_SCALE_3D also be handled?
| // Clear current selection | ||
| se->select_bone(-1); | ||
|
|
There was a problem hiding this comment.
| // Clear current selection | |
| se->select_bone(-1); |
This should not be necessary since bone_idx is -1 if no bones are found.
| // Only show selected gizmos if we have a skeleton3DEditor instance and it is in editor mode | ||
| if (track_type == Animation::TYPE_ROTATION_3D && se && se->is_edit_mode()) { | ||
| NodePath path = animation->track_get_path(key_edit->track); | ||
| // Node *root_path = nullptr; |
There was a problem hiding this comment.
| // Node *root_path = nullptr; |
There was a problem hiding this comment.
The biggest problem is that this is implemented as a too special case and hard coded only for the Skeleton3D. If we want to implement this, we need to implement a common method somewhere in the AnimationTrackEditor first that will handle Gizmo in Node2D and Node3D as well as Skeleton3D. So I am not against this PR, but there needs to be a more fundamental implementation.
Closes #95994