Skip to content

Commit

Permalink
Merge pull request #68747 from rune-scape/rune-stringname-unification
Browse files Browse the repository at this point in the history
GDScript: Unify StringName and String
  • Loading branch information
akien-mga committed Dec 9, 2022
2 parents 597e0c0 + e79be6c commit 907298d
Show file tree
Hide file tree
Showing 28 changed files with 1,463 additions and 267 deletions.
96 changes: 67 additions & 29 deletions core/variant/array.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include "core/templates/search_array.h"
#include "core/templates/vector.h"
#include "core/variant/callable.h"
#include "core/variant/dictionary.h"
#include "core/variant/variant.h"

class ArrayPrivate {
Expand Down Expand Up @@ -201,16 +202,21 @@ uint32_t Array::recursive_hash(int recursion_count) const {
}

bool Array::_assign(const Array &p_array) {
bool can_convert = p_array._p->typed.type == Variant::NIL;
can_convert |= _p->typed.type == Variant::STRING && p_array._p->typed.type == Variant::STRING_NAME;
can_convert |= _p->typed.type == Variant::STRING_NAME && p_array._p->typed.type == Variant::STRING;

if (_p->typed.type != Variant::OBJECT && _p->typed.type == p_array._p->typed.type) {
//same type or untyped, just reference, should be fine
_ref(p_array);
} else if (_p->typed.type == Variant::NIL) { //from typed to untyped, must copy, but this is cheap anyway
_p->array = p_array._p->array;
} else if (p_array._p->typed.type == Variant::NIL) { //from untyped to typed, must try to check if they are all valid
} else if (can_convert) { //from untyped to typed, must try to check if they are all valid
if (_p->typed.type == Variant::OBJECT) {
//for objects, it needs full validation, either can be converted or fail
for (int i = 0; i < p_array._p->array.size(); i++) {
if (!_p->typed.validate(p_array._p->array[i], "assign")) {
const Variant &element = p_array._p->array[i];
if (element.get_type() != Variant::OBJECT || !_p->typed.validate_object(element, "assign")) {
return false;
}
}
Expand Down Expand Up @@ -255,16 +261,20 @@ void Array::operator=(const Array &p_array) {

void Array::push_back(const Variant &p_value) {
ERR_FAIL_COND_MSG(_p->read_only, "Array is in read-only state.");
ERR_FAIL_COND(!_p->typed.validate(p_value, "push_back"));
_p->array.push_back(p_value);
Variant value = p_value;
ERR_FAIL_COND(!_p->typed.validate(value, "push_back"));
_p->array.push_back(value);
}

void Array::append_array(const Array &p_array) {
ERR_FAIL_COND_MSG(_p->read_only, "Array is in read-only state.");
for (int i = 0; i < p_array.size(); ++i) {
ERR_FAIL_COND(!_p->typed.validate(p_array[i], "append_array"));

Vector<Variant> validated_array = p_array._p->array;
for (int i = 0; i < validated_array.size(); ++i) {
ERR_FAIL_COND(!_p->typed.validate(validated_array.write[i], "append_array"));
}
_p->array.append_array(p_array._p->array);

_p->array.append_array(validated_array);
}

Error Array::resize(int p_new_size) {
Expand All @@ -274,20 +284,23 @@ Error Array::resize(int p_new_size) {

Error Array::insert(int p_pos, const Variant &p_value) {
ERR_FAIL_COND_V_MSG(_p->read_only, ERR_LOCKED, "Array is in read-only state.");
ERR_FAIL_COND_V(!_p->typed.validate(p_value, "insert"), ERR_INVALID_PARAMETER);
return _p->array.insert(p_pos, p_value);
Variant value = p_value;
ERR_FAIL_COND_V(!_p->typed.validate(value, "insert"), ERR_INVALID_PARAMETER);
return _p->array.insert(p_pos, value);
}

void Array::fill(const Variant &p_value) {
ERR_FAIL_COND_MSG(_p->read_only, "Array is in read-only state.");
ERR_FAIL_COND(!_p->typed.validate(p_value, "fill"));
_p->array.fill(p_value);
Variant value = p_value;
ERR_FAIL_COND(!_p->typed.validate(value, "fill"));
_p->array.fill(value);
}

void Array::erase(const Variant &p_value) {
ERR_FAIL_COND_MSG(_p->read_only, "Array is in read-only state.");
ERR_FAIL_COND(!_p->typed.validate(p_value, "erase"));
_p->array.erase(p_value);
Variant value = p_value;
ERR_FAIL_COND(!_p->typed.validate(value, "erase"));
_p->array.erase(value);
}

Variant Array::front() const {
Expand All @@ -306,15 +319,34 @@ Variant Array::pick_random() const {
}

int Array::find(const Variant &p_value, int p_from) const {
ERR_FAIL_COND_V(!_p->typed.validate(p_value, "find"), -1);
return _p->array.find(p_value, p_from);
if (_p->array.size() == 0) {
return -1;
}
Variant value = p_value;
ERR_FAIL_COND_V(!_p->typed.validate(value, "find"), -1);

int ret = -1;

if (p_from < 0 || size() == 0) {
return ret;
}

for (int i = p_from; i < size(); i++) {
if (StringLikeVariantComparator::compare(_p->array[i], value)) {
ret = i;
break;
}
}

return ret;
}

int Array::rfind(const Variant &p_value, int p_from) const {
if (_p->array.size() == 0) {
return -1;
}
ERR_FAIL_COND_V(!_p->typed.validate(p_value, "rfind"), -1);
Variant value = p_value;
ERR_FAIL_COND_V(!_p->typed.validate(value, "rfind"), -1);

if (p_from < 0) {
// Relative offset from the end
Expand All @@ -326,7 +358,7 @@ int Array::rfind(const Variant &p_value, int p_from) const {
}

for (int i = p_from; i >= 0; i--) {
if (_p->array[i] == p_value) {
if (StringLikeVariantComparator::compare(_p->array[i], value)) {
return i;
}
}
Expand All @@ -335,14 +367,15 @@ int Array::rfind(const Variant &p_value, int p_from) const {
}

int Array::count(const Variant &p_value) const {
ERR_FAIL_COND_V(!_p->typed.validate(p_value, "count"), 0);
Variant value = p_value;
ERR_FAIL_COND_V(!_p->typed.validate(value, "count"), 0);
if (_p->array.size() == 0) {
return 0;
}

int amount = 0;
for (int i = 0; i < _p->array.size(); i++) {
if (_p->array[i] == p_value) {
if (StringLikeVariantComparator::compare(_p->array[i], value)) {
amount++;
}
}
Expand All @@ -351,9 +384,10 @@ int Array::count(const Variant &p_value) const {
}

bool Array::has(const Variant &p_value) const {
ERR_FAIL_COND_V(!_p->typed.validate(p_value, "use 'has'"), false);
Variant value = p_value;
ERR_FAIL_COND_V(!_p->typed.validate(value, "use 'has'"), false);

return _p->array.find(p_value, 0) != -1;
return find(value) != -1;
}

void Array::remove_at(int p_pos) {
Expand All @@ -363,9 +397,10 @@ void Array::remove_at(int p_pos) {

void Array::set(int p_idx, const Variant &p_value) {
ERR_FAIL_COND_MSG(_p->read_only, "Array is in read-only state.");
ERR_FAIL_COND(!_p->typed.validate(p_value, "set"));
Variant value = p_value;
ERR_FAIL_COND(!_p->typed.validate(value, "set"));

operator[](p_idx) = p_value;
operator[](p_idx) = value;
}

const Variant &Array::get(int p_idx) const {
Expand Down Expand Up @@ -588,15 +623,17 @@ void Array::shuffle() {
}

int Array::bsearch(const Variant &p_value, bool p_before) {
ERR_FAIL_COND_V(!_p->typed.validate(p_value, "binary search"), -1);
Variant value = p_value;
ERR_FAIL_COND_V(!_p->typed.validate(value, "binary search"), -1);
SearchArray<Variant, _ArrayVariantSort> avs;
return avs.bisect(_p->array.ptrw(), _p->array.size(), p_value, p_before);
return avs.bisect(_p->array.ptrw(), _p->array.size(), value, p_before);
}

int Array::bsearch_custom(const Variant &p_value, const Callable &p_callable, bool p_before) {
ERR_FAIL_COND_V(!_p->typed.validate(p_value, "custom binary search"), -1);
Variant value = p_value;
ERR_FAIL_COND_V(!_p->typed.validate(value, "custom binary search"), -1);

return _p->array.bsearch_custom<CallableComparator>(p_value, p_before, p_callable);
return _p->array.bsearch_custom<CallableComparator>(value, p_before, p_callable);
}

void Array::reverse() {
Expand All @@ -606,8 +643,9 @@ void Array::reverse() {

void Array::push_front(const Variant &p_value) {
ERR_FAIL_COND_MSG(_p->read_only, "Array is in read-only state.");
ERR_FAIL_COND(!_p->typed.validate(p_value, "push_front"));
_p->array.insert(0, p_value);
Variant value = p_value;
ERR_FAIL_COND(!_p->typed.validate(value, "push_front"));
_p->array.insert(0, value);
}

Variant Array::pop_back() {
Expand Down
23 changes: 19 additions & 4 deletions core/variant/container_type_validate.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,22 +74,37 @@ struct ContainerTypeValidate {
return true;
}

_FORCE_INLINE_ bool validate(const Variant &p_variant, const char *p_operation = "use") {
// Coerces String and StringName into each other when needed.
_FORCE_INLINE_ bool validate(Variant &inout_variant, const char *p_operation = "use") {
if (type == Variant::NIL) {
return true;
}

if (type != p_variant.get_type()) {
if (p_variant.get_type() == Variant::NIL && type == Variant::OBJECT) {
if (type != inout_variant.get_type()) {
if (inout_variant.get_type() == Variant::NIL && type == Variant::OBJECT) {
return true;
}
if (type == Variant::STRING && inout_variant.get_type() == Variant::STRING_NAME) {
inout_variant = String(inout_variant);
return true;
} else if (type == Variant::STRING_NAME && inout_variant.get_type() == Variant::STRING) {
inout_variant = StringName(inout_variant);
return true;
}

ERR_FAIL_V_MSG(false, "Attempted to " + String(p_operation) + " a variable of type '" + Variant::get_type_name(p_variant.get_type()) + "' into a " + where + " of type '" + Variant::get_type_name(type) + "'.");
ERR_FAIL_V_MSG(false, "Attempted to " + String(p_operation) + " a variable of type '" + Variant::get_type_name(inout_variant.get_type()) + "' into a " + where + " of type '" + Variant::get_type_name(type) + "'.");
}

if (type != Variant::OBJECT) {
return true;
}

return validate_object(inout_variant, p_operation);
}

_FORCE_INLINE_ bool validate_object(const Variant &p_variant, const char *p_operation = "use") {
ERR_FAIL_COND_V(p_variant.get_type() != Variant::OBJECT, false);

#ifdef DEBUG_ENABLED
ObjectID object_id = p_variant;
if (object_id == ObjectID()) {
Expand Down
56 changes: 10 additions & 46 deletions core/variant/dictionary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
struct DictionaryPrivate {
SafeRefCount refcount;
Variant *read_only = nullptr; // If enabled, a pointer is used to a temporary value that is used to return read-only values.
HashMap<Variant, Variant, VariantHasher, VariantComparator> variant_map;
HashMap<Variant, Variant, VariantHasher, StringLikeVariantComparator> variant_map;
};

void Dictionary::get_key_list(List<Variant> *p_keys) const {
Expand Down Expand Up @@ -100,39 +100,20 @@ Variant &Dictionary::operator[](const Variant &p_key) {
}

const Variant &Dictionary::operator[](const Variant &p_key) const {
if (p_key.get_type() == Variant::STRING_NAME) {
const StringName *sn = VariantInternal::get_string_name(&p_key);
return _p->variant_map[sn->operator String()];
} else {
return _p->variant_map[p_key];
}
// Will not insert key, so no conversion is necessary.
return _p->variant_map[p_key];
}

const Variant *Dictionary::getptr(const Variant &p_key) const {
HashMap<Variant, Variant, VariantHasher, VariantComparator>::ConstIterator E;

if (p_key.get_type() == Variant::STRING_NAME) {
const StringName *sn = VariantInternal::get_string_name(&p_key);
E = ((const HashMap<Variant, Variant, VariantHasher, VariantComparator> *)&_p->variant_map)->find(sn->operator String());
} else {
E = ((const HashMap<Variant, Variant, VariantHasher, VariantComparator> *)&_p->variant_map)->find(p_key);
}

HashMap<Variant, Variant, VariantHasher, StringLikeVariantComparator>::ConstIterator E(_p->variant_map.find(p_key));
if (!E) {
return nullptr;
}
return &E->value;
}

Variant *Dictionary::getptr(const Variant &p_key) {
HashMap<Variant, Variant, VariantHasher, VariantComparator>::Iterator E;

if (p_key.get_type() == Variant::STRING_NAME) {
const StringName *sn = VariantInternal::get_string_name(&p_key);
E = ((HashMap<Variant, Variant, VariantHasher, VariantComparator> *)&_p->variant_map)->find(sn->operator String());
} else {
E = ((HashMap<Variant, Variant, VariantHasher, VariantComparator> *)&_p->variant_map)->find(p_key);
}
HashMap<Variant, Variant, VariantHasher, StringLikeVariantComparator>::Iterator E(_p->variant_map.find(p_key));
if (!E) {
return nullptr;
}
Expand All @@ -145,14 +126,7 @@ Variant *Dictionary::getptr(const Variant &p_key) {
}

Variant Dictionary::get_valid(const Variant &p_key) const {
HashMap<Variant, Variant, VariantHasher, VariantComparator>::ConstIterator E;

if (p_key.get_type() == Variant::STRING_NAME) {
const StringName *sn = VariantInternal::get_string_name(&p_key);
E = ((const HashMap<Variant, Variant, VariantHasher, VariantComparator> *)&_p->variant_map)->find(sn->operator String());
} else {
E = ((const HashMap<Variant, Variant, VariantHasher, VariantComparator> *)&_p->variant_map)->find(p_key);
}
HashMap<Variant, Variant, VariantHasher, StringLikeVariantComparator>::ConstIterator E(_p->variant_map.find(p_key));

if (!E) {
return Variant();
Expand All @@ -178,12 +152,7 @@ bool Dictionary::is_empty() const {
}

bool Dictionary::has(const Variant &p_key) const {
if (p_key.get_type() == Variant::STRING_NAME) {
const StringName *sn = VariantInternal::get_string_name(&p_key);
return _p->variant_map.has(sn->operator String());
} else {
return _p->variant_map.has(p_key);
}
return _p->variant_map.has(p_key);
}

bool Dictionary::has_all(const Array &p_keys) const {
Expand All @@ -206,12 +175,7 @@ Variant Dictionary::find_key(const Variant &p_value) const {

bool Dictionary::erase(const Variant &p_key) {
ERR_FAIL_COND_V_MSG(_p->read_only, false, "Dictionary is in read-only state.");
if (p_key.get_type() == Variant::STRING_NAME) {
const StringName *sn = VariantInternal::get_string_name(&p_key);
return _p->variant_map.erase(sn->operator String());
} else {
return _p->variant_map.erase(p_key);
}
return _p->variant_map.erase(p_key);
}

bool Dictionary::operator==(const Dictionary &p_dictionary) const {
Expand All @@ -238,7 +202,7 @@ bool Dictionary::recursive_equal(const Dictionary &p_dictionary, int recursion_c
}
recursion_count++;
for (const KeyValue<Variant, Variant> &this_E : _p->variant_map) {
HashMap<Variant, Variant, VariantHasher, VariantComparator>::ConstIterator other_E = ((const HashMap<Variant, Variant, VariantHasher, VariantComparator> *)&p_dictionary._p->variant_map)->find(this_E.key);
HashMap<Variant, Variant, VariantHasher, StringLikeVariantComparator>::ConstIterator other_E(p_dictionary._p->variant_map.find(this_E.key));
if (!other_E || !this_E.value.hash_compare(other_E->value, recursion_count)) {
return false;
}
Expand Down Expand Up @@ -360,7 +324,7 @@ const Variant *Dictionary::next(const Variant *p_key) const {
}
return nullptr;
}
HashMap<Variant, Variant, VariantHasher, VariantComparator>::Iterator E = _p->variant_map.find(*p_key);
HashMap<Variant, Variant, VariantHasher, StringLikeVariantComparator>::Iterator E = _p->variant_map.find(*p_key);

if (!E) {
return nullptr;
Expand Down
13 changes: 13 additions & 0 deletions core/variant/variant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3491,6 +3491,19 @@ bool Variant::hash_compare(const Variant &p_variant, int recursion_count) const
}
}

bool StringLikeVariantComparator::compare(const Variant &p_lhs, const Variant &p_rhs) {
if (p_lhs.hash_compare(p_rhs)) {
return true;
}
if (p_lhs.get_type() == Variant::STRING && p_rhs.get_type() == Variant::STRING_NAME) {
return *VariantInternal::get_string(&p_lhs) == *VariantInternal::get_string_name(&p_rhs);
}
if (p_lhs.get_type() == Variant::STRING_NAME && p_rhs.get_type() == Variant::STRING) {
return *VariantInternal::get_string_name(&p_lhs) == *VariantInternal::get_string(&p_rhs);
}
return false;
}

bool Variant::is_ref_counted() const {
return type == OBJECT && _get_obj().id.is_ref_counted();
}
Expand Down
Loading

0 comments on commit 907298d

Please sign in to comment.