Skip to content

Commit e232b80

Browse files
author
toasteater
committed
Add high-level API for method argument handling
This adds the `init::method::Varargs` type, moving logic for argument checking and error reporting out of the macro, into proper library code. This is the first step in simplifying the `godot_wrap_method` macro.
1 parent 9dd92ea commit e232b80

File tree

5 files changed

+263
-53
lines changed

5 files changed

+263
-53
lines changed

gdnative-core/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ approx = "0.3.2"
2323
euclid = "0.22.1"
2424
indexmap = "1.6.0"
2525
ahash = "0.4.5"
26+
thiserror = "1.0"
2627

2728
gdnative-impl-proc-macros = { path = "../impl/proc_macros", version = "=0.9.1" }
2829

gdnative-core/src/nativescript/init.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ use crate::private::get_api;
4545

4646
use super::emplace;
4747

48+
pub mod method;
4849
pub mod property;
4950

5051
pub use self::property::{Export, ExportInfo, PropertyBuilder, Usage as PropertyUsage};
Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
use std::fmt;
2+
use std::marker::PhantomData;
3+
4+
use crate::core_types::{FromVariant, FromVariantError, Variant};
5+
6+
/// Interface to a list of borrowed method arguments with a convenient interface
7+
/// for common operations with them. Can also be used as an iterator.
8+
pub struct Varargs<'a> {
9+
idx: usize,
10+
iter: std::slice::Iter<'a, &'a Variant>,
11+
}
12+
13+
impl<'a> Varargs<'a> {
14+
/// Returns the amount of arguments left.
15+
#[inline]
16+
pub fn len(&self) -> usize {
17+
self.iter.len()
18+
}
19+
20+
#[inline]
21+
pub fn is_empty(&self) -> bool {
22+
self.len() == 0
23+
}
24+
25+
/// Returns a builder for reading the next argument, that can be used to refine
26+
/// the error message on failure.
27+
#[inline]
28+
pub fn read<T: FromVariant>(&mut self) -> ArgBuilder<'_, 'a, T> {
29+
ArgBuilder {
30+
args: self,
31+
name: None,
32+
ty: None,
33+
_marker: PhantomData,
34+
}
35+
}
36+
37+
/// Returns the remaining arguments as a slice of `Variant`s.
38+
#[inline]
39+
pub fn as_slice(&self) -> &'a [&'a Variant] {
40+
self.iter.as_slice()
41+
}
42+
43+
/// Discard the rest of the arguments, and return an error if there is any.
44+
///
45+
/// # Errors
46+
///
47+
/// If there are any excess arguments left.
48+
#[inline]
49+
pub fn done(self) -> Result<(), ArgumentError<'a>> {
50+
if self.is_empty() {
51+
Ok(())
52+
} else {
53+
Err(ArgumentError::ExcessArguments {
54+
rest: self.as_slice(),
55+
})
56+
}
57+
}
58+
59+
/// Create a typed interface from raw pointers. This is an internal interface.
60+
///
61+
/// # Safety
62+
///
63+
/// `args` must point to an array of valid `godot_variant` pointers of at least `num_args` long.
64+
#[doc(hidden)]
65+
#[inline]
66+
pub unsafe fn from_sys(num_args: libc::c_int, args: *mut *mut sys::godot_variant) -> Self {
67+
let args = std::slice::from_raw_parts(args, num_args as usize);
68+
let args = std::mem::transmute::<&[*mut sys::godot_variant], &[&Variant]>(args);
69+
Self {
70+
idx: 0,
71+
iter: args.iter(),
72+
}
73+
}
74+
}
75+
76+
impl<'a> Iterator for Varargs<'a> {
77+
type Item = &'a Variant;
78+
#[inline]
79+
fn next(&mut self) -> Option<Self::Item> {
80+
self.iter.next().copied()
81+
}
82+
}
83+
84+
/// Builder for providing additional argument information for error reporting.
85+
pub struct ArgBuilder<'r, 'a, T> {
86+
args: &'r mut Varargs<'a>,
87+
name: Option<&'r str>,
88+
ty: Option<&'r str>,
89+
_marker: PhantomData<T>,
90+
}
91+
92+
impl<'r, 'a, T> ArgBuilder<'r, 'a, T> {
93+
/// Provides a name for this argument. If an old name is already set, it is
94+
/// silently replaced.
95+
#[inline]
96+
pub fn with_name(mut self, name: &'r str) -> Self {
97+
self.name = Some(name);
98+
self
99+
}
100+
101+
/// Provides a more readable type name for this argument. If an old name is
102+
/// already set, it is silently replaced. If no type name is given, a value
103+
/// from `std::any::type_name` is used.
104+
#[inline]
105+
pub fn with_type_name(mut self, ty: &'r str) -> Self {
106+
self.ty = Some(ty);
107+
self
108+
}
109+
}
110+
111+
impl<'r, 'a, T: FromVariant> ArgBuilder<'r, 'a, T> {
112+
/// Get the converted argument value.
113+
///
114+
/// # Errors
115+
///
116+
/// If the argument is missing, or cannot be converted to the desired type.
117+
#[inline]
118+
pub fn get(self) -> Result<T, ArgumentError<'r>> {
119+
let name = self.name;
120+
let idx = self.args.idx;
121+
122+
self.get_optional()
123+
.and_then(|arg| arg.ok_or(ArgumentError::Missing { idx, name }))
124+
}
125+
126+
/// Get the argument as optional.
127+
///
128+
/// # Errors
129+
///
130+
/// If the argument is present, but cannot be converted to the desired type.
131+
#[inline]
132+
pub fn get_optional(self) -> Result<Option<T>, ArgumentError<'r>> {
133+
let Self { args, name, ty, .. } = self;
134+
let idx = args.idx;
135+
136+
if let Some(arg) = args.iter.next() {
137+
args.idx += 1;
138+
T::from_variant(arg)
139+
.map(Some)
140+
.map_err(|err| ArgumentError::CannotConvert {
141+
idx,
142+
name,
143+
value: arg,
144+
ty: ty.unwrap_or_else(|| std::any::type_name::<T>()),
145+
err,
146+
})
147+
} else {
148+
Ok(None)
149+
}
150+
}
151+
}
152+
153+
#[derive(Debug)]
154+
pub enum ArgumentError<'a> {
155+
Missing {
156+
idx: usize,
157+
name: Option<&'a str>,
158+
},
159+
CannotConvert {
160+
idx: usize,
161+
name: Option<&'a str>,
162+
ty: &'a str,
163+
value: &'a Variant,
164+
err: FromVariantError,
165+
},
166+
ExcessArguments {
167+
rest: &'a [&'a Variant],
168+
},
169+
}
170+
171+
impl<'a> std::error::Error for ArgumentError<'a> {}
172+
173+
impl<'a> fmt::Display for ArgumentError<'a> {
174+
#[inline]
175+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
176+
use ArgumentError as E;
177+
178+
match self {
179+
E::Missing {
180+
idx,
181+
name: Some(name),
182+
} => {
183+
write!(f, "missing non-optional parameter `{}` (#{})", name, idx)
184+
}
185+
E::Missing { idx, name: None } => {
186+
write!(f, "missing non-optional parameter #{}", idx)
187+
}
188+
E::CannotConvert {
189+
idx,
190+
name: Some(name),
191+
value,
192+
ty,
193+
err,
194+
} => {
195+
write!(f,
196+
"cannot convert argument `{}` (#{}, {:?}) to {}: {} (non-primitive types may impose structural checks)",
197+
name, idx, value, ty, err
198+
)
199+
}
200+
E::CannotConvert {
201+
idx,
202+
name: None,
203+
value,
204+
ty,
205+
err,
206+
} => {
207+
write!(f,
208+
"cannot convert argument #{} ({:?}) to {}: {} (non-primitive types may impose structural checks)",
209+
idx, value, ty, err
210+
)
211+
}
212+
E::ExcessArguments { rest } => {
213+
if rest.len() > 1 {
214+
write!(
215+
f,
216+
"{} excessive arguments are given: {:?}",
217+
rest.len(),
218+
rest
219+
)
220+
} else {
221+
write!(f, "an excessive argument is given: {:?}", rest[0])
222+
}
223+
}
224+
}
225+
}
226+
}

