Fix "Create Custom Bone2D(s) from Node(s)" to create working bone chains #107797

Joey-Einerhand · 2025-06-21T11:33:01Z

before the new bone- and IK system in 4.0, the "Create Custom Bone2D(s) from Node(s)" editor button created an out-of-the-box working chain of bones. This can be seen in the pre-4.0 docs. Since 4.0, that has been broken.

This pull request aims to bring back that functionality.
When using the "Create Custom Bone2D(s) from Node(s)" button, the selected nodes should be used to create a correctly setup Bone2D chain, where Bone2Ds without Bone2D children or siblings should connect to an 'end' marker2D.

Fixes #92227

This is my first 'real' pull request to the Godot engine. I did my best to follow the docs. Please be really critical of my changes.

Notes:

Because the reparenting the nodes needs to happen sequentially, I've changed around some register_action() and commit_action() . To undo the "Create Custom Bone2D(s) from Node(s)" action, requires 4 undos. The way I did it is wrong. If you know of a better way, please tell me.
I'm pretty sure I got Clang to reformat my code, but please give the code style another pass.
Testers: I've added a minimum project to this PR so you don't have to start from scratch. make-bone-2d-mrp.zip
This does not fix the 'Bone lacks REST pose' warning. Should I include a fix for that in this PR, or does that need a seperate issue?
When using the 'Fixed' scenario, the angle constraint gizmos are drawn behind the sprites. Should I include a fix in this PR, or does that need a seperate issue?

KoBeWi · 2025-06-27T10:31:26Z

I'm getting a crash in the MRP from #92227:

