asgardius/virtualx-engine
1
0
Fork 0
Code Issues Pull requests Projects Releases 1 Packages Wiki Activity Actions

Merge pull request #96797 from Repiteo/core/typed-dictionary-bracket-fix

Browse source 
Core: Fix `operator[]` for typed dictionaries
This commit is contained in:
Rémi Verschelde 2024-09-16 13:35:02 +02:00
commit 08c5ce1d9c
No known key found for this signature in database
GPG key ID: C3336907360768E1
7 changed files with 85 additions and 47 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

61
core/variant/dictionary.cpp
View file

@ -83,35 +83,64 @@ 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];
} else {
*_p->read_only = Variant();
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 {
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];
}
}
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<Variant, Variant, VariantHasher, StringLikeVariantComparator>::ConstIterator E(_p->variant_map.find(p_key));
Variant key = p_key;
if (unlikely(!_p->typed_key.validate(key, "getptr"))) {
return nullptr;
}
HashMap<Variant, Variant, VariantHasher, StringLikeVariantComparator>::ConstIterator E(_p->variant_map.find(key));
if (!E) {
return nullptr;
}
return &E->value;
}
// WARNING: This method does not validate the value type.
Variant *Dictionary::getptr(const Variant &p_key) {
HashMap<Variant, Variant, VariantHasher, StringLikeVariantComparator>::Iterator E(_p->variant_map.find(p_key));
Variant key = p_key;
if (unlikely(!_p->typed_key.validate(key, "getptr"))) {
return nullptr;
}
HashMap<Variant, Variant, VariantHasher, StringLikeVariantComparator>::Iterator E(_p->variant_map.find(key));
if (!E) {
return nullptr;
}
@ -158,6 +187,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();
}

1
core/variant/dictionary.h
View file

@ -59,6 +59,7 @@ public:
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;

44
core/variant/variant_setget.cpp
View file

@ -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<Dictionary>::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<Variant>::encode(*ptr, member);
}
static void set(Variant *base, int64_t index, const Variant *value, bool *valid, bool *oob) {
if (VariantGetInternalPtr<Dictionary>::get_ptr(base)->is_read_only()) {
*valid = false;
*oob = true;
return;
}
(*VariantGetInternalPtr<Dictionary>::get_ptr(base))[index] = *value;
*oob = false;
*valid = true;
*valid = VariantGetInternalPtr<Dictionary>::get_ptr(base)->set(index, *value);
*oob = VariantGetInternalPtr<Dictionary>::get_ptr(base)->is_read_only();
}
static void validated_set(Variant *base, int64_t index, const Variant *value, bool *oob) {
if (VariantGetInternalPtr<Dictionary>::get_ptr(base)->is_read_only()) {
*oob = true;
return;
}
(*VariantGetInternalPtr<Dictionary>::get_ptr(base))[index] = *value;
*oob = false;
VariantGetInternalPtr<Dictionary>::get_ptr(base)->set(index, *value);
*oob = VariantGetInternalPtr<Dictionary>::get_ptr(base)->is_read_only();
}
static void ptr_set(void *base, int64_t index, const void *member) {
Dictionary &v = *reinterpret_cast<Dictionary *>(base);
v[index] = PtrToArg<Variant>::convert(member);
v.set(index, PtrToArg<Variant>::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<Variant>::encode(*ptr, value);
}
static void set(Variant *base, const Variant *key, const Variant *value, bool *r_valid) {
if (VariantGetInternalPtr<Dictionary>::get_ptr(base)->is_read_only()) {
*r_valid = false;
return;
}
(*VariantGetInternalPtr<Dictionary>::get_ptr(base))[*key] = *value;
*r_valid = true;
*r_valid = VariantGetInternalPtr<Dictionary>::get_ptr(base)->set(*key, *value);
}
static void ptr_set(void *base, const void *key, const void *value) {
Dictionary &v = *reinterpret_cast<Dictionary *>(base);
v[PtrToArg<Variant>::convert(key)] = PtrToArg<Variant>::convert(value);
v.set(PtrToArg<Variant>::convert(key), PtrToArg<Variant>::convert(value));
}
static bool has(const Variant *base, const Variant *key, bool *r_valid) {

7
modules/gdscript/tests/scripts/runtime/errors/typed_dictionary_assign_differently_typed_key.gd Normal file
View file

@ -0,0 +1,7 @@
func get_key() -> Variant:
return "key"
func test():
var typed: Dictionary[int, int]
typed[get_key()] = 0
print('not ok')

6
modules/gdscript/tests/scripts/runtime/errors/typed_dictionary_assign_differently_typed_key.out Normal file
View file

@ -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]'.

7
modules/gdscript/tests/scripts/runtime/errors/typed_dictionary_assign_differently_typed_value.gd Normal file
View file

@ -0,0 +1,7 @@
func get_value() -> Variant:
return "value"
func test():
var typed: Dictionary[int, int]
typed[0] = get_value()
print("not ok")

6
modules/gdscript/tests/scripts/runtime/errors/typed_dictionary_assign_differently_typed_value.out Normal file
View file

@ -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]'.