Skip to content

Commit

Permalink
Improved struct type checking.
Browse files Browse the repository at this point in the history
  • Loading branch information
@nlupugla
nlupugla committed Nov 12, 2024
1 parent ab4d72f commit be39501
Show file tree
Hide file tree
Showing 13 changed files with 161 additions and 148 deletions.
13 changes: 2 additions & 11 deletions core/core_bind.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1666,16 +1666,7 @@ TypedArray<Dictionary> ClassDB::class_get_struct_list(const StringName &p_class,
TypedArray<Dictionary> ret;
::ClassDB::get_struct_list(p_class, &structs, p_no_inheritance);
for (const StructInfo &struct_info : structs) {
Dictionary struct_dict;
for (int i = 0; i < struct_info.count; i++) {
Dictionary member_dict;
member_dict[SNAME("name")] = struct_info.names[i];
member_dict[SNAME("type")] = struct_info.types[i];
member_dict[SNAME("class_name")] = struct_info.class_names[i];
member_dict[SNAME("default_value")] = struct_info.default_values[i];
struct_dict[struct_info.name] = member_dict;
}
ret.push_back(struct_dict);
ret.push_back(StructInfo::Layout::to_dict(struct_info));
}
return ret;
}
Expand All @@ -1691,7 +1682,7 @@ TypedArray<Dictionary> ClassDB::class_get_struct_members(const StringName &p_cla
Dictionary dict;
dict[SNAME("name")] = struct_info->names[i];
dict[SNAME("type")] = struct_info->types[i];
dict[SNAME("class_name")] = struct_info->class_names[i];
dict[SNAME("type_name")] = struct_info->type_names[i];
dict[SNAME("default_value")] = struct_info->default_values[i];
ret.push_back(dict);
}
Expand Down
1 change: 1 addition & 0 deletions core/core_bind.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,7 @@ class ClassDB : public Object {

bool class_has_struct(const StringName &p_class, const StringName &p_struct, bool p_no_inheritance = false) const;
TypedArray<Dictionary> class_get_struct_list(const StringName &p_class, bool p_no_inheritance = false) const;
// TODO: class_get_struct_members seems kind of pointless. It should maybe just be class_get_struct or something.
TypedArray<Dictionary> class_get_struct_members(const StringName &p_class, const StringName &p_struct) const;

#ifdef TOOLS_ENABLED
Expand Down
52 changes: 26 additions & 26 deletions core/variant/array.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,22 +51,18 @@ class ArrayPrivate {
ContainerTypeValidate typed;

_FORCE_INLINE_ bool is_struct() const {
return !typed.is_array_of_structs && typed.struct_info != nullptr;
return typed.get_is_struct();
}

_FORCE_INLINE_ bool is_array_of_structs() const {
return typed.is_array_of_structs;
}

_FORCE_INLINE_ bool is_struct_array() const {
return false; // TODO: not supported yet
return typed.is_array_of_structs();
}

_FORCE_INLINE_ int32_t find_member_index(const StringName &p_member) const {
// TODO: is there a better way to do this than linear search?
ERR_FAIL_NULL_V_MSG(typed.struct_info, -1, "Can only find member on a Struct");
for (int32_t i = 0; i < typed.struct_info->count; i++) {
if (p_member == typed.struct_info->names[i]) {
ERR_FAIL_COND_V_MSG(!typed.get_is_struct(), -1, "Can only find member on a Struct");
for (int32_t i = 0; i < typed.get_struct_info()->count; i++) {
if (p_member == typed.get_struct_info()->names[i]) {
return i;
}
}
Expand All @@ -75,9 +71,9 @@ class ArrayPrivate {

_FORCE_INLINE_ int32_t rfind_member_index(const StringName &p_member) const {
// TODO: is there a better way to do this than linear search?
ERR_FAIL_NULL_V_MSG(typed.struct_info, -1, "Can only find member on a Struct");
for (int32_t i = typed.struct_info->count - 1; i >= 0; i--) {
if (p_member == typed.struct_info->names[i]) {
ERR_FAIL_COND_V_MSG(!typed.get_is_struct(), -1, "Can only find member on a Struct");
for (int32_t i = typed.get_struct_info()->count - 1; i >= 0; i--) {
if (p_member == typed.get_struct_info()->names[i]) {
return i;
}
}
Expand Down Expand Up @@ -250,6 +246,10 @@ void Array::operator=(const Array &p_array) {
_ref(p_array);
}

bool Array::can_reference(const Array &p_array) const {
return _p->typed.can_reference(p_array._p->typed);
}

void Array::assign(const Array &p_array) {
const ContainerTypeValidate &typed = _p->typed;
const ContainerTypeValidate &source_typed = p_array._p->typed;
Expand Down Expand Up @@ -363,7 +363,7 @@ Error Array::resize(int p_new_size) {
if (err || variant_type == Variant::NIL || variant_type == Variant::OBJECT) {
return err;
}
if (const StructInfo *info = _p->typed.struct_info) { // Typed array of structs
if (const StructInfo *info = _p->typed.get_struct_info()) { // Typed array of structs
for (int i = old_size; i < p_new_size; i++) {
_p->array.write[i] = Array(*info);
}
Expand Down Expand Up @@ -603,7 +603,7 @@ const Variant &Array::get_named(const StringName &p_member) const {

const StringName Array::get_member_name(int p_idx) const {
// TODO: probably need some error handling here.
return _p->typed.struct_info->names[p_idx];
return _p->typed.get_struct_info()->names[p_idx];
}

int Array::find_member(const StringName &p_member) const {
Expand All @@ -629,7 +629,7 @@ Array Array::duplicate(bool p_deep) const {
Array Array::recursive_duplicate(bool p_deep, int recursion_count) const {
Array new_arr;
if (const StructInfo *struct_info = get_struct_info()) {
new_arr.set_struct(*struct_info, is_array_of_structs());
new_arr.set_struct(*struct_info, is_struct());
} else {
new_arr._p->typed = _p->typed;
if (p_deep) {
Expand Down Expand Up @@ -1002,13 +1002,13 @@ void Array::set_typed(uint32_t p_type, const StringName &p_class_name, const Var
_p->typed = ContainerTypeValidate(Variant::Type(p_type), p_class_name, script, "TypedArray");
}

void Array::set_struct(const StructInfo &p_struct_info, bool p_is_array_of_structs) {
void Array::set_struct(const StructInfo &p_struct_info, bool p_is_struct) {
if (validate_set_type() != OK) {
return;
}
const int32_t size = p_struct_info.count;
_p->array.resize(size);
_p->typed = ContainerTypeValidate(p_struct_info, p_is_array_of_structs);
_p->typed = ContainerTypeValidate(p_struct_info, p_is_struct);
}

void Array::initialize_typed(uint32_t p_type, const StringName &p_class_name, const Variant &p_script) {
Expand All @@ -1019,11 +1019,11 @@ void Array::initialize_typed(uint32_t p_type, const StringName &p_class_name, co
_p->typed = ContainerTypeValidate(Variant::Type(p_type), p_class_name, script, "TypedArray");
}

void Array::initialize_struct_type(const StructInfo &p_struct_info, bool is_array_of_structs) {
if (!is_array_of_structs) {
void Array::initialize_struct_type(const StructInfo &p_struct_info, bool p_is_struct) {
if (p_is_struct) {
_p->array.resize(p_struct_info.count);
}
_p->typed = ContainerTypeValidate(p_struct_info, is_array_of_structs);
_p->typed = ContainerTypeValidate(p_struct_info, p_is_struct);
}

bool Array::is_typed() const {
Expand Down Expand Up @@ -1059,7 +1059,7 @@ Variant Array::get_typed_script() const {
}

const StructInfo *Array::get_struct_info() const {
return _p->typed.struct_info;
return _p->typed.get_struct_info();
}

Array Array::create_read_only() {
Expand Down Expand Up @@ -1087,27 +1087,27 @@ Array::Array(const Array &p_from, const StructInfo &p_struct_info) {
_p = memnew(ArrayPrivate);
_p->refcount.init(); // TODO: should this be _ref(p_from)?

initialize_struct_type(p_struct_info, p_from.is_array_of_structs());
initialize_struct_type(p_struct_info, true);
assign(p_from);
}

Array::Array(const Dictionary &p_from, const StructInfo &p_struct_info) {
_p = memnew(ArrayPrivate);
_p->refcount.init(); // TODO: should this be _ref(p_from)?

initialize_struct_type(p_struct_info, false);
initialize_struct_type(p_struct_info, true);
Variant *pw = _p->array.ptrw();
for (int32_t i = 0; i < p_struct_info.count; i++) {
pw[i] = p_from.has(p_struct_info.names[i]) ? p_from[p_struct_info.names[i]] : p_struct_info.default_values[i];
}
}

Array::Array(const StructInfo &p_struct_info, bool is_array_of_structs) {
Array::Array(const StructInfo &p_struct_info, bool p_is_struct) {
_p = memnew(ArrayPrivate);
_p->refcount.init();

initialize_struct_type(p_struct_info, is_array_of_structs);
if (!is_array_of_structs) {
initialize_struct_type(p_struct_info, p_is_struct);
if (p_is_struct) {
Variant *pw = _p->array.ptrw();
for (int32_t i = 0; i < p_struct_info.count; i++) {
pw[i] = p_struct_info.default_values[i].duplicate(true);
Expand Down
8 changes: 4 additions & 4 deletions core/variant/array.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ class StringName;
class Callable;
template <typename T>
class Vector;
struct StructMember;
struct ContainerTypeValidate;
struct StructInfo;
class Dictionary;
Expand Down Expand Up @@ -144,6 +143,7 @@ class Array {
uint32_t recursive_hash(int recursion_count) const;
void operator=(const Array &p_array);

bool can_reference(const Array &p_array) const;
void assign(const Array &p_array);
void push_back(const Variant &p_value);
_FORCE_INLINE_ void append(const Variant &p_value) { push_back(p_value); } //for python compatibility
Expand Down Expand Up @@ -201,11 +201,11 @@ class Array {

Error validate_set_type();
void set_typed(uint32_t p_type, const StringName &p_class_name, const Variant &p_script);
void set_struct(const StructInfo &p_struct_info, bool p_is_array_of_structs = false);
void set_struct(const StructInfo &p_struct_info, bool p_is_struct = true);

protected:
void initialize_typed(uint32_t p_type, const StringName &p_class_name, const Variant &p_script);
void initialize_struct_type(const StructInfo &p_struct_info, bool p_is_array_of_structs = false);
void initialize_struct_type(const StructInfo &p_struct_info, bool p_is_struct = true);

public:
bool is_typed() const;
Expand All @@ -226,7 +226,7 @@ class Array {
Array(const Array &p_from);
Array(const Array &p_from, const StructInfo &p_struct_info);
Array(const Dictionary &p_from, const StructInfo &p_struct_info);
Array(const StructInfo &p_struct_info, bool p_is_array_of_structs = false);
Array(const StructInfo &p_struct_info, bool p_is_struct = true);
Array();
~Array();
};
Expand Down
83 changes: 52 additions & 31 deletions core/variant/container_type_validate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,8 @@ struct ValidatedVariant {
Variant value;
bool valid;

ValidatedVariant(const Variant &p_value, const bool p_valid) {
value = p_value;
valid = p_valid;
ValidatedVariant(const Variant &p_value, const bool p_valid) :
value(p_value), valid(p_valid) {
}
};

Expand All @@ -50,25 +49,37 @@ struct ContainerTypeValidate {
StringName class_name;
Ref<Script> script;

bool is_array_of_structs = false;
const StructInfo *struct_info = nullptr; // TODO: if is_array_of_structs == true, then require struct_info != nullptr, but not sure the best way to enforce this.
const char *where = "container";

ContainerTypeValidate() {};
ContainerTypeValidate(const Variant::Type p_type, const StringName &p_class_name, const Ref<Script> &p_script, const char *p_where = "container") {
type = p_type;
class_name = p_class_name;
script = p_script;
struct_info = nullptr;
where = p_where;
private:
// is_struct must be false if struct_info == nullptr
// if is_struct is false and struct_info != nullptr, the container is a TypedArray of Structs.
bool is_struct = false;

Check failure on line 57 in core/variant/container_type_validate.h

View workflow job for this annotation

GitHub Actions / 🐧 Linux / Editor w/ Mono (target=editor) 

'bool ContainerTypeValidate::is_struct' [-Werror=reorder]

Check failure on line 57 in core/variant/container_type_validate.h

View workflow job for this annotation

GitHub Actions / 🐧 Linux / Editor w/ Mono (target=editor) 

'bool ContainerTypeValidate::is_struct' [-Werror=reorder]

Check failure on line 57 in core/variant/container_type_validate.h

View workflow job for this annotation

GitHub Actions / 🐧 Linux / Editor with doubles and GCC sanitizers (target=editor, tests=yes, dev_build=yes, scu_build=yes, precision=double, use_asan=yes, use_ubsan=yes, linker=gold) 

'bool ContainerTypeValidate::is_struct' [-Werror=reorder]
const StructInfo *struct_info = nullptr;

Check failure on line 58 in core/variant/container_type_validate.h

View workflow job for this annotation

GitHub Actions / 🐧 Linux / Editor w/ Mono (target=editor) 

'ContainerTypeValidate::struct_info' will be initialized after [-Werror=reorder]

Check failure on line 58 in core/variant/container_type_validate.h

View workflow job for this annotation

GitHub Actions / 🐧 Linux / Editor w/ Mono (target=editor) 

'ContainerTypeValidate::struct_info' will be initialized after [-Werror=reorder]

Check failure on line 58 in core/variant/container_type_validate.h

View workflow job for this annotation

GitHub Actions / 🐧 Linux / Editor with doubles and GCC sanitizers (target=editor, tests=yes, dev_build=yes, scu_build=yes, precision=double, use_asan=yes, use_ubsan=yes, linker=gold) 

'ContainerTypeValidate::struct_info' will be initialized after [-Werror=reorder]

public:
ContainerTypeValidate() {}
ContainerTypeValidate(const Variant::Type p_type, const StringName &p_class_name, const Ref<Script> &p_script, const char *p_where = "container") :
type(p_type), class_name(p_class_name), script(p_script), where(p_where) {
}
ContainerTypeValidate(const StructInfo &p_struct_info, bool p_is_struct = true) :

Check failure on line 65 in core/variant/container_type_validate.h

View workflow job for this annotation

GitHub Actions / 🐧 Linux / Editor w/ Mono (target=editor) 

when initialized here [-Werror=reorder]

Check failure on line 65 in core/variant/container_type_validate.h

View workflow job for this annotation

GitHub Actions / 🐧 Linux / Editor w/ Mono (target=editor) 

when initialized here [-Werror=reorder]

Check failure on line 65 in core/variant/container_type_validate.h

View workflow job for this annotation

GitHub Actions / 🐧 Linux / Editor with doubles and GCC sanitizers (target=editor, tests=yes, dev_build=yes, scu_build=yes, precision=double, use_asan=yes, use_ubsan=yes, linker=gold) 

when initialized here [-Werror=reorder]
type(Variant::ARRAY), class_name(p_struct_info.name), script(Ref<Script>()), where(p_is_struct ? "Struct" : "TypedArray"), struct_info(&p_struct_info), is_struct(p_is_struct) {
}

_FORCE_INLINE_ void set_struct_info(const StructInfo *p_struct_info) {
struct_info = p_struct_info;
if (!struct_info) {
is_struct = false;
}
}
_FORCE_INLINE_ const StructInfo *get_struct_info() const {
return struct_info;
}
ContainerTypeValidate(const StructInfo &p_struct_info, bool p_is_array_of_structs = false) {
type = Variant::ARRAY;
class_name = p_struct_info.name;
script = Ref<Script>();
is_array_of_structs = p_is_array_of_structs;
struct_info = &p_struct_info;
where = p_is_array_of_structs ? "TypedArray" : "Struct";
_FORCE_INLINE_ bool get_is_struct() const {
return is_struct;
}
_FORCE_INLINE_ bool is_array_of_structs() const {
return !is_struct && struct_info != nullptr;
}

_FORCE_INLINE_ bool can_reference(const ContainerTypeValidate &p_type) const {
Expand All @@ -78,7 +89,7 @@ struct ContainerTypeValidate {
if (type != p_type.type) {
return false;
}
if (is_array_of_structs != p_type.is_array_of_structs) {
if (is_struct != p_type.is_struct) {
return false;
}
if (!StructInfo::is_compatible(struct_info, p_type.struct_info)) {
Expand Down Expand Up @@ -108,10 +119,10 @@ struct ContainerTypeValidate {
}

_FORCE_INLINE_ bool operator==(const ContainerTypeValidate &p_type) const {
return type == p_type.type && class_name == p_type.class_name && script == p_type.script && is_array_of_structs == p_type.is_array_of_structs && StructInfo::is_compatible(struct_info, p_type.struct_info);
return type == p_type.type && class_name == p_type.class_name && script == p_type.script && is_struct == p_type.is_struct && StructInfo::is_compatible(struct_info, p_type.struct_info);
}
_FORCE_INLINE_ bool operator!=(const ContainerTypeValidate &p_type) const {
return type != p_type.type || class_name != p_type.class_name || script != p_type.script || is_array_of_structs != p_type.is_array_of_structs || !StructInfo::is_compatible(struct_info, p_type.struct_info);
return type != p_type.type || class_name != p_type.class_name || script != p_type.script || is_struct != p_type.is_struct || !StructInfo::is_compatible(struct_info, p_type.struct_info);
}

_FORCE_INLINE_ static ValidatedVariant validate_variant_type(const Variant::Type p_type, const Variant &p_variant, const char *p_where, const char *p_operation = "use") {
Expand Down Expand Up @@ -185,8 +196,8 @@ struct ContainerTypeValidate {
if (!ret.valid) {
return ret;
}

// Variant types match

if (type == Variant::ARRAY) {
const Array array = p_variant;
if (array.is_struct()) { // validating a struct into a typed array of structs
Expand All @@ -209,16 +220,26 @@ struct ContainerTypeValidate {
if (!ret.valid) {
return ret;
}

// Variant types match
if (variant_type == Variant::ARRAY) {
const Array array = p_variant;
// Valid if (the struct member is itself a struct and the other array is a compatible struct) or (neither the struct member nor the other array are a struct).
ret.valid = StructInfo::is_compatible(struct_info->struct_member_infos[p_struct_index], array.get_struct_info());
} else if (variant_type == Variant::OBJECT) {
ret.valid = validate_object(struct_info->class_names[p_struct_index], Ref<Script>(), p_variant, where, p_operation);

switch (variant_type) {
case Variant::ARRAY: {
const Array &default_array = struct_info->default_values[p_struct_index];
ret.valid = default_array.can_reference(p_variant);
return ret;
}
case Variant::DICTIONARY: {
const Dictionary &default_dict = struct_info->default_values[p_struct_index];
ret.valid = default_dict.can_reference(p_variant);
return ret;
}
case Variant::OBJECT: {
ret.valid = validate_object(struct_info->type_names[p_struct_index], static_cast<Ref<Script>>(struct_info->scripts[p_struct_index]), p_variant, where, p_operation);
return ret;
}
default:
return ret;
}
return ret;
}
};

Expand Down
4 changes: 4 additions & 0 deletions core/variant/dictionary.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,10 @@ Array Dictionary::values() const {
return varr;
}

bool Dictionary::can_reference(const Dictionary &p_dictionary) const {
return _p->typed_key.can_reference(p_dictionary._p->typed_key) && _p->typed_value.can_reference(p_dictionary._p->typed_value);
}

void Dictionary::assign(const Dictionary &p_dictionary) {
const ContainerTypeValidate &typed_key = _p->typed_key;
const ContainerTypeValidate &typed_key_source = p_dictionary._p->typed_key;
Expand Down
1 change: 1 addition & 0 deletions core/variant/dictionary.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ class Dictionary {
uint32_t recursive_hash(int recursion_count) const;
void operator=(const Dictionary &p_dictionary);

bool can_reference(const Dictionary &p_dictionary) const;
void assign(const Dictionary &p_dictionary);
const Variant *next(const Variant *p_key = nullptr) const;

Expand Down
4 changes: 2 additions & 2 deletions core/variant/struct.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ struct GetTypeInfo<Struct<T>> {
static const Variant::Type VARIANT_TYPE = Variant::ARRAY;
static const GodotTypeInfo::Metadata METADATA = GodotTypeInfo::METADATA_NONE;
static PropertyInfo get_class_info() {
return PropertyInfo(Variant::ARRAY, String(), PROPERTY_HINT_ARRAY_TYPE, T::Layout::get_struct_name());
return PropertyInfo(Variant::ARRAY, String(), PROPERTY_HINT_ARRAY_TYPE, T::get_struct_name());
}
};

Expand All @@ -139,7 +139,7 @@ struct GetTypeInfo<const Struct<T> &> {
static const Variant::Type VARIANT_TYPE = Variant::ARRAY;
static const GodotTypeInfo::Metadata METADATA = GodotTypeInfo::METADATA_NONE;
static PropertyInfo get_class_info() {
return PropertyInfo(Variant::ARRAY, String(), PROPERTY_HINT_ARRAY_TYPE, T::Layout::get_struct_name());
return PropertyInfo(Variant::ARRAY, String(), PROPERTY_HINT_ARRAY_TYPE, T::get_struct_name());
}
};

Expand Down
Loading

0 comments on commit be39501

Please sign in to comment.