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

Double-clicking a node without script when connecting signals crashes engine #34555

Closed
DleanJeans opened this issue Dec 23, 2019 · 5 comments · Fixed by #35123 or #36267
Closed

Double-clicking a node without script when connecting signals crashes engine #34555

DleanJeans opened this issue Dec 23, 2019 · 5 comments · Fixed by #35123 or #36267

Comments

@DleanJeans
Copy link

Godot version: 3.2 beta 4

OS/device including version: Windows 10

Issue description:

I'm trying to connect a signal to a built-in function (queue_free) of a node without a script. When I try to connect it by double-clicking, the engine crashes.
This, however, works in Advanced mode with no problems.

Steps to reproduce:

  1. Create a new scene + any node
  2. Connect any signal to a built-in function (say hide() to free())

Minimal reproduction project:

@qarmin
Copy link
Contributor

qarmin commented Dec 23, 2019

Backtrace

[1] /lib/x86_64-linux-gnu/libc.so.6(+0x46470) [0x7f2e74186470] (??:0)
[2] HashMap<Variant, List<Pair<Variant const*, Variant>, DefaultAllocator>::Element*, VariantHasher, VariantComparator, (unsigned char)3, (unsigned char)8>::getptr(Variant const&) const (/mnt/KubuntuWolne/godot/./core/hash_map.h:328)
[3] HashMap<Variant, List<Pair<Variant const*, Variant>, DefaultAllocator>::Element*, VariantHasher, VariantComparator, (unsigned char)3, (unsigned char)8>::has(Variant const&) const (/mnt/KubuntuWolne/godot/./core/hash_map.h:285)
[4] OrderedHashMap<Variant, Variant, VariantHasher, VariantComparator, (unsigned char)3, (unsigned char)8>::has(Variant const&) const (/mnt/KubuntuWolne/godot/./core/ordered_hash_map.h:241)
[5] Dictionary::has(Variant const&) const (/mnt/KubuntuWolne/godot/core/dictionary.cpp:136)
[6] Object::has_meta(String const&) const (/mnt/KubuntuWolne/godot/core/object.cpp:1032)
[7] SceneTreeEditor::_add_nodes(Node*, TreeItem*) (/mnt/KubuntuWolne/godot/editor/scene_tree_editor.cpp:195)
[8] SceneTreeEditor::_update_tree() (/mnt/KubuntuWolne/godot/editor/scene_tree_editor.cpp:556)
[9] SceneTreeEditor::set_selected(Node*, bool) (/mnt/KubuntuWolne/godot/editor/scene_tree_editor.cpp:720)
[10] SceneTreeDock::set_selected(Node*, bool) (/mnt/KubuntuWolne/godot/editor/scene_tree_dock.cpp:2165)
[11] EditorNode::push_item(Object*, String const&, bool) (/mnt/KubuntuWolne/godot/editor/editor_node.cpp:1607)
[12] SceneTreeDock::_node_selected() (/mnt/KubuntuWolne/godot/editor/scene_tree_dock.cpp:1204 (discriminator 2))
[13] MethodBind0::call(Object*, Variant const**, int, Variant::CallError&) (/mnt/KubuntuWolne/godot/./core/method_bind.gen.inc:61 (discriminator 4))
[14] Object::call(StringName const&, Variant const**, int, Variant::CallError&) (/mnt/KubuntuWolne/godot/core/object.cpp:921 (discriminator 1))
[15] MessageQueue::_call_function(Object*, StringName const&, Variant const*, int, bool) (/mnt/KubuntuWolne/godot/core/message_queue.cpp:250)
[16] MessageQueue::flush() (/mnt/KubuntuWolne/godot/core/message_queue.cpp:299)
[17] Main::iteration() (/mnt/KubuntuWolne/godot/main/main.cpp:1994)
[18] OS_X11::run() (/mnt/KubuntuWolne/godot/platform/x11/os_x11.cpp:3255)
[19] /usr/bin/godot(main+0x125) [0x13e396b] (/mnt/KubuntuWolne/godot/platform/x11/godot_x11.cpp:57)
[20] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf3) [0x7f2e741671e3] (??:0)
[21] /usr/bin/godot(_start+0x2e) [0x13e378e] (??:?)

@capnm
Copy link
Contributor

capnm commented Dec 23, 2019

I'm trying to connect a signal to a built-in function (queue_free) of a node without a script.

I think it's a bad idea ;-), see #27125 (comment)

@DleanJeans
Copy link
Author

I don't see why. I'm connecting GraphNode's close_request() to queue_free() so I can close it. It works normally except that it crashes when connecting in non-advanced mode.

@capnm
Copy link
Contributor

capnm commented Dec 23, 2019

It may work in some cases, but generally connecting a signal to a built-in function doesn't work as expected. Check the examples in #27125 e.g. #27125 (comment)

@asynts
Copy link
Contributor

asynts commented Jan 14, 2020

I had a very similar issue; it can be reproduced as follows:

  1. Create an node or a tree of nodes where no script is attached to any node.
  2. Open the dialog to connect any signal for that node.
  3. Enter any method name.
  4. Double click on any node.

Here is a video of me causing the crash


I've diagnosed the problem as follows:

Normally it wouldn't be possible to click on the "Connect" button if no node is selected, because the button would be disabled.

However, double clicking on a node is also hooked up to the same signal and double clicking is possible even if the "Connect" button is disabled.

Here is the method that is called when the "Connect" button is pressed, or if a node is double clicked:

 /*
 * Signal automatically called by parent dialog.
 */
void ConnectDialog::ok_pressed() {
    if (dst_method->get_text() == "") {
        error->set_text(TTR("Method in target node must be specified."));
        error->popup_centered_minsize();
        return;
    }
    Node *target = tree->get_selected();
    if (target->get_script().is_null()) {
        if (!target->has_method(dst_method->get_text())) {
            error->set_text(TTR("Target method not found. Specify a valid method or attach a script to the target node."));
            error->popup_centered_minsize();
            return;
        }
    }
    emit_signal("connected");
}

If no method name is provided the method returns early, no crash. Otherwise, the result of tree->get_selected() is de-referenced, causing a segmentation fault, because it is null.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants