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

is_connected returns false when it handler is registered #1293

Closed
Naros opened this issue Nov 1, 2023 · 2 comments · Fixed by #1294
Closed

is_connected returns false when it handler is registered #1293

Naros opened this issue Nov 1, 2023 · 2 comments · Fixed by #1294
Assignees
Labels
bug This has been identified as a bug confirmed topic:gdextension This relates to the new Godot 4 extension implementation
Milestone

Comments

@Naros
Copy link
Contributor

Naros commented Nov 1, 2023

Godot version

4.2

godot-cpp version

4.2

System information

Windows 11

Issue description

When verifying whether a code path is connected to another object when using is_connected, it always returns false, i.e.:

if (!obj->is_connected("changed", callable_mp(this, &MyClass::_on_changed)))
  obj->connect("changed", callable_mp(this, &MyClass::_on_changed));

When this code path is executed twice for the same obj reference, it should only register the connection once.

Steps to reproduce

  1. In a simple GDExtension example, add the following C++ header and implementation file:
// test.hpp

#include <godot_cpp/classes/node3d.hpp>
#include <godot_cpp/classes/resource.hpp>
#include <godot_cpp/templates/hash_map.hpp>
#include <godot_cpp/variant/utility_functions.hpp>

using namespace godot;

class CallableResource : public Resource
{
    GDCLASS(CallableResource, Resource);

    bool _value = false;

    static void _bind_methods() { }

    CallableResource() {}

    bool get_value() const { return _value; }
    void set_value(bool p_value) { _value = p_value; emit_changed(); }
};

class CallableNode : public Node3D
{
    GDCLASS(CallableNode, Node3D);

    HashMap<StringName, Ref<CallableResource>> _resources;

    static void _bind_methods();

    void _on_changed();

    CallableNode();

    bool get_value() const { return _resources["test"]->get_value(); }
    void set_value(bool p_value);

    int get_num_connections(Ref<CallableResource> res) { return res->get_signal_connection_list("changed").size(); }
};
// implementation
#include "test.h"

void CallableNode::_bind_methods()
{
    ClassDB::bind_method(D_METHOD("get_value"), &CallableNode::get_value);
    ClassDB::bind_method(D_METHOD("set_value", "value"), &CallableNode::set_value);
    ADD_PROPERTY(PropertyInfo(Variant::BOOL, "value"), "set_value", "get_value");
}

CallableNode::CallableNode()
{
    _resources["test"] = Ref<CallableResource>(memnew(CallableResource));
}

void CallableNode::_on_changed()
{
    UtilityFunctions::print("Resource 'changed' callback fired");
}

void CallableNode::set_value(bool p_value)
{
    Ref<CallableResource> res = _resources["test"];
    UtilityFunctions::print("Resource currently has ", get_num_connections(res), " active connections to 'changed'");

    if (res.is_valid() && !res->is_connected("changed", callable_mp(this, &CallableNode::_on_changed)))
    {
        UtilityFunctions::print("Connecting to resource's 'changed' signal.");
        res->connect("changed", callable_mp(this, &CallableNode::_on_changed));
        UtilityFunctions::print("Resource now has ", get_num_connections(res), " active connections to 'changed'");
    }
    else
    {
        UtilityFunctions::print("Callback already registered.");
    }

    if (res.is_valid())
    {
        res->set_value(p_value);
        notify_property_list_changed();
    }
}
  1. Register the two classes during start-up in EDITOR or SCENE scope.
  2. In a blank project using the GDExtension sample, create a test scene, and add the CallableNode to the scene.
  3. In the inspector dock, there will be a check-box that you can toggle.
  4. The first click establishes the first connection
  5. The subsequent clicks establishes a new connection per click
  6. Logs will report the number of connections for the changed signal before and after the toggle's logic.

Since there is only ever 1 CallableResource created and there is only 1 instance of the CallableNode, there should only ever be a single connection established; however the connections continue to accrue and the "Callback already registered" text is never output to the logs although it should on the 2+ clicks of the checkbox in the inspector.

Minimal reproduction project

N/A

@dsnopek dsnopek added bug This has been identified as a bug topic:gdextension This relates to the new Godot 4 extension implementation labels Nov 2, 2023
@dsnopek dsnopek added this to the 4.x milestone Nov 2, 2023
@dsnopek
Copy link
Collaborator

dsnopek commented Nov 2, 2023

Thanks! I'm able to reproduce this issue and I have an idea about a fix

@dsnopek
Copy link
Collaborator

dsnopek commented Nov 10, 2023

This would be fixed by PR #1294 which is still awaiting review

@akien-mga akien-mga modified the milestones: 4.x, 4.2 Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This has been identified as a bug confirmed topic:gdextension This relates to the new Godot 4 extension implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants