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.

Sign up for GitHub

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: Fix typed arrays #69248

Merged
merged 1 commit into from
Jan 31, 2023
+943 −539
Merged

GDScript: Fix typed arrays #69248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 7 additions & 7 deletions core/core_bind.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -717,7 +717,7 @@ TypedArray<PackedVector2Array> Geometry2D::decompose_polygon_in_convex(const Vec
TypedArray<PackedVector2Array> Geometry2D::merge_polygons(const Vector<Vector2> &p_polygon_a, const Vector<Vector2> &p_polygon_b) {
Vector<Vector<Point2>> polys = ::Geometry2D::merge_polygons(p_polygon_a, p_polygon_b);

Array ret;
TypedArray<PackedVector2Array> ret;

for (int i = 0; i < polys.size(); ++i) {
ret.push_back(polys[i]);
Expand All @@ -739,7 +739,7 @@ TypedArray<PackedVector2Array> Geometry2D::clip_polygons(const Vector<Vector2> &
TypedArray<PackedVector2Array> Geometry2D::intersect_polygons(const Vector<Vector2> &p_polygon_a, const Vector<Vector2> &p_polygon_b) {
Vector<Vector<Point2>> polys = ::Geometry2D::intersect_polygons(p_polygon_a, p_polygon_b);

Array ret;
TypedArray<PackedVector2Array> ret;

for (int i = 0; i < polys.size(); ++i) {
ret.push_back(polys[i]);
Expand All @@ -750,7 +750,7 @@ TypedArray<PackedVector2Array> Geometry2D::intersect_polygons(const Vector<Vecto
TypedArray<PackedVector2Array> Geometry2D::exclude_polygons(const Vector<Vector2> &p_polygon_a, const Vector<Vector2> &p_polygon_b) {
Vector<Vector<Point2>> polys = ::Geometry2D::exclude_polygons(p_polygon_a, p_polygon_b);

Array ret;
TypedArray<PackedVector2Array> ret;

for (int i = 0; i < polys.size(); ++i) {
ret.push_back(polys[i]);
Expand All @@ -761,7 +761,7 @@ TypedArray<PackedVector2Array> Geometry2D::exclude_polygons(const Vector<Vector2
TypedArray<PackedVector2Array> Geometry2D::clip_polyline_with_polygon(const Vector<Vector2> &p_polyline, const Vector<Vector2> &p_polygon) {
Vector<Vector<Point2>> polys = ::Geometry2D::clip_polyline_with_polygon(p_polyline, p_polygon);

Array ret;
TypedArray<PackedVector2Array> ret;

for (int i = 0; i < polys.size(); ++i) {
ret.push_back(polys[i]);
Expand All @@ -772,7 +772,7 @@ TypedArray<PackedVector2Array> Geometry2D::clip_polyline_with_polygon(const Vect
TypedArray<PackedVector2Array> Geometry2D::intersect_polyline_with_polygon(const Vector<Vector2> &p_polyline, const Vector<Vector2> &p_polygon) {
Vector<Vector<Point2>> polys = ::Geometry2D::intersect_polyline_with_polygon(p_polyline, p_polygon);

Array ret;
TypedArray<PackedVector2Array> ret;

for (int i = 0; i < polys.size(); ++i) {
ret.push_back(polys[i]);
Expand All @@ -783,7 +783,7 @@ TypedArray<PackedVector2Array> Geometry2D::intersect_polyline_with_polygon(const
TypedArray<PackedVector2Array> Geometry2D::offset_polygon(const Vector<Vector2> &p_polygon, real_t p_delta, PolyJoinType p_join_type) {
Vector<Vector<Point2>> polys = ::Geometry2D::offset_polygon(p_polygon, p_delta, ::Geometry2D::PolyJoinType(p_join_type));

Array ret;
TypedArray<PackedVector2Array> ret;

for (int i = 0; i < polys.size(); ++i) {
ret.push_back(polys[i]);
Expand All @@ -794,7 +794,7 @@ TypedArray<PackedVector2Array> Geometry2D::offset_polygon(const Vector<Vector2>
TypedArray<PackedVector2Array> Geometry2D::offset_polyline(const Vector<Vector2> &p_polygon, real_t p_delta, PolyJoinType p_join_type, PolyEndType p_end_type) {
Vector<Vector<Point2>> polys = ::Geometry2D::offset_polyline(p_polygon, p_delta, ::Geometry2D::PolyJoinType(p_join_type), ::Geometry2D::PolyEndType(p_end_type));

Array ret;
TypedArray<PackedVector2Array> ret;

for (int i = 0; i < polys.size(); ++i) {
ret.push_back(polys[i]);
Expand Down
12 changes: 10 additions & 2 deletions core/doc_data.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@

#include "doc_data.h"

String DocData::get_default_value_string(const Variant &p_value) {
if (p_value.get_type() == Variant::ARRAY) {
return Variant(Array(p_value, 0, StringName(), Variant())).get_construct_string().replace("\n", " ");
} else {
return p_value.get_construct_string().replace("\n", " ");
}
}

void DocData::return_doc_from_retinfo(DocData::MethodDoc &p_method, const PropertyInfo &p_retinfo) {
if (p_retinfo.type == Variant::INT && p_retinfo.hint == PROPERTY_HINT_INT_IS_POINTER) {
p_method.return_type = p_retinfo.hint_string;
Expand Down Expand Up @@ -105,7 +113,7 @@ void DocData::property_doc_from_scriptmemberinfo(DocData::PropertyDoc &p_propert
p_property.getter = p_memberinfo.getter;

if (p_memberinfo.has_default_value && p_memberinfo.default_value.get_type() != Variant::OBJECT) {
p_property.default_value = p_memberinfo.default_value.get_construct_string().replace("\n", "");
p_property.default_value = get_default_value_string(p_memberinfo.default_value);
}

p_property.overridden = false;
Expand Down Expand Up @@ -148,7 +156,7 @@ void DocData::method_doc_from_methodinfo(DocData::MethodDoc &p_method, const Met
int default_arg_index = i - (p_methodinfo.arguments.size() - p_methodinfo.default_arguments.size());
if (default_arg_index >= 0) {
Variant default_arg = p_methodinfo.default_arguments[default_arg_index];
argument.default_value = default_arg.get_construct_string().replace("\n", "");
argument.default_value = get_default_value_string(default_arg);
}
p_method.arguments.push_back(argument);
}
Expand Down
2 changes: 2 additions & 0 deletions core/doc_data.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,8 @@ class DocData {
}
};

static String get_default_value_string(const Variant &p_value);

static void return_doc_from_retinfo(DocData::MethodDoc &p_method, const PropertyInfo &p_retinfo);
static void argument_doc_from_arginfo(DocData::ArgumentDoc &p_argument, const PropertyInfo &p_arginfo);
static void property_doc_from_scriptmemberinfo(DocData::PropertyDoc &p_property, const ScriptMemberInfo &p_memberinfo);
Expand Down
7 changes: 7 additions & 0 deletions core/extension/gdextension_interface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,12 @@ static GDExtensionVariantPtr gdextension_array_operator_index_const(GDExtensionC
return (GDExtensionVariantPtr)&self->operator[](p_index);
}

void gdextension_array_ref(GDExtensionTypePtr p_self, GDExtensionConstTypePtr p_from) {
Array *self = (Array *)p_self;
const Array *from = (const Array *)p_from;
self->_ref(*from);
}

void gdextension_array_set_typed(GDExtensionTypePtr p_self, uint32_t p_type, GDExtensionConstStringNamePtr p_class_name, GDExtensionConstVariantPtr p_script) {
Array *self = reinterpret_cast<Array *>(p_self);
const StringName *class_name = reinterpret_cast<const StringName *>(p_class_name);
Expand Down Expand Up @@ -1136,6 +1142,7 @@ void gdextension_setup_interface(GDExtensionInterface *p_interface) {

gde_interface.array_operator_index = gdextension_array_operator_index;
gde_interface.array_operator_index_const = gdextension_array_operator_index_const;
gde_interface.array_ref = gdextension_array_ref;
gde_interface.array_set_typed = gdextension_array_set_typed;

/* Dictionary functions */
Expand Down
1 change: 1 addition & 0 deletions core/extension/gdextension_interface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,7 @@ typedef struct {

GDExtensionVariantPtr (*array_operator_index)(GDExtensionTypePtr p_self, GDExtensionInt p_index); // p_self should be an Array ptr
GDExtensionVariantPtr (*array_operator_index_const)(GDExtensionConstTypePtr p_self, GDExtensionInt p_index); // p_self should be an Array ptr
void (*array_ref)(GDExtensionTypePtr p_self, GDExtensionConstTypePtr p_from); // p_self should be an Array ptr
void (*array_set_typed)(GDExtensionTypePtr p_self, uint32_t p_type, GDExtensionConstStringNamePtr p_class_name, GDExtensionConstVariantPtr p_script); // p_self should be an Array ptr

/* Dictionary functions */
Expand Down
137 changes: 77 additions & 60 deletions core/variant/array.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ void Array::_ref(const Array &p_from) const {

_unref();

_p = p_from._p;
_p = _fp;
}

void Array::_unref() const {
Expand Down Expand Up @@ -191,62 +191,73 @@ uint32_t Array::recursive_hash(int recursion_count) const {
return hash_fmix32(h);
}

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;
vonagam marked this conversation as resolved.
Show resolved Hide resolved
void Array::operator=(const Array &p_array) {
if (this == &p_array) {
return;
}
_ref(p_array);
}

void Array::assign(const Array &p_array) {
const ContainerTypeValidate &typed = _p->typed;
const ContainerTypeValidate &source_typed = p_array._p->typed;

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
if (typed == source_typed || typed.type == Variant::NIL || (source_typed.type == Variant::OBJECT && typed.can_reference(source_typed))) {
// from same to same or
// from anything to variants or
// from subclasses to base classes
_p->array = p_array._p->array;
} 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++) {
const Variant &element = p_array._p->array[i];
if (element.get_type() != Variant::OBJECT || !_p->typed.validate_object(element, "assign")) {
return false;
}
}
_p->array = p_array._p->array; //then just copy, which is cheap anyway
return;
}

} else {
//for non objects, we need to check if there is a valid conversion, which needs to happen one by one, so this is the worst case.
Vector<Variant> new_array;
new_array.resize(p_array._p->array.size());
for (int i = 0; i < p_array._p->array.size(); i++) {
Variant src_val = p_array._p->array[i];
if (src_val.get_type() == _p->typed.type) {
new_array.write[i] = src_val;
} else if (Variant::can_convert_strict(src_val.get_type(), _p->typed.type)) {
Variant *ptr = &src_val;
Callable::CallError ce;
Variant::construct(_p->typed.type, new_array.write[i], (const Variant **)&ptr, 1, ce);
if (ce.error != Callable::CallError::CALL_OK) {
ERR_FAIL_V_MSG(false, "Unable to convert array index " + itos(i) + " from '" + Variant::get_type_name(src_val.get_type()) + "' to '" + Variant::get_type_name(_p->typed.type) + "'.");
}
} else {
ERR_FAIL_V_MSG(false, "Unable to convert array index " + itos(i) + " from '" + Variant::get_type_name(src_val.get_type()) + "' to '" + Variant::get_type_name(_p->typed.type) + "'.");
}
const Variant *source = p_array._p->array.ptr();
int size = p_array._p->array.size();

if ((source_typed.type == Variant::NIL && typed.type == Variant::OBJECT) || (source_typed.type == Variant::OBJECT && source_typed.can_reference(typed))) {
// from variants to objects or
// from base classes to subclasses
for (int i = 0; i < size; i++) {
const Variant &element = source[i];
if (element.get_type() != Variant::NIL && (element.get_type() != Variant::OBJECT || !typed.validate_object(element, "assign"))) {
ERR_FAIL_MSG(vformat(R"(Unable to convert array index %i from "%s" to "%s".)", i, Variant::get_type_name(element.get_type()), Variant::get_type_name(typed.type)));
}
}
_p->array = p_array._p->array;
return;
}

_p->array = new_array;
Vector<Variant> array;
array.resize(size);
Variant *data = array.ptrw();

if (source_typed.type == Variant::NIL && typed.type != Variant::OBJECT) {
// from variants to primitives
for (int i = 0; i < size; i++) {
const Variant *value = source + i;
if (value->get_type() == typed.type) {
data[i] = *value;
continue;
}
if (!Variant::can_convert_strict(value->get_type(), typed.type)) {
ERR_FAIL_MSG("Unable to convert array index " + itos(i) + " from '" + Variant::get_type_name(value->get_type()) + "' to '" + Variant::get_type_name(typed.type) + "'.");
}
Callable::CallError ce;
Variant::construct(typed.type, data[i], &value, 1, ce);
ERR_FAIL_COND_MSG(ce.error, vformat(R"(Unable to convert array index %i from "%s" to "%s".)", i, Variant::get_type_name(value->get_type()), Variant::get_type_name(typed.type)));
}
} else if (Variant::can_convert_strict(source_typed.type, typed.type)) {
// from primitives to different convertable primitives
for (int i = 0; i < size; i++) {
const Variant *value = source + i;
Callable::CallError ce;
Variant::construct(typed.type, data[i], &value, 1, ce);
ERR_FAIL_COND_MSG(ce.error, vformat(R"(Unable to convert array index %i from "%s" to "%s".)", i, Variant::get_type_name(value->get_type()), Variant::get_type_name(typed.type)));
}
} else if (_p->typed.can_reference(p_array._p->typed)) { //same type or compatible
_ref(p_array);
} else {
ERR_FAIL_V_MSG(false, "Assignment of arrays of incompatible types.");
ERR_FAIL_MSG(vformat(R"(Cannot assign contents of "Array[%s]" to "Array[%s]".)", Variant::get_type_name(source_typed.type), Variant::get_type_name(typed.type)));
}
return true;
}

void Array::operator=(const Array &p_array) {
if (this == &p_array) {
return;
}
_ref(p_array);
_p->array = array;
}

void Array::push_back(const Variant &p_value) {
Expand All @@ -269,7 +280,15 @@ void Array::append_array(const Array &p_array) {

Error Array::resize(int p_new_size) {
ERR_FAIL_COND_V_MSG(_p->read_only, ERR_LOCKED, "Array is in read-only state.");
return _p->array.resize(p_new_size);
Variant::Type &variant_type = _p->typed.type;
int old_size = _p->array.size();
Error err = _p->array.resize_zeroed(p_new_size);
if (!err && variant_type != Variant::NIL && variant_type != Variant::OBJECT) {
for (int i = old_size; i < p_new_size; i++) {
VariantInternal::initialize(&_p->array.write[i], variant_type);
}
}
return err;
}

Error Array::insert(int p_pos, const Variant &p_value) {
Expand Down Expand Up @@ -403,24 +422,22 @@ Array Array::duplicate(bool p_deep) const {

Array Array::recursive_duplicate(bool p_deep, int recursion_count) const {
Array new_arr;
new_arr._p->typed = _p->typed;

if (recursion_count > MAX_RECURSION) {
ERR_PRINT("Max recursion reached");
return new_arr;
}

int element_count = size();
new_arr.resize(element_count);
new_arr._p->typed = _p->typed;
if (p_deep) {
recursion_count++;
int element_count = size();
new_arr.resize(element_count);
for (int i = 0; i < element_count; i++) {
new_arr[i] = get(i).recursive_duplicate(true, recursion_count);
}
} else {
for (int i = 0; i < element_count; i++) {
new_arr[i] = get(i);
}
new_arr._p->array = _p->array;
}

return new_arr;
Expand Down Expand Up @@ -737,11 +754,7 @@ Array::Array(const Array &p_from, uint32_t p_type, const StringName &p_class_nam
_p = memnew(ArrayPrivate);
_p->refcount.init();
set_typed(p_type, p_class_name, p_script);
_assign(p_from);
}

bool Array::typed_assign(const Array &p_other) {
return _assign(p_other);
assign(p_from);
}

void Array::set_typed(uint32_t p_type, const StringName &p_class_name, const Variant &p_script) {
Expand All @@ -763,6 +776,10 @@ bool Array::is_typed() const {
return _p->typed.type != Variant::NIL;
}

bool Array::is_same_typed(const Array &p_other) const {
return _p->typed == p_other._p->typed;
}

uint32_t Array::get_typed_builtin() const {
return _p->typed.type;
}
Expand Down
9 changes: 4 additions & 5 deletions core/variant/array.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,11 @@ class Callable;

class Array {
mutable ArrayPrivate *_p;
void _ref(const Array &p_from) const;
void _unref() const;

protected:
bool _assign(const Array &p_array);

public:
void _ref(const Array &p_from) const;

Variant &operator[](int p_idx);
const Variant &operator[](int p_idx) const;

Expand All @@ -68,6 +66,7 @@ class Array {
uint32_t recursive_hash(int recursion_count) const;
void operator=(const Array &p_array);

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
void append_array(const Array &p_array);
Expand Down Expand Up @@ -120,9 +119,9 @@ class Array {

const void *id() const;

bool typed_assign(const Array &p_other);
void set_typed(uint32_t p_type, const StringName &p_class_name, const Variant &p_script);
bool is_typed() const;
bool is_same_typed(const Array &p_other) const;
uint32_t get_typed_builtin() const;
StringName get_typed_class_name() const;
Variant get_typed_script() const;
Expand Down
11 changes: 9 additions & 2 deletions core/variant/container_type_validate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,15 @@ struct ContainerTypeValidate {
return true;
}

_FORCE_INLINE_ bool operator==(const ContainerTypeValidate &p_type) const {
return type == p_type.type && class_name == p_type.class_name && script == p_type.script;
}
_FORCE_INLINE_ bool operator!=(const ContainerTypeValidate &p_type) const {
return type != p_type.type || class_name != p_type.class_name || script != p_type.script;
}

// Coerces String and StringName into each other when needed.
_FORCE_INLINE_ bool validate(Variant &inout_variant, const char *p_operation = "use") {
_FORCE_INLINE_ bool validate(Variant &inout_variant, const char *p_operation = "use") const {
if (type == Variant::NIL) {
return true;
}
Expand All @@ -102,7 +109,7 @@ struct ContainerTypeValidate {
return validate_object(inout_variant, p_operation);
}

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

#ifdef DEBUG_ENABLED
Expand Down
Loading