Skip to content

Commit 9196f8a

Browse files
committed
Change Packed*Array::get() to return Option<T>, now that there is []
1 parent 5b80c73 commit 9196f8a

File tree

3 files changed

+43
-38
lines changed

3 files changed

+43
-38
lines changed

godot-core/src/builtin/packed_array.rs

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -188,10 +188,16 @@ macro_rules! impl_packed_array {
188188
/// # Panics
189189
///
190190
/// If `index` is out of bounds.
191-
pub fn get(&self, index: usize) -> $Element {
192-
let ptr = self.ptr(index);
193-
// SAFETY: `ptr` just verified that the index is not out of bounds.
194-
unsafe { (*ptr).clone() }
191+
pub fn get(&self, index: usize) -> Option<$Element> {
192+
if index >= self.len() {
193+
return None;
194+
}
195+
196+
// SAFETY: bounds check above.
197+
unsafe {
198+
let ptr = self.ptr_unchecked(index);
199+
Some((*ptr).clone())
200+
}
195201
}
196202

197203
/// Finds the index of an existing value in a sorted array using binary search.
@@ -293,8 +299,7 @@ macro_rules! impl_packed_array {
293299
// elements to their new position, the overhead of retrieving this element is trivial.
294300
#[doc(alias = "remove_at")]
295301
pub fn remove(&mut self, index: usize) -> $Element {
296-
self.check_bounds(index);
297-
let element = self.get(index);
302+
let element = self[index].clone(); // panics on out-of-bounds
298303
self.as_inner().remove_at(to_i64(index));
299304
element
300305
}
@@ -334,9 +339,7 @@ macro_rules! impl_packed_array {
334339
/// If `index` is out of bounds.
335340
fn check_bounds(&self, index: usize) {
336341
let len = self.len();
337-
assert!(
338-
index < len,
339-
"Array index {index} is out of bounds: length is {len}");
342+
assert!(index < len, "Array index {index} is out of bounds: length is {len}");
340343
}
341344

342345
/// Returns a pointer to the element at the given index.
@@ -346,6 +349,7 @@ macro_rules! impl_packed_array {
346349
/// If `index` is out of bounds.
347350
fn ptr(&self, index: usize) -> *const $Element {
348351
self.check_bounds(index);
352+
349353
// SAFETY: We just checked that the index is not out of bounds.
350354
let ptr = unsafe {
351355
let item_ptr: *const $IndexRetType =
@@ -356,6 +360,18 @@ macro_rules! impl_packed_array {
356360
ptr
357361
}
358362

363+
/// Returns a pointer to the element at the given index, or `None` if `index` is out of bounds.
364+
///
365+
/// # Safety
366+
/// Bound checking is responsibility of the caller.
367+
unsafe fn ptr_unchecked(&self, index: usize) -> *const $Element {
368+
let item_ptr: *const $IndexRetType = interface_fn!($operator_index_const)(self.sys(), to_i64(index));
369+
let ptr = item_ptr as *const $Element;
370+
371+
assert!(!ptr.is_null());
372+
ptr
373+
}
374+
359375
/// Returns a mutable pointer to the element at the given index.
360376
///
361377
/// # Panics
@@ -477,11 +493,11 @@ macro_rules! impl_packed_array {
477493
/// Formats `PackedArray` to match Godot's string representation.
478494
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
479495
write!(f, "[")?;
480-
for i in 0..self.len() {
496+
for (i, elem) in self.as_slice().iter().enumerate() {
481497
if i != 0 {
482498
write!(f, ", ")?;
483499
}
484-
write!(f, "{}", self.get(i))?;
500+
write!(f, "{elem}")?;
485501
}
486502
write!(f, "]")
487503
}

itest/rust/src/builtin_tests/containers/packed_array_test.rs

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,8 @@ fn packed_array_from_iterator() {
2323
let array = PackedByteArray::from_iter([1, 2]);
2424

2525
assert_eq!(array.len(), 2);
26-
assert_eq!(array.get(0), 1);
27-
assert_eq!(array.get(1), 2);
28-
}
29-
30-
#[itest]
31-
fn packed_array_from() {
32-
let array = PackedByteArray::from(&[1, 2]);
33-
34-
assert_eq!(array.len(), 2);
35-
assert_eq!(array.get(0), 1);
36-
assert_eq!(array.get(1), 2);
26+
assert_eq!(array[0], 1);
27+
assert_eq!(array[1], 2);
3728
}
3829

3930
#[itest]
@@ -78,7 +69,7 @@ fn packed_array_clone() {
7869
let clone = array.clone();
7970
array.set(0, 3);
8071

81-
assert_eq!(clone.get(0), 1);
72+
assert_eq!(clone[0], 1);
8273
}
8374

8475
#[itest]
@@ -159,11 +150,9 @@ fn packed_array_index() {
159150
fn packed_array_get() {
160151
let array = PackedByteArray::from(&[1, 2]);
161152

162-
assert_eq!(array.get(0), 1);
163-
assert_eq!(array.get(1), 2);
164-
expect_panic("Array index 2 out of bounds: length is 2", || {
165-
array.get(2);
166-
});
153+
assert_eq!(array.get(0), Some(1));
154+
assert_eq!(array.get(1), Some(2));
155+
assert_eq!(array.get(2), None);
167156
}
168157

169158
#[itest]
@@ -200,7 +189,7 @@ fn packed_array_set() {
200189
let mut array = PackedByteArray::from(&[1, 2]);
201190

202191
array.set(0, 3);
203-
assert_eq!(array.get(0), 3);
192+
assert_eq!(array[0], 3);
204193

205194
expect_panic("Array index 2 out of bounds: length is 2", move || {
206195
array.set(2, 4);
@@ -214,7 +203,7 @@ fn packed_array_push() {
214203
array.push(3);
215204

216205
assert_eq!(array.len(), 3);
217-
assert_eq!(array.get(2), 3);
206+
assert_eq!(array[2], 3);
218207
}
219208

220209
#[itest]

itest/rust/src/object_tests/virtual_methods_test.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -381,20 +381,20 @@ fn test_virtual_method_with_return() {
381381
assert_eq!(arr.len(), arr_rust.len());
382382
// can't just assert_eq because the values of some floats change slightly
383383
assert_eq_approx!(
384-
arr.at(0).to::<PackedVector3Array>().get(0),
385-
arr_rust.at(0).to::<PackedVector3Array>().get(0),
384+
arr.at(0).to::<PackedVector3Array>()[0],
385+
arr_rust.at(0).to::<PackedVector3Array>()[0],
386386
);
387387
assert_eq_approx!(
388-
real::from_f32(arr.at(2).to::<PackedFloat32Array>().get(3)),
389-
real::from_f32(arr_rust.at(2).to::<PackedFloat32Array>().get(3)),
388+
real::from_f32(arr.at(2).to::<PackedFloat32Array>()[3]),
389+
real::from_f32(arr_rust.at(2).to::<PackedFloat32Array>()[3]),
390390
);
391391
assert_eq_approx!(
392-
arr.at(3).to::<PackedColorArray>().get(0),
393-
arr_rust.at(3).to::<PackedColorArray>().get(0),
392+
arr.at(3).to::<PackedColorArray>()[0],
393+
arr_rust.at(3).to::<PackedColorArray>()[0],
394394
);
395395
assert_eq_approx!(
396-
arr.at(4).to::<PackedVector2Array>().get(0),
397-
arr_rust.at(4).to::<PackedVector2Array>().get(0),
396+
arr.at(4).to::<PackedVector2Array>()[0],
397+
arr_rust.at(4).to::<PackedVector2Array>()[0],
398398
);
399399
assert_eq!(
400400
arr.at(6).to::<PackedByteArray>(),

0 commit comments

Comments
 (0)