VectorWriteProxy<Variant>::operator[](__int64 p_index) Line 54 (c:\godot_source\core\templates\vector.h:54)
Array::operator[](int p_idx) Line 106 (c:\godot_source\core\variant\array.cpp:106)
CanvasItemEditor::_popup_callback(int p_op) Line 4923 (c:\godot_source\editor\plugins\canvas_item_editor_plugin.cpp:4923)
call_with_variant_args_helper<CanvasItemEditor,int,0>(CanvasItemEditor * p_instance, void(CanvasItemEditor::*)(int) p_method, const Variant * * p_args, Callable::CallError & r_error, IndexSequence<0> __formal) Line 228 (c:\godot_source\core\variant\binder_common.h:228)
call_with_variant_args<CanvasItemEditor,int>(CanvasItemEditor * p_instance, void(CanvasItemEditor::*)(int) p_method, const Variant * * p_args, int p_argcount, Callable::CallError & r_error) Line 338 (c:\godot_source\core\variant\binder_common.h:338)
CallableCustomMethodPointer<CanvasItemEditor,void,int>::call(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 107 (c:\godot_source\core\object\callable_method_pointer.h:107)
Callable::callp(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 58 (c:\godot_source\core\variant\callable.cpp:58)
Object::emit_signalp(const StringName & p_name, const Variant * * p_args, int p_argcount) Line 1288 (c:\godot_source\core\object\object.cpp:1288)
Node::emit_signalp(const StringName & p_name, const Variant * * p_args, int p_argcount) Line 4130 (c:\godot_source\scene\main\node.cpp:4130)
Object::emit_signal<int>(const StringName & p_name, int <p_args_0>) Line 941 (c:\godot_source\core\object\object.h:941)
PopupMenu::activate_item(int p_idx) Line 2744 (c:\godot_source\scene\gui\popup_menu.cpp:2744)
PopupMenu::_input_from_window_internal(const Ref<InputEvent> & p_event) Line 686 (c:\godot_source\scene\gui\popup_menu.cpp:686)
PopupMenu::_input_from_window(const Ref<InputEvent> & p_event) Line 468 (c:\godot_source\scene\gui\popup_menu.cpp:468)
Window::_window_input(const Ref<InputEvent> & p_ev) Line 1793 (c:\godot_source\scene\main\window.cpp:1793)
call_with_variant_args_helper<Window,Ref<InputEvent> const &,0>(Window * p_instance, void(Window::*)(const Ref<InputEvent> &) p_method, const Variant * * p_args, Callable::CallError & r_error, IndexSequence<0> __formal) Line 223 (c:\godot_source\core\variant\binder_common.h:223)
call_with_variant_args<Window,Ref<InputEvent> const &>(Window * p_instance, void(Window::*)(const Ref<InputEvent> &) p_method, const Variant * * p_args, int p_argcount, Callable::CallError & r_error) Line 338 (c:\godot_source\core\variant\binder_common.h:338)
CallableCustomMethodPointer<Window,void,Ref<InputEvent> const &>::call(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 107 (c:\godot_source\core\object\callable_method_pointer.h:107)
Callable::callp(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 58 (c:\godot_source\core\variant\callable.cpp:58)
Callable::call<Ref<InputEvent>>(Ref<InputEvent> <p_args_0>) Line 953 (c:\godot_source\core\variant\variant.h:953)
DisplayServerWindows::_dispatch_input_event(const Ref<InputEvent> & p_event) Line 4412 (c:\godot_source\platform\windows\display_server_windows.cpp:4412)
DisplayServerWindows::_dispatch_input_events(const Ref<InputEvent> & p_event) Line 4383 (c:\godot_source\platform\windows\display_server_windows.cpp:4383)
Input::_parse_input_event_impl(const Ref<InputEvent> & p_event, bool p_is_emulated) Line 907 (c:\godot_source\core\input\input.cpp:907)
Input::flush_buffered_events() Line 1188 (c:\godot_source\core\input\input.cpp:1188)
DisplayServerWindows::process_events() Line 3802 (c:\godot_source\platform\windows\display_server_windows.cpp:3802)
OS_Windows::run() Line 2252 (c:\godot_source\platform\windows\os_windows.cpp:2252)
widechar_main(int argc, wchar_t * * argv) Line 97 (c:\godot_source\platform\windows\godot_windows.cpp:97)
_main() Line 122 (c:\godot_source\platform\windows\godot_windows.cpp:122)
main(int argc, char * * argv) Line 136 (c:\godot_source\platform\windows\godot_windows.cpp:136)
WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * lpCmdLine, int nCmdShow) Line 150 (c:\godot_source\platform\windows\godot_windows.cpp:150)

Seems like you are using wrong index in the for loop. You could change it to range loop, to avoid this problem altogether.

… bone chains

Joey-Einerhand · 2025-07-01T14:00:45Z

I'm getting a crash in the MRP from #92227:
Seems like you are using wrong index in the for loop. You could change it to range loop, to avoid this problem altogether.

Looks like I was too hasty in editing after failed checks, but I've changed it to a range loop now, thank you.

KoBeWi · 2025-07-04T10:35:57Z

editor/plugins/canvas_item_editor_plugin.cpp

@@ -4875,14 +4877,25 @@ void CanvasItemEditor::_popup_callback(int p_op) {
 		case SKELETON_MAKE_BONES: {
 			HashMap<Node *, Object *> &selection = editor_selection->get_selection();
 			Node *editor_root = get_tree()->get_edited_scene_root();
+			Vector<Node2D *> selected_nodes;


Suggested change

Vector<Node2D *> selected_nodes;

LocalVector<Node2D *> selected_nodes;

selected_nodes.reserve(selection.size());

Also move this after the condition below.

KoBeWi · 2025-07-04T10:38:09Z

editor/plugins/canvas_item_editor_plugin.cpp

-			undo_redo->create_action(TTR("Create Custom Bone2D(s) from Node(s)"));
+			// Store the objects from `get_selection()` because otherwise the references
+			// to the selected objects would disappear, because we're repeatedly
+			// reparenting them.
 			for (const KeyValue<Node *, Object *> &E : selection) {
 				Node2D *n2d = Object::cast_to<Node2D>(E.key);


Needs a null check, otherwise it will crash if a non-Node2D is selected.

KoBeWi · 2025-07-04T10:40:21Z

editor/plugins/canvas_item_editor_plugin.cpp

+			}
+
+			undo_redo->create_action(TTR("Create Custom Bone2D(s) from Node(s)"));
+			for (int i = 0; i < selected_nodes.size(); i++) {


This can also be a range loop.

KoBeWi · 2025-07-04T10:42:34Z

editor/plugins/canvas_item_editor_plugin.cpp

+
+			undo_redo->create_action(TTR("Create Custom Bone2D(s) from Node(s)"));
+			for (int i = 0; i < selected_nodes.size(); i++) {
+				undo_redo->create_action(TTR("Create Custom Bone2D from Node"));


What is this action for? Hitting the continue below will result in invalid undo state. Nested actions don't make sense if they are always nested.

What is this action for? Hitting the continue below will result in invalid undo state. Nested actions don't make sense if they are always nested.

The problem with how I set it up is, every action declared in an iteration in that for loop should be executed before the next iteration, so I create a new action within the loop which I execute at the end of that iteration.

So if I were to remove the nested actions, I'd have to remove the outer action (line 4894). This would mean the case SKELETON_MAKE_BONES contains multiple smaller actions, and not one big 'umbrella' action. Is that acceptable? Is there a better way to do this?

So this:

undo_redo->create_action(TTR("Create Custom Bone2D(s) from Node(s)")); for nodes: undo_redo->create_action(TTR("Create Custom Bone2D from Node")); // Add_do_methods undo_redo->commit_action() undo_redo->create_action(TTR("Connect bones to Marker2D end")); for nodes: // Add_do_methods undo_redo->commit_action() undo_redo->commit_action()

becomes:

for nodes: undo_redo->create_action(TTR("Create Custom Bone2D from Node")); // Add_do_methods undo_redo->commit_action() undo_redo->create_action(TTR("Connect bones to Marker2D end")); for nodes: // Add_do_methods undo_redo->commit_action()

commit_action() does not execute the action if it's nested, so your code should have the same effect without the inner actions.

commit_action() does not execute the action if it's nested

I'm not sure I understand. So nested actions are not supported? Because the result really is different if I removed the inner commit_action().

In my case, I need to execute the

for nodes: undo_redo->create_action(TTR("Create Custom Bone2D from Node")); // Add_do_methods undo_redo->commit_action()

nested commit_action(). This is because the loop is essentially:

for nodes: undo_redo->create_action(TTR("Create Custom Bone2D from Node")); // Find the parent of the current node // Use that in add_do_method(do_some_reparenting) undo_redo->commit_action()

The thing is, if I don't commit per loop iteration, the queued add_do_method()s use outdated variables. For example, the n2d_parent variable would be wrong after the 1st iteration. This is because the next node in the loop is moved by the previous when the action is commited. If I don't commit the action per loop, the parent referenced in

undo_redo->add_do_method(n2d_parent, "add_child", new_bone); undo_redo->add_do_reference(new_bone); undo_redo->add_do_method(n2d_parent, "remove_child", n2d); undo_redo->add_do_method(new_bone, "add_child", n2d);

are outdated.

Am I explaining this right?

I tried with and without nested actions. This is the result. Left side only has one 'big' action, "Create Custom Bone2D(s) from Node(s)" from my commit. The right side has both the 'big' action and the nested "Create Custom Bone2D from Node" action.

I'm not sure I understand. So nested actions are not supported?

They are supported, but they are only executed after the topmost action is committed.

godot/core/object/undo_redo.cpp

Lines 311 to 316 in 53be3b7

void UndoRedo::commit_action(bool p_execute) {

ERR_FAIL_COND(action_level <= 0);

action_level--;

if (action_level > 0) {

return; //still nested

}

Though it's weird that it would make difference in your case 🤔

That is weird... I've tried looking at the undo_redo & manager logic but this is going over my head.

In any case, as far as I understand it, what I'm trying to do shouldn't be supported. The alternative would be to pre-determine what the entire chain should look like after each node's actions would be executed, without actually executing them... I'm not sure if that's the way to go here?

I'll try to look into it more, but for now just fix the continue issue I mentioned initially. Every create_actions() needs a corresponding commit_action().

Found it. It's a bug in EditorUndoRedoManager. Your actions aren't getting nested at all.
I'll clean this up myself in a follow-up PR.

Nice! I'll wait and see how my implementation behaves when the bug is fixed. I'll probably have to predetermine the entire chain in that case.

KoBeWi · 2025-07-04T10:46:22Z

editor/plugins/canvas_item_editor_plugin.cpp

+			// Otherwise leave it to the default bone2D behavior.
+			// Bone2D without Bone2D children in a chain need their angle and length manually adjusted,
+			// `SKELETON_MAKE_BONES` should reduce manual input to create a chain.
+			undo_redo->create_action(TTR("Connect bones to Marker2D end"));


Another unnecessary nested action.

See above comment

KoBeWi · 2025-07-04T10:48:20Z

editor/plugins/canvas_item_editor_plugin.cpp

+			// `SKELETON_MAKE_BONES` should reduce manual input to create a chain.
+			undo_redo->create_action(TTR("Connect bones to Marker2D end"));
+			TypedArray<Marker2D> new_bone_children_marker2d = selected_nodes[0]->find_children("", "Marker2D", true, true);
+			for (int k = 0; k < selected_nodes.size(); k++) {


Also can be range loop. I think TypedArray uses Variant as iteration type.

AThousandShips changed the title ~~Fixed "Create Custom Bone2D(s) from Node(s)" to create working bone chains~~ Fix "Create Custom Bone2D(s) from Node(s)" to create working bone chains Jun 21, 2025

Joey-Einerhand force-pushed the master branch 3 times, most recently from f3606c0 to 82a7e68 Compare June 22, 2025 09:38

AThousandShips added bug topic:editor labels Jun 22, 2025

AThousandShips added this to the 4.5 milestone Jun 22, 2025

AThousandShips requested review from a team June 22, 2025 10:15

AThousandShips added the topic:2d label Jun 22, 2025

This was referenced Jun 22, 2025

The information in the skeleton section of the cutout animation tutorial is outdated godotengine/godot-docs#7517

Open

2D Skeletons tutorial seems outdated. Doesn't mention skeleton 2D node that's apparently needed. godotengine/godot-docs#7279

Closed

Fixed "Create Custom Bone2D(s) from Node(s)" button to create working…

e051ebd

… bone chains

Joey-Einerhand force-pushed the master branch from 82a7e68 to e051ebd Compare July 1, 2025 13:59

KoBeWi reviewed Jul 4, 2025

View reviewed changes

fire added this to Animation Team Issue Triage Jul 5, 2025

fire moved this to Ready for review in Animation Team Issue Triage Jul 5, 2025

	Vector<Node2D *> selected_nodes;
	LocalVector<Node2D *> selected_nodes;
	selected_nodes.reserve(selection.size());

	void UndoRedo::commit_action(bool p_execute) {
	ERR_FAIL_COND(action_level <= 0);
	action_level--;
	if (action_level > 0) {
	return; //still nested
	}

Uh oh!

Fix "Create Custom Bone2D(s) from Node(s)" to create working bone chains #107797

Are you sure you want to change the base?

Fix "Create Custom Bone2D(s) from Node(s)" to create working bone chains #107797

Conversation

Joey-Einerhand commented Jun 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KoBeWi commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Joey-Einerhand commented Jul 1, 2025

Uh oh!

KoBeWi Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KoBeWi Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Joey-Einerhand Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Joey-Einerhand Jul 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KoBeWi Jul 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KoBeWi Jul 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Joey-Einerhand Jul 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Joey-Einerhand commented Jun 21, 2025 •

edited

Loading

KoBeWi commented Jun 27, 2025 •

edited

Loading

KoBeWi Jul 4, 2025 •

edited

Loading

KoBeWi Jul 4, 2025 •

edited

Loading

Joey-Einerhand Jul 4, 2025 •

edited

Loading

Joey-Einerhand Jul 5, 2025 •

edited

Loading

KoBeWi Jul 5, 2025 •

edited

Loading

KoBeWi Jul 5, 2025 •

edited

Loading

Joey-Einerhand Jul 6, 2025 •

edited

Loading