Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide feature parity for NodePath with Godot #982

Merged
merged 2 commits into from
Dec 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,6 @@ exp.rs

# Mac specific
.DS_Store

# Windows specific
desktop.ini
5 changes: 5 additions & 0 deletions godot-codegen/src/special_cases/special_cases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,11 @@ pub fn is_builtin_method_exposed(builtin_ty: &TyName, godot_method_name: &str) -
| ("StringName", "to_wchar_buffer")

// NodePath
| ("NodePath", "is_absolute")
| ("NodePath", "is_empty")
| ("NodePath", "get_concatenated_names")
| ("NodePath", "get_concatenated_subnames")
//| ("NodePath", "get_as_property_path")

// Callable
| ("Callable", "call")
Expand Down
6 changes: 6 additions & 0 deletions godot-core/src/builtin/collections/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,10 @@ impl<T: ArrayElement> Array<T> {
/// Array elements are copied to the slice, but any reference types (such as `Array`,
/// `Dictionary` and `Object`) will still refer to the same value. To create a deep copy, use
/// [`subarray_deep()`][Self::subarray_deep] instead.
///
/// _Godot equivalent: `slice`_
#[doc(alias = "slice")]
// TODO(v0.3): change to i32 like NodePath::slice/subpath() and support+test negative indices.
pub fn subarray_shallow(&self, begin: usize, end: usize, step: Option<isize>) -> Self {
self.subarray_impl(begin, end, step, false)
}
Expand All @@ -511,7 +514,10 @@ impl<T: ArrayElement> Array<T> {
/// All nested arrays and dictionaries are duplicated and will not be shared with the original
/// array. Note that any `Object`-derived elements will still be shallow copied. To create a
/// shallow copy, use [`subarray_shallow()`][Self::subarray_shallow] instead.
///
/// _Godot equivalent: `slice`_
#[doc(alias = "slice")]
// TODO(v0.3): change to i32 like NodePath::slice/subpath() and support+test negative indices.
pub fn subarray_deep(&self, begin: usize, end: usize, step: Option<isize>) -> Self {
self.subarray_impl(begin, end, step, true)
}
Expand Down
1 change: 1 addition & 0 deletions godot-core/src/builtin/collections/packed_array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ macro_rules! impl_packed_array {
///
/// To obtain Rust slices, see [`as_slice`][Self::as_slice] and [`as_mut_slice`][Self::as_mut_slice].
#[doc(alias = "slice")]
// TODO(v0.3): change to i32 like NodePath::slice/subpath() and support+test negative indices.
pub fn subarray(&self, begin: usize, end: usize) -> Self {
let len = self.len();
let begin = begin.min(len);
Expand Down
110 changes: 108 additions & 2 deletions godot-core/src/builtin/string/node_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,80 @@ impl NodePath {
Self { opaque }
}

pub fn is_empty(&self) -> bool {
self.as_inner().is_empty()
/// Returns the node name at position `index`.
///
/// If you want to get a property name instead, check out [`get_subname()`][Self::get_subname].
///
/// # Example
/// ```no_run
/// # use godot::prelude::*;
/// let path = NodePath::from("../RigidBody2D/Sprite2D");
/// godot_print!("{}", path.get_name(0)); // ".."
/// godot_print!("{}", path.get_name(1)); // "RigidBody2D"
/// godot_print!("{}", path.get_name(2)); // "Sprite"
/// ```
///
/// # Panics
/// In Debug mode, if `index` is out of bounds. In Release, a Godot error is generated and the result is unspecified (but safe).
pub fn get_name(&self, index: usize) -> StringName {
let inner = self.as_inner();
let index = index as i64;

debug_assert!(
index < inner.get_name_count(),
"NodePath '{self}': name at index {index} is out of bounds"
);

inner.get_name(index)
}

/// Returns the node subname (property) at position `index`.
///
/// If you want to get a node name instead, check out [`get_name()`][Self::get_name].
///
/// # Example
/// ```no_run
/// # use godot::prelude::*;
/// let path = NodePath::from("Sprite2D:texture:resource_name");
/// godot_print!("{}", path.get_subname(0)); // "texture"
/// godot_print!("{}", path.get_subname(1)); // "resource_name"
/// ```
///
/// # Panics
/// In Debug mode, if `index` is out of bounds. In Release, a Godot error is generated and the result is unspecified (but safe).
pub fn get_subname(&self, index: usize) -> StringName {
let inner = self.as_inner();
let index = index as i64;

debug_assert!(
index < inner.get_subname_count(),
"NodePath '{self}': subname at index {index} is out of bounds"
);

inner.get_subname(index)
}

/// Returns the number of node names in the path. Property subnames are not included.
pub fn get_name_count(&self) -> usize {
self.as_inner()
.get_name_count()
.try_into()
.expect("Godot name counts are non-negative ints")
}

/// Returns the number of property names ("subnames") in the path. Each subname in the node path is listed after a colon character (`:`).
pub fn get_subname_count(&self) -> usize {
self.as_inner()
.get_subname_count()
.try_into()
.expect("Godot subname counts are non-negative ints")
}

/// Returns the total number of names + subnames.
///
/// This method does not exist in Godot and is provided in Rust for convenience.
pub fn get_total_count(&self) -> usize {
self.get_name_count() + self.get_subname_count()
}

/// Returns a 32-bit integer hash value representing the string.
Expand All @@ -53,6 +125,40 @@ impl NodePath {
.expect("Godot hashes are uint32_t")
}

/// Returns the range `begin..exclusive_end` as a new `NodePath`.
///
/// The absolute value of `begin` and `exclusive_end` will be clamped to [`get_total_count()`][Self::get_total_count].
/// So, to express "until the end", you can simply pass a large value for `exclusive_end`, such as `i32::MAX`.
///
/// If either `begin` or `exclusive_end` are negative, they will be relative to the end of the `NodePath`. \
/// For example, `path.subpath(0, -2)` is a shorthand for `path.subpath(0, path.get_total_count() - 2)`.
///
/// _Godot equivalent: `slice`_
///
/// # Compatibility
/// The `slice()` behavior for Godot <= 4.3 is unintuitive, see [#100954](https://github.com/godotengine/godot/pull/100954). godot-rust
/// automatically changes this to the fixed version for Godot 4.4+, even when used in older versions. So, the behavior is always the same.
// i32 used because it can be negative and many Godot APIs use this, see https://github.com/godot-rust/gdext/pull/982/files#r1893732978.
#[cfg(since_api = "4.3")]
#[doc(alias = "slice")]
pub fn subpath(&self, begin: i32, exclusive_end: i32) -> NodePath {
// Polyfill for bug https://github.com/godotengine/godot/pull/100954.
// TODO(v0.3) make polyfill (everything but last line) conditional if PR is merged in 4.4.
let name_count = self.get_name_count() as i32;
let subname_count = self.get_subname_count() as i32;
let total_count = name_count + subname_count;

let mut begin = begin.clamp(-total_count, total_count);
if begin < 0 {
begin += total_count;
}
if begin > name_count {
begin += 1;
}

self.as_inner().slice(begin as i64, exclusive_end as i64)
}

crate::meta::declare_arg_method! {
/// Use as argument for an [`impl AsArg<GString|StringName>`][crate::meta::AsArg] parameter.
///
Expand Down
12 changes: 4 additions & 8 deletions itest/rust/src/builtin_tests/string/gstring_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

use std::collections::HashSet;

use crate::framework::{expect_panic, itest};
use crate::framework::{expect_debug_panic_or_release_ok, itest};
use godot::builtin::{GString, PackedStringArray};

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

#[cfg(debug_assertions)]
expect_panic("Debug mode: unicode_at() out-of-bounds panics", || {
s.unicode_at(4);
});

// Release mode: out-of-bounds prints Godot error, but returns 0.
#[cfg(not(debug_assertions))]
assert_eq!(s.unicode_at(4), '\0');
expect_debug_panic_or_release_ok("unicode_at() out-of-bounds panics", || {
assert_eq!(s.unicode_at(4), '\0');
});
}

#[itest]
Expand Down
45 changes: 44 additions & 1 deletion itest/rust/src/builtin_tests/string/node_path_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

use std::collections::HashSet;

use crate::framework::itest;
use crate::framework::{expect_debug_panic_or_release_ok, itest};
use godot::builtin::{GString, NodePath};

#[itest]
Expand Down Expand Up @@ -83,3 +83,46 @@ fn node_path_with_null() {
assert_eq!(left, right);
}
}

#[itest]
#[cfg(since_api = "4.3")]
fn node_path_subpath() {
let path = NodePath::from("path/to/Node:with:props");
let parts = path.get_name_count() + path.get_subname_count();

assert_eq!(path.subpath(0, 1), "path".into());
assert_eq!(path.subpath(1, 2), "to".into());
assert_eq!(path.subpath(2, 3), "Node".into());
assert_eq!(path.subpath(3, 4), ":with".into());
assert_eq!(path.subpath(4, 5), ":props".into());

assert_eq!(path.subpath(1, -1), "to/Node:with".into());
assert_eq!(path.subpath(1, parts as i32 - 1), "to/Node:with".into());
assert_eq!(path.subpath(0, -2), "path/to/Node".into());
assert_eq!(path.subpath(-3, -1), "Node:with".into());
assert_eq!(path.subpath(-2, i32::MAX), ":with:props".into());
assert_eq!(path.subpath(-1, i32::MAX), ":props".into());
}

#[itest]
fn node_path_get_name() {
let path = NodePath::from("../RigidBody2D/Sprite2D");
assert_eq!(path.get_name(0), "..".into());
assert_eq!(path.get_name(1), "RigidBody2D".into());
assert_eq!(path.get_name(2), "Sprite2D".into());

expect_debug_panic_or_release_ok("NodePath::get_name() out of bounds", || {
assert_eq!(path.get_name(3), "".into());
})
}

#[itest]
fn node_path_get_subname() {
let path = NodePath::from("Sprite2D:texture:resource_name");
assert_eq!(path.get_subname(0), "texture".into());
assert_eq!(path.get_subname(1), "resource_name".into());

expect_debug_panic_or_release_ok("NodePath::get_subname() out of bounds", || {
assert_eq!(path.get_subname(2), "".into());
})
}
8 changes: 8 additions & 0 deletions itest/rust/src/framework/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,14 @@ pub fn expect_panic(context: &str, code: impl FnOnce()) {
);
}

pub fn expect_debug_panic_or_release_ok(_context: &str, code: impl FnOnce()) {
#[cfg(debug_assertions)]
expect_panic(_context, code);

#[cfg(not(debug_assertions))]
code()
}

/// Synchronously run a thread and return result. Panics are propagated to caller thread.
#[track_caller]
pub fn quick_thread<R, F>(f: F) -> R
Expand Down
Loading