Fixes to allow object-less callables throughout Godot

This fixes #81887
This commit is contained in:
Mai Lavelle 2023-09-26 15:58:57 -04:00
parent d31794c4a2
commit 5e15586ec2
9 changed files with 57 additions and 54 deletions

View File

@ -1211,8 +1211,7 @@ void Thread::_start_func(void *ud) {
Ref<Thread> t = *tud; Ref<Thread> t = *tud;
memdelete(tud); memdelete(tud);
Object *target_instance = t->target_callable.get_object(); if (!t->target_callable.is_valid()) {
if (!target_instance) {
t->running.clear(); 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())); 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()));
} }

View File

@ -1109,8 +1109,7 @@ Error Object::emit_signalp(const StringName &p_name, const Variant **p_args, int
Error err = OK; Error err = OK;
for (const Connection &c : slot_conns) { for (const Connection &c : slot_conns) {
Object *target = c.callable.get_object(); if (!c.callable.is_valid()) {
if (!target) {
// Target might have been deleted during signal callback, this is expected and OK. // Target might have been deleted during signal callback, this is expected and OK.
continue; continue;
} }
@ -1133,7 +1132,8 @@ Error Object::emit_signalp(const StringName &p_name, const Variant **p_args, int
continue; continue;
} }
#endif #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. //most likely object is not initialized yet, do not throw error.
} else { } else {
ERR_PRINT("Error calling from signal '" + String(p_name) + "' to callable: " + Variant::get_callable_error_text(c.callable, args, argc, ce) + "."); ERR_PRINT("Error calling from signal '" + String(p_name) + "' to callable: " + Variant::get_callable_error_text(c.callable, args, argc, ce) + ".");
@ -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) { 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."); 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(); if (p_callable.is_standard()) {
ERR_FAIL_NULL_V_MSG(target_object, ERR_INVALID_PARAMETER, "Cannot connect to '" + p_signal + "' to callable '" + p_callable + "': the callable object is null."); // 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); SignalData *s = signal_map.getptr(p_signal);
if (!s) { if (!s) {
@ -1352,6 +1358,8 @@ Error Object::connect(const StringName &p_signal, const Callable &p_callable, ui
} }
} }
Object *target_object = p_callable.get_object();
SignalData::Slot slot; SignalData::Slot slot;
Connection conn; Connection conn;
@ -1359,7 +1367,9 @@ Error Object::connect(const StringName &p_signal, const Callable &p_callable, ui
conn.signal = ::Signal(this, p_signal); conn.signal = ::Signal(this, p_signal);
conn.flags = p_flags; conn.flags = p_flags;
slot.conn = conn; slot.conn = conn;
if (target_object) {
slot.cE = target_object->connections.push_back(conn); slot.cE = target_object->connections.push_back(conn);
}
if (p_flags & CONNECT_REFERENCE_COUNTED) { if (p_flags & CONNECT_REFERENCE_COUNTED) {
slot.reference_count = 1; slot.reference_count = 1;
} }
@ -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) { 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."); 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); SignalData *s = signal_map.getptr(p_signal);
if (!s) { if (!s) {
bool signal_is_valid = ClassDB::has_signal(get_class_name(), p_signal) || bool signal_is_valid = ClassDB::has_signal(get_class_name(), p_signal) ||
@ -1420,7 +1427,13 @@ bool Object::_disconnect(const StringName &p_signal, const Callable &p_callable,
} }
} }
if (slot->cE) {
Object *target_object = p_callable.get_object();
if (target_object) {
target_object->connections.erase(slot->cE); target_object->connections.erase(slot->cE);
}
}
s->slot_map.erase(*p_callable.get_base_comparator()); s->slot_map.erase(*p_callable.get_base_comparator());
if (s->slot_map.is_empty() && ClassDB::has_signal(get_class_name(), p_signal)) { if (s->slot_map.is_empty() && ClassDB::has_signal(get_class_name(), p_signal)) {

View File

@ -136,17 +136,22 @@ void UndoRedo::add_do_method(const Callable &p_callable) {
ERR_FAIL_COND(action_level <= 0); ERR_FAIL_COND(action_level <= 0);
ERR_FAIL_COND((current_action + 1) >= actions.size()); ERR_FAIL_COND((current_action + 1) >= actions.size());
Object *object = p_callable.get_object(); ObjectID object_id = p_callable.get_object_id();
ERR_FAIL_NULL(object); Object *object = ObjectDB::get_instance(object_id);
ERR_FAIL_COND(object_id.is_valid() && object == nullptr);
Operation do_op; Operation do_op;
do_op.callable = p_callable; do_op.callable = p_callable;
do_op.object = p_callable.get_object_id(); do_op.object = object_id;
if (Object::cast_to<RefCounted>(object)) { if (Object::cast_to<RefCounted>(object)) {
do_op.ref = Ref<RefCounted>(Object::cast_to<RefCounted>(object)); do_op.ref = Ref<RefCounted>(Object::cast_to<RefCounted>(object));
} }
do_op.type = Operation::TYPE_METHOD; do_op.type = Operation::TYPE_METHOD;
do_op.name = p_callable.get_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); actions.write[current_action + 1].do_ops.push_back(do_op);
} }
@ -161,18 +166,23 @@ void UndoRedo::add_undo_method(const Callable &p_callable) {
return; return;
} }
Object *object = p_callable.get_object(); ObjectID object_id = p_callable.get_object_id();
ERR_FAIL_NULL(object); Object *object = ObjectDB::get_instance(object_id);
ERR_FAIL_COND(object_id.is_valid() && object == nullptr);
Operation undo_op; Operation undo_op;
undo_op.callable = p_callable; undo_op.callable = p_callable;
undo_op.object = p_callable.get_object_id(); undo_op.object = object_id;
if (Object::cast_to<RefCounted>(object)) { if (Object::cast_to<RefCounted>(object)) {
undo_op.ref = Ref<RefCounted>(Object::cast_to<RefCounted>(object)); undo_op.ref = Ref<RefCounted>(Object::cast_to<RefCounted>(object));
} }
undo_op.type = Operation::TYPE_METHOD; undo_op.type = Operation::TYPE_METHOD;
undo_op.force_keep_in_merge_ends = force_keep_in_merge_ends; undo_op.force_keep_in_merge_ends = force_keep_in_merge_ends;
undo_op.name = p_callable.get_method(); 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); actions.write[current_action + 1].undo_ops.push_back(undo_op);
} }

