Merge pull request #56668 from akien-mga/array-slice-nicer-bound-checks

This commit is contained in:
Rémi Verschelde 2022-01-18 13:22:35 +01:00 committed by GitHub
commit 9912492e93
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
16 changed files with 129 additions and 52 deletions

View file

@ -42,6 +42,7 @@
#include "core/templates/search_array.h"
#include "core/templates/sort_array.h"
#include <climits>
#include <initializer_list>
template <class T>
@ -144,25 +145,29 @@ public:
return ret;
}
Vector<T> slice(int p_begin, int p_end) const {
Vector<T> slice(int p_begin, int p_end = INT_MAX) const {
Vector<T> result;
if (p_end < 0) {
p_end += size() + 1;
const int s = size();
int begin = CLAMP(p_begin, -s, s);
if (begin < 0) {
begin += s;
}
int end = CLAMP(p_end, -s, s);
if (end < 0) {
end += s;
}
ERR_FAIL_INDEX_V(p_begin, size(), result);
ERR_FAIL_INDEX_V(p_end, size() + 1, result);
ERR_FAIL_COND_V(begin > end, result);
ERR_FAIL_COND_V(p_begin > p_end, result);
int result_size = p_end - p_begin;
int result_size = end - begin;
result.resize(result_size);
const T *const r = ptr();
T *const w = result.ptrw();
for (int i = 0; i < result_size; ++i) {
w[i] = r[p_begin + i];
w[i] = r[begin + i];
}
return result;

View file

@ -370,20 +370,24 @@ Array Array::slice(int p_begin, int p_end, int p_step, bool p_deep) const {
ERR_FAIL_COND_V_MSG(p_step == 0, result, "Slice step cannot be zero.");
if (p_end < 0) {
p_end += size() + 1;
const int s = size();
int begin = CLAMP(p_begin, -s, s);
if (begin < 0) {
begin += s;
}
int end = CLAMP(p_end, -s, s);
if (end < 0) {
end += s;
}
ERR_FAIL_INDEX_V(p_begin, size(), result);
ERR_FAIL_INDEX_V(p_end, size() + 1, result);
ERR_FAIL_COND_V_MSG(p_step > 0 && begin > end, result, "Slice is positive, but bounds is decreasing.");
ERR_FAIL_COND_V_MSG(p_step < 0 && begin < end, result, "Slice is negative, but bounds is increasing.");
ERR_FAIL_COND_V_MSG(p_step > 0 && p_begin > p_end, result, "Slice is positive, but bounds is decreasing");
ERR_FAIL_COND_V_MSG(p_step < 0 && p_begin < p_end, result, "Slice is negative, but bounds is increasing");
int result_size = (p_end - p_begin) / p_step;
int result_size = (end - begin) / p_step;
result.resize(result_size);
for (int src_idx = p_begin, dest_idx = 0; dest_idx < result_size; ++dest_idx) {
for (int src_idx = begin, dest_idx = 0; dest_idx < result_size; ++dest_idx) {
result[dest_idx] = p_deep ? get(src_idx).duplicate(true) : get(src_idx);
src_idx += p_step;
}

View file

@ -33,6 +33,8 @@
#include "core/typedefs.h"
#include <climits>
class Variant;
class ArrayPrivate;
class Object;
@ -102,7 +104,7 @@ public:
Array duplicate(bool p_deep = false) const;
Array recursive_duplicate(bool p_deep, int recursion_count) const;
Array slice(int p_begin, int p_end, int p_step = 1, bool p_deep = false) const;
Array slice(int p_begin, int p_end = INT_MAX, int p_step = 1, bool p_deep = false) const;
Array filter(const Callable &p_callable) const;
Array map(const Callable &p_callable) const;
Variant reduce(const Callable &p_callable, const Variant &p_accum) const;

View file

@ -1838,7 +1838,7 @@ static void _register_variant_builtin_methods() {
bind_method(Array, bsearch_custom, sarray("value", "func", "before"), varray(true));
bind_method(Array, reverse, sarray(), varray());
bind_method(Array, duplicate, sarray("deep"), varray(false));
bind_method(Array, slice, sarray("begin", "end", "step", "deep"), varray(1, false));
bind_method(Array, slice, sarray("begin", "end", "step", "deep"), varray(INT_MAX, 1, false));
bind_method(Array, filter, sarray("method"), varray());
bind_method(Array, map, sarray("method"), varray());
bind_method(Array, reduce, sarray("method", "accum"), varray(Variant()));
@ -1858,7 +1858,7 @@ static void _register_variant_builtin_methods() {
bind_method(PackedByteArray, resize, sarray("new_size"), varray());
bind_method(PackedByteArray, has, sarray("value"), varray());
bind_method(PackedByteArray, reverse, sarray(), varray());
bind_method(PackedByteArray, slice, sarray("begin", "end"), varray());
bind_method(PackedByteArray, slice, sarray("begin", "end"), varray(INT_MAX));
bind_method(PackedByteArray, sort, sarray(), varray());
bind_method(PackedByteArray, bsearch, sarray("value", "before"), varray(true));
bind_method(PackedByteArray, duplicate, sarray(), varray());
@ -1919,7 +1919,7 @@ static void _register_variant_builtin_methods() {
bind_method(PackedInt32Array, resize, sarray("new_size"), varray());
bind_method(PackedInt32Array, has, sarray("value"), varray());
bind_method(PackedInt32Array, reverse, sarray(), varray());
bind_method(PackedInt32Array, slice, sarray("begin", "end"), varray());
bind_method(PackedInt32Array, slice, sarray("begin", "end"), varray(INT_MAX));
bind_method(PackedInt32Array, to_byte_array, sarray(), varray());
bind_method(PackedInt32Array, sort, sarray(), varray());
bind_method(PackedInt32Array, bsearch, sarray("value", "before"), varray(true));
@ -1939,7 +1939,7 @@ static void _register_variant_builtin_methods() {
bind_method(PackedInt64Array, resize, sarray("new_size"), varray());
bind_method(PackedInt64Array, has, sarray("value"), varray());
bind_method(PackedInt64Array, reverse, sarray(), varray());
bind_method(PackedInt64Array, slice, sarray("begin", "end"), varray());
bind_method(PackedInt64Array, slice, sarray("begin", "end"), varray(INT_MAX));
bind_method(PackedInt64Array, to_byte_array, sarray(), varray());
bind_method(PackedInt64Array, sort, sarray(), varray());
bind_method(PackedInt64Array, bsearch, sarray("value", "before"), varray(true));
@ -1959,7 +1959,7 @@ static void _register_variant_builtin_methods() {
bind_method(PackedFloat32Array, resize, sarray("new_size"), varray());
bind_method(PackedFloat32Array, has, sarray("value"), varray());
bind_method(PackedFloat32Array, reverse, sarray(), varray());
bind_method(PackedFloat32Array, slice, sarray("begin", "end"), varray());
bind_method(PackedFloat32Array, slice, sarray("begin", "end"), varray(INT_MAX));
bind_method(PackedFloat32Array, to_byte_array, sarray(), varray());
bind_method(PackedFloat32Array, sort, sarray(), varray());
bind_method(PackedFloat32Array, bsearch, sarray("value", "before"), varray(true));
@ -1979,7 +1979,7 @@ static void _register_variant_builtin_methods() {
bind_method(PackedFloat64Array, resize, sarray("new_size"), varray());
bind_method(PackedFloat64Array, has, sarray("value"), varray());
bind_method(PackedFloat64Array, reverse, sarray(), varray());
bind_method(PackedFloat64Array, slice, sarray("begin", "end"), varray());
bind_method(PackedFloat64Array, slice, sarray("begin", "end"), varray(INT_MAX));
bind_method(PackedFloat64Array, to_byte_array, sarray(), varray());
bind_method(PackedFloat64Array, sort, sarray(), varray());
bind_method(PackedFloat64Array, bsearch, sarray("value", "before"), varray(true));
@ -1999,7 +1999,7 @@ static void _register_variant_builtin_methods() {
bind_method(PackedStringArray, resize, sarray("new_size"), varray());
bind_method(PackedStringArray, has, sarray("value"), varray());
bind_method(PackedStringArray, reverse, sarray(), varray());
bind_method(PackedStringArray, slice, sarray("begin", "end"), varray());
bind_method(PackedStringArray, slice, sarray("begin", "end"), varray(INT_MAX));
bind_method(PackedStringArray, to_byte_array, sarray(), varray());
bind_method(PackedStringArray, sort, sarray(), varray());
bind_method(PackedStringArray, bsearch, sarray("value", "before"), varray(true));
@ -2019,7 +2019,7 @@ static void _register_variant_builtin_methods() {
bind_method(PackedVector2Array, resize, sarray("new_size"), varray());
bind_method(PackedVector2Array, has, sarray("value"), varray());
bind_method(PackedVector2Array, reverse, sarray(), varray());
bind_method(PackedVector2Array, slice, sarray("begin", "end"), varray());
bind_method(PackedVector2Array, slice, sarray("begin", "end"), varray(INT_MAX));
bind_method(PackedVector2Array, to_byte_array, sarray(), varray());
bind_method(PackedVector2Array, sort, sarray(), varray());
bind_method(PackedVector2Array, bsearch, sarray("value", "before"), varray(true));
@ -2039,7 +2039,7 @@ static void _register_variant_builtin_methods() {
bind_method(PackedVector3Array, resize, sarray("new_size"), varray());
bind_method(PackedVector3Array, has, sarray("value"), varray());
bind_method(PackedVector3Array, reverse, sarray(), varray());
bind_method(PackedVector3Array, slice, sarray("begin", "end"), varray());
bind_method(PackedVector3Array, slice, sarray("begin", "end"), varray(INT_MAX));
bind_method(PackedVector3Array, to_byte_array, sarray(), varray());
bind_method(PackedVector3Array, sort, sarray(), varray());
bind_method(PackedVector3Array, bsearch, sarray("value", "before"), varray(true));
@ -2059,7 +2059,7 @@ static void _register_variant_builtin_methods() {
bind_method(PackedColorArray, resize, sarray("new_size"), varray());
bind_method(PackedColorArray, has, sarray("value"), varray());
bind_method(PackedColorArray, reverse, sarray(), varray());
bind_method(PackedColorArray, slice, sarray("begin", "end"), varray());
bind_method(PackedColorArray, slice, sarray("begin", "end"), varray(INT_MAX));
bind_method(PackedColorArray, to_byte_array, sarray(), varray());
bind_method(PackedColorArray, sort, sarray(), varray());
bind_method(PackedColorArray, bsearch, sarray("value", "before"), varray(true));

View file

@ -445,13 +445,14 @@
<method name="slice" qualifiers="const">
<return type="Array" />
<argument index="0" name="begin" type="int" />
<argument index="1" name="end" type="int" />
<argument index="1" name="end" type="int" default="2147483647" />
<argument index="2" name="step" type="int" default="1" />
<argument index="3" name="deep" type="bool" default="false" />
<description>
Returns the slice of the [Array], from [code]begin[/code] (inclusive) to [code]end[/code] (exclusive), as a new [Array].
If [code]end[/code] is negative, it will be relative to the end of the array.
If specified, [code]step[/code] is the relative index between source elements.
The absolute value of [code]begin[/code] and [code]end[/code] will be clamped to the array size, so the default value for [code]end[/code] makes it slice to the size of the array by default (i.e. [code]arr.slice(1)[/code] is a shorthand for [code]arr.slice(1, arr.size())[/code]).
If either [code]begin[/code] or [code]end[/code] are negative, they will be relative to the end of the array (i.e. [code]arr.slice(0, -2)[/code] is a shorthand for [code]arr.slice(0, arr.size() - 2)[/code]).
If specified, [code]step[/code] is the relative index between source elements. It can be negative, then [code]begin[/code] must be higher than [code]end[/code]. For example, [code][0, 1, 2, 3, 4, 5].slice(5, 1, -2)[/code] returns [code][5, 3][/code]).
If [code]deep[/code] is true, each element will be copied by value rather than by reference.
</description>
</method>

View file

@ -369,10 +369,11 @@
<method name="slice" qualifiers="const">
<return type="PackedByteArray" />
<argument index="0" name="begin" type="int" />
<argument index="1" name="end" type="int" />
<argument index="1" name="end" type="int" default="2147483647" />
<description>
Returns the slice of the [PackedByteArray], from [code]begin[/code] (inclusive) to [code]end[/code] (exclusive), as a new [PackedByteArray].
If [code]end[/code]is negative, it will be relative to the end of the array.
The absolute value of [code]begin[/code] and [code]end[/code] will be clamped to the array size, so the default value for [code]end[/code] makes it slice to the size of the array by default (i.e. [code]arr.slice(1)[/code] is a shorthand for [code]arr.slice(1, arr.size())[/code]).
If either [code]begin[/code] or [code]end[/code] are negative, they will be relative to the end of the array (i.e. [code]arr.slice(0, -2)[/code] is a shorthand for [code]arr.slice(0, arr.size() - 2)[/code]).
</description>
</method>
<method name="sort">

View file

@ -132,8 +132,11 @@
<method name="slice" qualifiers="const">
<return type="PackedColorArray" />
<argument index="0" name="begin" type="int" />
<argument index="1" name="end" type="int" />
<argument index="1" name="end" type="int" default="2147483647" />
<description>
Returns the slice of the [PackedColorArray], from [code]begin[/code] (inclusive) to [code]end[/code] (exclusive), as a new [PackedColorArray].
The absolute value of [code]begin[/code] and [code]end[/code] will be clamped to the array size, so the default value for [code]end[/code] makes it slice to the size of the array by default (i.e. [code]arr.slice(1)[/code] is a shorthand for [code]arr.slice(1, arr.size())[/code]).
If either [code]begin[/code] or [code]end[/code] are negative, they will be relative to the end of the array (i.e. [code]arr.slice(0, -2)[/code] is a shorthand for [code]arr.slice(0, arr.size() - 2)[/code]).
</description>
</method>
<method name="sort">

View file

@ -133,8 +133,11 @@
<method name="slice" qualifiers="const">
<return type="PackedFloat32Array" />
<argument index="0" name="begin" type="int" />
<argument index="1" name="end" type="int" />
<argument index="1" name="end" type="int" default="2147483647" />
<description>
Returns the slice of the [PackedFloat32Array], from [code]begin[/code] (inclusive) to [code]end[/code] (exclusive), as a new [PackedFloat32Array].
The absolute value of [code]begin[/code] and [code]end[/code] will be clamped to the array size, so the default value for [code]end[/code] makes it slice to the size of the array by default (i.e. [code]arr.slice(1)[/code] is a shorthand for [code]arr.slice(1, arr.size())[/code]).
If either [code]begin[/code] or [code]end[/code] are negative, they will be relative to the end of the array (i.e. [code]arr.slice(0, -2)[/code] is a shorthand for [code]arr.slice(0, arr.size() - 2)[/code]).
</description>
</method>
<method name="sort">

View file

@ -133,8 +133,11 @@
<method name="slice" qualifiers="const">
<return type="PackedFloat64Array" />
<argument index="0" name="begin" type="int" />
<argument index="1" name="end" type="int" />
<argument index="1" name="end" type="int" default="2147483647" />
<description>
Returns the slice of the [PackedFloat64Array], from [code]begin[/code] (inclusive) to [code]end[/code] (exclusive), as a new [PackedFloat64Array].
The absolute value of [code]begin[/code] and [code]end[/code] will be clamped to the array size, so the default value for [code]end[/code] makes it slice to the size of the array by default (i.e. [code]arr.slice(1)[/code] is a shorthand for [code]arr.slice(1, arr.size())[/code]).
If either [code]begin[/code] or [code]end[/code] are negative, they will be relative to the end of the array (i.e. [code]arr.slice(0, -2)[/code] is a shorthand for [code]arr.slice(0, arr.size() - 2)[/code]).
</description>
</method>
<method name="sort">

View file

@ -133,8 +133,11 @@
<method name="slice" qualifiers="const">
<return type="PackedInt32Array" />
<argument index="0" name="begin" type="int" />
<argument index="1" name="end" type="int" />
<argument index="1" name="end" type="int" default="2147483647" />
<description>
Returns the slice of the [PackedInt32Array], from [code]begin[/code] (inclusive) to [code]end[/code] (exclusive), as a new [PackedInt32Array].
The absolute value of [code]begin[/code] and [code]end[/code] will be clamped to the array size, so the default value for [code]end[/code] makes it slice to the size of the array by default (i.e. [code]arr.slice(1)[/code] is a shorthand for [code]arr.slice(1, arr.size())[/code]).
If either [code]begin[/code] or [code]end[/code] are negative, they will be relative to the end of the array (i.e. [code]arr.slice(0, -2)[/code] is a shorthand for [code]arr.slice(0, arr.size() - 2)[/code]).
</description>
</method>
<method name="sort">

View file

@ -133,8 +133,11 @@
<method name="slice" qualifiers="const">
<return type="PackedInt64Array" />
<argument index="0" name="begin" type="int" />
<argument index="1" name="end" type="int" />
<argument index="1" name="end" type="int" default="2147483647" />
<description>
Returns the slice of the [PackedInt64Array], from [code]begin[/code] (inclusive) to [code]end[/code] (exclusive), as a new [PackedInt64Array].
The absolute value of [code]begin[/code] and [code]end[/code] will be clamped to the array size, so the default value for [code]end[/code] makes it slice to the size of the array by default (i.e. [code]arr.slice(1)[/code] is a shorthand for [code]arr.slice(1, arr.size())[/code]).
If either [code]begin[/code] or [code]end[/code] are negative, they will be relative to the end of the array (i.e. [code]arr.slice(0, -2)[/code] is a shorthand for [code]arr.slice(0, arr.size() - 2)[/code]).
</description>
</method>
<method name="sort">

View file

@ -133,8 +133,11 @@
<method name="slice" qualifiers="const">
<return type="PackedStringArray" />
<argument index="0" name="begin" type="int" />
<argument index="1" name="end" type="int" />
<argument index="1" name="end" type="int" default="2147483647" />
<description>
Returns the slice of the [PackedStringArray], from [code]begin[/code] (inclusive) to [code]end[/code] (exclusive), as a new [PackedStringArray].
The absolute value of [code]begin[/code] and [code]end[/code] will be clamped to the array size, so the default value for [code]end[/code] makes it slice to the size of the array by default (i.e. [code]arr.slice(1)[/code] is a shorthand for [code]arr.slice(1, arr.size())[/code]).
If either [code]begin[/code] or [code]end[/code] are negative, they will be relative to the end of the array (i.e. [code]arr.slice(0, -2)[/code] is a shorthand for [code]arr.slice(0, arr.size() - 2)[/code]).
</description>
</method>
<method name="sort">

View file

@ -133,8 +133,11 @@
<method name="slice" qualifiers="const">
<return type="PackedVector2Array" />
<argument index="0" name="begin" type="int" />
<argument index="1" name="end" type="int" />
<argument index="1" name="end" type="int" default="2147483647" />
<description>
Returns the slice of the [PackedVector2Array], from [code]begin[/code] (inclusive) to [code]end[/code] (exclusive), as a new [PackedVector2Array].
The absolute value of [code]begin[/code] and [code]end[/code] will be clamped to the array size, so the default value for [code]end[/code] makes it slice to the size of the array by default (i.e. [code]arr.slice(1)[/code] is a shorthand for [code]arr.slice(1, arr.size())[/code]).
If either [code]begin[/code] or [code]end[/code] are negative, they will be relative to the end of the array (i.e. [code]arr.slice(0, -2)[/code] is a shorthand for [code]arr.slice(0, arr.size() - 2)[/code]).
</description>
</method>
<method name="sort">

View file

@ -132,8 +132,11 @@
<method name="slice" qualifiers="const">
<return type="PackedVector3Array" />
<argument index="0" name="begin" type="int" />
<argument index="1" name="end" type="int" />
<argument index="1" name="end" type="int" default="2147483647" />
<description>
Returns the slice of the [PackedVector3Array], from [code]begin[/code] (inclusive) to [code]end[/code] (exclusive), as a new [PackedVector3Array].
The absolute value of [code]begin[/code] and [code]end[/code] will be clamped to the array size, so the default value for [code]end[/code] makes it slice to the size of the array by default (i.e. [code]arr.slice(1)[/code] is a shorthand for [code]arr.slice(1, arr.size())[/code]).
If either [code]begin[/code] or [code]end[/code] are negative, they will be relative to the end of the array (i.e. [code]arr.slice(0, -2)[/code] is a shorthand for [code]arr.slice(0, arr.size() - 2)[/code]).
</description>
</method>
<method name="sort">

View file

@ -257,27 +257,42 @@ TEST_CASE("[Vector] Slice") {
vector.push_back(3);
vector.push_back(4);
Vector<int> slice0 = vector.slice(0, 0);
CHECK(slice0.size() == 0);
Vector<int> slice1 = vector.slice(1, 3);
CHECK(slice1.size() == 2);
CHECK(slice1[0] == 1);
CHECK(slice1[1] == 2);
Vector<int> slice2 = vector.slice(1, -1);
CHECK(slice2.size() == 4);
CHECK(slice2.size() == 3);
CHECK(slice2[0] == 1);
CHECK(slice2[1] == 2);
CHECK(slice2[2] == 3);
CHECK(slice2[3] == 4);
Vector<int> slice3 = vector.slice(3, -1);
Vector<int> slice3 = vector.slice(3);
CHECK(slice3.size() == 2);
CHECK(slice3[0] == 3);
CHECK(slice3[1] == 4);
Vector<int> slice4 = vector.slice(2, -2);
CHECK(slice4.size() == 2);
CHECK(slice4.size() == 1);
CHECK(slice4[0] == 2);
CHECK(slice4[1] == 3);
Vector<int> slice5 = vector.slice(-2);
CHECK(slice5.size() == 2);
CHECK(slice5[0] == 3);
CHECK(slice5[1] == 4);
Vector<int> slice6 = vector.slice(2, 42);
CHECK(slice6.size() == 3);
CHECK(slice6[0] == 2);
CHECK(slice6[1] == 3);
CHECK(slice6[2] == 4);
Vector<int> slice7 = vector.slice(5, 1);
CHECK(slice7.size() == 0);
}
TEST_CASE("[Vector] Find, has") {

View file

@ -254,27 +254,52 @@ TEST_CASE("[Array] slice()") {
array.push_back(3);
array.push_back(4);
Array slice0 = array.slice(0, 0);
CHECK(slice0.size() == 0);
Array slice1 = array.slice(1, 3);
CHECK(slice1.size() == 2);
CHECK(slice1[0] == Variant(1));
CHECK(slice1[1] == Variant(2));
Array slice2 = array.slice(1, -1);
CHECK(slice2.size() == 4);
CHECK(slice2.size() == 3);
CHECK(slice2[0] == Variant(1));
CHECK(slice2[1] == Variant(2));
CHECK(slice2[2] == Variant(3));
CHECK(slice2[3] == Variant(4));
Array slice3 = array.slice(3, -1);
Array slice3 = array.slice(3);
CHECK(slice3.size() == 2);
CHECK(slice3[0] == Variant(3));
CHECK(slice3[1] == Variant(4));
Array slice4 = array.slice(2, -2);
CHECK(slice4.size() == 2);
CHECK(slice4.size() == 1);
CHECK(slice4[0] == Variant(2));
CHECK(slice4[1] == Variant(3));
Array slice5 = array.slice(-2);
CHECK(slice5.size() == 2);
CHECK(slice5[0] == Variant(3));
CHECK(slice5[1] == Variant(4));
Array slice6 = array.slice(2, 42);
CHECK(slice6.size() == 3);
CHECK(slice6[0] == Variant(2));
CHECK(slice6[1] == Variant(3));
CHECK(slice6[2] == Variant(4));
Array slice7 = array.slice(4, 0, -2);
CHECK(slice7.size() == 2);
CHECK(slice7[0] == Variant(4));
CHECK(slice7[1] == Variant(2));
ERR_PRINT_OFF;
Array slice8 = array.slice(4, 1);
CHECK(slice8.size() == 0);
Array slice9 = array.slice(3, -4);
CHECK(slice9.size() == 0);
ERR_PRINT_ON;
}
TEST_CASE("[Array] Duplicate array") {