Browse Source

Merge pull request #48041 from akien-mga/3.x-clarify-freed-instance

Object: Make deleted object access raise errors, not warnings
Rémi Verschelde 4 years ago
parent
commit
2ece8c44b8
5 changed files with 15 additions and 13 deletions
  1. 1 1
      core/variant.cpp
  2. 2 2
      core/variant_call.cpp
  3. 9 9
      core/variant_op.cpp
  4. 1 0
      doc/classes/Node.xml
  5. 2 1
      doc/classes/Object.xml

+ 1 - 1
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();
 			}

+ 2 - 2
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;

+ 9 - 9
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<PropertyInfo> *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();
 			}

+ 1 - 0
doc/classes/Node.xml

@@ -564,6 +564,7 @@
 			</return>
 			<description>
 				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.
 			</description>
 		</method>
 		<method name="raise">

+ 2 - 1
doc/classes/Object.xml

@@ -201,7 +201,8 @@
 			<return type="void">
 			</return>
 			<description>
-				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.
 			</description>
 		</method>
 		<method name="get" qualifiers="const">