From 5fd8f9d71b21c7862b68ec05ff6fe5c3edeeb19b Mon Sep 17 00:00:00 2001 From: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Date: Mon, 18 Mar 2024 17:18:47 +0100 Subject: [PATCH 1/2] [Core] Make `Dictionary.get_or_add` check for read-only --- core/variant/dictionary.cpp | 1 + tests/core/variant/test_dictionary.h | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/core/variant/dictionary.cpp b/core/variant/dictionary.cpp index c094d532705e..f2658d24b258 100644 --- a/core/variant/dictionary.cpp +++ b/core/variant/dictionary.cpp @@ -179,6 +179,7 @@ Variant Dictionary::get_or_add(const Variant &p_key, const Variant &p_default) { ERR_FAIL_COND_V(!_p->typed_key.validate(key, "get"), p_default); const Variant *result = getptr(key); if (!result) { + ERR_FAIL_COND_V_MSG(_p->read_only, Variant(), "Dictionary is in read-only state."); Variant value = p_default; ERR_FAIL_COND_V(!_p->typed_value.validate(value, "add"), value); operator[](key) = value; diff --git a/tests/core/variant/test_dictionary.h b/tests/core/variant/test_dictionary.h index 46d0bc3e3078..526c4d8f19a4 100644 --- a/tests/core/variant/test_dictionary.h +++ b/tests/core/variant/test_dictionary.h @@ -93,6 +93,11 @@ TEST_CASE("[Dictionary] Assignment using bracket notation ([])") { map.make_read_only(); CHECK(int(map["This key does not exist"].get_type()) == Variant::NIL); CHECK(map.size() == length); + + ERR_PRINT_OFF; + CHECK(int(map.get_or_add("This key does not exist", String()).get_type()) == Variant::NIL); + CHECK(map.size() == length); + ERR_PRINT_ON; } TEST_CASE("[Dictionary] List init") { From a9a1694ad4b3f9e95edbcac7b38b57a31901f7f2 Mon Sep 17 00:00:00 2001 From: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Date: Mon, 18 Mar 2024 17:21:40 +0100 Subject: [PATCH 2/2] [Core] Add `set_safe` method to `Array` and `Dictionary` This returns an error, making it possible to check the behavior Also adds range checking for `Array.set` to match `Vector` and allow it to be use safely --- core/variant/array.cpp | 11 +++++++++++ core/variant/array.h | 1 + core/variant/dictionary.cpp | 10 ++++++++++ core/variant/dictionary.h | 1 + tests/core/variant/test_dictionary.h | 7 +++++++ 5 files changed, 30 insertions(+) diff --git a/core/variant/array.cpp b/core/variant/array.cpp index abde597f2023..0fb71f4f279d 100644 --- a/core/variant/array.cpp +++ b/core/variant/array.cpp @@ -490,9 +490,20 @@ void Array::set(int p_idx, const Variant &p_value) { Variant value = p_value; ERR_FAIL_COND(!_p->typed.validate(value, "set")); + ERR_FAIL_INDEX(p_idx, size()); operator[](p_idx) = value; } +Error Array::set_safe(int p_idx, const Variant &p_value) { + ERR_FAIL_COND_V_MSG(_p->read_only, ERR_LOCKED, "Array is in read-only state."); + Variant value = p_value; + ERR_FAIL_COND_V(!_p->typed.validate(value, "set"), ERR_INVALID_PARAMETER); + + ERR_FAIL_INDEX_V(p_idx, size(), ERR_PARAMETER_RANGE_ERROR); + operator[](p_idx) = value; + return OK; +} + const Variant &Array::get(int p_idx) const { return operator[](p_idx); } diff --git a/core/variant/array.h b/core/variant/array.h index 78b5d71c8883..c74eece08f55 100644 --- a/core/variant/array.h +++ b/core/variant/array.h @@ -118,6 +118,7 @@ class Array { const Variant &operator[](int p_idx) const; void set(int p_idx, const Variant &p_value); + Error set_safe(int p_idx, const Variant &p_value); const Variant &get(int p_idx) const; int size() const; diff --git a/core/variant/dictionary.cpp b/core/variant/dictionary.cpp index f2658d24b258..ab3961a4c613 100644 --- a/core/variant/dictionary.cpp +++ b/core/variant/dictionary.cpp @@ -198,6 +198,16 @@ bool Dictionary::set(const Variant &p_key, const Variant &p_value) { return true; } +Error Dictionary::set_safe(const Variant &p_key, const Variant &p_value) { + ERR_FAIL_COND_V_MSG(_p->read_only, ERR_LOCKED, "Dictionary is in read-only state."); + Variant key = p_key; + ERR_FAIL_COND_V(!_p->typed_key.validate(key, "set_safe"), ERR_INVALID_PARAMETER); + Variant value = p_value; + ERR_FAIL_COND_V(!_p->typed_value.validate(value, "set_safe"), ERR_INVALID_PARAMETER); + operator[](key) = p_value; + return OK; +} + int Dictionary::size() const { return _p->variant_map.size(); } diff --git a/core/variant/dictionary.h b/core/variant/dictionary.h index d7a222818e38..a5f6fe595665 100644 --- a/core/variant/dictionary.h +++ b/core/variant/dictionary.h @@ -62,6 +62,7 @@ class Dictionary { 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); + Error set_safe(const Variant &p_key, const Variant &p_value); int size() const; bool is_empty() const; diff --git a/tests/core/variant/test_dictionary.h b/tests/core/variant/test_dictionary.h index 526c4d8f19a4..8a82601f4c26 100644 --- a/tests/core/variant/test_dictionary.h +++ b/tests/core/variant/test_dictionary.h @@ -97,6 +97,13 @@ TEST_CASE("[Dictionary] Assignment using bracket notation ([])") { ERR_PRINT_OFF; CHECK(int(map.get_or_add("This key does not exist", String()).get_type()) == Variant::NIL); CHECK(map.size() == length); + + map.set("This key does not exist", String()); + CHECK_FALSE(map.has("This key does not exist")); + CHECK(map.size() == length); + + CHECK(map.set_safe("This key does not exist", String()) == ERR_LOCKED); + CHECK(map.size() == length); ERR_PRINT_ON; }