View File

@ -47,6 +47,13 @@ void Callable::callp(const Variant **p_arguments, int p_argcount, Variant &r_ret
r_call_error.expected = 0; r_call_error.expected = 0;
r_return_value = Variant(); r_return_value = Variant();
} else if (is_custom()) { } 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); custom->call(p_arguments, p_argcount, r_return_value, r_call_error);
} else { } else {
Object *obj = ObjectDB::get_instance(ObjectID(object)); Object *obj = ObjectDB::get_instance(ObjectID(object));

View File

@ -675,7 +675,7 @@ bool CallbackTweener::step(double &r_delta) {
return false; return false;
} }
if (!callback.get_object()) { if (!callback.is_valid()) {
return false; return false;
} }
@ -740,7 +740,7 @@ bool MethodTweener::step(double &r_delta) {
return false; return false;
} }
if (!callback.get_object()) { if (!callback.is_valid()) {
return false; return false;
} }

View File

@ -78,13 +78,6 @@ void GodotArea2D::set_space(GodotSpace2D *p_space) {
} }
void GodotArea2D::set_monitor_callback(const Callable &p_callback) { 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(); _unregister_shapes();
monitor_callback = p_callback; monitor_callback = p_callback;
@ -100,13 +93,6 @@ void GodotArea2D::set_monitor_callback(const Callable &p_callback) {
} }
void GodotArea2D::set_area_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(); _unregister_shapes();
area_monitor_callback = p_callback; area_monitor_callback = p_callback;

View File

@ -615,7 +615,7 @@ void GodotBody2D::integrate_velocities(real_t p_step) {
return; 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); get_space()->body_add_to_state_query_list(&direct_state_query_list);
} }
@ -676,7 +676,7 @@ void GodotBody2D::call_queries() {
Variant direct_state_variant = get_direct_state(); Variant direct_state_variant = get_direct_state();
if (fi_callback_data) { if (fi_callback_data) {
if (!fi_callback_data->callable.get_object()) { if (!fi_callback_data->callable.is_valid()) {
set_force_integration_callback(Callable()); set_force_integration_callback(Callable());
} else { } else {
const Variant *vp[2] = { &direct_state_variant, &fi_callback_data->udata }; const Variant *vp[2] = { &direct_state_variant, &fi_callback_data->udata };
@ -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); body_state_callback.call(direct_state_variant);
} }
} }
@ -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) { 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) { if (!fi_callback_data) {
fi_callback_data = memnew(ForceIntegrationCallbackData); fi_callback_data = memnew(ForceIntegrationCallbackData);
} }

View File

@ -87,12 +87,6 @@ void GodotArea3D::set_space(GodotSpace3D *p_space) {
} }
void GodotArea3D::set_monitor_callback(const Callable &p_callback) { 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(); _unregister_shapes();
monitor_callback = p_callback; monitor_callback = p_callback;
@ -108,12 +102,6 @@ void GodotArea3D::set_monitor_callback(const Callable &p_callback) {
} }
void GodotArea3D::set_area_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(); _unregister_shapes();
area_monitor_callback = p_callback; area_monitor_callback = p_callback;

View File

@ -674,7 +674,7 @@ void GodotBody3D::integrate_velocities(real_t p_step) {
return; 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); get_space()->body_add_to_state_query_list(&direct_state_query_list);
} }
@ -759,7 +759,7 @@ void GodotBody3D::call_queries() {
Variant direct_state_variant = get_direct_state(); Variant direct_state_variant = get_direct_state();
if (fi_callback_data) { if (fi_callback_data) {
if (!fi_callback_data->callable.get_object()) { if (!fi_callback_data->callable.is_valid()) {
set_force_integration_callback(Callable()); set_force_integration_callback(Callable());
} else { } else {
const Variant *vp[2] = { &direct_state_variant, &fi_callback_data->udata }; const Variant *vp[2] = { &direct_state_variant, &fi_callback_data->udata };
@ -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); body_state_callback.call(direct_state_variant);
} }
} }
@ -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) { 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) { if (!fi_callback_data) {
fi_callback_data = memnew(ForceIntegrationCallbackData); fi_callback_data = memnew(ForceIntegrationCallbackData);
} }