GDScript: Improve error messages for invalid indexing

These errors are very common when using an invalid property name
or calling on an object of the wrong type, and the previous message
was a bit cryptic for users.

Co-authored-by: Rémi Verschelde <rverschelde@gmail.com>
Co-authored-by: golfinq <golfinqz@gmail.com>

core/variant/variant.h

@@ -703,9 +703,20 @@ public:
 	bool has_key(const Variant &p_key, bool &r_valid) const;
 
 	/* Generic */
-
+	enum VariantSetError {
-	void set(const Variant &p_index, const Variant &p_value, bool *r_valid = nullptr);
+		SET_OK,
-	Variant get(const Variant &p_index, bool *r_valid = nullptr) const;
+		SET_KEYED_ERR,
+		SET_NAMED_ERR,
+		SET_INDEXED_ERR
+	};
+	enum VariantGetError {
+		GET_OK,
+		GET_KEYED_ERR,
+		GET_NAMED_ERR,
+		GET_INDEXED_ERR
+	};
+	void set(const Variant &p_index, const Variant &p_value, bool *r_valid = nullptr, VariantSetError *err_code = nullptr);
+	Variant get(const Variant &p_index, bool *r_valid = nullptr, VariantGetError *err_code = nullptr) const;
 	bool in(const Variant &p_index, bool *r_valid = nullptr) const;
 
 	bool iter_init(Variant &r_iter, bool &r_valid) const;

core/variant/variant_setget.cpp

@@ -1166,30 +1166,48 @@ bool Variant::has_key(const Variant &p_key, bool &r_valid) const {
 	}
 }
 
