Skip to content

Commit 5f22fe2

Browse files
committed
Finalize NodePath methods + subpath/slice polyfill + tests
1 parent 427e682 commit 5f22fe2

File tree

6 files changed

+158
-73
lines changed

6 files changed

+158
-73
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,10 @@ impl<T: ArrayElement> Array<T> {
495495
/// Array elements are copied to the slice, but any reference types (such as `Array`,
496496
/// `Dictionary` and `Object`) will still refer to the same value. To create a deep copy, use
497497
/// [`subarray_deep()`][Self::subarray_deep] instead.
498+
///
499+
/// _Godot equivalent: `slice`_
498500
#[doc(alias = "slice")]
501+
// TODO(v0.3): change to i32 like NodePath::slice/subpath() and support+test negative indices.
499502
pub fn subarray_shallow(&self, begin: usize, end: usize, step: Option<isize>) -> Self {
500503
self.subarray_impl(begin, end, step, false)
501504
}
@@ -511,7 +514,10 @@ impl<T: ArrayElement> Array<T> {
511514
/// All nested arrays and dictionaries are duplicated and will not be shared with the original
512515
/// array. Note that any `Object`-derived elements will still be shallow copied. To create a
513516
/// shallow copy, use [`subarray_shallow()`][Self::subarray_shallow] instead.
517+
///
518+
/// _Godot equivalent: `slice`_
514519
#[doc(alias = "slice")]
520+
// TODO(v0.3): change to i32 like NodePath::slice/subpath() and support+test negative indices.
515521
pub fn subarray_deep(&self, begin: usize, end: usize, step: Option<isize>) -> Self {
516522
self.subarray_impl(begin, end, step, true)
517523
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ macro_rules! impl_packed_array {
212212
///
213213
/// To obtain Rust slices, see [`as_slice`][Self::as_slice] and [`as_mut_slice`][Self::as_mut_slice].
214214
#[doc(alias = "slice")]
215+
// TODO(v0.3): change to i32 like NodePath::slice/subpath() and support+test negative indices.
215216
pub fn subarray(&self, begin: usize, end: usize) -> Self {
216217
let len = self.len();
217218
let begin = begin.min(len);

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

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

8-
use std::{
9-
fmt,
10-
ops::{
11-
Bound::{Excluded, Included, Unbounded},
12-
RangeBounds,
13-
},
14-
};
8+
use std::fmt;
159

1610
use godot_ffi as sys;
1711
use godot_ffi::{ffi_methods, GodotFfi};
@@ -47,33 +41,80 @@ impl NodePath {
4741
Self { opaque }
4842
}
4943

44+
/// Returns the node name at position `index`.
45+
///
46+
/// If you want to get a property name instead, check out [`get_subname()`][Self::get_subname].
47+
///
48+
/// # Example
49+
/// ```no_run
50+
/// # use godot::prelude::*;
51+
/// let path = NodePath::from("../RigidBody2D/Sprite2D");
52+
/// godot_print!("{}", path.get_name(0)); // ".."
53+
/// godot_print!("{}", path.get_name(1)); // "RigidBody2D"
54+
/// godot_print!("{}", path.get_name(2)); // "Sprite"
55+
/// ```
56+
///
57+
/// # Panics
58+
/// In Debug mode, if `index` is out of bounds. In Release, a Godot error is generated and the result is unspecified (but safe).
59+
pub fn get_name(&self, index: usize) -> StringName {
60+
let inner = self.as_inner();
61+
let index = index as i64;
62+
63+
debug_assert!(
64+
index < inner.get_name_count(),
65+
"NodePath '{self}': name at index {index} is out of bounds"
66+
);
67+
68+
inner.get_name(index)
69+
}
70+
71+
/// Returns the node subname (property) at position `index`.
72+
///
73+
/// If you want to get a node name instead, check out [`get_name()`][Self::get_name].
74+
///
75+
/// # Example
76+
/// ```no_run
77+
/// # use godot::prelude::*;
78+
/// let path = NodePath::from("Sprite2D:texture:resource_name");
79+
/// godot_print!("{}", path.get_subname(0)); // "texture"
80+
/// godot_print!("{}", path.get_subname(1)); // "resource_name"
81+
/// ```
82+
///
83+
/// # Panics
84+
/// In Debug mode, if `index` is out of bounds. In Release, a Godot error is generated and the result is unspecified (but safe).
85+
pub fn get_subname(&self, index: usize) -> StringName {
86+
let inner = self.as_inner();
87+
let index = index as i64;
88+
89+
debug_assert!(
90+
index < inner.get_subname_count(),
91+
"NodePath '{self}': subname at index {index} is out of bounds"
92+
);
93+
94+
inner.get_subname(index)
95+
}
96+
5097
/// Returns the number of node names in the path. Property subnames are not included.
51-
pub fn get_name_count(&self) -> u32 {
98+
pub fn get_name_count(&self) -> usize {
5299
self.as_inner()
53100
.get_name_count()
54101
.try_into()
55-
.expect("Godot name counts are non negative int")
56-
}
57-
58-
/// Returns the node name indicated by `idx`, starting from 0. If `idx` is out of bounds, [`None`] is returned.
59-
/// See also [`NodePath::get_subname_count`] and [`NodePath::get_name_count`].
60-
pub fn get_name(&self, idx: u32) -> Option<StringName> {
61-
let inner = self.as_inner();
62-
let idx = idx as i64;
63-
// This check checks both data being empty (get_name_count returns 0) and index out of bounds.
64-
if idx >= inner.get_name_count() {
65-
None
66-
} else {
67-
Some(inner.get_name(idx))
68-
}
102+
.expect("Godot name counts are non-negative ints")
69103
}
70104

71105
/// Returns the number of property names ("subnames") in the path. Each subname in the node path is listed after a colon character (`:`).
72-
pub fn get_subname_count(&self) -> u32 {
106+
pub fn get_subname_count(&self) -> usize {
73107
self.as_inner()
74108
.get_subname_count()
75109
.try_into()
76-
.expect("Godot subname counts are non negative int")
110+
.expect("Godot subname counts are non-negative ints")
111+
}
112+
113+
/// Returns the total number of names + subnames.
114+
///
115+
/// This method does not exist in Godot and is provided in Rust for convenience.
116+
pub fn get_total_count(&self) -> usize {
117+
self.get_name_count() + self.get_subname_count()
77118
}
78119

79120
/// Returns a 32-bit integer hash value representing the string.
@@ -84,48 +125,38 @@ impl NodePath {
84125
.expect("Godot hashes are uint32_t")
85126
}
86127

87-
/// Returns the property name indicated by `idx`, starting from 0. If `idx` is out of bounds, [`None`] is returned.
88-
/// See also [`NodePath::get_subname_count`].
89-
pub fn get_subname(&self, idx: u32) -> Option<StringName> {
90-
let inner = self.as_inner();
91-
let idx = idx as i64;
92-
// This check checks both data being empty (get_subname_count returns 0) and index out of bounds.
93-
if idx >= inner.get_subname_count() {
94-
None
95-
} else {
96-
Some(inner.get_subname(idx))
128+
/// Returns the range `begin..exclusive_end` as a new `NodePath`.
129+
///
130+
/// The absolute value of `begin` and `exclusive_end` will be clamped to [`get_total_count()`][Self::get_total_count].
131+
/// So, to express "until the end", you can simply pass a large value for `exclusive_end`, such as `i32::MAX`.
132+
///
133+
/// If either `begin` or `exclusive_end` are negative, they will be relative to the end of the `NodePath`. \
134+
/// For example, `path.subpath(0, -2)` is a shorthand for `path.subpath(0, path.get_total_count() - 2)`.
135+
///
136+
/// _Godot equivalent: `slice`_
137+
///
138+
/// # Compatibility
139+
/// The `slice()` behavior for Godot <= 4.3 is unintuitive, see [#100954](https://github.com/godotengine/godot/pull/100954). godot-rust
140+
/// automatically changes this to the fixed version for Godot 4.4+, even when used in older versions. So, the behavior is always the same.
141+
// i32 used because it can be negative and many Godot APIs use this, see https://github.com/godot-rust/gdext/pull/982/files#r1893732978.
142+
#[cfg(since_api = "4.3")]
143+
#[doc(alias = "slice")]
144+
pub fn subpath(&self, begin: i32, exclusive_end: i32) -> NodePath {
145+
// Polyfill for bug https://github.com/godotengine/godot/pull/100954.
146+
// TODO(v0.3) make polyfill (everything but last line) conditional if PR is merged in 4.4.
147+
let name_count = self.get_name_count() as i32;
148+
let subname_count = self.get_subname_count() as i32;
149+
let total_count = name_count + subname_count;
150+
151+
let mut begin = begin.clamp(-total_count, total_count);
152+
if begin < 0 {
153+
begin += total_count;
154+
}
155+
if begin > name_count {
156+
begin += 1;
97157
}
98-
}
99158

100-
/// Returns the slice of the [`NodePath`] as a new [`NodePath`]
101-
pub fn slice(&self, range: impl RangeBounds<i64>) -> NodePath {
102-
self.as_inner().slice(
103-
match range.start_bound() {
104-
Excluded(&start) => {
105-
if start == -1 {
106-
// Default end from godot, since the start is excluded.
107-
i32::MAX as i64
108-
} else {
109-
start + 1
110-
}
111-
}
112-
Included(&start) => start,
113-
Unbounded => 0,
114-
},
115-
match range.end_bound() {
116-
Excluded(&end) => end,
117-
Included(&end) => {
118-
if end == -1 {
119-
// Default end from godot, since the end is excluded.
120-
i32::MAX as i64
121-
} else {
122-
end + 1
123-
}
124-
}
125-
// Default end from godot.
126-
Unbounded => i32::MAX as i64,
127-
},
128-
)
159+
self.as_inner().slice(begin as i64, exclusive_end as i64)
129160
}
130161

131162
crate::meta::declare_arg_method! {

itest/rust/src/builtin_tests/string/gstring_test.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
use std::collections::HashSet;
99

10-
use crate::framework::{expect_panic, itest};
10+
use crate::framework::{expect_debug_panic_or_release_ok, itest};
1111
use godot::builtin::{GString, PackedStringArray};
1212

1313
// TODO use tests from godot-rust/gdnative
@@ -98,14 +98,10 @@ fn string_unicode_at() {
9898
assert_eq!(s.unicode_at(2), 'A');
9999
assert_eq!(s.unicode_at(3), '💡');
100100

101-
#[cfg(debug_assertions)]
102-
expect_panic("Debug mode: unicode_at() out-of-bounds panics", || {
103-
s.unicode_at(4);
104-
});
105-
106101
// Release mode: out-of-bounds prints Godot error, but returns 0.
107-
#[cfg(not(debug_assertions))]
108-
assert_eq!(s.unicode_at(4), '\0');
102+
expect_debug_panic_or_release_ok("unicode_at() out-of-bounds panics", || {
103+
assert_eq!(s.unicode_at(4), '\0');
104+
});
109105
}
110106

111107
#[itest]

itest/rust/src/builtin_tests/string/node_path_test.rs

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
use std::collections::HashSet;
99

10-
use crate::framework::itest;
10+
use crate::framework::{expect_debug_panic_or_release_ok, itest};
1111
use godot::builtin::{GString, NodePath};
1212

1313
#[itest]
@@ -83,3 +83,46 @@ fn node_path_with_null() {
8383
assert_eq!(left, right);
8484
}
8585
}
86+
87+
#[itest]
88+
#[cfg(since_api = "4.3")]
89+
fn node_path_subpath() {
90+
let path = NodePath::from("path/to/Node:with:props");
91+
let parts = path.get_name_count() + path.get_subname_count();
92+
93+
assert_eq!(path.subpath(0, 1), "path".into());
94+
assert_eq!(path.subpath(1, 2), "to".into());
95+
assert_eq!(path.subpath(2, 3), "Node".into());
96+
assert_eq!(path.subpath(3, 4), ":with".into());
97+
assert_eq!(path.subpath(4, 5), ":props".into());
98+
99+
assert_eq!(path.subpath(1, -1), "to/Node:with".into());
100+
assert_eq!(path.subpath(1, parts as i32 - 1), "to/Node:with".into());
101+
assert_eq!(path.subpath(0, -2), "path/to/Node".into());
102+
assert_eq!(path.subpath(-3, -1), "Node:with".into());
103+
assert_eq!(path.subpath(-2, i32::MAX), ":with:props".into());
104+
assert_eq!(path.subpath(-1, i32::MAX), ":props".into());
105+
}
106+
107+
#[itest]
108+
fn node_path_get_name() {
109+
let path = NodePath::from("../RigidBody2D/Sprite2D");
110+
assert_eq!(path.get_name(0), "..".into());
111+
assert_eq!(path.get_name(1), "RigidBody2D".into());
112+
assert_eq!(path.get_name(2), "Sprite2D".into());
113+
114+
expect_debug_panic_or_release_ok("NodePath::get_name() out of bounds", || {
115+
assert_eq!(path.get_name(3), "".into());
116+
})
117+
}
118+
119+
#[itest]
120+
fn node_path_get_subname() {
121+
let path = NodePath::from("Sprite2D:texture:resource_name");
122+
assert_eq!(path.get_subname(0), "texture".into());
123+
assert_eq!(path.get_subname(1), "resource_name".into());
124+
125+
expect_debug_panic_or_release_ok("NodePath::get_subname() out of bounds", || {
126+
assert_eq!(path.get_subname(2), "".into());
127+
})
128+
}

itest/rust/src/framework/mod.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,14 @@ pub fn expect_panic(context: &str, code: impl FnOnce()) {
144144
);
145145
}
146146

147+
pub fn expect_debug_panic_or_release_ok(_context: &str, code: impl FnOnce()) {
148+
#[cfg(debug_assertions)]
149+
expect_panic(_context, code);
150+
151+
#[cfg(not(debug_assertions))]
152+
code()
153+
}
154+
147155
/// Synchronously run a thread and return result. Panics are propagated to caller thread.
148156
#[track_caller]
149157
pub fn quick_thread<R, F>(f: F) -> R

0 commit comments

Comments
 (0)