Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GDScript: Unify StringName and String #68747

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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