From 7752a0d8d13e1052e6cb0f0199bd6cbb20e3abe8 Mon Sep 17 00:00:00 2001 From: Ninni Pipping Date: Thu, 6 Jul 2023 15:03:17 +0200 Subject: [PATCH] Fix range error for `Array.slice` --- core/variant/array.cpp | 12 ++++++++---- doc/classes/Array.xml | 1 + tests/core/variant/test_array.h | 28 +++++++++++++++++++++++----- 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/core/variant/array.cpp b/core/variant/array.cpp index 5215142dd37..5a0ded6c014 100644 --- a/core/variant/array.cpp +++ b/core/variant/array.cpp @@ -454,17 +454,21 @@ Array Array::slice(int p_begin, int p_end, int p_step, bool p_deep) const { const int s = size(); - int begin = CLAMP(p_begin, -s, s); + if (s == 0 || (p_begin < -s && p_step < 0) || (p_begin >= s && p_step > 0)) { + return result; + } + + int begin = CLAMP(p_begin, -s, s - 1); if (begin < 0) { begin += s; } - int end = CLAMP(p_end, -s, s); + int end = CLAMP(p_end, -s - 1, s); if (end < 0) { end += s; } - 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 && begin > end, result, "Slice step is positive, but bounds are decreasing."); + ERR_FAIL_COND_V_MSG(p_step < 0 && begin < end, result, "Slice step is negative, but bounds are increasing."); int result_size = (end - begin) / p_step + (((end - begin) % p_step != 0) ? 1 : 0); result.resize(result_size); diff --git a/doc/classes/Array.xml b/doc/classes/Array.xml index f62c44aa0df..999cc6fa299 100644 --- a/doc/classes/Array.xml +++ b/doc/classes/Array.xml @@ -584,6 +584,7 @@ If either [param begin] or [param end] 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, [param step] is the relative index between source elements. It can be negative, then [param begin] must be higher than [param end]. For example, [code][0, 1, 2, 3, 4, 5].slice(5, 1, -2)[/code] returns [code][5, 3][/code]. If [param deep] is true, each element will be copied by value rather than by reference. + [b]Note:[/b] To include the first element when [param step] is negative, use [code]arr.slice(begin, -arr.size() - 1, step)[/code] (i.e. [code][0, 1, 2].slice(1, -4, -1)[/code] returns [code][1, 0][/code]). diff --git a/tests/core/variant/test_array.h b/tests/core/variant/test_array.h index ccb02ed5fac..228d77b3b50 100644 --- a/tests/core/variant/test_array.h +++ b/tests/core/variant/test_array.h @@ -304,13 +304,31 @@ TEST_CASE("[Array] slice()") { CHECK(slice8[1] == Variant(3)); CHECK(slice8[2] == Variant(1)); - ERR_PRINT_OFF; - Array slice9 = array.slice(4, 1); - CHECK(slice9.size() == 0); + Array slice9 = array.slice(10, 0, -2); + CHECK(slice9.size() == 3); + CHECK(slice9[0] == Variant(5)); + CHECK(slice9[1] == Variant(3)); + CHECK(slice9[2] == Variant(1)); - Array slice10 = array.slice(3, -4); - CHECK(slice10.size() == 0); + Array slice10 = array.slice(2, -10, -1); + CHECK(slice10.size() == 3); + CHECK(slice10[0] == Variant(2)); + CHECK(slice10[1] == Variant(1)); + CHECK(slice10[2] == Variant(0)); + + ERR_PRINT_OFF; + Array slice11 = array.slice(4, 1); + CHECK(slice11.size() == 0); + + Array slice12 = array.slice(3, -4); + CHECK(slice12.size() == 0); ERR_PRINT_ON; + + Array slice13 = Array().slice(1); + CHECK(slice13.size() == 0); + + Array slice14 = array.slice(6); + CHECK(slice14.size() == 0); } TEST_CASE("[Array] Duplicate array") {