From 1c9203ad6873758df92aa5ba2081869524372c27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Verschelde?= Date: Tue, 20 Apr 2021 11:33:26 +0200 Subject: [PATCH] Object: Make deleted object access raise errors, not warnings Clarify doc about not decaying to `null` for `free` and `queue_free`. Part of #45639. --- core/variant.cpp | 2 +- core/variant_call.cpp | 4 ++-- core/variant_op.cpp | 18 +++++++++--------- doc/classes/Node.xml | 1 + doc/classes/Object.xml | 3 ++- 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/core/variant.cpp b/core/variant.cpp index 2eb4d98e93..9a068c0fcc 100644 --- a/core/variant.cpp +++ b/core/variant.cpp @@ -1769,7 +1769,7 @@ Variant::operator RID() const { Object *obj = likely(_get_obj().rc) ? _get_obj().rc->get_ptr() : NULL; if (unlikely(!obj)) { if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { - WARN_PRINT("Attempted get RID on a deleted object."); + ERR_PRINT("Attempted get RID on a deleted object."); } return RID(); } diff --git a/core/variant_call.cpp b/core/variant_call.cpp index 4cdaf13565..eb0e766842 100644 --- a/core/variant_call.cpp +++ b/core/variant_call.cpp @@ -1139,7 +1139,7 @@ void Variant::call_ptr(const StringName &p_method, const Variant **p_args, int p if (!obj) { #ifdef DEBUG_ENABLED if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { - WARN_PRINT("Attempted call on a deleted object."); + ERR_PRINT("Attempted method call on a deleted object."); } #endif r_error.error = CallError::CALL_ERROR_INSTANCE_IS_NULL; @@ -1318,7 +1318,7 @@ bool Variant::has_method(const StringName &p_method) const { if (!obj) { #ifdef DEBUG_ENABLED if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { - WARN_PRINT("Attempted method check on a deleted object."); + ERR_PRINT("Attempted method check on a deleted object."); } #endif return false; diff --git a/core/variant_op.cpp b/core/variant_op.cpp index fb67f18691..bd3c46a2ee 100644 --- a/core/variant_op.cpp +++ b/core/variant_op.cpp @@ -1517,7 +1517,7 @@ void Variant::set_named(const StringName &p_index, const Variant &p_value, bool #ifdef DEBUG_ENABLED if (unlikely(!obj)) { if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { - WARN_PRINT("Attempted set on a deleted object."); + ERR_PRINT("Attempted set on a deleted object."); } break; } @@ -1685,7 +1685,7 @@ Variant Variant::get_named(const StringName &p_index, bool *r_valid) const { if (r_valid) *r_valid = false; if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { - WARN_PRINT("Attempted get on a deleted object."); + ERR_PRINT("Attempted get on a deleted object."); } return Variant(); } @@ -2170,7 +2170,7 @@ void Variant::set(const Variant &p_index, const Variant &p_value, bool *r_valid) #ifdef DEBUG_ENABLED valid = false; if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { - WARN_PRINT("Attempted set on a deleted object."); + ERR_PRINT("Attempted set on a deleted object."); } #endif return; @@ -2540,7 +2540,7 @@ Variant Variant::get(const Variant &p_index, bool *r_valid) const { #ifdef DEBUG_ENABLED valid = false; if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { - WARN_PRINT("Attempted get on a deleted object."); + ERR_PRINT("Attempted get on a deleted object."); } #endif return Variant(); @@ -2603,7 +2603,7 @@ bool Variant::in(const Variant &p_index, bool *r_valid) const { *r_valid = false; } if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { - WARN_PRINT("Attempted 'in' on a deleted object."); + ERR_PRINT("Attempted 'in' on a deleted object."); } #endif return false; @@ -2866,7 +2866,7 @@ void Variant::get_property_list(List *p_list) const { if (unlikely(!obj)) { #ifdef DEBUG_ENABLED if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { - WARN_PRINT("Attempted get property list on a deleted object."); + ERR_PRINT("Attempted get property list on a deleted object."); } #endif return; @@ -2944,7 +2944,7 @@ bool Variant::iter_init(Variant &r_iter, bool &valid) const { if (unlikely(!obj)) { valid = false; if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { - WARN_PRINT("Attempted iteration start on a deleted object."); + ERR_PRINT("Attempted iteration start on a deleted object."); } return false; } @@ -3111,7 +3111,7 @@ bool Variant::iter_next(Variant &r_iter, bool &valid) const { if (unlikely(!obj)) { valid = false; if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { - WARN_PRINT("Attempted iteration check next on a deleted object."); + ERR_PRINT("Attempted iteration check next on a deleted object."); } return false; } @@ -3269,7 +3269,7 @@ Variant Variant::iter_get(const Variant &r_iter, bool &r_valid) const { if (unlikely(!obj)) { r_valid = false; if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { - WARN_PRINT("Attempted iteration get next on a deleted object."); + ERR_PRINT("Attempted iteration get next on a deleted object."); } return Variant(); } diff --git a/doc/classes/Node.xml b/doc/classes/Node.xml index 7265dae42c..0cf20ba076 100644 --- a/doc/classes/Node.xml +++ b/doc/classes/Node.xml @@ -564,6 +564,7 @@ Queues a node for deletion at the end of the current frame. When deleted, all of its child nodes will be deleted as well. This method ensures it's safe to delete the node, contrary to [method Object.free]. Use [method Object.is_queued_for_deletion] to check whether a node will be deleted at the end of the frame. + [b]Important:[/b] If you have a variable pointing to a node, it will [i]not[/i] be assigned to [code]null[/code] once the node is freed. Instead, it will point to a [i]previously freed instance[/i] and you should validate it with [method @GDScript.is_instance_valid] before attempting to call its methods or access its properties. diff --git a/doc/classes/Object.xml b/doc/classes/Object.xml index 6fc117014d..193bf28a32 100644 --- a/doc/classes/Object.xml +++ b/doc/classes/Object.xml @@ -201,7 +201,8 @@ - Deletes the object from memory. Any pre-existing reference to the freed object will become invalid, e.g. [code]is_instance_valid(object)[/code] will return [code]false[/code]. + Deletes the object from memory immediately. For [Node]s, you may want to use [method Node.queue_free] to queue the node for safe deletion at the end of the current frame. + [b]Important:[/b] If you have a variable pointing to an object, it will [i]not[/i] be assigned to [code]null[/code] once the object is freed. Instead, it will point to a [i]previously freed instance[/i] and you should validate it with [method @GDScript.is_instance_valid] before attempting to call its methods or access its properties.