From 26edc6cd417babbe9acd1cd3041b4f166d3c126e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20J=2E=20Est=C3=A9banez?= Date: Tue, 21 Sep 2021 10:30:34 +0200 Subject: [PATCH] Promote object validity checks to release builds Extra: - Optimized the debug-only check about why the object is null to determine if it's because it has been deleted (the RC is enough; no need to check the ObjectDB). - Because of the previous point. the debugger being attached is not required anymore for giving the "Object was deleted" error; from now, it only matters that it's a debug build. - `is_instance_valid()` is now trustworthy. It will return `true` if, and only if, the last object assigned to a `Variant` is still alive (and not if a new object happened to be created at the same memory address of the old one). - Replacements of `instance_validate()` are used where possible `Variant::is_invalid_object()` is introduced to help with that. (GDScript's `is_instance_valid()` is good.) --- core/io/marshalls.cpp | 8 ++-- core/object.cpp | 6 --- core/object.h | 9 +--- core/object_rc.h | 4 -- core/variant.cpp | 56 ++++------------------- core/variant.h | 21 ++------- core/variant_call.cpp | 8 ++-- core/variant_op.cpp | 44 +++++++++--------- modules/gdscript/gdscript_function.cpp | 48 ++++++++++--------- modules/gdscript/gdscript_function.h | 4 +- modules/gdscript/gdscript_functions.cpp | 2 +- scene/animation/tween.cpp | 11 ----- scene/debugger/script_debugger_remote.cpp | 2 +- 13 files changed, 71 insertions(+), 152 deletions(-) diff --git a/core/io/marshalls.cpp b/core/io/marshalls.cpp index 7c6bd718694..0c5e1589b0d 100644 --- a/core/io/marshalls.cpp +++ b/core/io/marshalls.cpp @@ -786,10 +786,9 @@ Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len, bo } } break; case Variant::OBJECT: { -#ifdef DEBUG_ENABLED - // Test for potential wrong values sent by the debugger when it breaks. + // Test for potential wrong values sent by the debugger when it breaks or freed objects. Object *obj = p_variant; - if (!obj || !ObjectDB::instance_validate(obj)) { + if (!obj) { // Object is invalid, send a NULL instead. if (buf) { encode_uint32(Variant::NIL, buf); @@ -797,7 +796,6 @@ Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len, bo r_len += 4; return OK; } -#endif // DEBUG_ENABLED if (!p_full_objects) { flags |= ENCODE_FLAG_OBJECT_AS_ID; } @@ -1090,7 +1088,7 @@ Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len, bo if (buf) { Object *obj = p_variant; ObjectID id = 0; - if (obj && ObjectDB::instance_validate(obj)) { + if (obj) { id = obj->get_instance_id(); } diff --git a/core/object.cpp b/core/object.cpp index 86ede3ce289..a479a81d2c9 100644 --- a/core/object.cpp +++ b/core/object.cpp @@ -961,7 +961,6 @@ void Object::cancel_delete() { _predelete_ok = true; } -#ifdef DEBUG_ENABLED ObjectRC *Object::_use_rc() { // The RC object is lazily created the first time it's requested; // that way, there's no need to allocate and release it at all if this Object @@ -989,7 +988,6 @@ ObjectRC *Object::_use_rc() { rc = _rc.load(std::memory_order_acquire); } } -#endif void Object::set_script_and_instance(const RefPtr &p_script, ScriptInstance *p_instance) { //this function is not meant to be used in any of these ways @@ -1927,9 +1925,7 @@ Object::Object() { _emitting = false; memset(_script_instance_bindings, 0, sizeof(void *) * MAX_SCRIPT_INSTANCE_BINDINGS); script_instance = nullptr; -#ifdef DEBUG_ENABLED _rc.store(nullptr, std::memory_order_release); -#endif #ifdef TOOLS_ENABLED _edited = false; @@ -1942,14 +1938,12 @@ Object::Object() { } Object::~Object() { -#ifdef DEBUG_ENABLED ObjectRC *rc = _rc.load(std::memory_order_acquire); if (rc) { if (rc->invalidate()) { memdelete(rc); } } -#endif if (script_instance) { memdelete(script_instance); diff --git a/core/object.h b/core/object.h index a795ee83ef4..220fa645d2f 100644 --- a/core/object.h +++ b/core/object.h @@ -41,9 +41,7 @@ #include "core/variant.h" #include "core/vmap.h" -#ifdef DEBUG_ENABLED -#include // For ObjectRC* -#endif +#include #define VARIANT_ARG_LIST const Variant &p_arg1 = Variant(), const Variant &p_arg2 = Variant(), const Variant &p_arg3 = Variant(), const Variant &p_arg4 = Variant(), const Variant &p_arg5 = Variant() #define VARIANT_ARG_PASS p_arg1, p_arg2, p_arg3, p_arg4, p_arg5 @@ -476,9 +474,7 @@ private: int _predelete_ok; Set change_receptors; ObjectID _instance_id; -#ifdef DEBUG_ENABLED std::atomic _rc; -#endif bool _predelete(); void _postinitialize(); bool _can_translate; @@ -590,9 +586,7 @@ public: return &ptr; } -#ifdef DEBUG_ENABLED ObjectRC *_use_rc(); -#endif bool _is_gpl_reversed() const { return false; } @@ -798,6 +792,7 @@ public: static void debug_objects(DebugFunc p_func); static int get_object_count(); + // This one may give false positives because a new object may be allocated at the same memory of a previously freed one _FORCE_INLINE_ static bool instance_validate(Object *p_ptr) { rw_lock.read_lock(); diff --git a/core/object_rc.h b/core/object_rc.h index eb9f7012ec8..56ad8b9d8b4 100644 --- a/core/object_rc.h +++ b/core/object_rc.h @@ -31,8 +31,6 @@ #ifndef OBJECTRC_H #define OBJECTRC_H -#ifdef DEBUG_ENABLED - #include "core/os/memory.h" #include "core/typedefs.h" @@ -77,5 +75,3 @@ public: }; #endif - -#endif diff --git a/core/variant.cpp b/core/variant.cpp index 032e0c447cb..c7851fb837d 100644 --- a/core/variant.cpp +++ b/core/variant.cpp @@ -817,11 +817,15 @@ ObjectID Variant::get_object_instance_id() const { if (is_ref() && _get_obj().ref.is_null()) { return 0; } else { - return _get_obj().obj->get_instance_id(); + return _get_obj().rc->get_ptr()->get_instance_id(); } #endif } +bool Variant::is_invalid_object() const { + return type == OBJECT && _get_obj().rc && !_get_obj().rc->get_ptr(); +} + void Variant::reference(const Variant &p_variant) { switch (type) { case NIL: @@ -896,11 +900,9 @@ void Variant::reference(const Variant &p_variant) { } break; case OBJECT: { memnew_placement(_data._mem, ObjData(p_variant._get_obj())); -#ifdef DEBUG_ENABLED - if (_get_obj().rc) { + if (likely(_get_obj().rc)) { _get_obj().rc->increment(); } -#endif } break; case NODE_PATH: { memnew_placement(_data._mem, NodePath(*reinterpret_cast(p_variant._data._mem))); @@ -1018,7 +1020,6 @@ void Variant::clear() { reinterpret_cast(_data._mem)->~NodePath(); } break; case OBJECT: { -#ifdef DEBUG_ENABLED if (likely(_get_obj().rc)) { if (unlikely(_get_obj().rc->decrement())) { memdelete(_get_obj().rc); @@ -1026,10 +1027,6 @@ void Variant::clear() { } else { _get_obj().ref.unref(); } -#else - _get_obj().obj = NULL; - _get_obj().ref.unref(); -#endif } break; case _RID: { // not much need probably @@ -1511,18 +1508,12 @@ String Variant::stringify(List &stack) const { } break; case OBJECT: { Object *obj = _OBJ_PTR(*this); - if (obj) { - if (_get_obj().ref.is_null() && !ObjectDB::get_instance(obj->get_instance_id())) { - return "[Deleted Object]"; - } - + if (likely(obj)) { return obj->to_string(); } else { -#ifdef DEBUG_ENABLED - if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { + if (_get_obj().rc) { return "[Deleted Object]"; } -#endif return "[Object:null]"; } } break; @@ -1678,20 +1669,13 @@ Variant::operator RID() const { if (!_get_obj().ref.is_null()) { return _get_obj().ref.get_rid(); } else { -#ifdef DEBUG_ENABLED Object *obj = likely(_get_obj().rc) ? _get_obj().rc->get_ptr() : nullptr; if (unlikely(!obj)) { - if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { + if (_get_obj().rc) { ERR_PRINT("Attempted get RID on a deleted object."); } return RID(); } -#else - Object *obj = _get_obj().obj; - if (unlikely(!obj)) { - return RID(); - } -#endif Variant::CallError ce; Variant ret = obj->call(CoreStringNames::get_singleton()->get_rid, nullptr, 0, ce); if (ce.error == Variant::CallError::CALL_OK && ret.get_type() == Variant::_RID) { @@ -1714,22 +1698,14 @@ Variant::operator Object *() const { } Variant::operator Node *() const { if (type == OBJECT) { -#ifdef DEBUG_ENABLED Object *obj = _get_obj().rc ? _get_obj().rc->get_ptr() : nullptr; -#else - Object *obj = _get_obj().obj; -#endif return Object::cast_to(obj); } return nullptr; } Variant::operator Control *() const { if (type == OBJECT) { -#ifdef DEBUG_ENABLED Object *obj = _get_obj().rc ? _get_obj().rc->get_ptr() : nullptr; -#else - Object *obj = _get_obj().obj; -#endif return Object::cast_to(obj); } return nullptr; @@ -2182,12 +2158,7 @@ Variant::Variant(const NodePath &p_node_path) { Variant::Variant(const RefPtr &p_resource) { type = OBJECT; memnew_placement(_data._mem, ObjData); -#ifdef DEBUG_ENABLED _get_obj().rc = nullptr; -#else - REF *ref = reinterpret_cast(p_resource.get_data()); - _get_obj().obj = ref->ptr(); -#endif _get_obj().ref = p_resource; } @@ -2204,15 +2175,10 @@ Variant::Variant(const Object *p_object) { Reference *ref = Object::cast_to(obj); if (unlikely(ref)) { *reinterpret_cast *>(_get_obj().ref.get_data()) = Ref(ref); -#ifdef DEBUG_ENABLED _get_obj().rc = nullptr; } else { _get_obj().rc = likely(obj) ? obj->_use_rc() : nullptr; -#endif } -#if !defined(DEBUG_ENABLED) - _get_obj().obj = obj; -#endif } Variant::Variant(const Dictionary &p_dictionary) { @@ -2490,19 +2456,15 @@ void Variant::operator=(const Variant &p_variant) { *reinterpret_cast(_data._mem) = *reinterpret_cast(p_variant._data._mem); } break; case OBJECT: { -#ifdef DEBUG_ENABLED if (likely(_get_obj().rc)) { if (unlikely(_get_obj().rc->decrement())) { memdelete(_get_obj().rc); } } -#endif *reinterpret_cast(_data._mem) = p_variant._get_obj(); -#ifdef DEBUG_ENABLED if (likely(_get_obj().rc)) { _get_obj().rc->increment(); } -#endif } break; case NODE_PATH: { *reinterpret_cast(_data._mem) = *reinterpret_cast(p_variant._data._mem); diff --git a/core/variant.h b/core/variant.h index fdaa2b695c3..e0dbd902279 100644 --- a/core/variant.h +++ b/core/variant.h @@ -73,22 +73,12 @@ typedef PoolVector PoolColorArray; #define GCC_ALIGNED_8 #endif -// With DEBUG_ENABLED, the pointer to a deleted object stored in ObjectRC is set to nullptr, -// so _OBJ_PTR is not useful for checks in which we want to act as if we still believed the -// object is alive; e.g., comparing a Variant that points to a deleted object with NIL, -// should return false regardless DEBUG_ENABLED is defined or not. -// So in cases like that we use _UNSAFE_OBJ_PROXY_PTR, which serves that purpose. With DEBUG_ENABLED -// it won't be the real pointer to the object for non-Reference types, but that's fine. -// We just need it to be unique for each object, to be comparable and not to be forced to NULL -// when the object is freed. -#ifdef DEBUG_ENABLED #define _REF_OBJ_PTR(m_variant) (reinterpret_cast *>((m_variant)._get_obj().ref.get_data())->ptr()) #define _OBJ_PTR(m_variant) ((m_variant)._get_obj().rc ? (m_variant)._get_obj().rc->get_ptr() : _REF_OBJ_PTR(m_variant)) +// _UNSAFE_OBJ_PROXY_PTR is needed for comparing an object Variant against NIL or compare two object Variants. +// It's guaranteed to be unique per object, in contrast to the pointer stored in the RC structure, +// which is set to null when the object is destroyed. #define _UNSAFE_OBJ_PROXY_PTR(m_variant) ((m_variant)._get_obj().rc ? reinterpret_cast((m_variant)._get_obj().rc) : reinterpret_cast(_REF_OBJ_PTR(m_variant))) -#else -#define _OBJ_PTR(m_variant) ((m_variant)._get_obj().obj) -#define _UNSAFE_OBJ_PROXY_PTR(m_variant) _OBJ_PTR(m_variant) -#endif class Variant { public: @@ -149,13 +139,9 @@ private: Type type; struct ObjData { -#ifdef DEBUG_ENABLED // Will be null for every type deriving from Reference as they have their // own reference count mechanism ObjectRC *rc; -#else - Object *obj; -#endif // Always initialized, but will be null if the Ref<> assigned was null // or this Variant is not even holding a Reference-derived object RefPtr ref; @@ -193,6 +179,7 @@ public: bool is_one() const; ObjectID get_object_instance_id() const; + bool is_invalid_object() const; operator bool() const; operator signed int() const; diff --git a/core/variant_call.cpp b/core/variant_call.cpp index 195afcd5575..b9547928915 100644 --- a/core/variant_call.cpp +++ b/core/variant_call.cpp @@ -1162,9 +1162,9 @@ void Variant::call_ptr(const StringName &p_method, const Variant **p_args, int p if (type == Variant::OBJECT) { //call object Object *obj = _OBJ_PTR(*this); - if (!obj) { + if (unlikely(!obj)) { #ifdef DEBUG_ENABLED - if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { + if (_get_obj().rc) { ERR_PRINT("Attempted method call on a deleted object."); } #endif @@ -1374,9 +1374,9 @@ Variant Variant::construct(const Variant::Type p_type, const Variant **p_args, i bool Variant::has_method(const StringName &p_method) const { if (type == OBJECT) { Object *obj = _OBJ_PTR(*this); - if (!obj) { + if (unlikely(!obj)) { #ifdef DEBUG_ENABLED - if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { + if (_get_obj().rc) { ERR_PRINT("Attempted method check on a deleted object."); } #endif diff --git a/core/variant_op.cpp b/core/variant_op.cpp index e88a438f658..162a26bbcbf 100644 --- a/core/variant_op.cpp +++ b/core/variant_op.cpp @@ -1529,14 +1529,14 @@ void Variant::set_named(const StringName &p_index, const Variant &p_value, bool } break; case OBJECT: { Object *obj = _OBJ_PTR(*this); -#ifdef DEBUG_ENABLED if (unlikely(!obj)) { - if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { +#ifdef DEBUG_ENABLED + if (_get_obj().rc) { ERR_PRINT("Attempted set on a deleted object."); } +#endif break; } -#endif obj->set(p_index, p_value, &valid); } break; @@ -1684,17 +1684,17 @@ Variant Variant::get_named(const StringName &p_index, bool *r_valid) const { } break; case OBJECT: { Object *obj = _OBJ_PTR(*this); -#ifdef DEBUG_ENABLED if (unlikely(!obj)) { if (r_valid) { *r_valid = false; } - if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { +#ifdef DEBUG_ENABLED + if (_get_obj().rc) { ERR_PRINT("Attempted get on a deleted object."); } +#endif return Variant(); } -#endif return obj->get(p_index, r_valid); @@ -2169,9 +2169,9 @@ void Variant::set(const Variant &p_index, const Variant &p_value, bool *r_valid) case OBJECT: { Object *obj = _OBJ_PTR(*this); if (unlikely(!obj)) { -#ifdef DEBUG_ENABLED valid = false; - if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { +#ifdef DEBUG_ENABLED + if (_get_obj().rc) { ERR_PRINT("Attempted set on a deleted object."); } #endif @@ -2520,9 +2520,9 @@ Variant Variant::get(const Variant &p_index, bool *r_valid) const { case OBJECT: { Object *obj = _OBJ_PTR(*this); if (unlikely(!obj)) { -#ifdef DEBUG_ENABLED valid = false; - if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { +#ifdef DEBUG_ENABLED + if (_get_obj().rc) { ERR_PRINT("Attempted get on a deleted object."); } #endif @@ -2578,11 +2578,11 @@ bool Variant::in(const Variant &p_index, bool *r_valid) const { case OBJECT: { Object *obj = _OBJ_PTR(*this); if (unlikely(!obj)) { -#ifdef DEBUG_ENABLED if (r_valid) { *r_valid = false; } - if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { +#ifdef DEBUG_ENABLED + if (_get_obj().rc) { ERR_PRINT("Attempted 'in' on a deleted object."); } #endif @@ -2832,7 +2832,7 @@ void Variant::get_property_list(List *p_list) const { Object *obj = _OBJ_PTR(*this); if (unlikely(!obj)) { #ifdef DEBUG_ENABLED - if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { + if (_get_obj().rc) { ERR_PRINT("Attempted get property list on a deleted object."); } #endif @@ -2903,15 +2903,15 @@ bool Variant::iter_init(Variant &r_iter, bool &valid) const { } break; case OBJECT: { Object *obj = _OBJ_PTR(*this); -#ifdef DEBUG_ENABLED if (unlikely(!obj)) { valid = false; - if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { +#ifdef DEBUG_ENABLED + if (_get_obj().rc) { ERR_PRINT("Attempted iteration start on a deleted object."); } +#endif return false; } -#endif Variant::CallError ce; ce.error = Variant::CallError::CALL_OK; Array ref; @@ -3077,15 +3077,15 @@ bool Variant::iter_next(Variant &r_iter, bool &valid) const { } break; case OBJECT: { Object *obj = _OBJ_PTR(*this); -#ifdef DEBUG_ENABLED if (unlikely(!obj)) { valid = false; - if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { +#ifdef DEBUG_ENABLED + if (_get_obj().rc) { ERR_PRINT("Attempted iteration check next on a deleted object."); } +#endif return false; } -#endif Variant::CallError ce; ce.error = Variant::CallError::CALL_OK; Array ref; @@ -3233,15 +3233,15 @@ Variant Variant::iter_get(const Variant &r_iter, bool &r_valid) const { } break; case OBJECT: { Object *obj = _OBJ_PTR(*this); -#ifdef DEBUG_ENABLED if (unlikely(!obj)) { r_valid = false; - if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) { +#ifdef DEBUG_ENABLED + if (_get_obj().rc) { ERR_PRINT("Attempted iteration get next on a deleted object."); } +#endif return Variant(); } -#endif Variant::CallError ce; ce.error = Variant::CallError::CALL_OK; const Variant *refp[] = { &r_iter }; diff --git a/modules/gdscript/gdscript_function.cpp b/modules/gdscript/gdscript_function.cpp index 27ad6060d2a..d4542949da2 100644 --- a/modules/gdscript/gdscript_function.cpp +++ b/modules/gdscript/gdscript_function.cpp @@ -133,16 +133,16 @@ static String _get_var_type(const Variant *p_var) { if (p_var->get_type() == Variant::OBJECT) { Object *bobj = *p_var; if (!bobj) { - basestr = "null instance"; - } else { - if (ObjectDB::instance_validate(bobj)) { - if (bobj->get_script_instance()) { - basestr = bobj->get_class() + " (" + bobj->get_script_instance()->get_script()->get_path().get_file() + ")"; - } else { - basestr = bobj->get_class(); - } - } else { + if (p_var->is_invalid_object()) { basestr = "previously freed instance"; + } else { + basestr = "null instance"; + } + } else { + if (bobj->get_script_instance()) { + basestr = bobj->get_class() + " (" + bobj->get_script_instance()->get_script()->get_path().get_file() + ")"; + } else { + basestr = bobj->get_class(); } } @@ -364,6 +364,9 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a } } +#ifdef DEBUG_ENABLED + Variant instance = p_instance; +#endif static_ref = script; String err_text; @@ -466,7 +469,11 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a GET_VARIANT_PTR(dst, 3); #ifdef DEBUG_ENABLED - if (b->get_type() != Variant::OBJECT || b->operator Object *() == nullptr) { + if (a->is_invalid_object()) { + err_text = "Left operand of 'is' was already freed."; + OPCODE_BREAK; + } + if (b->is_invalid_object()) { err_text = "Right operand of 'is' is not a class."; OPCODE_BREAK; } @@ -477,13 +484,6 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a Object *obj_A = *a; Object *obj_B = *b; -#ifdef DEBUG_ENABLED - if (!ObjectDB::instance_validate(obj_A)) { - err_text = "Left operand of 'is' was already freed."; - OPCODE_BREAK; - } -#endif // DEBUG_ENABLED - GDScript *scr_B = Object::cast_to(obj_B); if (scr_B) { @@ -1264,17 +1264,15 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a Object *obj = argobj->operator Object *(); String signal = argname->operator String(); + if (argobj->is_invalid_object()) { + err_text = "First argument of yield() is a previously freed instance."; + OPCODE_BREAK; + } #ifdef DEBUG_ENABLED if (!obj) { err_text = "First argument of yield() is null."; OPCODE_BREAK; } - if (ScriptDebugger::get_singleton()) { - if (!ObjectDB::instance_validate(obj)) { - err_text = "First argument of yield() is a previously freed instance."; - OPCODE_BREAK; - } - } if (signal.length() == 0) { err_text = "Second argument of yield() is an empty string (for signal name)."; OPCODE_BREAK; @@ -1525,7 +1523,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a //error // function, file, line, error, explanation String err_file; - if (p_instance && ObjectDB::instance_validate(p_instance->owner) && p_instance->script->is_valid() && p_instance->script->path != "") { + if (p_instance && !instance.is_invalid_object() && p_instance->script->is_valid() && p_instance->script->path != "") { err_file = p_instance->script->path; } else if (script) { err_file = script->path; @@ -1534,7 +1532,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a err_file = ""; } String err_func = name; - if (p_instance && ObjectDB::instance_validate(p_instance->owner) && p_instance->script->is_valid() && p_instance->script->name != "") { + if (p_instance && !instance.is_invalid_object() && p_instance->script->is_valid() && p_instance->script->name != "") { err_func = p_instance->script->name + "." + err_func; } int err_line = line; diff --git a/modules/gdscript/gdscript_function.h b/modules/gdscript/gdscript_function.h index ebd9e79206c..d4deeeb8a2a 100644 --- a/modules/gdscript/gdscript_function.h +++ b/modules/gdscript/gdscript_function.h @@ -81,7 +81,7 @@ struct GDScriptDataType { } Object *obj = p_variant.operator Object *(); - if (!obj || !ObjectDB::instance_validate(obj)) { + if (!obj) { return false; } @@ -104,7 +104,7 @@ struct GDScriptDataType { } Object *obj = p_variant.operator Object *(); - if (!obj || !ObjectDB::instance_validate(obj)) { + if (!obj) { return false; } diff --git a/modules/gdscript/gdscript_functions.cpp b/modules/gdscript/gdscript_functions.cpp index 1a3df027f16..12e18e28bb1 100644 --- a/modules/gdscript/gdscript_functions.cpp +++ b/modules/gdscript/gdscript_functions.cpp @@ -1382,7 +1382,7 @@ void GDScriptFunctions::call(Function p_func, const Variant **p_args, int p_arg_ r_ret = false; } else { Object *obj = *p_args[0]; - r_ret = ObjectDB::instance_validate(obj); + r_ret = obj != nullptr; } } break; diff --git a/scene/animation/tween.cpp b/scene/animation/tween.cpp index 973f36f675d..7cc6a27e383 100644 --- a/scene/animation/tween.cpp +++ b/scene/animation/tween.cpp @@ -1321,7 +1321,6 @@ bool Tween::_build_interpolation(InterpolateType p_interpolation_type, Object *p // Give it the object ERR_FAIL_COND_V_MSG(p_object == nullptr, false, "Invalid object provided to Tween."); - ERR_FAIL_COND_V_MSG(!ObjectDB::instance_validate(p_object), false, "Invalid object provided to Tween."); data.id = p_object->get_instance_id(); // Validate the initial and final values @@ -1439,7 +1438,6 @@ bool Tween::interpolate_callback(Object *p_object, real_t p_duration, String p_c // Check that the target object is valid ERR_FAIL_COND_V(p_object == nullptr, false); - ERR_FAIL_COND_V(!ObjectDB::instance_validate(p_object), false); // Duration cannot be negative ERR_FAIL_COND_V(p_duration < 0, false); @@ -1499,7 +1497,6 @@ bool Tween::interpolate_deferred_callback(Object *p_object, real_t p_duration, S // Check that the target object is valid ERR_FAIL_COND_V(p_object == nullptr, false); - ERR_FAIL_COND_V(!ObjectDB::instance_validate(p_object), false); // No negative durations allowed ERR_FAIL_COND_V(p_duration < 0, false); @@ -1574,9 +1571,7 @@ bool Tween::follow_property(Object *p_object, NodePath p_property, Variant p_ini // Confirm the source and target objects are valid ERR_FAIL_COND_V(p_object == nullptr, false); - ERR_FAIL_COND_V(!ObjectDB::instance_validate(p_object), false); ERR_FAIL_COND_V(p_target == nullptr, false); - ERR_FAIL_COND_V(!ObjectDB::instance_validate(p_target), false); // No negative durations ERR_FAIL_COND_V(p_duration < 0, false); @@ -1642,9 +1637,7 @@ bool Tween::follow_method(Object *p_object, StringName p_method, Variant p_initi // Verify the source and target objects are valid ERR_FAIL_COND_V(p_object == nullptr, false); - ERR_FAIL_COND_V(!ObjectDB::instance_validate(p_object), false); ERR_FAIL_COND_V(p_target == nullptr, false); - ERR_FAIL_COND_V(!ObjectDB::instance_validate(p_target), false); // No negative durations ERR_FAIL_COND_V(p_duration < 0, false); @@ -1712,9 +1705,7 @@ bool Tween::targeting_property(Object *p_object, NodePath p_property, Object *p_ // Verify both objects are valid ERR_FAIL_COND_V(p_object == nullptr, false); - ERR_FAIL_COND_V(!ObjectDB::instance_validate(p_object), false); ERR_FAIL_COND_V(p_initial == nullptr, false); - ERR_FAIL_COND_V(!ObjectDB::instance_validate(p_initial), false); // No negative durations ERR_FAIL_COND_V(p_duration < 0, false); @@ -1785,9 +1776,7 @@ bool Tween::targeting_method(Object *p_object, StringName p_method, Object *p_in // Make sure the given objects are valid ERR_FAIL_COND_V(p_object == nullptr, false); - ERR_FAIL_COND_V(!ObjectDB::instance_validate(p_object), false); ERR_FAIL_COND_V(p_initial == nullptr, false); - ERR_FAIL_COND_V(!ObjectDB::instance_validate(p_initial), false); // No negative durations ERR_FAIL_COND_V(p_duration < 0, false); diff --git a/scene/debugger/script_debugger_remote.cpp b/scene/debugger/script_debugger_remote.cpp index 89a382a5f5d..d16044af84d 100644 --- a/scene/debugger/script_debugger_remote.cpp +++ b/scene/debugger/script_debugger_remote.cpp @@ -101,7 +101,7 @@ void ScriptDebuggerRemote::_put_variable(const String &p_name, const Variant &p_ packet_peer_stream->put_var(p_name); Variant var = p_variable; - if (p_variable.get_type() == Variant::OBJECT && !ObjectDB::instance_validate(p_variable)) { + if (p_variable.get_type() == Variant::OBJECT && p_variable.operator Object *() == nullptr) { var = Variant(); }