Skip to content

Commit 51ab4e0

Browse files
lilizoeylilizoey
committed
Changes from review
1 parent 157ffea commit 51ab4e0

File tree

19 files changed

+164
-134
lines changed

19 files changed

+164
-134
lines changed

‎godot-codegen/src/class_generator.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -925,7 +925,7 @@ fn make_native_structure(
925925
}
926926

927927
impl FromGodot for *mut #class_name {
928-
fn try_from_godot(via: Self::Via) -> Result<Self, crate::builtin::ConvertError> {
928+
fn try_from_godot(via: Self::Via) -> Result<Self, crate::builtin::meta::ConvertError> {
929929
Ok(via as Self)
930930
}
931931
}
@@ -941,7 +941,7 @@ fn make_native_structure(
941941
}
942942

943943
impl FromGodot for *const #class_name {
944-
fn try_from_godot(via: Self::Via) -> Result<Self, crate::builtin::ConvertError> {
944+
fn try_from_godot(via: Self::Via) -> Result<Self, crate::builtin::meta::ConvertError> {
945945
Ok(via as Self)
946946
}
947947
}

‎godot-codegen/src/util.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -337,9 +337,9 @@ pub fn make_enum_definition(enum_: &Enum) -> TokenStream {
337337
}
338338

339339
impl crate::builtin::meta::FromGodot for #enum_name {
340-
fn try_from_godot(via: Self::Via) -> std::result::Result<Self, crate::builtin::ConvertError> {
340+
fn try_from_godot(via: Self::Via) -> std::result::Result<Self, crate::builtin::meta::ConvertError> {
341341
<Self as crate::obj::EngineEnum>::try_from_ord(via)
342-
.ok_or_else(|| crate::builtin::ConvertError::from(crate::builtin::meta::FromGodotError::InvalidEnum).with_value(via))
342+
.ok_or_else(|| crate::builtin::meta::FromGodotError::InvalidEnum.into_error(via))
343343
}
344344
}
345345

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

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ use std::marker::PhantomData;
1414
use sys::{ffi_methods, interface_fn, GodotFfi};
1515

1616
use super::meta::{
17-
FromGodot, FromGodotError, FromVariantError, GodotConvert, GodotFfiVariant, GodotType, ToGodot,
17+
ConvertError, FromGodot, FromGodotError, FromVariantError, GodotConvert, GodotFfiVariant,
18+
GodotType, ToGodot,
1819
};
1920

2021
/// Godot's `Array` type.
@@ -346,11 +347,11 @@ impl<T: GodotType> Array<T> {
346347
if self_ty == target_ty {
347348
Ok(self)
348349
} else {
349-
Err(ConvertError::from(FromGodotError::BadArrayType {
350+
Err(FromGodotError::BadArrayType {
350351
expected: target_ty,
351352
got: self_ty,
352-
})
353-
.with_value(self))
353+
}
354+
.into_error(self))
354355
}
355356
}
356357