-void Variant::set(const Variant &p_index, const Variant &p_value, bool *r_valid) {
+void Variant::set(const Variant &p_index, const Variant &p_value, bool *r_valid, VariantSetError *err_code) {
+	if (err_code) {
+		*err_code = VariantSetError::SET_OK;
+	}
 	if (type == DICTIONARY || type == OBJECT) {
 		bool valid;
 		set_keyed(p_index, p_value, valid);
 		if (r_valid) {
 			*r_valid = valid;
+			if (!valid && err_code) {
+				*err_code = VariantSetError::SET_KEYED_ERR;
+			}
 		}
 	} else {
 		bool valid = false;
 		if (p_index.get_type() == STRING_NAME) {
 			set_named(*VariantGetInternalPtr<StringName>::get_ptr(&p_index), p_value, valid);
+			if (!valid && err_code) {
+				*err_code = VariantSetError::SET_NAMED_ERR;
+			}
 		} else if (p_index.get_type() == INT) {
 			bool obb;
 			set_indexed(*VariantGetInternalPtr<int64_t>::get_ptr(&p_index), p_value, valid, obb);
 			if (obb) {
 				valid = false;
+				if (err_code) {
+					*err_code = VariantSetError::SET_INDEXED_ERR;
+				}
 			}
 		} else if (p_index.get_type() == STRING) { // less efficient version of named
 			set_named(*VariantGetInternalPtr<String>::get_ptr(&p_index), p_value, valid);
+			if (!valid && err_code) {
+				*err_code = VariantSetError::SET_NAMED_ERR;
+			}
 		} else if (p_index.get_type() == FLOAT) { // less efficient version of indexed
 			bool obb;
 			set_indexed(*VariantGetInternalPtr<double>::get_ptr(&p_index), p_value, valid, obb);
 			if (obb) {
 				valid = false;
+				if (err_code) {
+					*err_code = VariantSetError::SET_INDEXED_ERR;
+				}
 			}
 		}
 		if (r_valid) {
@@ -1198,31 +1216,49 @@ void Variant::set(const Variant &p_index, const Variant &p_value, bool *r_valid)
 	}
 }
 
-Variant Variant::get(const Variant &p_index, bool *r_valid) const {
+Variant Variant::get(const Variant &p_index, bool *r_valid, VariantGetError *err_code) const {
+	if (err_code) {
+		*err_code = VariantGetError::GET_OK;
+	}
 	Variant ret;
 	if (type == DICTIONARY || type == OBJECT) {
 		bool valid;
 		ret = get_keyed(p_index, valid);
 		if (r_valid) {
 			*r_valid = valid;
+			if (!valid && err_code) {
+				*err_code = VariantGetError::GET_KEYED_ERR;
+			}
 		}
 	} else {
 		bool valid = false;
 		if (p_index.get_type() == STRING_NAME) {
 			ret = get_named(*VariantGetInternalPtr<StringName>::get_ptr(&p_index), valid);
+			if (!valid && err_code) {
+				*err_code = VariantGetError::GET_NAMED_ERR;
+			}
 		} else if (p_index.get_type() == INT) {
 			bool obb;
 			ret = get_indexed(*VariantGetInternalPtr<int64_t>::get_ptr(&p_index), valid, obb);
 			if (obb) {
 				valid = false;
+				if (err_code) {
+					*err_code = VariantGetError::GET_INDEXED_ERR;
+				}
 			}
 		} else if (p_index.get_type() == STRING) { // less efficient version of named
 			ret = get_named(*VariantGetInternalPtr<String>::get_ptr(&p_index), valid);
+			if (!valid && err_code) {
+				*err_code = VariantGetError::GET_NAMED_ERR;
+			}
 		} else if (p_index.get_type() == FLOAT) { // less efficient version of indexed
 			bool obb;
 			ret = get_indexed(*VariantGetInternalPtr<double>::get_ptr(&p_index), valid, obb);
 			if (obb) {
 				valid = false;
+				if (err_code) {
+					*err_code = VariantGetError::GET_INDEXED_ERR;
+				}
 			}
 		}
 		if (r_valid) {

modules/gdscript/gdscript_analyzer.cpp

@@ -3595,7 +3595,7 @@ void GDScriptAnalyzer::reduce_identifier_from_base(GDScriptParser::IdentifierNod
 			switch (base.builtin_type) {
 				case Variant::NIL: {
 					if (base.is_hard_type()) {
-						push_error(vformat(R"(Invalid get index "%s" on base Nil)", name), p_identifier);
+						push_error(vformat(R"(Cannot get property "%s" on a null object.)", name), p_identifier);
 					}
 					return;
 				}

modules/gdscript/gdscript_vm.cpp

@@ -888,8 +888,12 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 				GET_VARIANT_PTR(value, 2);
 
 				bool valid;
+#ifdef DEBUG_ENABLED
+				Variant::VariantSetError err_code;
+				dst->set(*index, *value, &valid, &err_code);
+#else
 				dst->set(*index, *value, &valid);
-
+#endif
 #ifdef DEBUG_ENABLED
 				if (!valid) {
 					Object *obj = dst->get_validated_object();
@@ -906,7 +910,10 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 						} else {
 							v = "of type '" + _get_var_type(index) + "'";
 						}
-						err_text = "Invalid set index " + v + " (on base: '" + _get_var_type(dst) + "') with value of type '" + _get_var_type(value) + "'";
+						err_text = "Invalid assignment of property or key " + v + " with value of type '" + _get_var_type(value) + "' on a base object of type '" + _get_var_type(dst) + "'.";
+						if (err_code == Variant::VariantSetError::SET_INDEXED_ERR) {
+							err_text = "Invalid assignment of index " + v + " (on base: '" + _get_var_type(dst) + "') with value of type '" + _get_var_type(value) + "'.";
+						}
 					}
 					OPCODE_BREAK;
 				}
@@ -937,7 +944,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 					} else {
 						v = "of type '" + _get_var_type(index) + "'";
 					}
-					err_text = "Invalid set index " + v + " (on base: '" + _get_var_type(dst) + "') with value of type '" + _get_var_type(value) + "'";
+					err_text = "Invalid assignment of property or key " + v + " with value of type '" + _get_var_type(value) + "' on a base object of type '" + _get_var_type(dst) + "'.";
 					OPCODE_BREAK;
 				}
 #endif
@@ -987,7 +994,8 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 				bool valid;
 #ifdef DEBUG_ENABLED
 				// Allow better error message in cases where src and dst are the same stack position.
-				Variant ret = src->get(*index, &valid);
+				Variant::VariantGetError err_code;
+				Variant ret = src->get(*index, &valid, &err_code);
 #else
 				*dst = src->get(*index, &valid);
 
@@ -1000,7 +1008,10 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 					} else {
 						v = "of type '" + _get_var_type(index) + "'";
 					}
-					err_text = "Invalid get index " + v + " (on base: '" + _get_var_type(src) + "').";
+					err_text = "Invalid access to property or key " + v + " on a base object of type '" + _get_var_type(src) + "'.";
+					if (err_code == Variant::VariantGetError::GET_INDEXED_ERR) {
+						err_text = "Invalid access of index " + v + " on a base object of type: '" + _get_var_type(src) + "'.";
+					}
 					OPCODE_BREAK;
 				}
 				*dst = ret;
@@ -1036,7 +1047,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 					} else {
 						v = "of type '" + _get_var_type(key) + "'";
 					}
-					err_text = "Invalid get index " + v + " (on base: '" + _get_var_type(src) + "').";
+					err_text = "Invalid access to property or key " + v + " on a base object of type '" + _get_var_type(src) + "'.";
 					OPCODE_BREAK;
 				}
 				*dst = ret;
