diff --git a/core/variant/dictionary.cpp b/core/variant/dictionary.cpp index 0754814d35fc..a3a56a4227a6 100644 --- a/core/variant/dictionary.cpp +++ b/core/variant/dictionary.cpp @@ -83,27 +83,53 @@ Variant Dictionary::get_value_at_index(int p_index) const { return Variant(); } +/// @warning This operator does not validate the value type. For scripting/extensions this is done +/// in `variant_setget.cpp`. Consider using `set()` if the data might be invalid. Variant &Dictionary::operator[](const Variant &p_key) { - if (unlikely(_p->read_only)) { - if (likely(_p->variant_map.has(p_key))) { - *_p->read_only = _p->variant_map[p_key]; + Variant key = p_key; + if (unlikely(!_p->typed_key.validate(key, "use `operator[]`"))) { + if (unlikely(!_p->typed_fallback)) { + _p->typed_fallback = memnew(Variant); + } + VariantInternal::initialize(_p->typed_fallback, _p->typed_value.type); + return *_p->typed_fallback; + } else if (unlikely(_p->read_only)) { + if (likely(_p->variant_map.has(key))) { + *_p->read_only = _p->variant_map[key]; } else { - *_p->read_only = Variant(); + VariantInternal::initialize(_p->read_only, _p->typed_value.type); } - return *_p->read_only; } else { - return _p->variant_map[p_key]; + if (unlikely(!_p->variant_map.has(key))) { + VariantInternal::initialize(&_p->variant_map[key], _p->typed_value.type); + } + return _p->variant_map[key]; } } +/// @warning This operator does not validate the value type. For scripting/extensions this is done +/// in `variant_setget.cpp`. Consider using `set()` if the data might be invalid. const Variant &Dictionary::operator[](const Variant &p_key) const { - // Will not insert key, so no conversion is necessary. - return _p->variant_map[p_key]; + Variant key = p_key; + if (unlikely(!_p->typed_key.validate(key, "use `operator[]`"))) { + if (unlikely(!_p->typed_fallback)) { + _p->typed_fallback = memnew(Variant); + } + VariantInternal::initialize(_p->typed_fallback, _p->typed_value.type); + return *_p->typed_fallback; + } else { + // Will not insert key, so no initialization is necessary. + return _p->variant_map[key]; + } } const Variant *Dictionary::getptr(const Variant &p_key) const { - HashMap::ConstIterator E(_p->variant_map.find(p_key)); + Variant key = p_key; + if (unlikely(!_p->typed_key.validate(key, "getptr"))) { + return nullptr; + } + HashMap::ConstIterator E(_p->variant_map.find(key)); if (!E) { return nullptr; } @@ -111,7 +137,11 @@ const Variant *Dictionary::getptr(const Variant &p_key) const { } Variant *Dictionary::getptr(const Variant &p_key) { - HashMap::Iterator E(_p->variant_map.find(p_key)); + Variant key = p_key; + if (unlikely(!_p->typed_key.validate(key, "getptr"))) { + return nullptr; + } + HashMap::Iterator E(_p->variant_map.find(key)); if (!E) { return nullptr; } @@ -158,6 +188,16 @@ Variant Dictionary::get_or_add(const Variant &p_key, const Variant &p_default) { return *result; } +bool Dictionary::set(const Variant &p_key, const Variant &p_value) { + ERR_FAIL_COND_V_MSG(_p->read_only, false, "Dictionary is in read-only state."); + Variant key = p_key; + ERR_FAIL_COND_V(!_p->typed_key.validate(key, "set"), false); + Variant value = p_value; + ERR_FAIL_COND_V(!_p->typed_value.validate(value, "set"), false); + _p->variant_map[key] = value; + return true; +} + int Dictionary::size() const { return _p->variant_map.size(); } diff --git a/core/variant/dictionary.h b/core/variant/dictionary.h index 57fbefc8f251..5f3ce40219bd 100644 --- a/core/variant/dictionary.h +++ b/core/variant/dictionary.h @@ -59,6 +59,7 @@ class Dictionary { Variant get_valid(const Variant &p_key) const; Variant get(const Variant &p_key, const Variant &p_default) const; Variant get_or_add(const Variant &p_key, const Variant &p_default); + bool set(const Variant &p_key, const Variant &p_value); int size() const; bool is_empty() const; diff --git a/core/variant/variant_setget.cpp b/core/variant/variant_setget.cpp index b60ff83cf198..1652f81d9994 100644 --- a/core/variant/variant_setget.cpp +++ b/core/variant/variant_setget.cpp @@ -252,20 +252,7 @@ void Variant::set_named(const StringName &p_member, const Variant &p_value, bool } } else if (type == Variant::DICTIONARY) { Dictionary &dict = *VariantGetInternalPtr::get_ptr(this); - - if (dict.is_read_only()) { - r_valid = false; - return; - } - - Variant *v = dict.getptr(p_member); - if (v) { - *v = p_value; - } else { - dict[p_member] = p_value; - } - - r_valid = true; + r_valid = dict.set(p_member, p_value); } else { r_valid = false; } @@ -721,26 +708,16 @@ struct VariantIndexedSetGet_Dictionary { PtrToArg::encode(*ptr, member); } static void set(Variant *base, int64_t index, const Variant *value, bool *valid, bool *oob) { - if (VariantGetInternalPtr::get_ptr(base)->is_read_only()) { - *valid = false; - *oob = true; - return; - } - (*VariantGetInternalPtr::get_ptr(base))[index] = *value; - *oob = false; - *valid = true; + *valid = VariantGetInternalPtr::get_ptr(base)->set(index, *value); + *oob = VariantGetInternalPtr::get_ptr(base)->is_read_only(); } static void validated_set(Variant *base, int64_t index, const Variant *value, bool *oob) { - if (VariantGetInternalPtr::get_ptr(base)->is_read_only()) { - *oob = true; - return; - } - (*VariantGetInternalPtr::get_ptr(base))[index] = *value; - *oob = false; + VariantGetInternalPtr::get_ptr(base)->set(index, *value); + *oob = VariantGetInternalPtr::get_ptr(base)->is_read_only(); } static void ptr_set(void *base, int64_t index, const void *member) { Dictionary &v = *reinterpret_cast(base); - v[index] = PtrToArg::convert(member); + v.set(index, PtrToArg::convert(member)); } static Variant::Type get_index_type() { return Variant::NIL; } static uint32_t get_index_usage() { return PROPERTY_USAGE_DEFAULT; } @@ -1010,16 +987,11 @@ struct VariantKeyedSetGetDictionary { PtrToArg::encode(*ptr, value); } static void set(Variant *base, const Variant *key, const Variant *value, bool *r_valid) { - if (VariantGetInternalPtr::get_ptr(base)->is_read_only()) { - *r_valid = false; - return; - } - (*VariantGetInternalPtr::get_ptr(base))[*key] = *value; - *r_valid = true; + *r_valid = VariantGetInternalPtr::get_ptr(base)->set(*key, *value); } static void ptr_set(void *base, const void *key, const void *value) { Dictionary &v = *reinterpret_cast(base); - v[PtrToArg::convert(key)] = PtrToArg::convert(value); + v.set(PtrToArg::convert(key), PtrToArg::convert(value)); } static bool has(const Variant *base, const Variant *key, bool *r_valid) { diff --git a/modules/gdscript/tests/scripts/runtime/errors/typed_dictionary_assign_differently_typed_key.gd b/modules/gdscript/tests/scripts/runtime/errors/typed_dictionary_assign_differently_typed_key.gd new file mode 100644 index 000000000000..2f0b3bd0ebfa --- /dev/null +++ b/modules/gdscript/tests/scripts/runtime/errors/typed_dictionary_assign_differently_typed_key.gd @@ -0,0 +1,7 @@ +func get_key() -> Variant: + return "key" + +func test(): + var typed: Dictionary[int, int] + typed[get_key()] = 0 + print('not ok') diff --git a/modules/gdscript/tests/scripts/runtime/errors/typed_dictionary_assign_differently_typed_key.out b/modules/gdscript/tests/scripts/runtime/errors/typed_dictionary_assign_differently_typed_key.out new file mode 100644 index 000000000000..5f6dd7f641ec --- /dev/null +++ b/modules/gdscript/tests/scripts/runtime/errors/typed_dictionary_assign_differently_typed_key.out @@ -0,0 +1,6 @@ +GDTEST_RUNTIME_ERROR +>> SCRIPT ERROR +>> on function: test() +>> runtime/errors/typed_dictionary_assign_differently_typed_key.gd +>> 6 +>> Invalid assignment of property or key 'key' with value of type 'int' on a base object of type 'Dictionary[int, int]'. diff --git a/modules/gdscript/tests/scripts/runtime/errors/typed_dictionary_assign_differently_typed_value.gd b/modules/gdscript/tests/scripts/runtime/errors/typed_dictionary_assign_differently_typed_value.gd new file mode 100644 index 000000000000..b171159aedd0 --- /dev/null +++ b/modules/gdscript/tests/scripts/runtime/errors/typed_dictionary_assign_differently_typed_value.gd @@ -0,0 +1,7 @@ +func get_value() -> Variant: + return "value" + +func test(): + var typed: Dictionary[int, int] + typed[0] = get_value() + print("not ok") diff --git a/modules/gdscript/tests/scripts/runtime/errors/typed_dictionary_assign_differently_typed_value.out b/modules/gdscript/tests/scripts/runtime/errors/typed_dictionary_assign_differently_typed_value.out new file mode 100644 index 000000000000..f766d142611f --- /dev/null +++ b/modules/gdscript/tests/scripts/runtime/errors/typed_dictionary_assign_differently_typed_value.out @@ -0,0 +1,6 @@ +GDTEST_RUNTIME_ERROR +>> SCRIPT ERROR +>> on function: test() +>> runtime/errors/typed_dictionary_assign_differently_typed_value.gd +>> 6 +>> Invalid assignment of property or key '0' with value of type 'String' on a base object of type 'Dictionary[int, int]'.