@@ -801,11 +802,11 @@ impl<T: GodotType> GodotFfiVariant for Array<T> {
801802

802803
fn ffi_from_variant(variant: &Variant) -> Result<Self, ConvertError> {
803804
if variant.get_type() != Self::variant_type() {
804-
return Err(ConvertError::from(FromVariantError::BadType {
805+
return Err(FromVariantError::BadType {
805806
expected: Self::variant_type(),
806807
got: variant.get_type(),
807-
})
808-
.with_value(variant.clone()));
808+
}
809+
.into_error(variant.clone()));
809810
}
810811

811812
let array = unsafe {

‎godot-core/src/builtin/meta/godot_convert/convert_error.rs

Lines changed: 67 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -22,37 +22,54 @@ pub struct ConvertError {
2222
}
2323

2424
impl ConvertError {
25-
/// Create a new error for a conversion.
26-
fn new(kind: ErrorKind) -> Self {
25+
/// Create a new custom error for a conversion.
26+
pub fn new() -> Self {
2727
Self {
28-
kind,
28+
kind: ErrorKind::Custom,
2929
cause: None,
3030
value: None,
3131
}
3232
}
3333

34-
/// Create a new custom error for a conversion.
35-
pub fn custom() -> Self {
36-
Self::new(ErrorKind::Custom)
34+
fn custom() -> Self {
35+
Self {
36+
kind: ErrorKind::Custom,
37+
cause: None,
38+
value: None,
39+
}
3740
}
3841

39-
/// Add a rust-error as an underlying cause for the conversion error.
40-
pub fn with_cause<C: Into<Cause>>(mut self, cause: C) -> Self {
41-
self.cause = Some(cause.into());
42-
self
42+
/// Create a new custom error for a conversion with the value that failed to convert.
43+
pub fn new_with_value<V: fmt::Debug + 'static>(value: V) -> Self {
44+
let mut err = Self::custom();
45+
err.value = Some(Box::new(value));
46+
err
47+
}
48+
49+
/// Create a new custom error with a rust-error as an underlying cause for the conversion error.
50+
pub fn new_with_cause<C: Into<Cause>>(cause: C) -> Self {
51+
let mut err = Self::custom();
52+
err.cause = Some(cause.into());
53+
err
54+
}
55+
56+
/// Create a new custom error with a rust-error as an underlying cause for the conversion error, and the
57+
/// value that failed to convert.
58+
pub fn new_with_cause_value<C: Into<Cause>, V: fmt::Debug + 'static>(
59+
cause: C,
60+
value: V,
61+
) -> Self {
62+
let mut err = Self::custom();
63+
err.cause = Some(cause.into());
64+
err.value = Some(Box::new(value));
65+
err
4366
}
4467

4568
/// Returns the rust-error that caused this error, if one exists.
4669
pub fn cause(&self) -> Option<&(dyn Error + Send + Sync)> {
4770
self.cause.as_deref()
4871
}
4972

50-
/// Add the value that failed to be converted.
51-
pub fn with_value<V: fmt::Debug + 'static>(mut self, value: V) -> Self {
52-
self.value = Some(Box::new(value));
53-
self
54-
}
55-
5673
/// Returns the value that failed to convert, if one exists.
5774
pub fn value(&self) -> Option<&(dyn fmt::Debug)> {
5875
self.value.as_deref()
@@ -63,6 +80,12 @@ impl ConvertError {
6380
}
6481
}
6582

83+
impl Default for ConvertError {
84+
fn default() -> Self {
85+
Self::new()
86+
}
87+
}
88+
6689
impl fmt::Display for ConvertError {
6790
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
6891
match (self.description(), self.cause.as_ref()) {
@@ -89,7 +112,6 @@ impl Error for ConvertError {
89112
}
90113

91114
#[derive(Debug, PartialEq, Eq)]
92-
#[non_exhaustive]
93115
pub(crate) enum ErrorKind {
94116
FromGodot(FromGodotError),
95117
FromFfi(FromFfiError),
@@ -109,8 +131,7 @@ impl ErrorKind {
109131
}
110132

111133
/// Conversion failed during a [`FromGodot`](crate::builtin::meta::FromGodot) call.
112-
#[derive(Debug, PartialEq, Eq)]
113-
#[non_exhaustive]
134+
#[derive(Eq, PartialEq, Debug)]
114135
pub(crate) enum FromGodotError {
115136
BadArrayType {
116137
expected: array_inner::TypeInfo,
@@ -121,6 +142,14 @@ pub(crate) enum FromGodotError {
121142
}
122143

123144
impl FromGodotError {
145+
pub fn into_error<V: fmt::Debug + 'static>(self, value: V) -> ConvertError {
146+
ConvertError {
147+
kind: ErrorKind::FromGodot(self),
148+
cause: None,
149+
value: Some(Box::new(value)),
150+
}
151+
}
152+
124153
fn description(&self) -> String {
125154
match self {
126155
Self::BadArrayType { expected, got } => {
@@ -157,14 +186,8 @@ impl FromGodotError {
157186
}
158187
}
159188

160-
impl From<FromGodotError> for ConvertError {
161-
fn from(from_godot: FromGodotError) -> Self {
162-
ConvertError::new(ErrorKind::FromGodot(from_godot))
163-
}
164-
}
165-
166189
/// Conversion failed during a [`GodotType::try_from_ffi()`](crate::builtin::meta::GodotType::try_from_ffi()) call.
167-
#[derive(Debug, PartialEq, Eq)]
190+
#[derive(Eq, PartialEq, Debug)]
168191
#[non_exhaustive]
169192
pub(crate) enum FromFfiError {
170193
NullRawGd,
@@ -178,6 +201,14 @@ pub(crate) enum FromFfiError {
178201
}
179202

180203
impl FromFfiError {
204+
pub fn into_error<V: fmt::Debug + 'static>(self, value: V) -> ConvertError {
205+
ConvertError {
206+
kind: ErrorKind::FromFfi(self),
207+
cause: None,
208+
value: Some(Box::new(value)),
209+
}
210+
}
211+
181212
fn description(&self) -> String {
182213
let target = match self {
183214
Self::NullRawGd => return "`Gd` cannot be null".into(),
@@ -194,14 +225,7 @@ impl FromFfiError {
194225
}
195226
}
196227

197-
impl From<FromFfiError> for ConvertError {
198-
fn from(from_ffi: FromFfiError) -> Self {
199-
ConvertError::new(ErrorKind::FromFfi(from_ffi))
200-
}
201-
}
202-
203-
#[derive(Debug, PartialEq, Eq)]
204-
#[non_exhaustive]
228+
#[derive(Eq, PartialEq, Debug)]
205229
pub(crate) enum FromVariantError {
206230
/// Variant type does not match expected type
207231
BadType {
@@ -212,27 +236,25 @@ pub(crate) enum FromVariantError {
212236
WrongClass {
213237
expected: ClassName,
214238
},
215-
216-
/// Variant value cannot be represented in target type
217-
BadValue,
218239
}
219240

220241
impl FromVariantError {
242+
pub fn into_error<V: fmt::Debug + 'static>(self, value: V) -> ConvertError {
243+
ConvertError {
244+
kind: ErrorKind::FromVariant(self),
245+
cause: None,
246+
value: Some(Box::new(value)),
247+
}
248+
}
249+
221250
fn description(&self) -> String {
222251
match self {
223252
Self::BadType { expected, got } => {
224253
format!("expected Variant of type `{expected:?}` but got Variant of type `{got:?}`")
225254
}
226255
Self::WrongClass { expected } => {
227-
format!("got variant of wrong class, expected class `{expected}`")
256+
format!("expected class `{expected}`, got variant with wrong class")
228257
}
229-
Self::BadValue => "Variant value cannot be represented in target type".into(),
230258
}
231259
}
232260
}
233-
234-
impl From<FromVariantError> for ConvertError {
235-
fn from(from_variant: FromVariantError) -> Self {
236-
ConvertError::new(ErrorKind::FromVariant(from_variant))
237-
}
238-
}

‎godot-core/src/builtin/meta/godot_convert/impls.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@
44
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
55
*/
66

7-
use crate::builtin::{
8-
meta::{impl_godot_as_self, FromGodot, GodotConvert, GodotType, ToGodot},
9-
ConvertError, Variant,
7+
use crate::builtin::meta::{
8+
impl_godot_as_self, ConvertError, FromGodot, GodotConvert, GodotType, ToGodot,
109
};
10+
use crate::builtin::Variant;
1111
use godot_ffi as sys;
1212

1313
// The following ToGodot/FromGodot/Convert impls are auto-generated for each engine type, co-located with their definitions:
@@ -137,7 +137,7 @@ macro_rules! impl_godot_scalar {
137137
}
138138

139139
fn try_from_ffi(ffi: Self::Ffi) -> Result<Self, ConvertError> {
140-
Self::try_from(ffi).map_err(|_| ConvertError::from($err).with_value(ffi))
140+
Self::try_from(ffi).map_err(|_| $err.into_error(ffi))
141141
}
142142

143143
$(

‎godot-core/src/builtin/meta/godot_convert/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ macro_rules! impl_godot_as_self {
116116

117117
impl $crate::builtin::meta::FromGodot for $T {
118118
#[inline]
119-
fn try_from_godot(via: Self::Via) -> Result<Self, $crate::builtin::ConvertError> {
119+
fn try_from_godot(via: Self::Via) -> Result<Self, $crate::builtin::meta::ConvertError> {
120120
Ok(via)
121121
}
122122
}

‎godot-core/src/builtin/meta/return_marshal.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,10 @@
44
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
55
*/
66

7-
use crate::builtin::ConvertError;
87
use crate::obj::{Gd, GodotClass};
98
use crate::sys;
109

11-
use super::{FromGodot, GodotType};
10+
use super::{ConvertError, FromGodot, GodotType};
1211

1312
/// Specifies how the return type is marshalled in a ptrcall.
1413
#[doc(hidden)]

‎godot-core/src/builtin/meta/signature.rs

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,8 @@ macro_rules! impl_varcall_signature_for_tuple {
211211
check_varcall_error(&err, method_name, &explicit_args, varargs);
212212
});
213213

214-
<Self::Ret as FromGodot>::try_from_variant(&variant).unwrap_or_else(|err| return_error::<Self::Ret>(method_name, err))
214+
let result = <Self::Ret as FromGodot>::try_from_variant(&variant);
215+
result.unwrap_or_else(|err| return_error::<Self::Ret>(method_name, err))
215216
}
216217

217218
// Note: this is doing a ptrcall, but uses variant conversions for it
@@ -234,9 +235,10 @@ macro_rules! impl_varcall_signature_for_tuple {
234235
type_ptrs.extend(varargs.iter().map(sys::GodotFfi::sys_const));
235236

236237
// Important: this calls from_sys_init_default().
237-
PtrcallReturnT::<$R>::call(|return_ptr| {
238+
let result = PtrcallReturnT::<$R>::call(|return_ptr| {
238239
utility_fn(return_ptr, type_ptrs.as_ptr(), type_ptrs.len() as i32);
239-
}).unwrap_or_else(|err| return_error::<Self::Ret>(method_name, err))
240+
});
241+
result.unwrap_or_else(|err| return_error::<Self::Ret>(method_name, err))
240242
}
241243

242244
#[inline]
@@ -313,9 +315,10 @@ macro_rules! impl_ptrcall_signature_for_tuple {
313315
)*
314316
];
315317

316-
Rr::call(|return_ptr| {
318+
let result = Rr::call(|return_ptr| {
317319
class_fn(method_bind, object_ptr, type_ptrs.as_ptr(), return_ptr);
318-
}).unwrap_or_else(|err| return_error::<Self::Ret>(method_name, err))
320+
});
321+
result.unwrap_or_else(|err| return_error::<Self::Ret>(method_name, err))
319322
}
320323

321324
#[inline]
@@ -339,9 +342,10 @@ macro_rules! impl_ptrcall_signature_for_tuple {
339342
)*
340343
];
341344

342-
Rr::call(|return_ptr| {
345+
let result = Rr::call(|return_ptr| {
343346
builtin_fn(type_ptr, type_ptrs.as_ptr(), return_ptr, type_ptrs.len() as i32);
344-
}).unwrap_or_else(|err| return_error::<Self::Ret>(method_name, err))
347+
});
348+
result.unwrap_or_else(|err| return_error::<Self::Ret>(method_name, err))
345349
}
346350

347351
#[inline]
@@ -364,9 +368,10 @@ macro_rules! impl_ptrcall_signature_for_tuple {
364368
)*
365369
];
366370

367-
PtrcallReturnT::<$R>::call(|return_ptr| {
371+
let result = PtrcallReturnT::<$R>::call(|return_ptr| {
368372
utility_fn(return_ptr, arg_ptrs.as_ptr(), arg_ptrs.len() as i32);
369-
}).unwrap_or_else(|err| return_error::<Self::Ret>(method_name, err))
373+
});
374+
result.unwrap_or_else(|err| return_error::<Self::Ret>(method_name, err))
370375
}
371376
}
372377
};
@@ -382,8 +387,8 @@ unsafe fn varcall_arg<P: FromGodot, const N: isize>(
382387
) -> P {
383388
let variant_ref = &*Variant::ptr_from_sys(*args_ptr.offset(N));
384389

385-
P::try_from_variant(variant_ref)
386-
.unwrap_or_else(|err| param_error::<P>(method_name, N as i32, err))
390+
let result = P::try_from_variant(variant_ref);
391+
result.unwrap_or_else(|err| param_error::<P>(method_name, N as i32, err))
387392
}
388393

389394
/// Moves `ret_val` into `ret`.

‎godot-core/src/builtin/mod.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ pub use vectors::*;
5858

5959
/// Meta-information about variant types, properties and class names.
6060
pub mod meta;
61-
pub use meta::ConvertError;
6261

6362
/// Math-related functions and traits like [`ApproxEq`][math::ApproxEq].
6463
pub mod math;

0 commit comments

Comments
 (0)