@@ -1101,7 +1112,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 					if (read_only_property) {
 						err_text = vformat(R"(Cannot set value into property "%s" (on base "%s") because it is read-only.)", String(*index), _get_var_type(dst));
 					} else {
-						err_text = "Invalid set index '" + String(*index) + "' (on base: '" + _get_var_type(dst) + "') with value of type '" + _get_var_type(value) + "'.";
+						err_text = "Invalid assignment of property or key '" + String(*index) + "' with value of type '" + _get_var_type(value) + "' on a base object of type '" + _get_var_type(dst) + "'.";
 					}
 					OPCODE_BREAK;
 				}
@@ -1146,7 +1157,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 #endif
 #ifdef DEBUG_ENABLED
 				if (!valid) {
-					err_text = "Invalid get index '" + index->operator String() + "' (on base: '" + _get_var_type(src) + "').";
+					err_text = "Invalid access to property or key '" + index->operator String() + "' on a base object of type '" + _get_var_type(src) + "'.";
 					OPCODE_BREAK;
 				}
 				*dst = ret;
@@ -2619,7 +2630,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 						Variant::construct(ret_type, retvalue, const_cast<const Variant **>(&r), 1, ce);
 					} else {
 #ifdef DEBUG_ENABLED
-						err_text = vformat(R"(Trying to return value of type "%s" from a function which the return type is "%s".)",
+						err_text = vformat(R"(Trying to return value of type "%s" from a function whose return type is "%s".)",
 								Variant::get_type_name(r->get_type()), Variant::get_type_name(ret_type));
 #endif // DEBUG_ENABLED
 
@@ -2649,9 +2660,9 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 
 				if (r->get_type() != Variant::ARRAY) {
 #ifdef DEBUG_ENABLED
-					err_text = vformat(R"(Trying to return a value of type "%s" where expected return type is "Array[%s]".)",
+					err_text = vformat(R"(Trying to return value of type "%s" from a function whose return type is "Array[%s]".)",
-							_get_var_type(r), _get_element_type(builtin_type, native_type, *script_type));
+							Variant::get_type_name(r->get_type()), Variant::get_type_name(builtin_type));
-#endif // DEBUG_ENABLED
+#endif
 					OPCODE_BREAK;
 				}
 
@@ -2682,7 +2693,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 				GD_ERR_BREAK(!nc);
 
 				if (r->get_type() != Variant::OBJECT && r->get_type() != Variant::NIL) {
-					err_text = vformat(R"(Trying to return value of type "%s" from a function which the return type is "%s".)",
+					err_text = vformat(R"(Trying to return value of type "%s" from a function whose return type is "%s".)",
 							Variant::get_type_name(r->get_type()), nc->get_name());
 					OPCODE_BREAK;
 				}
@@ -2700,7 +2711,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 #endif // DEBUG_ENABLED
 				if (ret_obj && !ClassDB::is_parent_class(ret_obj->get_class_name(), nc->get_name())) {
 #ifdef DEBUG_ENABLED
-					err_text = vformat(R"(Trying to return value of type "%s" from a function which the return type is "%s".)",
+					err_text = vformat(R"(Trying to return value of type "%s" from a function whose return type is "%s".)",
 							ret_obj->get_class_name(), nc->get_name());
 #endif // DEBUG_ENABLED
 					OPCODE_BREAK;
@@ -2723,7 +2734,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 
 				if (r->get_type() != Variant::OBJECT && r->get_type() != Variant::NIL) {
 #ifdef DEBUG_ENABLED
-					err_text = vformat(R"(Trying to return value of type "%s" from a function which the return type is "%s".)",
+					err_text = vformat(R"(Trying to return value of type "%s" from a function whose return type is "%s".)",
 							Variant::get_type_name(r->get_type()), GDScript::debug_get_script_name(Ref<Script>(base_type)));
 #endif // DEBUG_ENABLED
 					OPCODE_BREAK;
@@ -2745,7 +2756,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 					ScriptInstance *ret_inst = ret_obj->get_script_instance();
 					if (!ret_inst) {
 #ifdef DEBUG_ENABLED
-						err_text = vformat(R"(Trying to return value of type "%s" from a function which the return type is "%s".)",
+						err_text = vformat(R"(Trying to return value of type "%s" from a function whose return type is "%s".)",
 								ret_obj->get_class_name(), GDScript::debug_get_script_name(Ref<GDScript>(base_type)));
 #endif // DEBUG_ENABLED
 						OPCODE_BREAK;
@@ -2764,7 +2775,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 
 					if (!valid) {
 #ifdef DEBUG_ENABLED
-						err_text = vformat(R"(Trying to return value of type "%s" from a function which the return type is "%s".)",
+						err_text = vformat(R"(Trying to return value of type "%s" from a function whose return type is "%s".)",
 								GDScript::debug_get_script_name(ret_obj->get_script_instance()->get_script()), GDScript::debug_get_script_name(Ref<GDScript>(base_type)));
 #endif // DEBUG_ENABLED
 						OPCODE_BREAK;

modules/gdscript/tests/scripts/analyzer/errors/outer_class_constants.out

@@ -3,4 +3,4 @@ GDTEST_RUNTIME_ERROR
 >> on function: test()
 >> analyzer/errors/outer_class_constants.gd
 >> 8
->> Invalid get index 'OUTER_CONST' (on base: 'GDScript').
+>> Invalid access to property or key 'OUTER_CONST' on a base object of type 'GDScript'.

modules/gdscript/tests/scripts/analyzer/errors/outer_class_constants_as_variant.out

@@ -3,4 +3,4 @@ GDTEST_RUNTIME_ERROR
 >> on function: test()
 >> analyzer/errors/outer_class_constants_as_variant.gd
 >> 9
->> Invalid get index 'OUTER_CONST' (on base: 'GDScript').
+>> Invalid access to property or key 'OUTER_CONST' on a base object of type 'GDScript'.

modules/gdscript/tests/scripts/analyzer/errors/outer_class_instance_constants.out

@@ -3,4 +3,4 @@ GDTEST_RUNTIME_ERROR
 >> on function: test()
 >> analyzer/errors/outer_class_instance_constants.gd
 >> 8
->> Invalid get index 'OUTER_CONST' (on base: 'RefCounted (Inner)').
+>> Invalid access to property or key 'OUTER_CONST' on a base object of type 'RefCounted (Inner)'.

modules/gdscript/tests/scripts/analyzer/errors/outer_class_instance_constants_as_variant.out

@@ -3,4 +3,4 @@ GDTEST_RUNTIME_ERROR
 >> on function: test()
 >> analyzer/errors/outer_class_instance_constants_as_variant.gd
 >> 9
->> Invalid get index 'OUTER_CONST' (on base: 'RefCounted (Inner)').
+>> Invalid access to property or key 'OUTER_CONST' on a base object of type 'RefCounted (Inner)'.

modules/gdscript/tests/scripts/runtime/errors/constant_array_is_deep.out

@@ -3,4 +3,4 @@ GDTEST_RUNTIME_ERROR
 >> on function: test()
 >> runtime/errors/constant_array_is_deep.gd
 >> 6
->> Invalid set index '0' (on base: 'Dictionary') with value of type 'int'
+>> Invalid assignment of property or key '0' with value of type 'int' on a base object of type 'Dictionary'.

modules/gdscript/tests/scripts/runtime/errors/constant_dictionary_is_deep.out

@@ -3,4 +3,4 @@ GDTEST_RUNTIME_ERROR
 >> on function: test()
 >> runtime/errors/constant_dictionary_is_deep.gd
 >> 6
->> Invalid set index '0' (on base: 'Array') with value of type 'int'
+>> Invalid assignment of index '0' (on base: 'Array') with value of type 'int'.