Skip to content

Commit 21b8906

Browse files
committed
Implement trait GodotRange, used to convert Rust ranges to values expected by Godot. Support negative indices for Array. Fix Array docs.
1 parent b0ba40e commit 21b8906

File tree

13 files changed

+275
-132
lines changed

13 files changed

+275
-132
lines changed

godot-core/src/builtin/collections/array.rs

Lines changed: 63 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
use std::cell::OnceCell;
99
use std::marker::PhantomData;
10+
use std::ops::RangeBounds;
1011
use std::{cmp, fmt};
1112

1213
use godot_ffi as sys;
@@ -15,6 +16,7 @@ use sys::{ffi_methods, interface_fn, GodotFfi};
1516
use crate::builtin::*;
1617
use crate::meta;
1718
use crate::meta::error::{ConvertError, FromGodotError, FromVariantError};
19+
use crate::meta::godot_range::GodotRange;
1820
use crate::meta::{
1921
element_godot_type_name, element_variant_type, ArrayElement, AsArg, ClassName, ElementType,
2022
ExtVariantType, FromGodot, GodotConvert, GodotFfiVariant, GodotType, PropertyHintInfo, RefArg,
@@ -526,57 +528,98 @@ impl<T: ArrayElement> Array<T> {
526528
result.with_cache(self)
527529
}
528530

529-
/// Returns a sub-range `begin..end`, as a new array.
531+
/// Returns a sub-range `begin..end` as a new `Array`.
530532
///
531533
/// The values of `begin` (inclusive) and `end` (exclusive) will be clamped to the array size.
532534
///
535+
/// If either `begin` or `end` are negative, their value is relative to the end of the array.
536+
///
537+
/// # Example
538+
/// ```no_run
539+
/// # use godot::builtin::array;
540+
/// assert_eq!(array![0, 1, 2, 3, 4, 5].subarray_shallow(-1..-5, None), array![5, 3]);
541+
/// ```
542+
///
543+
/// If `end` is not specified, the range spans through whole array.
544+
///
545+
/// # Example
546+
/// ```no_run
547+
/// # use godot::builtin::array;
548+
/// assert_eq!(array![0, 1, 2, 3, 4, 5].subarray_shallow(1.., None), array![1, 2, 3, 4, 5]);
549+
/// ```
550+
///
533551
/// If specified, `step` is the relative index between source elements. It can be negative,
534-
/// in which case `begin` must be higher than `end`. For example,
535-
/// `Array::from(&[0, 1, 2, 3, 4, 5]).slice(5, 1, -2)` returns `[5, 3]`.
552+
/// in which case `begin` must be higher than `end`.
553+
///
554+
/// # Example
555+
/// ```no_run
556+
/// # use godot::builtin::array;
557+
/// assert_eq!(array![0, 1, 2, 3, 4, 5].subarray_shallow(-1..-5, Some(-2)), array![5, 3]);
558+
/// ```
536559
///
537560
/// Array elements are copied to the slice, but any reference types (such as `Array`,
538561
/// `Dictionary` and `Object`) will still refer to the same value. To create a deep copy, use
539562
/// [`subarray_deep()`][Self::subarray_deep] instead.
540563
///
541564
/// _Godot equivalent: `slice`_
542565
#[doc(alias = "slice")]
543-
// TODO(v0.3): change to i32 like NodePath::slice/subpath() and support+test negative indices.
544-
pub fn subarray_shallow(&self, begin: usize, end: usize, step: Option<isize>) -> Self {
545-
self.subarray_impl(begin, end, step, false)
566+
pub fn subarray_shallow(&self, range: impl RangeBounds<i32>, step: Option<i32>) -> Self {
567+
self.subarray_impl(range, step, false)
546568
}
547569

548-
/// Returns a sub-range `begin..end`, as a new `Array`.
570+
/// Returns a sub-range `begin..end` as a new `Array`.
549571
///
550572
/// The values of `begin` (inclusive) and `end` (exclusive) will be clamped to the array size.
551573
///
574+
/// If either `begin` or `end` are negative, their value is relative to the end of the array.
575+
///
576+
/// # Example
577+
/// ```no_run
578+
/// # use godot::builtin::array;
579+
/// assert_eq!(array![0, 1, 2, 3, 4, 5].subarray_deep(-1..-5, None), array![5, 3]);
580+
/// ```
581+
///
582+
/// If `end` is not specified, the range spans through whole array.
583+
///
584+
/// # Example
585+
/// ```no_run
586+
/// # use godot::builtin::array;
587+
/// assert_eq!(array![0, 1, 2, 3, 4, 5].subarray_deep(1.., None), array![1, 2, 3, 4, 5]);
588+
/// ```
589+
///
552590
/// If specified, `step` is the relative index between source elements. It can be negative,
553-
/// in which case `begin` must be higher than `end`. For example,
554-
/// `Array::from(&[0, 1, 2, 3, 4, 5]).slice(5, 1, -2)` returns `[5, 3]`.
591+
/// in which case `begin` must be higher than `end`.
592+
///
593+
/// # Example
594+
/// ```no_run
595+
/// # use godot::builtin::array;
596+
/// assert_eq!(array![0, 1, 2, 3, 4, 5].subarray_deep(-1..-5, Some(-2)), array![5, 3]);
597+
/// ```
555598
///
556599
/// All nested arrays and dictionaries are duplicated and will not be shared with the original
557600
/// array. Note that any `Object`-derived elements will still be shallow copied. To create a
558601
/// shallow copy, use [`subarray_shallow()`][Self::subarray_shallow] instead.
559602
///
560603
/// _Godot equivalent: `slice`_
561604
#[doc(alias = "slice")]
562-
// TODO(v0.3): change to i32 like NodePath::slice/subpath() and support+test negative indices.
563-
pub fn subarray_deep(&self, begin: usize, end: usize, step: Option<isize>) -> Self {
564-
self.subarray_impl(begin, end, step, true)
605+
pub fn subarray_deep(&self, range: impl RangeBounds<i32>, step: Option<i32>) -> Self {
606+
self.subarray_impl(range, step, true)
565607
}
566608

567-
fn subarray_impl(&self, begin: usize, end: usize, step: Option<isize>, deep: bool) -> Self {
609+
// Note: Godot will clamp values by itself.
610+
fn subarray_impl(&self, range: impl GodotRange<i32>, step: Option<i32>, deep: bool) -> Self {
568611
assert_ne!(step, Some(0), "subarray: step cannot be zero");
569612

570-
let len = self.len();
571-
let begin = begin.min(len);
572-
let end = end.min(len);
573613
let step = step.unwrap_or(1);
614+
let (begin, end) = range.to_godot_range_fromto();
615+
616+
// Unbounded upper bounds are represented by `i32::MAX` instead of `i64::MAX`,
617+
// since Godot treats some indexes as 32-bit despite being declared `i64` in GDExtension API.
618+
let end = end.unwrap_or(i32::MAX as i64);
574619

575620
// SAFETY: The type of the array is `T` and we convert the returned array to an `Array<T>` immediately.
576-
let subarray: VariantArray = unsafe {
577-
self.as_inner()
578-
.slice(to_i64(begin), to_i64(end), step.try_into().unwrap(), deep)
579-
};
621+
let subarray: VariantArray =
622+
unsafe { self.as_inner().slice(begin, end, step as i64, deep) };
580623

581624
// SAFETY: slice() returns a typed array with the same type as Self.
582625
let result = unsafe { subarray.assume_type() };

godot-core/src/builtin/collections/packed_array.rs

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#![allow(clippy::result_unit_err)]
1111

1212
use std::iter::FromIterator;
13+
use std::ops::RangeBounds;
1314
use std::{fmt, ops, ptr};
1415

1516
use godot_ffi as sys;
@@ -226,18 +227,33 @@ impl<T: PackedArrayElement> PackedArray<T> {
226227

227228
/// Returns a sub-range `begin..end`, as a new packed array.
228229
///
229-
/// This method is called `slice()` in Godot.
230230
/// The values of `begin` (inclusive) and `end` (exclusive) will be clamped to the array size.
231231
///
232+
/// If either `begin` or `end` are negative, their value is relative to the end of the array.
233+
///
234+
/// # Example
235+
/// ```no_run
236+
/// # use godot::builtin::PackedArray;
237+
/// let array = PackedArray::from([10, 20, 30, 40, 50]);
238+
/// assert_eq!(array.subarray(-4..-2), PackedArray::from([20, 30]));
239+
/// ```
240+
///
241+
/// If `end` is not specified, the resulting subarray will span to the end of the array.
242+
///
243+
/// # Example
244+
/// ```no_run
245+
/// # use godot::builtin::PackedArray;
246+
/// let array = PackedArray::from([10, 20, 30, 40, 50]);
247+
/// assert_eq!(array.subarray(2..), PackedArray::from([30, 40, 50]));
248+
/// ```
249+
///
232250
/// To obtain Rust slices, see [`as_slice`][Self::as_slice] and [`as_mut_slice`][Self::as_mut_slice].
251+
///
252+
/// _Godot equivalent: `slice`_
233253
#[doc(alias = "slice")]
234-
// TODO(v0.3): change to i32 like NodePath::slice/subpath() and support+test negative indices.
235-
pub fn subarray(&self, begin: usize, end: usize) -> Self {
236-
let len = self.len();
237-
let begin = begin.min(len);
238-
let end = end.min(len);
239-
240-
T::op_slice(self.as_inner(), to_i64(begin), to_i64(end))
254+
// Note: Godot will clamp values by itself.
255+
pub fn subarray(&self, range: impl RangeBounds<i32>) -> Self {
256+
T::op_slice(self.as_inner(), range)
241257
}
242258

243259
/// Returns a shared Rust slice of the array.

godot-core/src/builtin/collections/packed_array_element.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,13 @@
44
* License, v. 2.0. If a copy of the MPL was not distributed with this
55
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
66
*/
7+
use std::ops::RangeBounds;
78

89
use sys::{interface_fn, GodotFfi, SysPtr};
910

1011
use crate::builtin::collections::extend_buffer::{ExtendBuffer, ExtendBufferTrait};
1112
use crate::builtin::PackedArray;
13+
use crate::meta::godot_range::GodotRange;
1214
use crate::meta::{CowArg, FromGodot, GodotType, ToGodot};
1315
use crate::registry::property::builtin_type_string;
1416
use crate::{builtin, sys};
@@ -126,7 +128,7 @@ pub trait PackedArrayElement: GodotType + Clone + ToGodot + FromGodot {
126128
fn op_append_array(inner: Self::Inner<'_>, other: &PackedArray<Self>);
127129

128130
#[doc(hidden)]
129-
fn op_slice(inner: Self::Inner<'_>, begin: i64, end: i64) -> PackedArray<Self>;
131+
fn op_slice(inner: Self::Inner<'_>, range: impl RangeBounds<i32>) -> PackedArray<Self>;
130132

131133
#[doc(hidden)]
132134
fn op_find(inner: Self::Inner<'_>, value: CowArg<'_, Self>, from: i64) -> i64;
@@ -285,8 +287,9 @@ macro_rules! impl_packed_array_element {
285287
inner.append_array(other);
286288
}
287289

288-
fn op_slice(inner: Self::Inner<'_>, begin: i64, end: i64) -> PackedArray<Self> {
289-
inner.slice(begin, end)
290+
fn op_slice(inner: Self::Inner<'_>, range: impl RangeBounds<i32>) -> PackedArray<Self> {
291+
let (begin, end) = range.to_godot_range_fromto();
292+
inner.slice(begin, end.unwrap_or(i32::MAX as i64))
290293
}
291294

292295
fn op_find(inner: Self::Inner<'_>, value: CowArg<'_, Self>, from: i64) -> i64 {

godot-core/src/builtin/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,9 @@ pub mod inner {
113113
pub use crate::gen::builtin_classes::*;
114114
}
115115

116+
// ----------------------------------------------------------------------------------------------------------------------------------------------
117+
// Conversion functions
118+
116119
pub(crate) fn to_i64(i: usize) -> i64 {
117120
i.try_into().unwrap()
118121
}

godot-core/src/builtin/string/mod.rs

Lines changed: 0 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@ mod node_path;
1313
mod string_macros;
1414
mod string_name;
1515

16-
use std::ops;
17-
1816
pub use gstring::*;
1917
pub use node_path::NodePath;
2018
pub use string_name::*;
@@ -69,71 +67,6 @@ pub enum Encoding {
6967

7068
// ----------------------------------------------------------------------------------------------------------------------------------------------
7169

72-
/// Returns a tuple of `(from, len)` from a Rust range.
73-
///
74-
/// Unbounded upper bounds are represented by `len = -1`.
75-
fn to_godot_fromlen_neg1<R>(range: R) -> (i64, i64)
76-
where
77-
R: ops::RangeBounds<usize>,
78-
{
79-
let from = match range.start_bound() {
80-
ops::Bound::Included(&n) => n as i64,
81-
ops::Bound::Excluded(&n) => (n as i64) + 1,
82-
ops::Bound::Unbounded => 0,
83-
};
84-
85-
let len = match range.end_bound() {
86-
ops::Bound::Included(&n) => {
87-
let to = (n + 1) as i64;
88-
debug_assert!(
89-
from <= to,
90-
"range: start ({from}) > inclusive end ({n}) + 1"
91-
);
92-
to - from
93-
}
94-
ops::Bound::Excluded(&n) => {
95-
let to = n as i64;
96-
debug_assert!(from <= to, "range: start ({from}) > exclusive end ({to})");
97-
to - from
98-
}
99-
ops::Bound::Unbounded => -1,
100-
};
101-
102-
(from, len)
103-
}
104-
105-
/// Returns a tuple of `(from, len)` from a Rust range.
106-
///
107-
/// Unbounded upper bounds are represented by `i32::MAX` (yes, not `i64::MAX` -- since Godot treats some indexes as 32-bit despite being
108-
/// declared `i64` in GDExtension API).
109-
fn to_godot_fromlen_i32max<R>(range: R) -> (i64, i64)
110-
where
111-
R: ops::RangeBounds<usize>,
112-
{
113-
let (from, len) = to_godot_fromlen_neg1(range);
114-
if len == -1 {
115-
// Use i32 here because Godot may wrap around larger values (see Rustdoc).
116-
(from, i32::MAX as i64)
117-
} else {
118-
(from, len)
119-
}
120-
}
121-
122-
/// Returns a tuple of `(from, to)` from a Rust range.
123-
///
124-
/// Unbounded upper bounds are represented by `to = 0`.
125-
fn to_godot_fromto<R>(range: R) -> (i64, i64)
126-
where
127-
R: ops::RangeBounds<usize>,
128-
{
129-
let (from, len) = to_godot_fromlen_neg1(range);
130-
if len == -1 {
131-
(from, 0)
132-
} else {
133-
(from, from + len)
134-
}
135-
}
136-
13770
fn populated_or_none(s: GString) -> Option<GString> {
13871
if s.is_empty() {
13972
None

godot-core/src/builtin/string/node_path.rs

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,14 @@
55
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
66
*/
77

8-
use std::fmt;
8+
use std::{fmt, ops};
99

1010
use godot_ffi as sys;
1111
use godot_ffi::{ffi_methods, ExtVariantType, GdextBuild, GodotFfi};
1212

1313
use super::{GString, StringName};
1414
use crate::builtin::inner;
15+
use crate::meta::godot_range::GodotRange;
1516

1617
/// A pre-parsed scene tree path.
1718
///
@@ -127,10 +128,25 @@ impl NodePath {
127128
/// Returns the range `begin..exclusive_end` as a new `NodePath`.
128129
///
129130
/// The absolute value of `begin` and `exclusive_end` will be clamped to [`get_total_count()`][Self::get_total_count].
130-
/// So, to express "until the end", you can simply pass a large value for `exclusive_end`, such as `i32::MAX`.
131+
/// If upper bound is not defined `exclusive_end` will span to the end of the `NodePath`.
131132
///
132-
/// If either `begin` or `exclusive_end` are negative, they will be relative to the end of the `NodePath`. \
133-
/// For example, `path.subpath(0, -2)` is a shorthand for `path.subpath(0, path.get_total_count() - 2)`.
133+
/// # Example
134+
/// ```no_run
135+
/// # use godot::builtin::NodePath;
136+
/// assert_eq!(NodePath::from("path/to/Node:with:props").subpath(-1..), ":props".into());
137+
/// ```
138+
///
139+
/// If either `begin` or `exclusive_end` are negative, they will be relative to the end of the `NodePath`.
140+
///
141+
/// # Example
142+
/// ```no_run
143+
/// # use godot::builtin::NodePath;
144+
/// let path = NodePath::from("path/to/Node:with:props");
145+
/// let total_count = path.get_total_count() as i32;
146+
/// // Both are equal to "path/to/Node".
147+
/// assert_eq!(path.subpath(0..-2), path.subpath(0..total_count - 2));
148+
///
149+
/// ```
134150
///
135151
/// _Godot equivalent: `slice`_
136152
///
@@ -140,16 +156,17 @@ impl NodePath {
140156
// i32 used because it can be negative and many Godot APIs use this, see https://github.com/godot-rust/gdext/pull/982/files#r1893732978.
141157
#[cfg(since_api = "4.3")]
142158
#[doc(alias = "slice")]
143-
pub fn subpath(&self, begin: i32, exclusive_end: i32) -> NodePath {
159+
pub fn subpath(&self, range: impl ops::RangeBounds<i32>) -> NodePath {
160+
let (from, exclusive_end) = range.to_godot_range_fromto();
144161
// Polyfill for bug https://github.com/godotengine/godot/pull/100954, fixed in 4.4.
145162
let begin = if GdextBuild::since_api("4.4") {
146-
begin
163+
from
147164
} else {
148-
let name_count = self.get_name_count() as i32;
149-
let subname_count = self.get_subname_count() as i32;
165+
let name_count = self.get_name_count() as i64;
166+
let subname_count = self.get_subname_count() as i64;
150167
let total_count = name_count + subname_count;
151168

152-
let mut begin = begin.clamp(-total_count, total_count);
169+
let mut begin = from.clamp(-total_count, total_count);
153170
if begin < 0 {
154171
begin += total_count;
155172
}
@@ -159,7 +176,8 @@ impl NodePath {
159176
begin
160177
};
161178

162-
self.as_inner().slice(begin as i64, exclusive_end as i64)
179+
self.as_inner()
180+
.slice(begin, exclusive_end.unwrap_or(i32::MAX as i64))
163181
}
164182

165183
crate::meta::declare_arg_method! {

0 commit comments

Comments
 (0)