Skip to content

Commit

Permalink
Fixes to allow object-less callables throughout Godot
Browse files Browse the repository at this point in the history
This fixes #81887
  • Loading branch information
maiself committed Oct 6, 2023
1 parent d31794c commit 5e15586
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 54 deletions.
3 changes: 1 addition & 2 deletions core/core_bind.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1211,8 +1211,7 @@ void Thread::_start_func(void *ud) {
Ref<Thread> t = *tud;
memdelete(tud);

Object *target_instance = t->target_callable.get_object();
if (!target_instance) {
if (!t->target_callable.is_valid()) {
t->running.clear();
ERR_FAIL_MSG(vformat("Could not call function '%s' on previously freed instance to start thread %s.", t->target_callable.get_method(), t->get_id()));
}
Expand Down
33 changes: 23 additions & 10 deletions core/object/object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1109,8 +1109,7 @@ Error Object::emit_signalp(const StringName &p_name, const Variant **p_args, int
Error err = OK;

for (const Connection &c : slot_conns) {
Object *target = c.callable.get_object();
if (!target) {
if (!c.callable.is_valid()) {
// Target might have been deleted during signal callback, this is expected and OK.
continue;
}
Expand All @@ -1133,7 +1132,8 @@ Error Object::emit_signalp(const StringName &p_name, const Variant **p_args, int
continue;
}
#endif
if (ce.error == Callable::CallError::CALL_ERROR_INVALID_METHOD && !ClassDB::class_exists(target->get_class_name())) {
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) + ".");
Expand Down Expand Up @@ -1313,8 +1313,14 @@ void Object::get_signals_connected_to_this(List<Connection> *p_connections) cons
Error Object::connect(const StringName &p_signal, const Callable &p_callable, uint32_t p_flags) {
ERR_FAIL_COND_V_MSG(p_callable.is_null(), ERR_INVALID_PARAMETER, "Cannot connect to '" + p_signal + "': the provided callable is null.");

Object *target_object = p_callable.get_object();
ERR_FAIL_NULL_V_MSG(target_object, ERR_INVALID_PARAMETER, "Cannot connect to '" + p_signal + "' to callable '" + p_callable + "': the callable object is null.");
if (p_callable.is_standard()) {
// FIXME: This branch should probably removed in favor of the `is_valid()` branch, but there exist some classes
// that call `connect()` before they are fully registered with ClassDB. Until all such classes can be found
// and registered soon enough this branch is needed to allow `connect()` to succeed.
ERR_FAIL_NULL_V_MSG(p_callable.get_object(), ERR_INVALID_PARAMETER, "Cannot connect to '" + p_signal + "' to callable '" + p_callable + "': the callable object is null.");
} else {
ERR_FAIL_COND_V_MSG(!p_callable.is_valid(), ERR_INVALID_PARAMETER, "Cannot connect to '" + p_signal + "': the provided callable is not valid: " + p_callable);
}

SignalData *s = signal_map.getptr(p_signal);
if (!s) {
Expand Down Expand Up @@ -1352,14 +1358,18 @@ Error Object::connect(const StringName &p_signal, const Callable &p_callable, ui
}
}

Object *target_object = p_callable.get_object();

SignalData::Slot slot;

Connection conn;
conn.callable = target;
conn.signal = ::Signal(this, p_signal);
conn.flags = p_flags;
slot.conn = conn;
slot.cE = target_object->connections.push_back(conn);
if (target_object) {
slot.cE = target_object->connections.push_back(conn);
}
if (p_flags & CONNECT_REFERENCE_COUNTED) {
slot.reference_count = 1;
}
Expand Down Expand Up @@ -1398,9 +1408,6 @@ void Object::disconnect(const StringName &p_signal, const Callable &p_callable)
bool Object::_disconnect(const StringName &p_signal, const Callable &p_callable, bool p_force) {
ERR_FAIL_COND_V_MSG(p_callable.is_null(), false, "Cannot disconnect from '" + p_signal + "': the provided callable is null.");

Object *target_object = p_callable.get_object();
ERR_FAIL_NULL_V_MSG(target_object, false, "Cannot disconnect '" + p_signal + "' from callable '" + p_callable + "': the callable object is null.");

SignalData *s = signal_map.getptr(p_signal);
if (!s) {
bool signal_is_valid = ClassDB::has_signal(get_class_name(), p_signal) ||
Expand All @@ -1420,7 +1427,13 @@ bool Object::_disconnect(const StringName &p_signal, const Callable &p_callable,
}
}

target_object->connections.erase(slot->cE);
if (slot->cE) {
Object *target_object = p_callable.get_object();
if (target_object) {
target_object->connections.erase(slot->cE);
}
}

s->slot_map.erase(*p_callable.get_base_comparator());

if (s->slot_map.is_empty() && ClassDB::has_signal(get_class_name(), p_signal)) {
Expand Down
22 changes: 16 additions & 6 deletions core/object/undo_redo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,17 +136,22 @@ void UndoRedo::add_do_method(const Callable &p_callable) {
ERR_FAIL_COND(action_level <= 0);
ERR_FAIL_COND((current_action + 1) >= actions.size());

Object *object = p_callable.get_object();
ERR_FAIL_NULL(object);
ObjectID object_id = p_callable.get_object_id();
Object *object = ObjectDB::get_instance(object_id);
ERR_FAIL_COND(object_id.is_valid() && object == nullptr);

Operation do_op;
do_op.callable = p_callable;
do_op.object = p_callable.get_object_id();
do_op.object = object_id;
if (Object::cast_to<RefCounted>(object)) {
do_op.ref = Ref<RefCounted>(Object::cast_to<RefCounted>(object));
}
do_op.type = Operation::TYPE_METHOD;
do_op.name = p_callable.get_method();
if (do_op.name == StringName()) {
// There's no `get_method()` for custom callables, so use `operator String()` instead.
do_op.name = static_cast<String>(p_callable);
}

actions.write[current_action + 1].do_ops.push_back(do_op);
}
Expand All @@ -161,18 +166,23 @@ void UndoRedo::add_undo_method(const Callable &p_callable) {
return;
}

Object *object = p_callable.get_object();
ERR_FAIL_NULL(object);
ObjectID object_id = p_callable.get_object_id();
Object *object = ObjectDB::get_instance(object_id);
ERR_FAIL_COND(object_id.is_valid() && object == nullptr);

Operation undo_op;
undo_op.callable = p_callable;
undo_op.object = p_callable.get_object_id();
undo_op.object = object_id;
if (Object::cast_to<RefCounted>(object)) {
undo_op.ref = Ref<RefCounted>(Object::cast_to<RefCounted>(object));
}
undo_op.type = Operation::TYPE_METHOD;
undo_op.force_keep_in_merge_ends = force_keep_in_merge_ends;
undo_op.name = p_callable.get_method();
if (undo_op.name == StringName()) {
// There's no `get_method()` for custom callables, so use `operator String()` instead.
undo_op.name = static_cast<String>(p_callable);
}

actions.write[current_action + 1].undo_ops.push_back(undo_op);
}
Expand Down
7 changes: 7 additions & 0 deletions core/variant/callable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@ void Callable::callp(const Variant **p_arguments, int p_argcount, Variant &r_ret
r_call_error.expected = 0;
r_return_value = Variant();
} else if (is_custom()) {
if (!is_valid()) {
r_call_error.error = CallError::CALL_ERROR_INSTANCE_IS_NULL;
r_call_error.argument = 0;
r_call_error.expected = 0;
r_return_value = Variant();
return;
}
custom->call(p_arguments, p_argcount, r_return_value, r_call_error);
} else {
Object *obj = ObjectDB::get_instance(ObjectID(object));
Expand Down
4 changes: 2 additions & 2 deletions scene/animation/tween.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,7 @@ bool CallbackTweener::step(double &r_delta) {
return false;
}

if (!callback.get_object()) {
if (!callback.is_valid()) {
return false;
}

Expand Down Expand Up @@ -740,7 +740,7 @@ bool MethodTweener::step(double &r_delta) {
return false;
}

if (!callback.get_object()) {
if (!callback.is_valid()) {
return false;
}

Expand Down
14 changes: 0 additions & 14 deletions servers/physics_2d/godot_area_2d.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,6 @@ void GodotArea2D::set_space(GodotSpace2D *p_space) {
}

void GodotArea2D::set_monitor_callback(const Callable &p_callback) {
ObjectID id = p_callback.get_object_id();

if (id == monitor_callback.get_object_id()) {
monitor_callback = p_callback;
return;
}

_unregister_shapes();

monitor_callback = p_callback;
Expand All @@ -100,13 +93,6 @@ void GodotArea2D::set_monitor_callback(const Callable &p_callback) {
}

void GodotArea2D::set_area_monitor_callback(const Callable &p_callback) {
ObjectID id = p_callback.get_object_id();

if (id == area_monitor_callback.get_object_id()) {
area_monitor_callback = p_callback;
return;
}

_unregister_shapes();

area_monitor_callback = p_callback;
Expand Down
8 changes: 4 additions & 4 deletions servers/physics_2d/godot_body_2d.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,7 @@ void GodotBody2D::integrate_velocities(real_t p_step) {
return;
}

if (fi_callback_data || body_state_callback.get_object()) {
if (fi_callback_data || body_state_callback.is_valid()) {
get_space()->body_add_to_state_query_list(&direct_state_query_list);
}

Expand Down Expand Up @@ -676,7 +676,7 @@ void GodotBody2D::call_queries() {
Variant direct_state_variant = get_direct_state();

if (fi_callback_data) {
if (!fi_callback_data->callable.get_object()) {
if (!fi_callback_data->callable.is_valid()) {
set_force_integration_callback(Callable());
} else {
const Variant *vp[2] = { &direct_state_variant, &fi_callback_data->udata };
Expand All @@ -692,7 +692,7 @@ void GodotBody2D::call_queries() {
}
}

if (body_state_callback.get_object()) {
if (body_state_callback.is_valid()) {
body_state_callback.call(direct_state_variant);
}
}
Expand All @@ -719,7 +719,7 @@ void GodotBody2D::set_state_sync_callback(const Callable &p_callable) {
}

void GodotBody2D::set_force_integration_callback(const Callable &p_callable, const Variant &p_udata) {
if (p_callable.get_object()) {
if (p_callable.is_valid()) {
if (!fi_callback_data) {
fi_callback_data = memnew(ForceIntegrationCallbackData);
}
Expand Down
12 changes: 0 additions & 12 deletions servers/physics_3d/godot_area_3d.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,6 @@ void GodotArea3D::set_space(GodotSpace3D *p_space) {
}

void GodotArea3D::set_monitor_callback(const Callable &p_callback) {
ObjectID id = p_callback.get_object_id();
if (id == monitor_callback.get_object_id()) {
monitor_callback = p_callback;
return;
}

_unregister_shapes();

monitor_callback = p_callback;
Expand All @@ -108,12 +102,6 @@ void GodotArea3D::set_monitor_callback(const Callable &p_callback) {
}

void GodotArea3D::set_area_monitor_callback(const Callable &p_callback) {
ObjectID id = p_callback.get_object_id();
if (id == area_monitor_callback.get_object_id()) {
area_monitor_callback = p_callback;
return;
}

_unregister_shapes();

area_monitor_callback = p_callback;
Expand Down
8 changes: 4 additions & 4 deletions servers/physics_3d/godot_body_3d.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ void GodotBody3D::integrate_velocities(real_t p_step) {
return;
}

if (fi_callback_data || body_state_callback.get_object()) {
if (fi_callback_data || body_state_callback.is_valid()) {
get_space()->body_add_to_state_query_list(&direct_state_query_list);
}

Expand Down Expand Up @@ -759,7 +759,7 @@ void GodotBody3D::call_queries() {
Variant direct_state_variant = get_direct_state();

if (fi_callback_data) {
if (!fi_callback_data->callable.get_object()) {
if (!fi_callback_data->callable.is_valid()) {
set_force_integration_callback(Callable());
} else {
const Variant *vp[2] = { &direct_state_variant, &fi_callback_data->udata };
Expand All @@ -771,7 +771,7 @@ void GodotBody3D::call_queries() {
}
}

if (body_state_callback.get_object()) {
if (body_state_callback.is_valid()) {
body_state_callback.call(direct_state_variant);
}
}
Expand All @@ -798,7 +798,7 @@ void GodotBody3D::set_state_sync_callback(const Callable &p_callable) {
}

void GodotBody3D::set_force_integration_callback(const Callable &p_callable, const Variant &p_udata) {
if (p_callable.get_object()) {
if (p_callable.is_valid()) {
if (!fi_callback_data) {
fi_callback_data = memnew(ForceIntegrationCallbackData);
}
Expand Down

0 comments on commit 5e15586

Please sign in to comment.