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

One Shot signals do not disconnect before emission #95222

Closed
anty567567 opened this issue Aug 7, 2024 · 1 comment
Closed

One Shot signals do not disconnect before emission #95222

anty567567 opened this issue Aug 7, 2024 · 1 comment
Milestone

Comments

@anty567567
Copy link

Tested versions

  • Reproducible in Godot v4.2.2.stable.official [15073af]

System information

Windows 10, CPU - i9 12900K, GPU RTX 3070Ti

Issue description

When the callback for a signal that was connected using the CONNECTION_ONE_SHOT flag is processed, the connection is not disconnected as is suggested here in the source code:

image

This is shown by trying to make the same connection in the callback function and it throwing an error that the connection already exists:
E 0:00:27:0611 FamiliarAI.gd:42 @ make_decision(): Signal 'outgoing_done_acting' is already connected to given callable 'Node3D(FamiliarAI.gd)::recover' in that object.
<C++ Error> Method/function failed. Returning: ERR_INVALID_PARAMETER
<C++ Source> core/object/object.cpp:1358 @ connect()

Steps to reproduce

Run the mrp provided to see that the signal does not disconnect before calling the callable.

Minimal reproduction project (MRP)

mrp.zip

@kleonc
Copy link
Member

kleonc commented Aug 7, 2024

Tested versions

  • Reproducible in Godot v4.2.2.stable.official [15073af]

Issue description

When the callback for a signal that was connected using the CONNECTION_ONE_SHOT flag is processed, the connection is not disconnected as is suggested here in the source code:

image

That's not the source for v4.2.2.stable.official [15073af], here's the actual source (check the commit hash):

godot/core/object/object.cpp

Lines 1094 to 1166 in 15073af

List<_ObjectSignalDisconnectData> disconnect_data;
// Ensure that disconnecting the signal or even deleting the object
// will not affect the signal calling.
LocalVector<Connection> slot_conns;
slot_conns.resize(s->slot_map.size());
{
uint32_t idx = 0;
for (const KeyValue<Callable, SignalData::Slot> &slot_kv : s->slot_map) {
slot_conns[idx++] = slot_kv.value.conn;
}
DEV_ASSERT(idx == s->slot_map.size());
}
OBJ_DEBUG_LOCK
Error err = OK;
for (const Connection &c : slot_conns) {
if (!c.callable.is_valid()) {
// Target might have been deleted during signal callback, this is expected and OK.
continue;
}
const Variant **args = p_args;
int argc = p_argcount;
if (c.flags & CONNECT_DEFERRED) {
MessageQueue::get_singleton()->push_callablep(c.callable, args, argc, true);
} else {
Callable::CallError ce;
_emitting = true;
Variant ret;
c.callable.callp(args, argc, ret, ce);
_emitting = false;
if (ce.error != Callable::CallError::CALL_OK) {
#ifdef DEBUG_ENABLED
if (c.flags & CONNECT_PERSIST && Engine::get_singleton()->is_editor_hint() && (script.is_null() || !Ref<Script>(script)->is_tool())) {
continue;
}
#endif
Object *target = c.callable.get_object();
if (ce.error == Callable::CallError::CALL_ERROR_INVALID_METHOD && target && !ClassDB::class_exists(target->get_class_name())) {
//most likely object is not initialized yet, do not throw error.
} else {
ERR_PRINT("Error calling from signal '" + String(p_name) + "' to callable: " + Variant::get_callable_error_text(c.callable, args, argc, ce) + ".");
err = ERR_METHOD_NOT_FOUND;
}
}
}
bool disconnect = c.flags & CONNECT_ONE_SHOT;
#ifdef TOOLS_ENABLED
if (disconnect && (c.flags & CONNECT_PERSIST) && Engine::get_singleton()->is_editor_hint()) {
//this signal was connected from the editor, and is being edited. just don't disconnect for now
disconnect = false;
}
#endif
if (disconnect) {
_ObjectSignalDisconnectData dd;
dd.signal = p_name;
dd.callable = c.callable;
disconnect_data.push_back(dd);
}
}
while (!disconnect_data.is_empty()) {
const _ObjectSignalDisconnectData &dd = disconnect_data.front()->get();
_disconnect(dd.signal, dd.callable);
disconnect_data.pop_front();
}


Fixed by #89451 (since v4.3.dev6.official [89850d5]).

MRP of course still results in stack overflow, according to the code. But no more errors. From v4.3.rc2.official [3978628]:
Godot_v4 3-rc2_win64_CVtP4qt5dQ

@kleonc kleonc closed this as completed Aug 7, 2024
@kleonc kleonc added this to the 4.3 milestone Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants