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.)
This commit is contained in:
Pedro J. Estébanez 2021-09-21 10:30:34 +02:00
parent 1bbc7c9c3a
commit 26edc6cd41
13 changed files with 71 additions and 152 deletions

View file

@ -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<GDScript>(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 = "<built-in>";
}
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;