Provide feature parity for NodePath with Godot by sylbeth · Pull Request #982 · godot-rust/gdext (original) (raw)

In my opinion, consistency and principle of least surprise are important API design guidelines.

Yes, absolutely, totally agree to this, no questions asked. I will follow precedent and if it's decided it should be changed, I'll change it myself if asked. My bad for not knowing the library better.

There aren't any other places in godot-rust where negative ranges are used as wrap-around.

Then, if it's ever considered, it shouldn't be that hard to change (the commit is here for easy copy paste)

There is also the precedent of Array::subarray_{deep|shallow} and Packed*Array::subarray() , which are all called slice in Godot, and currently take usize.

I would absolutely advocate for them taking isize instead. For the mentioned earlier, i64 is not intuitive for indices or slices, isize I feel makes more sense. It would provide feature parity with Godot too. For now, I'll do exactly as those methods and use usize and clamp the value to the length. No default value for end as it is not provided in these methods. To do it, I'd use the precedent from deep/shallow and make subpath and subpath_from, at most.

Btw, maybe this method should also be called subpath instead of slice, with a doc alias 🤔

Agreed, I don't know the API for the doc alias, so if you could point me to that, I'll use it.