gdnative-core/src/nativescript/macros.rs

Lines changed: 33 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ macro_rules! godot_wrap_method_inner {
106106

107107
use std::panic::{self, AssertUnwindSafe};
108108
use $crate::nativescript::{NativeClass, Instance, RefInstance, OwnerArg};
109+
use $crate::nativescript::init::method::Varargs;
109110
use $crate::object::{GodotObject, Ref, TRef};
110111

111112
if user_data.is_null() {
@@ -132,66 +133,47 @@ macro_rules! godot_wrap_method_inner {
132133
let this: TRef<'_, <$type_name as NativeClass>::Base, _> = this.assume_safe_unchecked();
133134
let __instance: RefInstance<'_, $type_name, _> = RefInstance::from_raw_unchecked(this, user_data);
134135

135-
let num_args = num_args as isize;
136-
137-
let num_required_params = $crate::godot_wrap_method_parameter_count!($($pname,)*);
138-
if num_args < num_required_params {
139-
$crate::godot_error!("Incorrect number of parameters: required {} but got {}", num_required_params, num_args);
140-
return $crate::core_types::Variant::new();
141-
}
142-
143-
let num_optional_params = $crate::godot_wrap_method_parameter_count!($($opt_pname,)*);
144-
let num_max_params = num_required_params + num_optional_params;
145-
if num_args > num_max_params {
146-
$crate::godot_error!("Incorrect number of parameters: expected at most {} but got {}", num_max_params, num_args);
147-
return $crate::core_types::Variant::new();
148-
}
136+
let mut args = Varargs::from_sys(num_args, args);
137+
let mut is_failure = false;
149138

150139
let mut offset = 0;
151140
$(
152-
let _variant: &$crate::core_types::Variant = ::std::mem::transmute(&mut **(args.offset(offset)));
153-
let $pname = match <$pty as $crate::core_types::FromVariant>::from_variant(_variant) {
154-
Ok(val) => val,
155-
Err(err) => {
156-
$crate::godot_error!(
157-
"Cannot convert argument #{idx} ({name}) to {ty}: {err} (non-primitive types may impose structural checks)",
158-
idx = offset + 1,
159-
name = stringify!($pname),
160-
ty = stringify!($pty),
161-
err = err,
162-
);
163-
return $crate::core_types::Variant::new();
164-
},
165-
};
166-
167-
offset += 1;
141+
let $pname = args.read()
142+
.with_name(stringify!($pname))
143+
.with_type_name(stringify!($pty))
144+
.get()
145+
.map_err(|err| {
146+
$crate::godot_error!("{}", err);
147+
is_failure = true;
148+
})
149+
.ok();
168150
)*
169151

170152
$(
171-
let $opt_pname = if offset < num_args {
172-
let _variant: &$crate::core_types::Variant = ::std::mem::transmute(&mut **(args.offset(offset)));
153+
let $opt_pname = args.read()
154+
.with_name(stringify!($opt_pname))
155+
.with_type_name(stringify!($opt_pty))
156+
.get_optional()
157+
.map_err(|err| {
158+
$crate::godot_error!("{}", err);
159+
is_failure = true;
160+
})
161+
.ok()
162+
.flatten()
163+
.unwrap_or_default();
164+
)*
173165

174-
let $opt_pname = match <$opt_pty as $crate::core_types::FromVariant>::from_variant(_variant) {
175-
Ok(val) => val,
176-
Err(err) => {
177-
$crate::godot_error!(
178-
"Cannot convert argument #{idx} ({name}) to {ty}: {err} (non-primitive types may impose structural checks)",
179-
idx = offset + 1,
180-
name = stringify!($opt_pname),
181-
ty = stringify!($opt_pty),
182-
err = err,
183-
);
184-
return $crate::core_types::Variant::new();
185-
},
186-
};
166+
if let Err(err) = args.done() {
167+
$crate::godot_error!("{}", err);
168+
is_failure = true;
169+
}
187170

188-
offset += 1;
171+
if is_failure {
172+
return $crate::core_types::Variant::new();
173+
}
189174

190-
$opt_pname
191-
}
192-
else {
193-
<$opt_pty as ::std::default::Default>::default()
194-
};
175+
$(
176+
let $pname = $pname.unwrap();
195177
)*
196178

197179
let __ret = __instance

test/project/main.gd

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ func _assert_choose(expected, foo, fun, a, which, b):
6565
func _test_optional_args():
6666
print(" -- _test_optional_args")
6767
print(" -- expected error messages for edge cases:")
68-
print(" -- Incorrect number of parameters: required 2 but got 1")
69-
print(" -- Incorrect number of parameters: expected at most 5 but got 6")
68+
print(" -- missing non-optional parameter `b` (#1)")
69+
print(" -- 1 excessive argument is given: [I64(6)]")
7070
print(" -- the test is successful when and only when these errors are shown")
7171

7272
var script = NativeScript.new()

0 commit comments

Comments
 (0)