Skip to content

Commit 0c9fdc4

Browse files
YarwinBromeon
authored andcommitted
Create SignedRange trait.
Add `wrapped` utility function to handle signed ranges.
1 parent 21b8906 commit 0c9fdc4

File tree

12 files changed

+219
-209
lines changed

12 files changed

+219
-209
lines changed

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

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

88
use std::cell::OnceCell;
99
use std::marker::PhantomData;
10-
use std::ops::RangeBounds;
1110
use std::{cmp, fmt};
1211

1312
use godot_ffi as sys;
@@ -16,7 +15,7 @@ use sys::{ffi_methods, interface_fn, GodotFfi};
1615
use crate::builtin::*;
1716
use crate::meta;
1817
use crate::meta::error::{ConvertError, FromGodotError, FromVariantError};
19-
use crate::meta::godot_range::GodotRange;
18+
use crate::meta::signed_range::SignedRange;
2019
use crate::meta::{
2120
element_godot_type_name, element_variant_type, ArrayElement, AsArg, ClassName, ElementType,
2221
ExtVariantType, FromGodot, GodotConvert, GodotFfiVariant, GodotType, PropertyHintInfo, RefArg,
@@ -110,6 +109,35 @@ use crate::registry::property::{BuiltinExport, Export, Var};
110109
/// // ...and so on.
111110
/// ```
112111
///
112+
/// # Working with signed ranges and steps
113+
///
114+
/// For negative indices, use [`wrapped()`](crate::meta::wrapped).
115+
///
116+
/// ```no_run
117+
/// # use godot::builtin::array;
118+
/// # use godot::meta::wrapped;
119+
/// let arr = array![0, 1, 2, 3, 4, 5];
120+
///
121+
/// // The values of `begin` (inclusive) and `end` (exclusive) will be clamped to the array size.
122+
/// let clamped_array = arr.subarray_deep(999..99999, None);
123+
/// assert_eq!(clamped_array, array![]);
124+
///
125+
/// // If either `begin` or `end` is negative, its value is relative to the end of the array.
126+
/// let sub = arr.subarray_shallow(wrapped(-1..-5), None);
127+
/// assert_eq!(sub, array![5, 3]);
128+
///
129+
/// // If `end` is not specified, the range spans through whole array.
130+
/// let sub = arr.subarray_deep(1.., None);
131+
/// assert_eq!(sub, array![1, 2, 3, 4, 5]);
132+
/// let other_clamped_array = arr.subarray_shallow(5.., Some(2));
133+
/// assert_eq!(other_clamped_array, array![5]);
134+
///
135+
/// // If specified, `step` is the relative index between source elements. It can be negative,
136+
/// // in which case `begin` must be higher than `end`.
137+
/// let sub = arr.subarray_shallow(wrapped(-1..-5), Some(-2));
138+
/// assert_eq!(sub, array![5, 3]);
139+
/// ```
140+
///
113141
/// # Thread safety
114142
///
115143
/// Usage is safe if the `Array` is used on a single thread only. Concurrent reads on
@@ -530,91 +558,34 @@ impl<T: ArrayElement> Array<T> {
530558

531559
/// Returns a sub-range `begin..end` as a new `Array`.
532560
///
533-
/// The values of `begin` (inclusive) and `end` (exclusive) will be clamped to the array size.
534-
///
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-
///
551-
/// If specified, `step` is the relative index between source elements. It can be negative,
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-
/// ```
559-
///
560561
/// Array elements are copied to the slice, but any reference types (such as `Array`,
561562
/// `Dictionary` and `Object`) will still refer to the same value. To create a deep copy, use
562563
/// [`subarray_deep()`][Self::subarray_deep] instead.
563564
///
564565
/// _Godot equivalent: `slice`_
565566
#[doc(alias = "slice")]
566-
pub fn subarray_shallow(&self, range: impl RangeBounds<i32>, step: Option<i32>) -> Self {
567+
pub fn subarray_shallow(&self, range: impl SignedRange, step: Option<i32>) -> Self {
567568
self.subarray_impl(range, step, false)
568569
}
569570

570571
/// Returns a sub-range `begin..end` as a new `Array`.
571572
///
572-
/// The values of `begin` (inclusive) and `end` (exclusive) will be clamped to the array size.
573-
///
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-
///
590-
/// If specified, `step` is the relative index between source elements. It can be negative,
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-
/// ```
598-
///
599573
/// All nested arrays and dictionaries are duplicated and will not be shared with the original
600574
/// array. Note that any `Object`-derived elements will still be shallow copied. To create a
601575
/// shallow copy, use [`subarray_shallow()`][Self::subarray_shallow] instead.
602576
///
603577
/// _Godot equivalent: `slice`_
604578
#[doc(alias = "slice")]
605-
pub fn subarray_deep(&self, range: impl RangeBounds<i32>, step: Option<i32>) -> Self {
579+
pub fn subarray_deep(&self, range: impl SignedRange, step: Option<i32>) -> Self {
606580
self.subarray_impl(range, step, true)
607581
}
608582

609583
// Note: Godot will clamp values by itself.
610-
fn subarray_impl(&self, range: impl GodotRange<i32>, step: Option<i32>, deep: bool) -> Self {
584+
fn subarray_impl(&self, range: impl SignedRange, step: Option<i32>, deep: bool) -> Self {
611585
assert_ne!(step, Some(0), "subarray: step cannot be zero");
612586

613587
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.
588+
let (begin, end) = range.signed();
618589
let end = end.unwrap_or(i32::MAX as i64);
619590

620591
// SAFETY: The type of the array is `T` and we convert the returned array to an `Array<T>` immediately.

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

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

1212
use std::iter::FromIterator;
13-
use std::ops::RangeBounds;
1413
use std::{fmt, ops, ptr};
1514

1615
use godot_ffi as sys;
@@ -20,6 +19,7 @@ use crate::builtin::collections::extend_buffer::ExtendBufferTrait;
2019
use crate::builtin::*;
2120
use crate::classes::file_access::CompressionMode;
2221
use crate::meta;
22+
use crate::meta::signed_range::SignedRange;
2323
use crate::meta::{AsArg, FromGodot, GodotConvert, PackedArrayElement, ToGodot};
2424
use crate::obj::EngineEnum;
2525
use crate::registry::property::{Export, Var};
@@ -228,31 +228,29 @@ impl<T: PackedArrayElement> PackedArray<T> {
228228
/// Returns a sub-range `begin..end`, as a new packed array.
229229
///
230230
/// The values of `begin` (inclusive) and `end` (exclusive) will be clamped to the array size.
231+
/// To obtain Rust slices, see [`as_slice`][Self::as_slice] and [`as_mut_slice`][Self::as_mut_slice].
231232
///
232-
/// If either `begin` or `end` are negative, their value is relative to the end of the array.
233+
/// # Usage
234+
/// For negative indices, use [`wrapped()`](crate::meta::wrapped).
233235
///
234-
/// # Example
235236
/// ```no_run
236237
/// # use godot::builtin::PackedArray;
238+
/// # use godot::meta::wrapped;
237239
/// let array = PackedArray::from([10, 20, 30, 40, 50]);
238-
/// assert_eq!(array.subarray(-4..-2), PackedArray::from([20, 30]));
239-
/// ```
240240
///
241-
/// If `end` is not specified, the resulting subarray will span to the end of the array.
241+
/// // If either `begin` or `end` is negative, its value is relative to the end of the array.
242+
/// let sub = array.subarray(wrapped(-4..-2));
243+
/// assert_eq!(sub, PackedArray::from([20, 30]));
242244
///
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]));
245+
/// // If `end` is not specified, the resulting subarray will span to the end of the array.
246+
/// let sub = array.subarray(2..);
247+
/// assert_eq!(sub, PackedArray::from([30, 40, 50]));
248248
/// ```
249249
///
250-
/// To obtain Rust slices, see [`as_slice`][Self::as_slice] and [`as_mut_slice`][Self::as_mut_slice].
251-
///
252250
/// _Godot equivalent: `slice`_
253251
#[doc(alias = "slice")]
254252
// Note: Godot will clamp values by itself.
255-
pub fn subarray(&self, range: impl RangeBounds<i32>) -> Self {
253+
pub fn subarray(&self, range: impl SignedRange) -> Self {
256254
T::op_slice(self.as_inner(), range)
257255
}
258256

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,12 @@
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;
87

98
use sys::{interface_fn, GodotFfi, SysPtr};
109

1110
use crate::builtin::collections::extend_buffer::{ExtendBuffer, ExtendBufferTrait};
1211
use crate::builtin::PackedArray;
13-
use crate::meta::godot_range::GodotRange;
12+
use crate::meta::signed_range::SignedRange;
1413
use crate::meta::{CowArg, FromGodot, GodotType, ToGodot};
1514
use crate::registry::property::builtin_type_string;
1615
use crate::{builtin, sys};
@@ -128,7 +127,7 @@ pub trait PackedArrayElement: GodotType + Clone + ToGodot + FromGodot {
128127
fn op_append_array(inner: Self::Inner<'_>, other: &PackedArray<Self>);
129128

130129
#[doc(hidden)]
131-
fn op_slice(inner: Self::Inner<'_>, range: impl RangeBounds<i32>) -> PackedArray<Self>;
130+
fn op_slice(inner: Self::Inner<'_>, range: impl SignedRange) -> PackedArray<Self>;
132131

133132
#[doc(hidden)]
134133
fn op_find(inner: Self::Inner<'_>, value: CowArg<'_, Self>, from: i64) -> i64;
@@ -287,8 +286,8 @@ macro_rules! impl_packed_array_element {
287286
inner.append_array(other);
288287
}
289288

290-
fn op_slice(inner: Self::Inner<'_>, range: impl RangeBounds<i32>) -> PackedArray<Self> {
291-
let (begin, end) = range.to_godot_range_fromto();
289+
fn op_slice(inner: Self::Inner<'_>, range: impl $crate::meta::signed_range::SignedRange) -> PackedArray<Self> {
290+
let (begin, end) = range.signed();
292291
inner.slice(begin, end.unwrap_or(i32::MAX as i64))
293292
}
294293

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

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

8-
use std::{fmt, ops};
8+
use std::fmt;
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;
15+
use crate::meta::signed_range::SignedRange;
1616

1717
/// A pre-parsed scene tree path.
1818
///
@@ -128,36 +128,37 @@ impl NodePath {
128128
/// Returns the range `begin..exclusive_end` as a new `NodePath`.
129129
///
130130
/// The absolute value of `begin` and `exclusive_end` will be clamped to [`get_total_count()`][Self::get_total_count].
131-
/// If upper bound is not defined `exclusive_end` will span to the end of the `NodePath`.
132131
///
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`.
132+
/// # Usage
133+
/// For negative indices, use [`wrapped()`](crate::meta::wrapped).
140134
///
141-
/// # Example
142135
/// ```no_run
143136
/// # use godot::builtin::NodePath;
137+
/// # use godot::meta::wrapped;
144138
/// 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));
148139
///
140+
/// // If upper bound is not defined, `exclusive_end` will span to the end of the `NodePath`.
141+
/// let sub = path.subpath(2..);
142+
/// assert_eq!(sub, ":props".into());
143+
///
144+
/// // If either `begin` or `exclusive_end` are negative, they will be relative to the end of the `NodePath`.
145+
/// let total_count = path.get_total_count();
146+
/// let wrapped_sub = path.subpath(wrapped(0..-2));
147+
/// let normal_sub = path.subpath(0..total_count - 2);
148+
/// // Both are equal to "path/to/Node".
149+
/// assert_eq!(wrapped_sub, normal_sub);
149150
/// ```
150151
///
151152
/// _Godot equivalent: `slice`_
152153
///
153154
/// # Compatibility
154-
/// The `slice()` behavior for Godot <= 4.3 is unintuitive, see [#100954](https://github.com/godotengine/godot/pull/100954). godot-rust
155+
/// The `slice()` behavior for Godot <= 4.3 is unintuitive, see [#100954](https://github.com/godotengine/godot/pull/100954). Godot-rust
155156
/// automatically changes this to the fixed version for Godot 4.4+, even when used in older versions. So, the behavior is always the same.
156157
// i32 used because it can be negative and many Godot APIs use this, see https://github.com/godot-rust/gdext/pull/982/files#r1893732978.
157158
#[cfg(since_api = "4.3")]
158159
#[doc(alias = "slice")]
159-
pub fn subpath(&self, range: impl ops::RangeBounds<i32>) -> NodePath {
160-
let (from, exclusive_end) = range.to_godot_range_fromto();
160+
pub fn subpath(&self, range: impl SignedRange) -> NodePath {
161+
let (from, exclusive_end) = range.signed();
161162
// Polyfill for bug https://github.com/godotengine/godot/pull/100954, fixed in 4.4.
162163
let begin = if GdextBuild::since_api("4.4") {
163164
from

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

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -81,15 +81,13 @@ macro_rules! impl_shared_string_api {
8181

8282
/// Count how many times `what` appears within `range`. Use `..` for full string search.
8383
pub fn count(&self, what: impl AsArg<GString>, range: impl std::ops::RangeBounds<usize>) -> usize {
84-
use $crate::meta::godot_range::GodotRange;
85-
let (from, to) = range.to_godot_range_fromto_checked(0);
84+
let (from, to) = $crate::meta::signed_range::to_godot_range_fromto(range);
8685
self.as_inner().count(what, from, to) as usize
8786
}
8887

8988
/// Count how many times `what` appears within `range`, case-insensitively. Use `..` for full string search.
9089
pub fn countn(&self, what: impl AsArg<GString>, range: impl std::ops::RangeBounds<usize>) -> usize {
91-
use $crate::meta::godot_range::GodotRange;
92-
let (from, to) = range.to_godot_range_fromto_checked(0);
90+
let (from, to) = $crate::meta::signed_range::to_godot_range_fromto(range);
9391
self.as_inner().countn(what, from, to) as usize
9492
}
9593

@@ -123,8 +121,7 @@ macro_rules! impl_shared_string_api {
123121
/// Returns a substring of this, as another `GString`.
124122
// TODO is there no efficient way to implement this for StringName by interning?
125123
pub fn substr(&self, range: impl std::ops::RangeBounds<usize>) -> GString {
126-
use $crate::meta::godot_range::GodotRange;
127-
let (from, len) = range.to_godot_range_fromlen(-1);
124+
let (from, len) = $crate::meta::signed_range::to_godot_range_fromlen(range, -1);
128125

129126
self.as_inner().substr(from, len)
130127
}
@@ -166,11 +163,7 @@ macro_rules! impl_shared_string_api {
166163

167164
/// Returns a copy of the string without the specified index range.
168165
pub fn erase(&self, range: impl std::ops::RangeBounds<usize>) -> GString {
169-
use $crate::meta::godot_range::GodotRange;
170-
// Unbounded upper bounds are represented by `i32::MAX` instead of `i64::MAX`,
171-
// since Godot treats some indexes as 32-bit despite being declared `i64` in GDExtension API.
172-
let (from, len) = range.to_godot_range_fromlen(i32::MAX as i64);
173-
166+
let (from, len) = $crate::meta::signed_range::to_godot_range_fromlen(range, i32::MAX as i64);
174167
self.as_inner().erase(from, len)
175168
}
176169

0 commit comments

Comments
 (0)