Skip to content

Commit

Permalink
Merge pull request #48041 from akien-mga/3.x-clarify-freed-instance
Browse files Browse the repository at this point in the history
Object: Make deleted object access raise errors, not warnings
  • Loading branch information
akien-mga authored Apr 20, 2021
2 parents f4653f6 + 1c9203a commit 2ece8c4
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 13 deletions.
2 changes: 1 addition & 1 deletion core/variant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
4 changes: 2 additions & 2 deletions core/variant_call.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
18 changes: 9 additions & 9 deletions core/variant_op.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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();
}
Expand Down
1 change: 1 addition & 0 deletions doc/classes/Node.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down
3 changes: 2 additions & 1 deletion doc/classes/Object.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down

0 comments on commit 2ece8c4

Please sign in to comment.