diff --git a/examples/dodge-the-creeps/rust/src/player.rs b/examples/dodge-the-creeps/rust/src/player.rs index 3d2bffb74..345e1dab4 100644 --- a/examples/dodge-the-creeps/rust/src/player.rs +++ b/examples/dodge-the-creeps/rust/src/player.rs @@ -24,7 +24,7 @@ impl Player { .base() .get_node_as::("CollisionShape2D"); - collision_shape.set_deferred("disabled".into(), true.to_variant()); + collision_shape.set_deferred("disabled".into(), &true.to_variant()); } #[func] diff --git a/godot-codegen/src/context.rs b/godot-codegen/src/context.rs index 752420e2c..a1e9d7548 100644 --- a/godot-codegen/src/context.rs +++ b/godot-codegen/src/context.rs @@ -64,14 +64,19 @@ impl<'a> Context<'a> { } // Populate class lookup by name - println!("-- add engine class {}", class_name.description()); engine_classes.insert(class_name.clone(), class); // Populate derived-to-base relations if let Some(base) = class.inherits.as_ref() { let base_name = TyName::from_godot(base); - println!(" -- inherits {}", base_name.description()); + println!( + "* Add engine class {} <- inherits {}", + class_name.description(), + base_name.description() + ); ctx.inheritance_tree.insert(class_name.clone(), base_name); + } else { + println!("* Add engine class {}", class_name.description()); } // Populate notification constants (first, only for classes that declare them themselves). @@ -238,6 +243,12 @@ impl<'a> Context<'a> { self.builtin_types.contains(ty_name) } + pub fn is_builtin_copy(&self, ty_name: &str) -> bool { + debug_assert!(!ty_name.starts_with("Packed")); // Already handled separately. + + !matches!(ty_name, "Variant" | "VariantArray" | "Dictionary") + } + pub fn is_native_structure(&self, ty_name: &str) -> bool { self.native_structures_types.contains(ty_name) } diff --git a/godot-codegen/src/conv/type_conversions.rs b/godot-codegen/src/conv/type_conversions.rs index 83002c3c3..7b6290a04 100644 --- a/godot-codegen/src/conv/type_conversions.rs +++ b/godot-codegen/src/conv/type_conversions.rs @@ -20,6 +20,7 @@ use crate::util::ident; // ---------------------------------------------------------------------------------------------------------------------------------------------- // Godot -> Rust types +/// Returns `(identifier, is_copy)` for a hardcoded Rust type, if it exists. fn to_hardcoded_rust_ident(full_ty: &GodotTy) -> Option<&str> { let ty = full_ty.ty.as_str(); let meta = full_ty.meta.as_deref(); @@ -95,13 +96,25 @@ pub(crate) fn to_rust_type_abi(ty: &str, ctx: &mut Context) -> (RustTy, bool) { "Object*" => { is_obj = true; RustTy::RawPointer { - inner: Box::new(RustTy::BuiltinIdent(ident("c_void"))), + inner: Box::new(RustTy::BuiltinIdent { + ty: ident("c_void"), + is_copy: true, + }), is_const: false, } } - "int" => RustTy::BuiltinIdent(ident("i32")), - "float" => RustTy::BuiltinIdent(ident("f32")), - "double" => RustTy::BuiltinIdent(ident("f64")), + "int" => RustTy::BuiltinIdent { + ty: ident("i32"), + is_copy: true, + }, + "float" => RustTy::BuiltinIdent { + ty: ident("f32"), + is_copy: true, + }, + "double" => RustTy::BuiltinIdent { + ty: ident("f64"), + is_copy: true, + }, _ => to_rust_type(ty, None, ctx), }; @@ -137,6 +150,7 @@ fn to_rust_type_uncached(full_ty: &GodotTy, ctx: &mut Context) -> RustTy { if is_builtin_type_scalar(ty) { ident(ty) } else { + // Convert as-is. Includes StringName and NodePath. TyName::from_godot(ty).rust_ty } } @@ -163,7 +177,10 @@ fn to_rust_type_uncached(full_ty: &GodotTy, ctx: &mut Context) -> RustTy { // Only place where meta is relevant is here. if !ty.starts_with("typedarray::") { if let Some(hardcoded) = to_hardcoded_rust_ident(full_ty) { - return RustTy::BuiltinIdent(ident(hardcoded)); + return RustTy::BuiltinIdent { + ty: ident(hardcoded), + is_copy: ctx.is_builtin_copy(hardcoded), + }; } } @@ -182,7 +199,10 @@ fn to_rust_type_uncached(full_ty: &GodotTy, ctx: &mut Context) -> RustTy { } else if let Some(packed_arr_ty) = ty.strip_prefix("Packed") { // Don't trigger on PackedScene ;P if packed_arr_ty.ends_with("Array") { - return RustTy::BuiltinIdent(rustify_ty(ty)); + return RustTy::BuiltinIdent { + ty: rustify_ty(ty), + is_copy: false, // Packed arrays are not Copy. + }; } } else if let Some(elem_ty) = ty.strip_prefix("typedarray::") { let rust_elem_ty = to_rust_type(elem_ty, full_ty.meta.as_ref(), ctx); @@ -200,8 +220,12 @@ fn to_rust_type_uncached(full_ty: &GodotTy, ctx: &mut Context) -> RustTy { // Note: do not check if it's a known engine class, because that will not work in minimal mode (since not all classes are stored) if ctx.is_builtin(ty) || ctx.is_native_structure(ty) { - // Unchanged - RustTy::BuiltinIdent(rustify_ty(ty)) + // Unchanged. + // Native structures might not all be Copy, but they should have value semantics. + RustTy::BuiltinIdent { + ty: rustify_ty(ty), + is_copy: ctx.is_builtin_copy(ty), + } } else { let ty = rustify_ty(ty); let qualified_class = quote! { crate::classes::#ty }; @@ -263,7 +287,9 @@ fn to_rust_expr_inner(expr: &str, ty: &RustTy, is_inner: bool) -> TokenStream { "{}" => return quote! { Dictionary::new() }, "null" => { return match ty { - RustTy::BuiltinIdent(ident) if ident == "Variant" => quote! { Variant::nil() }, + RustTy::BuiltinIdent { ty: ident, .. } if ident == "Variant" => { + quote! { Variant::nil() } + } RustTy::EngineClass { .. } => { quote! { Gd::null_arg() } } @@ -273,8 +299,8 @@ fn to_rust_expr_inner(expr: &str, ty: &RustTy, is_inner: bool) -> TokenStream { // empty string appears only for Callable/Rid in 4.0; default ctor syntax in 4.1+ "" | "RID()" | "Callable()" if !is_inner => { return match ty { - RustTy::BuiltinIdent(ident) if ident == "Rid" => quote! { Rid::Invalid }, - RustTy::BuiltinIdent(ident) if ident == "Callable" => { + RustTy::BuiltinIdent { ty: ident, .. } if ident == "Rid" => quote! { Rid::Invalid }, + RustTy::BuiltinIdent { ty: ident, .. } if ident == "Callable" => { quote! { Callable::invalid() } } _ => panic!("empty string not representable in target type {ty:?}"), @@ -295,9 +321,11 @@ fn to_rust_expr_inner(expr: &str, ty: &RustTy, is_inner: bool) -> TokenStream { is_bitfield: false, .. } => quote! { crate::obj::EngineEnum::from_ord(#lit) }, - RustTy::BuiltinIdent(ident) if ident == "Variant" => quote! { Variant::from(#lit) }, + RustTy::BuiltinIdent { ty: ident, .. } if ident == "Variant" => { + quote! { Variant::from(#lit) } + } - RustTy::BuiltinIdent(ident) + RustTy::BuiltinIdent { ty: ident, .. } if ident == "i64" || ident == "f64" || unmap_meta(ty).is_some() => { suffixed_lit(num, ident) @@ -313,7 +341,9 @@ fn to_rust_expr_inner(expr: &str, ty: &RustTy, is_inner: bool) -> TokenStream { // Float literals (some floats already handled by integer literals) if let Ok(num) = expr.parse::() { return match ty { - RustTy::BuiltinIdent(ident) if ident == "f64" || unmap_meta(ty).is_some() => { + RustTy::BuiltinIdent { ty: ident, .. } + if ident == "f64" || unmap_meta(ty).is_some() => + { suffixed_lit(num, ident) } _ if is_inner => { @@ -331,7 +361,7 @@ fn to_rust_expr_inner(expr: &str, ty: &RustTy, is_inner: bool) -> TokenStream { quote! { #expr } } else { match ty { - RustTy::BuiltinIdent(ident) + RustTy::BuiltinIdent { ty: ident, .. } if ident == "GString" || ident == "StringName" || ident == "NodePath" => { quote! { #ident::from(#expr) } @@ -429,16 +459,28 @@ fn gdscript_to_rust_expr() { // The 'None' type is used to simulate absence of type information. Some tests are commented out, because this functionality is not // yet needed. If we ever want to reuse to_rust_expr() in other contexts, we could re-enable them. - let ty_int = RustTy::BuiltinIdent(ident("i64")); + let ty_int = RustTy::BuiltinIdent { + ty: ident("i64"), + is_copy: true, + }; let ty_int = Some(&ty_int); - let ty_int_u16 = RustTy::BuiltinIdent(ident("u16")); + let ty_int_u16 = RustTy::BuiltinIdent { + ty: ident("u16"), + is_copy: true, + }; let ty_int_u16 = Some(&ty_int_u16); - let ty_float = RustTy::BuiltinIdent(ident("f64")); + let ty_float = RustTy::BuiltinIdent { + ty: ident("f64"), + is_copy: true, + }; let ty_float = Some(&ty_float); - let ty_float_f32 = RustTy::BuiltinIdent(ident("f32")); + let ty_float_f32 = RustTy::BuiltinIdent { + ty: ident("f32"), + is_copy: true, + }; let ty_float_f32 = Some(&ty_float_f32); let ty_enum = RustTy::EngineEnum { @@ -455,7 +497,10 @@ fn gdscript_to_rust_expr() { }; let ty_bitfield = Some(&ty_bitfield); - let ty_variant = RustTy::BuiltinIdent(ident("Variant")); + let ty_variant = RustTy::BuiltinIdent { + ty: ident("Variant"), + is_copy: false, + }; let ty_variant = Some(&ty_variant); // let ty_object = RustTy::EngineClass { @@ -464,13 +509,22 @@ fn gdscript_to_rust_expr() { // }; // let ty_object = Some(&ty_object); - let ty_string = RustTy::BuiltinIdent(ident("GString")); + let ty_string = RustTy::BuiltinIdent { + ty: ident("GString"), + is_copy: true, + }; let ty_string = Some(&ty_string); - let ty_stringname = RustTy::BuiltinIdent(ident("StringName")); + let ty_stringname = RustTy::BuiltinIdent { + ty: ident("StringName"), + is_copy: true, + }; let ty_stringname = Some(&ty_stringname); - let ty_nodepath = RustTy::BuiltinIdent(ident("NodePath")); + let ty_nodepath = RustTy::BuiltinIdent { + ty: ident("NodePath"), + is_copy: true, + }; let ty_nodepath = Some(&ty_nodepath); #[rustfmt::skip] @@ -581,7 +635,7 @@ fn gdscript_to_rust_expr() { /// /// Avoids dragging along the meta type through [`RustTy::BuiltinIdent`]. pub(crate) fn unmap_meta(rust_ty: &RustTy) -> Option { - let RustTy::BuiltinIdent(rust_ty) = rust_ty else { + let RustTy::BuiltinIdent { ty: rust_ty, .. } = rust_ty else { return None; }; diff --git a/godot-codegen/src/generator/builtins.rs b/godot-codegen/src/generator/builtins.rs index 5006b2546..c6c86cfe7 100644 --- a/godot-codegen/src/generator/builtins.rs +++ b/godot-codegen/src/generator/builtins.rs @@ -95,7 +95,10 @@ fn make_builtin_class( ) -> GeneratedBuiltin { let godot_name = &class.name().godot_ty; - let RustTy::BuiltinIdent(outer_class) = conv::to_rust_type(godot_name, None, ctx) else { + let RustTy::BuiltinIdent { + ty: outer_class, .. + } = conv::to_rust_type(godot_name, None, ctx) + else { panic!("Rust type `{}` categorized wrong", godot_name) }; let inner_class = class.inner_name(); diff --git a/godot-codegen/src/generator/default_parameters.rs b/godot-codegen/src/generator/default_parameters.rs index 87b761631..24c588e69 100644 --- a/godot-codegen/src/generator/default_parameters.rs +++ b/godot-codegen/src/generator/default_parameters.rs @@ -11,7 +11,7 @@ use crate::models::domain::{FnParam, FnQualifier, Function, RustTy, TyName}; use crate::util::{ident, safe_ident}; use crate::{conv, special_cases}; use proc_macro2::{Ident, TokenStream}; -use quote::{format_ident, quote}; +use quote::{format_ident, quote, ToTokens}; pub fn make_function_definition_with_defaults( sig: &dyn Function, @@ -40,28 +40,61 @@ pub fn make_function_definition_with_defaults( let Extender { builder_ty, builder_lifetime, - builder_anon_lifetime, builder_methods, builder_fields, builder_args, builder_inits, - } = make_extender(sig, object_fn_param, default_fn_params); - - let receiver_param = &code.receiver.param; - let receiver_self = &code.receiver.self_prefix; + } = make_extender( + sig.name(), + object_fn_param, + &required_fn_params, + &default_fn_params, + ); + // ExBuilder::new() constructor signature. let FnParamTokens { - params: required_params_impl_asarg, + //params: required_params_builder_constructor, + params: required_params_class_methods, + func_general_lifetime, .. - } = functions_common::make_params_exprs(required_fn_params.iter().cloned(), true, true); - + } = functions_common::make_params_exprs( + required_fn_params.iter().cloned(), + true, + true, + false, + true, + ); + let required_params_builder_constructor = &required_params_class_methods; + + // Forwarded args by some_function() and some_function_ex(). let FnParamTokens { arg_names: required_args_internal, .. - } = functions_common::make_params_exprs(required_fn_params.into_iter(), false, false); + } = functions_common::make_params_exprs( + required_fn_params.into_iter(), + false, + false, + false, + false, + ); let return_decl = &sig.return_value().decl; + // If either the builder has a lifetime (non-static/global method), or one of its parameters is a reference, + // then we need to annotate the _ex() function with an explicit lifetime. Also adjust &self -> &'a self. + let receiver_self = &code.receiver.self_prefix; + + let (simple_receiver_param, extended_receiver_param, func_or_builder_lifetime); + if let Some(func_lt) = func_general_lifetime.as_ref().or(builder_lifetime.as_ref()) { + simple_receiver_param = &code.receiver.param; + extended_receiver_param = &code.receiver.param_lifetime_a; + func_or_builder_lifetime = Some(func_lt); + } else { + simple_receiver_param = &code.receiver.param; + extended_receiver_param = &code.receiver.param; + func_or_builder_lifetime = None; + }; + // Technically, the builder would not need a lifetime -- it could just maintain an `object_ptr` copy. // However, this increases the risk that it is used out of place (not immediately for a default-param call). // Ideally we would require &mut, but then we would need `mut Gd` objects everywhere. @@ -83,7 +116,7 @@ pub fn make_function_definition_with_defaults( impl #builder_lifetime #builder_ty #builder_lifetime { fn new( #object_param - #( #required_params_impl_asarg, )* + #( #required_params_builder_constructor, )* ) -> Self { Self { #( #builder_inits, )* @@ -102,22 +135,26 @@ pub fn make_function_definition_with_defaults( }; let functions = quote! { - #[doc = #default_parameter_usage] + // Simple function: + // Lifetime is set if any parameter is a reference. + #[doc = #default_parameter_usage] #[inline] - #vis fn #simple_fn_name( - #receiver_param - #( #required_params_impl_asarg, )* + #vis fn #simple_fn_name #func_general_lifetime ( + #simple_receiver_param + #( #required_params_class_methods, )* ) #return_decl { #receiver_self #extended_fn_name( #( #required_args_internal, )* ).done() } + // _ex() function: + // Lifetime is set if any parameter is a reference OR if the method is not static/global (and thus can refer to self). #[inline] - #vis fn #extended_fn_name( - #receiver_param - #( #required_params_impl_asarg, )* - ) -> #builder_ty #builder_anon_lifetime { + #vis fn #extended_fn_name #func_or_builder_lifetime ( + #extended_receiver_param + #( #required_params_class_methods, )* + ) -> #builder_ty #builder_lifetime { #builder_ty::new( #object_arg #( #required_args_internal, )* @@ -147,14 +184,29 @@ struct ExtenderReceiver { struct Extender { builder_ty: Ident, - builder_lifetime: TokenStream, - builder_anon_lifetime: TokenStream, + /// `None` if function is global/associated. + builder_lifetime: Option, builder_methods: Vec, builder_fields: Vec, builder_args: Vec, builder_inits: Vec, } +#[derive(Copy, Clone, Debug)] +enum ExtenderFieldConversion { + /// Regular type. + None, + + /// Object argument. + ObjectArg, + + /// Pass-by-ref type. + Reference, + + /// Pass-by-ref type, but a default param means a value needs to be stored. + ReferenceWithDefault, +} + fn make_extender_doc(sig: &dyn Function, extended_fn_name: &Ident) -> (String, TokenStream) { // Not in the above match, because this is true for both static/instance methods. // Static/instance is determined by first argument (always use fully qualified function call syntax). @@ -220,27 +272,34 @@ fn make_extender_receiver(sig: &dyn Function) -> ExtenderReceiver { } fn make_extender( - sig: &dyn Function, - object_fn_param: Option, - default_fn_params: Vec<&FnParam>, + fn_name: &str, + receiver_param: Option, + required_params: &[&FnParam], + default_params: &[&FnParam], ) -> Extender { // Note: could build a documentation string with default values here, but the Rust tokens are not very readable, // and often not helpful, such as Enum::from_ord(13). Maybe one day those could be resolved and curated. - let (lifetime, anon_lifetime) = if sig.qualifier().is_static_or_global() { - (TokenStream::new(), TokenStream::new()) + let all_fn_params = receiver_param + .iter() + .chain(required_params.iter().cloned()) + .chain(default_params.iter().cloned()); + let len = all_fn_params.size_hint().0; + + // If builder is a method with a receiver OR any *required* parameter is by-ref, use lifetime. + // Default parameters cannot be by-ref, since they need to store a default value. Potential optimization later. + let lifetime = if receiver_param.is_some() // !sig.qualifier().is_static_or_global() + || required_params.iter().any(|p| p.type_.is_pass_by_ref()) + { + Some(quote! { <'a> }) } else { - (quote! { <'a> }, quote! { <'_> }) + None }; - let all_fn_params = object_fn_param.iter().chain(sig.params().iter()); - let len = all_fn_params.size_hint().0; - let mut result = Extender { - builder_ty: format_ident!("Ex{}", conv::to_pascal_case(sig.name())), + builder_ty: format_ident!("Ex{}", conv::to_pascal_case(fn_name)), builder_lifetime: lifetime, - builder_anon_lifetime: anon_lifetime, - builder_methods: Vec::with_capacity(default_fn_params.len()), + builder_methods: Vec::with_capacity(default_params.len()), builder_fields: Vec::with_capacity(len), builder_args: Vec::with_capacity(len), builder_inits: Vec::with_capacity(len), @@ -253,20 +312,24 @@ fn make_extender( default_value, } = param; - let (field_type, needs_conversion) = type_.default_extender_field_decl(); + let (field_type, conversion) = default_extender_field_decl(type_, default_value.is_some()); // Initialize with default parameters where available, forward constructor args otherwise let init = if let Some(value) = default_value { - make_field_init(name, Some(value), needs_conversion) + make_field_init(name, Some(value), conversion) } else { - //quote! { #name } - make_field_init(name, None, needs_conversion) + make_field_init(name, None, conversion) }; - let builder_arg = if needs_conversion { - quote! { self.#name.cow_as_object_arg() } - } else { - quote! { self.#name } + let builder_arg = match conversion { + ExtenderFieldConversion::None => quote! { self.#name }, + ExtenderFieldConversion::ObjectArg => quote! { self.#name.cow_as_object_arg() }, + + // Two cases: + // * Param is required -> field type RefArg<'a, T> -> pass as-is. + // * Param is default -> field type T -> pass as &self. + ExtenderFieldConversion::ReferenceWithDefault => quote! { &self.#name }, + ExtenderFieldConversion::Reference => quote! { self.#name }, }; result.builder_fields.push(quote! { #name: #field_type }); @@ -274,10 +337,10 @@ fn make_extender( result.builder_inits.push(init); } - for param in default_fn_params { + for param in default_params { let FnParam { name, type_, .. } = param; let param_type = type_.param_decl(); - let (_, field_needs_conversion) = type_.default_extender_field_decl(); + let (_, field_needs_conversion) = default_extender_field_decl(type_, true); let field_init = make_field_init(name, Some("e! { value }), field_needs_conversion); @@ -298,16 +361,46 @@ fn make_extender( result } +/// Returns `( , )`. +fn default_extender_field_decl( + ty: &RustTy, + is_default_param: bool, +) -> (TokenStream, ExtenderFieldConversion) { + match ty { + RustTy::EngineClass { inner_class, .. } => { + let cow_tokens = quote! { ObjectCow }; + (cow_tokens, ExtenderFieldConversion::ObjectArg) + } + + // Default parameters cannot be stored by reference, as they would need a backing storage. Alternative: Cow. + ty if ty.is_pass_by_ref() => { + if is_default_param { + let ref_tokens = quote! { #ty }; + (ref_tokens, ExtenderFieldConversion::ReferenceWithDefault) + } else { + let ref_tokens = quote! { &'a #ty }; + (ref_tokens, ExtenderFieldConversion::Reference) + } + } + + other => (other.to_token_stream(), ExtenderFieldConversion::None), + } +} + // Rewrite the above using match fn make_field_init( name: &Ident, expr: Option<&TokenStream>, // None if the parameter has the same name as the field, and can use shorthand syntax. - needs_conversion: bool, + conversion: ExtenderFieldConversion, ) -> TokenStream { - match (needs_conversion, expr) { - (true, Some(expr)) => quote! { #name: #expr.consume_object() }, - (true, None) /*. */ => quote! { #name: #name.consume_object() }, - (false, Some(expr)) => quote! { #name: #expr }, - (false, None) /*. */ => quote! { #name }, + type Conv = ExtenderFieldConversion; + + match (conversion, expr) { + (Conv::ObjectArg, Some(expr)) => quote! { #name: #expr.consume_object() }, + (Conv::ObjectArg, None) /*. */ => quote! { #name: #name.consume_object() }, + + // Currently no differentiation between None|Reference|ReferenceWithDefault; this may change... + (Conv::None | Conv::Reference | Conv::ReferenceWithDefault, Some(expr)) => quote! { #name: #expr }, + (Conv::None | Conv::Reference | Conv::ReferenceWithDefault, None) /*. */ => quote! { #name }, } } diff --git a/godot-codegen/src/generator/enums.rs b/godot-codegen/src/generator/enums.rs index aea0d44bc..719d04294 100644 --- a/godot-codegen/src/generator/enums.rs +++ b/godot-codegen/src/generator/enums.rs @@ -112,7 +112,9 @@ pub fn make_enum_definition_with( } impl crate::meta::ToGodot for #name { - fn to_godot(&self) -> Self::Via { + type ToVia<'v> = #ord_type; + + fn to_godot(&self) -> Self::ToVia<'_> { ::ord(*self) } } diff --git a/godot-codegen/src/generator/functions_common.rs b/godot-codegen/src/generator/functions_common.rs index 9a5f44c45..22c37b336 100644 --- a/godot-codegen/src/generator/functions_common.rs +++ b/godot-codegen/src/generator/functions_common.rs @@ -8,7 +8,7 @@ use crate::generator::default_parameters; use crate::models::domain::{FnParam, FnQualifier, Function, RustTy}; use crate::special_cases; -use crate::util::safe_ident; +use crate::util::{lifetime, safe_ident}; use proc_macro2::{Ident, TokenStream}; use quote::{format_ident, quote}; @@ -16,6 +16,9 @@ pub struct FnReceiver { /// `&self`, `&mut self`, (none) pub param: TokenStream, + /// `&self`, `&mut self`, (none) + pub param_lifetime_a: TokenStream, + /// `ptr::null_mut()`, `self.object_ptr`, `self.sys_ptr`, (none) pub ffi_arg: TokenStream, @@ -28,6 +31,7 @@ impl FnReceiver { pub fn global_function() -> FnReceiver { FnReceiver { param: TokenStream::new(), + param_lifetime_a: TokenStream::new(), ffi_arg: TokenStream::new(), self_prefix: TokenStream::new(), } @@ -89,14 +93,46 @@ impl FnDefinitions { pub struct FnParamTokens { pub params: Vec, pub param_types: Vec, + pub param_lifetime_count: usize, pub arg_names: Vec, + pub func_general_lifetime: Option, } impl FnParamTokens { - pub fn push_regular(&mut self, param_name: &Ident, param_ty: &RustTy) { - self.params.push(quote! { #param_name: #param_ty }); - self.arg_names.push(quote! { #param_name }); - self.param_types.push(quote! { #param_ty }); + pub fn push_regular( + &mut self, + param_name: &Ident, + param_ty: &RustTy, + // TODO refactor into single enum + by_value: bool, + arg_is_ref_arg: bool, + explicit_lifetimes: bool, + ) { + if by_value { + self.params.push(quote! { #param_name: #param_ty }); + self.param_types.push(quote! { #param_ty }); + self.arg_names.push(quote! { #param_name }); + } else { + if explicit_lifetimes { + self.params.push(quote! { #param_name: &'a #param_ty }); + } else { + self.params.push(quote! { #param_name: & #param_ty }); + } + + if arg_is_ref_arg { + let lft = lifetime(&format!("a{}", self.param_lifetime_count)); + + self.param_types.push(quote! { RefArg<#lft, #param_ty> }); + self.arg_names.push(quote! { RefArg::new(#param_name) }); + + self.param_lifetime_count += 1; + } else { + self.param_types.push(quote! { #param_ty }); + self.arg_names.push(quote! { #param_name }); + + self.func_general_lifetime = Some(quote! { <'a> }); + } + } } } @@ -139,14 +175,19 @@ pub fn make_function_definition( let FnParamTokens { params, param_types, + param_lifetime_count, arg_names, + func_general_lifetime: fn_lifetime, } = if sig.is_virtual() { make_params_exprs_virtual(sig.params().iter(), sig) } else { + // primary_function() if not default-params, or full_function() otherwise. make_params_exprs( sig.params().iter(), !has_default_params, // For *_full function, we don't need impl AsObjectArg parameters !has_default_params, // or arg.as_object_arg() calls. + true, // but we do need RefArg. + false, ) }; @@ -169,9 +210,25 @@ pub fn make_function_definition( default_structs_code = TokenStream::new(); }; - let return_ty = &sig.return_value().type_tokens(); - let call_sig = quote! { - ( #return_ty, #(#param_types),* ) + let call_sig_decl = { + let return_ty = &sig.return_value().type_tokens(); + + // Build <'a0, 'a1, ...> for lifetimes. + let param_lifetimes = if param_lifetime_count > 0 { + let mut tokens = quote! { < }; + for i in 0..param_lifetime_count { + let i = lifetime(&format!("a{i}")); + tokens.extend(quote! { #i, }); + } + tokens.extend(quote! { > }); + tokens + } else { + TokenStream::new() + }; + + quote! { + type CallSig #param_lifetimes = ( #return_ty, #(#param_types),* ); + } }; let return_decl = &sig.return_value().decl; @@ -187,7 +244,7 @@ pub fn make_function_definition( quote! { #maybe_safety_doc - #maybe_unsafe fn #primary_fn_name( + #maybe_unsafe fn #primary_fn_name ( #receiver_param #( #params, )* ) #return_decl #fn_body @@ -202,12 +259,12 @@ pub fn make_function_definition( if code.receiver.param.is_empty() { quote! { #maybe_safety_doc - #vis #maybe_unsafe fn #primary_fn_name( + #vis #maybe_unsafe fn #primary_fn_name ( #receiver_param #( #params, )* varargs: &[Variant] ) #return_decl { - type CallSig = #call_sig; + #call_sig_decl let args = (#( #arg_names, )*); @@ -231,6 +288,8 @@ pub fn make_function_definition( sig.params().iter(), !has_default_params, // For *_full function, we don't need impl AsObjectArg parameters false, // or arg.as_object_arg() calls. + true, // but we do need RefArg. + false, ); quote! { @@ -238,7 +297,7 @@ pub fn make_function_definition( /// This is a _varcall_ method, meaning parameters and return values are passed as `Variant`. /// It can detect call failures and will panic in such a case. #maybe_safety_doc - #vis #maybe_unsafe fn #primary_fn_name( + #vis #maybe_unsafe fn #primary_fn_name ( #receiver_param #( #params, )* varargs: &[Variant] @@ -256,7 +315,7 @@ pub fn make_function_definition( #( #params, )* varargs: &[Variant] ) #try_return_decl { - type CallSig = #call_sig; + #call_sig_decl let args = (#( #arg_names, )*); @@ -273,11 +332,11 @@ pub fn make_function_definition( quote! { #maybe_safety_doc - #vis #maybe_unsafe fn #primary_fn_name( + #vis #maybe_unsafe fn #primary_fn_name #fn_lifetime ( #receiver_param #( #params, )* ) #return_decl { - type CallSig = #call_sig; + #call_sig_decl let args = (#( #arg_names, )*); @@ -300,11 +359,11 @@ pub fn make_function_definition( pub fn make_receiver(qualifier: FnQualifier, ffi_arg_in: TokenStream) -> FnReceiver { assert_ne!(qualifier, FnQualifier::Global, "expected class"); - let param = match qualifier { - FnQualifier::Const => quote! { &self, }, - FnQualifier::Mut => quote! { &mut self, }, - FnQualifier::Static => quote! {}, - FnQualifier::Global => quote! {}, + let (param, param_lifetime_a) = match qualifier { + FnQualifier::Const => (quote! { &self, }, quote! { &'a self, }), + FnQualifier::Mut => (quote! { &mut self, }, quote! { &'a mut self, }), + FnQualifier::Static => (quote! {}, quote! {}), + FnQualifier::Global => (quote! {}, quote! {}), }; let (ffi_arg, self_prefix); @@ -318,6 +377,7 @@ pub fn make_receiver(qualifier: FnQualifier, ffi_arg_in: TokenStream) -> FnRecei FnReceiver { param, + param_lifetime_a, ffi_arg, self_prefix, } @@ -338,6 +398,8 @@ pub(crate) fn make_params_exprs<'a>( method_args: impl Iterator, param_is_impl_asarg: bool, arg_is_asarg: bool, + arg_is_ref_arg: bool, + explicit_lifetimes: bool, ) -> FnParamTokens { let mut ret = FnParamTokens::default(); @@ -369,8 +431,19 @@ pub(crate) fn make_params_exprs<'a>( ret.param_types.push(quote! { #object_arg }); } + // Arrays and non-Copy builtins: pass by ref. + ty if ty.is_pass_by_ref() => { + ret.push_regular( + param_name, + param_ty, + false, + arg_is_ref_arg, + explicit_lifetimes, + ); + } + // All other methods and parameter types: standard handling. - _ => ret.push_regular(param_name, param_ty), + _ => ret.push_regular(param_name, param_ty, true, false, false), } } @@ -403,7 +476,8 @@ pub(crate) fn make_params_exprs_virtual<'a>( } // All other methods and parameter types: standard handling. - _ => ret.push_regular(param_name, param_ty), + // For now, virtual methods always receive their parameter by value. + _ => ret.push_regular(param_name, param_ty, true, false, false), } } diff --git a/godot-codegen/src/generator/native_structures.rs b/godot-codegen/src/generator/native_structures.rs index dc7eac77e..23a3ad850 100644 --- a/godot-codegen/src/generator/native_structures.rs +++ b/godot-codegen/src/generator/native_structures.rs @@ -105,7 +105,9 @@ fn make_native_structure( } impl ToGodot for *mut #class_name { - fn to_godot(&self) -> Self::Via { + type ToVia<'v> = i64; + + fn to_godot(&self) -> Self::ToVia<'_> { *self as i64 } } @@ -121,7 +123,9 @@ fn make_native_structure( } impl ToGodot for *const #class_name { - fn to_godot(&self) -> Self::Via { + type ToVia<'v> = i64; + + fn to_godot(&self) -> Self::ToVia<'_> { *self as i64 } } diff --git a/godot-codegen/src/models/domain.rs b/godot-codegen/src/models/domain.rs index bb91bccb5..e2cbe849b 100644 --- a/godot-codegen/src/models/domain.rs +++ b/godot-codegen/src/models/domain.rs @@ -530,6 +530,17 @@ impl FnParam { } } +impl fmt::Debug for FnParam { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let def_val = self + .default_value + .as_ref() + .map_or(String::new(), |v| format!(" (default {v})")); + + write!(f, "{}: {}{}", self.name, self.type_, def_val) + } +} + // ---------------------------------------------------------------------------------------------------------------------------------------------- pub struct FnReturn { @@ -589,7 +600,7 @@ pub struct GodotTy { #[derive(Clone, Debug)] pub enum RustTy { /// `bool`, `Vector3i`, `Array` - BuiltinIdent(Ident), + BuiltinIdent { ty: Ident, is_copy: bool }, /// `Array` /// @@ -646,29 +657,27 @@ impl RustTy { } } - /// Returns `( , )`. - pub fn default_extender_field_decl(&self) -> (TokenStream, bool) { - match self { - RustTy::EngineClass { inner_class, .. } => { - let cow_tokens = quote! { ObjectCow }; - (cow_tokens, true) - } - other => (other.to_token_stream(), false), - } - } - pub fn return_decl(&self) -> TokenStream { match self { Self::EngineClass { tokens, .. } => quote! { -> Option<#tokens> }, other => quote! { -> #other }, } } + + pub fn is_pass_by_ref(&self) -> bool { + matches!( + self, + RustTy::BuiltinIdent { is_copy: false, .. } + | RustTy::BuiltinArray { .. } + | RustTy::EngineArray { .. } + ) + } } impl ToTokens for RustTy { fn to_tokens(&self, tokens: &mut TokenStream) { match self { - RustTy::BuiltinIdent(ident) => ident.to_tokens(tokens), + RustTy::BuiltinIdent { ty: ident, .. } => ident.to_tokens(tokens), RustTy::BuiltinArray { elem_type } => elem_type.to_tokens(tokens), RustTy::RawPointer { inner, @@ -686,6 +695,12 @@ impl ToTokens for RustTy { } } +impl fmt::Display for RustTy { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.to_token_stream().to_string().replace(" ", "")) + } +} + // ---------------------------------------------------------------------------------------------------------------------------------------------- /// Contains multiple naming conventions for types (classes, builtin classes, enums). diff --git a/godot-codegen/src/special_cases/codegen_special_cases.rs b/godot-codegen/src/special_cases/codegen_special_cases.rs index 9b58a08ca..06913b8dc 100644 --- a/godot-codegen/src/special_cases/codegen_special_cases.rs +++ b/godot-codegen/src/special_cases/codegen_special_cases.rs @@ -36,7 +36,7 @@ fn is_type_excluded(ty: &str, ctx: &mut Context) -> bool { fn is_rust_type_excluded(ty: &RustTy) -> bool { match ty { - RustTy::BuiltinIdent(_) => false, + RustTy::BuiltinIdent { .. } => false, RustTy::BuiltinArray { .. } => false, RustTy::RawPointer { inner, .. } => is_rust_type_excluded(inner), RustTy::EngineArray { elem_class, .. } => is_class_excluded(elem_class.as_str()), diff --git a/godot-codegen/src/util.rs b/godot-codegen/src/util.rs index 0c62f2adc..0abc101a7 100644 --- a/godot-codegen/src/util.rs +++ b/godot-codegen/src/util.rs @@ -11,7 +11,7 @@ use crate::models::domain::ClassCodegenLevel; use crate::models::json::JsonClass; use crate::special_cases; -use proc_macro2::{Ident, Literal, TokenStream}; +use proc_macro2::{Ident, Literal, Punct, Spacing, TokenStream, TokenTree}; use quote::{format_ident, quote}; // ---------------------------------------------------------------------------------------------------------------------------------------------- @@ -25,7 +25,7 @@ pub fn make_imports() -> TokenStream { quote! { use godot_ffi as sys; use crate::builtin::*; - use crate::meta::{ClassName, PtrcallSignatureTuple, VarcallSignatureTuple}; + use crate::meta::{ClassName, PtrcallSignatureTuple, RefArg, VarcallSignatureTuple}; use crate::classes::native::*; use crate::classes::Object; use crate::obj::{Gd, AsObjectArg}; @@ -86,6 +86,13 @@ pub fn ident(s: &str) -> Ident { format_ident!("{}", s) } +pub fn lifetime(s: &str) -> TokenStream { + let tk_apostrophe = TokenTree::Punct(Punct::new('\'', Spacing::Joint)); + let tk_lifetime = TokenTree::Ident(ident(s)); + + TokenStream::from_iter([tk_apostrophe, tk_lifetime]) +} + // This function is duplicated in godot-macros\src\util\mod.rs #[rustfmt::skip] pub fn safe_ident(s: &str) -> Ident { diff --git a/godot-core/src/builtin/callable.rs b/godot-core/src/builtin/callable.rs index 6e5da598f..a88ff657d 100644 --- a/godot-core/src/builtin/callable.rs +++ b/godot-core/src/builtin/callable.rs @@ -173,14 +173,14 @@ impl Callable { /// - If called on an invalid Callable then no error is printed, and `NIL` is returned. /// /// _Godot equivalent: `callv`_ - pub fn callv(&self, arguments: VariantArray) -> Variant { + pub fn callv(&self, arguments: &VariantArray) -> Variant { self.as_inner().callv(arguments) } /// Returns a copy of this Callable with one or more arguments bound, reading them from an array. /// /// _Godot equivalent: `bindv`_ - pub fn bindv(&self, arguments: VariantArray) -> Self { + pub fn bindv(&self, arguments: &VariantArray) -> Self { self.as_inner().bindv(arguments) } diff --git a/godot-core/src/builtin/collections/array.rs b/godot-core/src/builtin/collections/array.rs index f7f424467..fc69a7d40 100644 --- a/godot-core/src/builtin/collections/array.rs +++ b/godot-core/src/builtin/collections/array.rs @@ -4,7 +4,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ - +use std::borrow::Borrow; use std::fmt; use std::marker::PhantomData; @@ -196,12 +196,12 @@ impl Array { /// Returns `true` if the array contains the given value. Equivalent of `has` in GDScript. pub fn contains(&self, value: &T) -> bool { - self.as_inner().has(value.to_variant()) + self.as_inner().has(&value.to_variant()) } /// Returns the number of times a value is in the array. pub fn count(&self, value: &T) -> usize { - to_usize(self.as_inner().count(value.to_variant())) + to_usize(self.as_inner().count(&value.to_variant())) } /// Returns the number of elements in the array. Equivalent of `size()` in Godot. @@ -258,26 +258,31 @@ impl Array { /// Sets the value at the specified index. /// + /// `value` uses `Borrow` to accept either by value or by reference. + /// /// # Panics /// /// If `index` is out of bounds. - pub fn set(&mut self, index: usize, value: T) { + pub fn set(&mut self, index: usize, value: impl Borrow) { let ptr_mut = self.ptr_mut(index); + let variant = value.borrow().to_variant(); + // SAFETY: `ptr_mut` just checked that the index is not out of bounds. - unsafe { - value.to_variant().move_into_var_ptr(ptr_mut); - } + unsafe { variant.move_into_var_ptr(ptr_mut) }; } /// Appends an element to the end of the array. /// + /// `value` uses `Borrow` to accept either by value or by reference. + /// /// _Godot equivalents: `append` and `push_back`_ #[doc(alias = "append")] #[doc(alias = "push_back")] - pub fn push(&mut self, value: T) { + pub fn push(&mut self, value: impl Borrow) { // SAFETY: The array has type `T` and we're writing a value of type `T` to it. - unsafe { self.as_inner_mut() }.push_back(value.to_variant()); + let mut inner = unsafe { self.as_inner_mut() }; + inner.push_back(&value.borrow().to_variant()); } /// Adds an element at the beginning of the array, in O(n). @@ -286,7 +291,7 @@ impl Array { /// The larger the array, the slower `push_front()` will be. pub fn push_front(&mut self, value: T) { // SAFETY: The array has type `T` and we're writing a value of type `T` to it. - unsafe { self.as_inner_mut() }.push_front(value.to_variant()); + unsafe { self.as_inner_mut() }.push_front(&value.to_variant()); } /// Removes and returns the last element of the array. Returns `None` if the array is empty. @@ -328,7 +333,7 @@ impl Array { ); // SAFETY: The array has type `T` and we're writing a value of type `T` to it. - unsafe { self.as_inner_mut() }.insert(to_i64(index), value.to_variant()); + unsafe { self.as_inner_mut() }.insert(to_i64(index), &value.to_variant()); } /// ⚠️ Removes and returns the element at the specified index. Equivalent of `pop_at` in GDScript. @@ -356,14 +361,14 @@ impl Array { /// elements after the removed element. pub fn erase(&mut self, value: &T) { // SAFETY: We don't write anything to the array. - unsafe { self.as_inner_mut() }.erase(value.to_variant()); + unsafe { self.as_inner_mut() }.erase(&value.to_variant()); } /// Assigns the given value to all elements in the array. This can be used together with /// `resize` to create an array with a given size and initialized elements. pub fn fill(&mut self, value: &T) { // SAFETY: The array has type `T` and we're writing values of type `T` to it. - unsafe { self.as_inner_mut() }.fill(value.to_variant()); + unsafe { self.as_inner_mut() }.fill(&value.to_variant()); } /// Resizes the array to contain a different number of elements. @@ -381,7 +386,7 @@ impl Array { // If new_size < original_size then this is an empty iterator and does nothing. for i in original_size..new_size { - self.set(i, value.to_godot()); + self.set(i, value); } } @@ -403,9 +408,9 @@ impl Array { } /// Appends another array at the end of this array. Equivalent of `append_array` in GDScript. - pub fn extend_array(&mut self, other: Array) { + pub fn extend_array(&mut self, other: &Array) { // SAFETY: `append_array` will only read values from `other`, and all types can be converted to `Variant`. - let other: VariantArray = unsafe { other.assume_type::() }; + let other: &VariantArray = unsafe { other.assume_type_ref::() }; // SAFETY: `append_array` will only write values gotten from `other` into `self`, and all values in `other` are guaranteed // to be of type `T`. @@ -532,7 +537,7 @@ impl Array { /// not found. Starts searching at index `from`; pass `None` to search the entire array. pub fn find(&self, value: &T, from: Option) -> Option { let from = to_i64(from.unwrap_or(0)); - let index = self.as_inner().find(value.to_variant(), from); + let index = self.as_inner().find(&value.to_variant(), from); if index >= 0 { Some(index.try_into().unwrap()) } else { @@ -545,7 +550,7 @@ impl Array { /// array. pub fn rfind(&self, value: &T, from: Option) -> Option { let from = from.map(to_i64).unwrap_or(-1); - let index = self.as_inner().rfind(value.to_variant(), from); + let index = self.as_inner().rfind(&value.to_variant(), from); // It's not documented, but `rfind` returns -1 if not found. if index >= 0 { Some(to_usize(index)) @@ -562,7 +567,7 @@ impl Array { /// /// Calling `bsearch` on an unsorted array results in unspecified behavior. pub fn bsearch(&self, value: &T) -> usize { - to_usize(self.as_inner().bsearch(value.to_variant(), true)) + to_usize(self.as_inner().bsearch(&value.to_variant(), true)) } /// Finds the index of an existing value in a sorted array using binary search. @@ -580,7 +585,7 @@ impl Array { pub fn bsearch_custom(&self, value: &T, func: Callable) -> usize { to_usize( self.as_inner() - .bsearch_custom(value.to_variant(), func, true), + .bsearch_custom(&value.to_variant(), func, true), ) } @@ -829,6 +834,19 @@ impl Array { } } + /// Returns a clone of the array without checking the resulting type. + /// + /// # Safety + /// Should be used only in scenarios where the caller can guarantee that the resulting array will have the correct type, + /// or when an incorrect Rust type is acceptable (passing raw arrays to Godot FFI). + unsafe fn clone_unchecked(&self) -> Self { + Self::new_with_uninit(|self_ptr| { + let ctor = sys::builtin_fn!(array_construct_copy); + let args = [self.sys()]; + ctor(self_ptr, args.as_ptr()); + }) + } + /// Whether this array is untyped and holds `Variant` elements (compile-time check). /// /// Used as `if` statement in trait impls. Avoids defining yet another trait or non-local overridden function just for this case; @@ -888,12 +906,15 @@ impl GodotConvert for Array { } impl ToGodot for Array { - fn to_godot(&self) -> Self::Via { - self.clone() - } + type ToVia<'v> = Self::Via; - fn into_godot(self) -> Self::Via { - self + fn to_godot(&self) -> Self::ToVia<'_> { + // SAFETY: only safe when passing to FFI in a context where Rust-side type doesn't matter. + // TODO: avoid unsafety with either of the following: + // * OutArray -- https://github.com/godot-rust/gdext/pull/806. + // * Instead of cloning, use ArgRef>. + unsafe { self.clone_unchecked() } + //self.clone() } fn to_variant(&self) -> Variant { @@ -945,13 +966,8 @@ impl fmt::Display for Array { impl Clone for Array { fn clone(&self) -> Self { // SAFETY: `self` is a valid array, since we have a reference that keeps it alive. - let copy = unsafe { - Self::new_with_uninit(|self_ptr| { - let ctor = sys::builtin_fn!(array_construct_copy); - let args = [self.sys()]; - ctor(self_ptr, args.as_ptr()); - }) - }; + // Type-check follows below. + let copy = unsafe { self.clone_unchecked() }; // Double-check copy's runtime type in Debug mode. if cfg!(debug_assertions) { @@ -1032,8 +1048,9 @@ impl GodotType for Array { type Ffi = Self; fn to_ffi(&self) -> Self::Ffi { - // `to_ffi` is sometimes intentionally called with an array in an invalid state. - self.clone() + // SAFETY: we may pass type-transmuted arrays to FFI (e.g. Array as Array). This would fail the regular + // type-check in clone(), so we disable it. Type invariants are upheld by the "front-end" APIs. + unsafe { self.clone_unchecked() } } fn into_ffi(self) -> Self::Ffi { @@ -1145,10 +1162,10 @@ impl Extend for Array { // Unfortunately the GDExtension API does not offer the equivalent of `Vec::reserve`. // Otherwise, we could use it to pre-allocate based on `iter.size_hint()`. // - // A faster implementation using `resize()` and direct pointer writes might still be - // possible. + // A faster implementation using `resize()` and direct pointer writes might still be possible. + // Note that this could technically also use iter(), since no moves need to happen (however Extend requires IntoIterator). for item in iter.into_iter() { - self.push(item); + self.push(&item); } } } diff --git a/godot-core/src/builtin/collections/dictionary.rs b/godot-core/src/builtin/collections/dictionary.rs index a09555b33..828bc1fbb 100644 --- a/godot-core/src/builtin/collections/dictionary.rs +++ b/godot-core/src/builtin/collections/dictionary.rs @@ -134,7 +134,7 @@ impl Dictionary { /// /// _Godot equivalent: `dict.get(key, null)`_ pub fn get_or_nil(&self, key: K) -> Variant { - self.as_inner().get(key.to_variant(), Variant::nil()) + self.as_inner().get(&key.to_variant(), &Variant::nil()) } /// Returns `true` if the dictionary contains the given key. @@ -143,14 +143,14 @@ impl Dictionary { #[doc(alias = "has")] pub fn contains_key(&self, key: K) -> bool { let key = key.to_variant(); - self.as_inner().has(key) + self.as_inner().has(&key) } /// Returns `true` if the dictionary contains all the given keys. /// /// _Godot equivalent: `has_all`_ #[doc(alias = "has_all")] - pub fn contains_all_keys(&self, keys: VariantArray) -> bool { + pub fn contains_all_keys(&self, keys: &VariantArray) -> bool { self.as_inner().has_all(keys) } @@ -177,7 +177,7 @@ impl Dictionary { /// _Godot equivalent: `find_key`_ #[doc(alias = "find_key")] pub fn find_key_by_value(&self, value: V) -> Option { - let key = self.as_inner().find_key(value.to_variant()); + let key = self.as_inner().find_key(&value.to_variant()); if !key.is_nil() || self.contains_key(key.clone()) { Some(key) @@ -224,7 +224,7 @@ impl Dictionary { pub fn remove(&mut self, key: K) -> Option { let key = key.to_variant(); let old_value = self.get(key.clone()); - self.as_inner().erase(key); + self.as_inner().erase(&key); old_value } @@ -256,7 +256,7 @@ impl Dictionary { /// /// _Godot equivalent: `merge`_ #[doc(alias = "merge")] - pub fn extend_dictionary(&mut self, other: Self, overwrite: bool) { + pub fn extend_dictionary(&mut self, other: &Self, overwrite: bool) { self.as_inner().merge(other, overwrite) } @@ -481,7 +481,7 @@ impl<'a> DictionaryIter<'a> { return None; } - let value = self.dictionary.as_inner().get(key.clone(), Variant::nil()); + let value = self.dictionary.as_inner().get(&key, &Variant::nil()); Some((key, value)) } diff --git a/godot-core/src/builtin/collections/packed_array.rs b/godot-core/src/builtin/collections/packed_array.rs index 3427af0d2..06891b57c 100644 --- a/godot-core/src/builtin/collections/packed_array.rs +++ b/godot-core/src/builtin/collections/packed_array.rs @@ -184,7 +184,7 @@ macro_rules! impl_packed_array { /// Appends another array at the end of this array. Equivalent of `append_array` in GDScript. pub fn extend_array(&mut self, other: &$PackedArray) { - self.as_inner().append_array(other.clone()); + self.as_inner().append_array(other); } /// Converts this array to a Rust vector, making a copy of its contents. diff --git a/godot-core/src/builtin/string/mod.rs b/godot-core/src/builtin/string/mod.rs index ceafd2cba..002ff3c48 100644 --- a/godot-core/src/builtin/string/mod.rs +++ b/godot-core/src/builtin/string/mod.rs @@ -24,7 +24,10 @@ impl GodotConvert for &str { } impl ToGodot for &str { - fn to_godot(&self) -> Self::Via { + type ToVia<'v> = GString + where Self: 'v; + + fn to_godot(&self) -> Self::ToVia<'_> { GString::from(*self) } } @@ -34,7 +37,9 @@ impl GodotConvert for String { } impl ToGodot for String { - fn to_godot(&self) -> Self::Via { + type ToVia<'v> = Self::Via; + + fn to_godot(&self) -> Self::ToVia<'_> { GString::from(self) } } diff --git a/godot-core/src/builtin/vectors/vector_axis.rs b/godot-core/src/builtin/vectors/vector_axis.rs index 540ce7c30..e47ffe262 100644 --- a/godot-core/src/builtin/vectors/vector_axis.rs +++ b/godot-core/src/builtin/vectors/vector_axis.rs @@ -61,7 +61,9 @@ macro_rules! impl_vector_axis_enum { } impl ToGodot for $AxisEnum { - fn to_godot(&self) -> Self::Via { + type ToVia<'v> = i32; + + fn to_godot(&self) -> Self::ToVia<'_> { self.ord() } } diff --git a/godot-core/src/global/mod.rs b/godot-core/src/global/mod.rs index 170a73c5d..b8525a17c 100644 --- a/godot-core/src/global/mod.rs +++ b/godot-core/src/global/mod.rs @@ -50,7 +50,7 @@ pub fn instance_from_id(instance_id: i64) -> Option> #[deprecated = "Instance utilities in `godot::global` will be removed. Use methods on `Gd` and `InstanceId` instead.\n\ For detailed reasons, see https://github.com/godot-rust/gdext/pull/892."] pub fn is_instance_valid(instance: Variant) -> bool { - crate::gen::utilities::is_instance_valid(instance) + crate::gen::utilities::is_instance_valid(&instance) } #[deprecated = "Instance utilities in `godot::global` will be removed. Use methods on `Gd` and `InstanceId` instead.\n\ diff --git a/godot-core/src/lib.rs b/godot-core/src/lib.rs index a2b621ed7..e32a4b7e4 100644 --- a/godot-core/src/lib.rs +++ b/godot-core/src/lib.rs @@ -48,6 +48,7 @@ compile_error!("Generating editor docs for Rust symbols requires at least Godot #[allow(clippy::let_unit_value)] // let args = (); #[allow(clippy::wrong_self_convention)] // to_string() is const #[allow(clippy::upper_case_acronyms)] // TODO remove this line once we transform names +#[allow(clippy::needless_lifetimes)] // the following explicit lifetimes could be elided: 'a #[allow(unreachable_code, clippy::unimplemented)] // TODO remove once #153 is implemented mod gen { include!(concat!(env!("OUT_DIR"), "/mod.rs")); diff --git a/godot-core/src/meta/godot_convert/impls.rs b/godot-core/src/meta/godot_convert/impls.rs index ea04a8159..84de15e67 100644 --- a/godot-core/src/meta/godot_convert/impls.rs +++ b/godot-core/src/meta/godot_convert/impls.rs @@ -91,13 +91,14 @@ where impl ToGodot for Option where Option: GodotType, + for<'f> T::ToVia<'f>: GodotType, { - fn to_godot(&self) -> Self::Via { - self.as_ref().map(ToGodot::to_godot) - } + type ToVia<'v> = Option> + // type ToVia<'v> = Self::Via + where Self: 'v; - fn into_godot(self) -> Self::Via { - self.map(ToGodot::into_godot) + fn to_godot(&self) -> Self::ToVia<'_> { + self.as_ref().map(ToGodot::to_godot) } fn to_variant(&self) -> Variant { @@ -225,7 +226,9 @@ macro_rules! impl_godot_scalar { } impl ToGodot for $T { - fn to_godot(&self) -> Self::Via { + type ToVia<'v> = Self::Via; + + fn to_godot(&self) -> Self::ToVia<'_> { *self } } @@ -308,7 +311,9 @@ impl GodotConvert for u64 { } impl ToGodot for u64 { - fn to_godot(&self) -> Self::Via { + type ToVia<'v> = u64; + + fn to_godot(&self) -> Self::ToVia<'_> { *self } @@ -346,7 +351,9 @@ impl GodotConvert for Vec { } impl ToGodot for Vec { - fn to_godot(&self) -> Self::Via { + type ToVia<'v> = Array; + + fn to_godot(&self) -> Self::ToVia<'_> { Array::from(self.as_slice()) } } @@ -362,7 +369,9 @@ impl GodotConvert for [T; LEN] { } impl ToGodot for [T; LEN] { - fn to_godot(&self) -> Self::Via { + type ToVia<'v> = Array; + + fn to_godot(&self) -> Self::ToVia<'_> { Array::from(self) } } @@ -400,7 +409,10 @@ impl GodotConvert for &[T] { } impl ToGodot for &[T] { - fn to_godot(&self) -> Self::Via { + type ToVia<'v> = Array + where Self: 'v; + + fn to_godot(&self) -> Self::ToVia<'_> { Array::from(*self) } } @@ -419,7 +431,9 @@ macro_rules! impl_pointer_convert { } impl ToGodot for $Ptr { - fn to_godot(&self) -> Self::Via { + type ToVia<'v> = i64; + + fn to_godot(&self) -> Self::ToVia<'_> { *self as i64 } } diff --git a/godot-core/src/meta/godot_convert/mod.rs b/godot-core/src/meta/godot_convert/mod.rs index 0998c2a15..e9ee39108 100644 --- a/godot-core/src/meta/godot_convert/mod.rs +++ b/godot-core/src/meta/godot_convert/mod.rs @@ -40,15 +40,12 @@ pub trait GodotConvert { /// /// Please read the [`godot::meta` module docs][crate::meta] for further information about conversions. pub trait ToGodot: Sized + GodotConvert { - /// Converts this type to the Godot type by reference, usually by cloning. - fn to_godot(&self) -> Self::Via; + type ToVia<'v>: GodotType + where + Self: 'v; - /// Converts this type to the Godot type. - /// - /// This can in some cases enable minor optimizations, such as avoiding reference counting operations. - fn into_godot(self) -> Self::Via { - self.to_godot() - } + /// Converts this type to the Godot type by reference, usually by cloning. + fn to_godot(&self) -> Self::ToVia<'_>; /// Converts this type to a [Variant]. fn to_variant(&self) -> Variant { @@ -98,8 +95,16 @@ pub trait FromGodot: Sized + GodotConvert { } } -pub(crate) fn into_ffi(value: T) -> ::Ffi { - value.into_godot().into_ffi() +// Note: removing the implicit lifetime (by taking value: T instead of &T) causes issues due to allegedly returning a lifetime +// to a local variable, even though the result Ffi is 'static by definition. +#[allow(clippy::needless_lifetimes)] // eliding causes error: missing generics for associated type `godot_convert::ToGodot::ToVia` +pub(crate) fn into_ffi<'v, T: ToGodot>(value: &'v T) -> as GodotType>::Ffi { + let by_ref = value.to_godot(); + by_ref.to_ffi() +} + +pub(crate) fn into_ffi_variant(value: &T) -> Variant { + GodotFfiVariant::ffi_to_variant(&into_ffi(value)) } pub(crate) fn try_from_ffi( @@ -117,14 +122,11 @@ macro_rules! impl_godot_as_self { } impl $crate::meta::ToGodot for $T { - #[inline] - fn to_godot(&self) -> Self::Via { - self.clone() - } + type ToVia<'v> = Self::Via; #[inline] - fn into_godot(self) -> Self::Via { - self + fn to_godot(&self) -> Self::ToVia<'_> { + self.clone() } } diff --git a/godot-core/src/meta/mod.rs b/godot-core/src/meta/mod.rs index dde9ce2b6..b5a2e4a6f 100644 --- a/godot-core/src/meta/mod.rs +++ b/godot-core/src/meta/mod.rs @@ -39,6 +39,7 @@ mod class_name; mod godot_convert; mod method_info; mod property_info; +mod ref_arg; mod sealed; mod signature; mod traits; @@ -54,6 +55,8 @@ pub(crate) use traits::{GodotFfiVariant, GodotNullableFfi}; use crate::registry::method::MethodParamOrReturnInfo; +#[doc(hidden)] +pub use ref_arg::*; #[doc(hidden)] pub use signature::*; diff --git a/godot-core/src/meta/ref_arg.rs b/godot-core/src/meta/ref_arg.rs new file mode 100644 index 000000000..eadc7e3c3 --- /dev/null +++ b/godot-core/src/meta/ref_arg.rs @@ -0,0 +1,57 @@ +/* + * Copyright (c) godot-rust; Bromeon and contributors. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ +use crate::meta::error::ConvertError; +use crate::meta::{FromGodot, GodotConvert, ToGodot}; +use std::fmt; + +pub struct RefArg<'r, T> { + pub(crate) shared_ref: &'r T, +} + +impl<'r, T> RefArg<'r, T> { + pub fn new(shared_ref: &'r T) -> Self { + RefArg { shared_ref } + } +} + +impl<'r, T> GodotConvert for RefArg<'r, T> +where + T: GodotConvert, +{ + type Via = T::Via; +} + +impl<'r, T> ToGodot for RefArg<'r, T> +where + T: ToGodot, +{ + type ToVia<'v> = T::ToVia<'v> + where Self: 'v; + + fn to_godot(&self) -> Self::ToVia<'_> { + self.shared_ref.to_godot() + } +} + +// TODO refactor signature tuples into separate in+out traits, so FromGodot is no longer needed. +impl<'r, T> FromGodot for RefArg<'r, T> +where + T: FromGodot, +{ + fn try_from_godot(_via: Self::Via) -> Result { + unreachable!("RefArg should only be passed *to* Godot, not *from*.") + } +} + +impl<'r, T> fmt::Debug for RefArg<'r, T> +where + T: fmt::Debug, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "&{:?}", self.shared_ref) + } +} diff --git a/godot-core/src/meta/signature.rs b/godot-core/src/meta/signature.rs index 2ef77bfb7..33223dc86 100644 --- a/godot-core/src/meta/signature.rs +++ b/godot-core/src/meta/signature.rs @@ -4,6 +4,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + use std::borrow::Cow; use std::fmt; use std::fmt::Debug; @@ -13,7 +14,7 @@ use sys::{BuiltinMethodBind, ClassMethodBind, GodotFfi, UtilityFunctionBind}; use crate::builtin::Variant; use crate::meta::error::{CallError, ConvertError}; -use crate::meta::godot_convert::{into_ffi, try_from_ffi}; +use crate::meta::godot_convert::{into_ffi, into_ffi_variant, try_from_ffi}; use crate::meta::*; use crate::obj::{GodotClass, InstanceId}; @@ -224,7 +225,7 @@ macro_rules! impl_varcall_signature_for_tuple { let explicit_args = [ $( - GodotFfiVariant::ffi_to_variant(&into_ffi($pn)), + into_ffi_variant(&$pn), )* ]; @@ -269,7 +270,7 @@ macro_rules! impl_varcall_signature_for_tuple { let object_call_script_method = sys::interface_fn!(object_call_script_method); let explicit_args = [ $( - GodotFfiVariant::ffi_to_variant(&into_ffi($pn)), + into_ffi_variant(&$pn), )* ]; @@ -304,7 +305,7 @@ macro_rules! impl_varcall_signature_for_tuple { let explicit_args: [Variant; $PARAM_COUNT] = [ $( - GodotFfiVariant::ffi_to_variant(&into_ffi($pn)), + into_ffi_variant(&$pn), )* ]; @@ -391,7 +392,7 @@ macro_rules! impl_ptrcall_signature_for_tuple { #[allow(clippy::let_unit_value)] let marshalled_args = ( $( - into_ffi($pn), + into_ffi(&$pn), )* ); @@ -422,7 +423,7 @@ macro_rules! impl_ptrcall_signature_for_tuple { #[allow(clippy::let_unit_value)] let marshalled_args = ( $( - into_ffi($pn), + into_ffi(&$pn), )* ); @@ -450,7 +451,7 @@ macro_rules! impl_ptrcall_signature_for_tuple { #[allow(clippy::let_unit_value)] let marshalled_args = ( $( - into_ffi($pn), + into_ffi(&$pn), )* ); @@ -547,7 +548,9 @@ unsafe fn ptrcall_return( _call_ctx: &CallContext, call_type: sys::PtrcallType, ) { - let val = into_ffi(ret_val); + //let val = into_ffi(ret_val); + let val = ret_val.to_godot().to_ffi(); + val.move_return_ptr(ret, call_type); } diff --git a/godot-core/src/meta/traits.rs b/godot-core/src/meta/traits.rs index 68a3a49ef..b536b38cb 100644 --- a/godot-core/src/meta/traits.rs +++ b/godot-core/src/meta/traits.rs @@ -38,12 +38,11 @@ pub trait GodotFfiVariant: Sized + GodotFfi { // type. For instance [`i32`] does not implement `GodotFfi` because it cannot represent all values of // Godot's `int` type, however it does implement `GodotType` because we can set the metadata of values with // this type to indicate that they are 32 bits large. -pub trait GodotType: - GodotConvert + ToGodot + FromGodot + sealed::Sealed + 'static +pub trait GodotType: GodotConvert + sealed::Sealed + Sized + 'static // 'static is not technically required, but it simplifies a few things (limits e.g. ObjectArg). { #[doc(hidden)] - type Ffi: GodotFfiVariant; + type Ffi: GodotFfiVariant + 'static; #[doc(hidden)] fn to_ffi(&self) -> Self::Ffi; @@ -145,7 +144,7 @@ pub trait GodotType: message = "`Array` can only store element types supported in Godot arrays (no nesting).", label = "has invalid element type" )] -pub trait ArrayElement: GodotType + sealed::Sealed { +pub trait ArrayElement: GodotType + ToGodot + FromGodot + sealed::Sealed { /// Returns the representation of this type as a type string. /// /// Used for elements in arrays (the latter despite `ArrayElement` not having a direct relation). diff --git a/godot-core/src/obj/gd.rs b/godot-core/src/obj/gd.rs index 096bcc264..f85cf8d0c 100644 --- a/godot-core/src/obj/gd.rs +++ b/godot-core/src/obj/gd.rs @@ -679,15 +679,12 @@ impl GodotConvert for Gd { } impl ToGodot for Gd { - fn to_godot(&self) -> Self::Via { + type ToVia<'v> = Gd; + + fn to_godot(&self) -> Self::ToVia<'_> { self.raw.check_rtti("to_godot"); self.clone() } - - fn into_godot(self) -> Self::Via { - self.raw.check_rtti("into_godot"); - self - } } impl FromGodot for Gd { diff --git a/godot-core/src/obj/instance_id.rs b/godot-core/src/obj/instance_id.rs index 2c2b96e4a..0c0e85a89 100644 --- a/godot-core/src/obj/instance_id.rs +++ b/godot-core/src/obj/instance_id.rs @@ -94,7 +94,9 @@ impl GodotConvert for InstanceId { } impl ToGodot for InstanceId { - fn to_godot(&self) -> Self::Via { + type ToVia<'v> = i64; + + fn to_godot(&self) -> Self::ToVia<'_> { self.to_i64() } } diff --git a/godot-core/src/obj/object_arg.rs b/godot-core/src/obj/object_arg.rs index e3058ac8a..1736d973a 100644 --- a/godot-core/src/obj/object_arg.rs +++ b/godot-core/src/obj/object_arg.rs @@ -254,15 +254,14 @@ impl GodotConvert for ObjectArg { } impl ToGodot for ObjectArg { - fn to_godot(&self) -> Self::Via { - (*self).clone() - } + type ToVia<'v> = Self; - fn into_godot(self) -> Self::Via { - self + fn to_godot(&self) -> Self::ToVia<'_> { + (*self).clone() } } +// TODO refactor signature tuples into separate in+out traits, so FromGodot is no longer needed. impl FromGodot for ObjectArg { fn try_from_godot(_via: Self::Via) -> Result { unreachable!("ObjectArg should only be passed *to* Godot, not *from*.") diff --git a/godot-core/src/obj/raw_gd.rs b/godot-core/src/obj/raw_gd.rs index 20a24e5bb..d7714a4ab 100644 --- a/godot-core/src/obj/raw_gd.rs +++ b/godot-core/src/obj/raw_gd.rs @@ -536,12 +536,10 @@ impl GodotConvert for RawGd { } impl ToGodot for RawGd { - fn to_godot(&self) -> Self::Via { - self.clone() - } + type ToVia<'v> = Self; - fn into_godot(self) -> Self::Via { - self + fn to_godot(&self) -> Self::ToVia<'_> { + self.clone() } } diff --git a/godot-core/src/tools/gfile.rs b/godot-core/src/tools/gfile.rs index 8f9944c2c..b47baa455 100644 --- a/godot-core/src/tools/gfile.rs +++ b/godot-core/src/tools/gfile.rs @@ -153,7 +153,7 @@ impl GFile { pub fn open_encrypted( path: impl Into, flags: ModeFlags, - key: PackedByteArray, + key: &PackedByteArray, ) -> std::io::Result { let path: GString = path.into(); let fa = FileAccess::open_encrypted(path.clone(), flags, key).ok_or_else(|| { @@ -591,7 +591,7 @@ impl GFile { #[doc(alias = "store_csv_line")] pub fn write_csv_line( &mut self, - values: PackedStringArray, + values: &PackedStringArray, delim: impl Into, ) -> std::io::Result<()> { self.fa.store_csv_line_ex(values).delim(delim.into()).done(); @@ -611,7 +611,7 @@ impl GFile { #[doc(alias = "store_var")] pub fn write_variant(&mut self, value: Variant, full_objects: bool) -> std::io::Result<()> { self.fa - .store_var_ex(value) + .store_var_ex(&value) .full_objects(full_objects) .done(); self.clear_file_length(); @@ -753,7 +753,7 @@ impl Write for GFile { fn write(&mut self, buf: &[u8]) -> std::io::Result { self.pack_into_write_buffer(buf); self.fa - .store_buffer(self.write_buffer.subarray(0, buf.len())); + .store_buffer(&self.write_buffer.subarray(0, buf.len())); self.clear_file_length(); self.check_error()?; diff --git a/godot-macros/src/derive/derive_to_godot.rs b/godot-macros/src/derive/derive_to_godot.rs index 8a7f167f9..2eaa65487 100644 --- a/godot-macros/src/derive/derive_to_godot.rs +++ b/godot-macros/src/derive/derive_to_godot.rs @@ -42,13 +42,12 @@ fn make_togodot_for_newtype_struct(name: &Ident, field: &NewtypeStruct) -> Token quote! { impl ::godot::meta::ToGodot for #name { + // For now by-value, may change. + type ToVia<'v> = #via_type; + fn to_godot(&self) -> #via_type { ::godot::meta::ToGodot::to_godot(&self.#field_name) } - - fn into_godot(self) -> #via_type { - ::godot::meta::ToGodot::into_godot(self.#field_name) - } } } } @@ -67,6 +66,8 @@ fn make_togodot_for_int_enum( quote! { impl ::godot::meta::ToGodot for #name { + type ToVia<'v> = #int; + #[allow(unused_parens)] // Error "unnecessary parentheses around block return value"; comes from ord expressions like (1 + 2). fn to_godot(&self) -> #int { match self { @@ -86,6 +87,8 @@ fn make_togodot_for_string_enum(name: &Ident, enum_: &CStyleEnum) -> TokenStream quote! { impl ::godot::meta::ToGodot for #name { + type ToVia<'v> = ::godot::builtin::GString; + fn to_godot(&self) -> ::godot::builtin::GString { match self { #( diff --git a/itest/rust/build.rs b/itest/rust/build.rs index 39d17d95f..33e326f4b 100644 --- a/itest/rust/build.rs +++ b/itest/rust/build.rs @@ -86,8 +86,10 @@ macro_rules! push_newtype { } impl godot::meta::ToGodot for $name { + type ToVia<'v> = $T; + #[allow(clippy::clone_on_copy)] - fn to_godot(&self) -> Self::Via { + fn to_godot(&self) -> Self::ToVia<'_> { self.0.clone() } } diff --git a/itest/rust/src/builtin_tests/containers/array_test.rs b/itest/rust/src/builtin_tests/containers/array_test.rs index 01677dd2e..ba09a655a 100644 --- a/itest/rust/src/builtin_tests/containers/array_test.rs +++ b/itest/rust/src/builtin_tests/containers/array_test.rs @@ -293,7 +293,7 @@ fn array_insert() { fn array_extend() { let mut array = array![1, 2]; let other = array![3, 4]; - array.extend_array(other); + array.extend_array(&other); assert_eq!(array, array![1, 2, 3, 4]); } @@ -383,7 +383,7 @@ fn untyped_array_pass_to_godot_func() { node.queue_free(); // Do not leak even if the test fails. assert_eq!( - node.callv(StringName::from("has_signal"), varray!["tree_entered"]), + node.callv(StringName::from("has_signal"), &varray!["tree_entered"]), true.to_variant() ); } @@ -416,11 +416,11 @@ fn typed_array_pass_to_godot_func() { 4, false, Format::L8, - PackedByteArray::from(&[255, 0, 255, 0, 0, 255, 0, 255]), + &PackedByteArray::from(&[255, 0, 255, 0, 0, 255, 0, 255]), ); let images = array![image]; let mut texture = Texture2DArray::new_gd(); - let error = texture.create_from_images(images); + let error = texture.create_from_images(&images); assert_eq!(error, Error::OK); assert_eq!((texture.get_width(), texture.get_height()), (2, 4)); diff --git a/itest/rust/src/builtin_tests/containers/callable_test.rs b/itest/rust/src/builtin_tests/containers/callable_test.rs index 1407b9fe0..c5e6bded9 100644 --- a/itest/rust/src/builtin_tests/containers/callable_test.rs +++ b/itest/rust/src/builtin_tests/containers/callable_test.rs @@ -85,19 +85,19 @@ fn callable_call() { let callable = obj.callable("foo"); assert_eq!(obj.bind().value, 0); - callable.callv(varray![10]); + callable.callv(&varray![10]); assert_eq!(obj.bind().value, 10); // Too many arguments: this call fails, its logic is not applied. // In the future, panic should be propagated to caller. - callable.callv(varray![20, 30]); + callable.callv(&varray![20, 30]); assert_eq!(obj.bind().value, 10); // TODO(bromeon): this causes a Rust panic, but since call() is routed to Godot, the panic is handled at the FFI boundary. // Can there be a way to notify the caller about failed calls like that? - assert_eq!(callable.callv(varray!["string"]), Variant::nil()); + assert_eq!(callable.callv(&varray!["string"]), Variant::nil()); - assert_eq!(Callable::invalid().callv(varray![1, 2, 3]), Variant::nil()); + assert_eq!(Callable::invalid().callv(&varray![1, 2, 3]), Variant::nil()); } #[itest] @@ -106,11 +106,11 @@ fn callable_call_return() { let callable = obj.callable("bar"); assert_eq!( - callable.callv(varray![10]), + callable.callv(&varray![10]), 10.to_variant().stringify().to_variant() ); // errors in godot but does not crash - assert_eq!(callable.callv(varray!["string"]), Variant::nil()); + assert_eq!(callable.callv(&varray!["string"]), Variant::nil()); } #[itest] @@ -137,10 +137,10 @@ fn callable_call_engine() { fn callable_bindv() { let obj = CallableTestObj::new_gd(); let callable = obj.callable("bar"); - let callable_bound = callable.bindv(varray![10]); + let callable_bound = callable.bindv(&varray![10]); assert_eq!( - callable_bound.callv(varray![]), + callable_bound.callv(&varray![]), 10.to_variant().stringify().to_variant() ); } @@ -179,14 +179,14 @@ pub mod custom_callable { assert!(callable.is_custom()); assert!(callable.object().is_none()); - let sum1 = callable.callv(varray![1, 2, 4, 8]); + let sum1 = callable.callv(&varray![1, 2, 4, 8]); assert_eq!(sum1, 15.to_variant()); - let sum2 = callable.callv(varray![5]); + let sum2 = callable.callv(&varray![5]); assert_eq!(sum2, 5.to_variant()); // Important to test 0 arguments, as the FFI call passes a null pointer for the argument array. - let sum3 = callable.callv(varray![]); + let sum3 = callable.callv(&varray![]); assert_eq!(sum3, 0.to_variant()); } @@ -215,10 +215,10 @@ pub mod custom_callable { assert!(callable.is_custom()); assert!(callable.object().is_none()); - let sum1 = callable.callv(varray![3, 9, 2, 1]); + let sum1 = callable.callv(&varray![3, 9, 2, 1]); assert_eq!(sum1, 15.to_variant()); - let sum2 = callable.callv(varray![4]); + let sum2 = callable.callv(&varray![4]); assert_eq!(sum2, 19.to_variant()); } @@ -302,7 +302,7 @@ pub mod custom_callable { panic!("TEST: {}", received_callable.fetch_add(1, Ordering::SeqCst)) }); - assert_eq!(Variant::nil(), callable.callv(varray![])); + assert_eq!(Variant::nil(), callable.callv(&varray![])); assert_eq!(1, received.load(Ordering::SeqCst)); } @@ -312,7 +312,7 @@ pub mod custom_callable { let received = Arc::new(AtomicU32::new(0)); let callable = Callable::from_custom(PanicCallable(received.clone())); - assert_eq!(Variant::nil(), callable.callv(varray![])); + assert_eq!(Variant::nil(), callable.callv(&varray![])); assert_eq!(1, received.load(Ordering::SeqCst)); } diff --git a/itest/rust/src/builtin_tests/containers/dictionary_test.rs b/itest/rust/src/builtin_tests/containers/dictionary_test.rs index 50f42fdf0..ef7896502 100644 --- a/itest/rust/src/builtin_tests/containers/dictionary_test.rs +++ b/itest/rust/src/builtin_tests/containers/dictionary_test.rs @@ -287,7 +287,7 @@ fn dictionary_extend() { "bar": "new", "baz": Variant::nil(), }; - dictionary.extend_dictionary(other, false); + dictionary.extend_dictionary(&other, false); assert_eq!(dictionary.get("bar"), Some(true.to_variant())); assert_eq!(dictionary.get("baz"), Some(Variant::nil())); @@ -297,7 +297,7 @@ fn dictionary_extend() { let other = dict! { "bar": "new", }; - dictionary.extend_dictionary(other, true); + dictionary.extend_dictionary(&other, true); assert_eq!(dictionary.get("bar"), Some("new".to_variant())); } @@ -345,12 +345,12 @@ fn dictionary_contains_keys() { assert!(dictionary.contains_key("foo"), "key = \"foo\""); assert!(dictionary.contains_key("bar"), "key = \"bar\""); assert!( - dictionary.contains_all_keys(varray!["foo", "bar"]), + dictionary.contains_all_keys(&varray!["foo", "bar"]), "keys = [\"foo\", \"bar\"]" ); assert!(!dictionary.contains_key("missing"), "key = \"missing\""); assert!( - !dictionary.contains_all_keys(varray!["foo", "bar", "missing"]), + !dictionary.contains_all_keys(&varray!["foo", "bar", "missing"]), "keys = [\"foo\", \"bar\", \"missing\"]" ); } diff --git a/itest/rust/src/builtin_tests/containers/variant_test.rs b/itest/rust/src/builtin_tests/containers/variant_test.rs index 19707d93d..04f0575d2 100644 --- a/itest/rust/src/builtin_tests/containers/variant_test.rs +++ b/itest/rust/src/builtin_tests/containers/variant_test.rs @@ -179,6 +179,9 @@ fn variant_get_type() { let variant = gstr("hello").to_variant(); assert_eq!(variant.get_type(), VariantType::STRING); + let variant = sname("hello").to_variant(); + assert_eq!(variant.get_type(), VariantType::STRING_NAME); + let variant = TEST_BASIS.to_variant(); assert_eq!(variant.get_type(), VariantType::BASIS) } @@ -263,14 +266,14 @@ fn variant_evaluate() { evaluate(VariantOperator::MULTIPLY, 5, 2.5, 12.5); evaluate(VariantOperator::EQUAL, gstr("hello"), gstr("hello"), true); - evaluate(VariantOperator::EQUAL, gstr("hello"), gname("hello"), true); - evaluate(VariantOperator::EQUAL, gname("rust"), gstr("rust"), true); - evaluate(VariantOperator::EQUAL, gname("rust"), gname("rust"), true); + evaluate(VariantOperator::EQUAL, gstr("hello"), sname("hello"), true); + evaluate(VariantOperator::EQUAL, sname("rust"), gstr("rust"), true); + evaluate(VariantOperator::EQUAL, sname("rust"), sname("rust"), true); evaluate(VariantOperator::NOT_EQUAL, gstr("hello"), gstr("hallo"), true); - evaluate(VariantOperator::NOT_EQUAL, gstr("hello"), gname("hallo"), true); - evaluate(VariantOperator::NOT_EQUAL, gname("rust"), gstr("rest"), true); - evaluate(VariantOperator::NOT_EQUAL, gname("rust"), gname("rest"), true); + evaluate(VariantOperator::NOT_EQUAL, gstr("hello"), sname("hallo"), true); + evaluate(VariantOperator::NOT_EQUAL, sname("rust"), gstr("rest"), true); + evaluate(VariantOperator::NOT_EQUAL, sname("rust"), sname("rest"), true); evaluate_fail(VariantOperator::EQUAL, 1, true); evaluate_fail(VariantOperator::EQUAL, 0, false); @@ -391,28 +394,7 @@ fn variant_null_object_is_nil() { } #[itest] -fn variant_type_correct() { - assert_eq!(Variant::nil().get_type(), VariantType::NIL); - assert_eq!(0.to_variant().get_type(), VariantType::INT); - assert_eq!(3.8.to_variant().get_type(), VariantType::FLOAT); - assert_eq!(false.to_variant().get_type(), VariantType::BOOL); - assert_eq!("string".to_variant().get_type(), VariantType::STRING); - assert_eq!( - StringName::from("string_name").to_variant().get_type(), - VariantType::STRING_NAME - ); - assert_eq!( - VariantArray::default().to_variant().get_type(), - VariantType::ARRAY - ); - assert_eq!( - Dictionary::default().to_variant().get_type(), - VariantType::DICTIONARY - ); -} - -#[itest] -fn variant_stringify_correct() { +fn variant_stringify() { assert_eq!("value".to_variant().stringify(), gstr("value")); assert_eq!(Variant::nil().stringify(), gstr("")); assert_eq!(true.to_variant().stringify(), gstr("true")); @@ -428,7 +410,7 @@ fn variant_stringify_correct() { } #[itest] -fn variant_booleanize_correct() { +fn variant_booleanize() { assert!(gstr("string").to_variant().booleanize()); assert!(10.to_variant().booleanize()); assert!(varray![""].to_variant().booleanize()); @@ -442,7 +424,7 @@ fn variant_booleanize_correct() { } #[itest] -fn variant_hash_correct() { +fn variant_hash() { let hash_is_not_0 = [ dict! {}.to_variant(), gstr("").to_variant(), @@ -583,6 +565,6 @@ fn gstr(s: &str) -> GString { GString::from(s) } -fn gname(s: &str) -> StringName { +fn sname(s: &str) -> StringName { StringName::from(s) } diff --git a/itest/rust/src/builtin_tests/convert_test.rs b/itest/rust/src/builtin_tests/convert_test.rs index 050095a2a..e95933f2f 100644 --- a/itest/rust/src/builtin_tests/convert_test.rs +++ b/itest/rust/src/builtin_tests/convert_test.rs @@ -106,7 +106,9 @@ impl GodotConvert for ConvertedStruct { } impl ToGodot for ConvertedStruct { - fn to_godot(&self) -> Self::Via { + type ToVia<'v> = Dictionary; + + fn to_godot(&self) -> Self::ToVia<'_> { dict! { "a": self.a, "b": self.b, diff --git a/itest/rust/src/engine_tests/utilities_test.rs b/itest/rust/src/engine_tests/utilities_test.rs index c4ffa747a..62a95d8a3 100644 --- a/itest/rust/src/engine_tests/utilities_test.rs +++ b/itest/rust/src/engine_tests/utilities_test.rs @@ -18,7 +18,7 @@ use godot::obj::NewAlloc; #[itest] fn utilities_abs() { let input = Variant::from(-7); - let output = abs(input); + let output = abs(&input); assert_eq!(output, Variant::from(7)); } @@ -26,7 +26,7 @@ fn utilities_abs() { #[itest] fn utilities_sign() { let input = Variant::from(-7); - let output = sign(input); + let output = sign(&input); assert_eq!(output, Variant::from(-1)); } @@ -48,13 +48,17 @@ fn utilities_str() { #[itest] fn utilities_wrap() { - let output = wrap(Variant::from(3.4), Variant::from(2.0), Variant::from(3.0)); + let output = wrap( + &Variant::from(3.4), + &Variant::from(2.0), + &Variant::from(3.0), + ); assert_eq!(output, Variant::from(2.4)); let output = wrap( - Variant::from(-5.7), - Variant::from(-3.0), - Variant::from(-2.0), + &Variant::from(-5.7), + &Variant::from(-3.0), + &Variant::from(-2.0), ); assert_eq!(output, Variant::from(-2.7)); } @@ -62,15 +66,15 @@ fn utilities_wrap() { #[itest] fn utilities_max() { let output = max( - Variant::from(1.0), - Variant::from(3.0), + &Variant::from(1.0), + &Variant::from(3.0), &[Variant::from(5.0), Variant::from(7.0)], ); assert_eq!(output, Variant::from(7.0)); let output = max( - Variant::from(-1.0), - Variant::from(-3.0), + &Variant::from(-1.0), + &Variant::from(-3.0), &[Variant::from(-5.0), Variant::from(-7.0)], ); assert_eq!(output, Variant::from(-1.0)); diff --git a/itest/rust/src/object_tests/enum_test.rs b/itest/rust/src/object_tests/enum_test.rs index 6431f19b9..1ed679cf7 100644 --- a/itest/rust/src/object_tests/enum_test.rs +++ b/itest/rust/src/object_tests/enum_test.rs @@ -14,7 +14,7 @@ use godot::global::{Orientation, Key}; use std::collections::HashSet; #[itest] -fn enum_ords_correct() { +fn enum_ords() { use godot::obj::EngineEnum; assert_eq!(CursorShape::CURSOR_ARROW.ord(), 0); assert_eq!(CursorShape::CURSOR_IBEAM.ord(), 1); @@ -73,7 +73,7 @@ fn add_surface_from_arrays() { } #[itest] -fn enum_as_str_correct() { +fn enum_as_str() { use godot::obj::EngineEnum; assert_eq!(Orientation::Vertical.as_str(), "VERTICAL"); assert_eq!(Orientation::Horizontal.as_str(), "HORIZONTAL"); @@ -86,7 +86,7 @@ fn enum_as_str_correct() { } #[itest] -fn enum_godot_name_correct() { +fn enum_godot_name() { use godot::obj::EngineEnum; assert_eq!(Orientation::Vertical.godot_name(), Orientation::Vertical.as_str()); assert_eq!(Orientation::Horizontal.godot_name(), Orientation::Vertical.as_str()); diff --git a/itest/rust/src/object_tests/object_arg_test.rs b/itest/rust/src/object_tests/object_arg_test.rs index cb2ed341a..2f4b8ac6c 100644 --- a/itest/rust/src/object_tests/object_arg_test.rs +++ b/itest/rust/src/object_tests/object_arg_test.rs @@ -17,8 +17,8 @@ use crate::object_tests::object_test::{user_refc_instance, RefcPayload}; fn object_arg_owned() { with_objects(|manual, refc| { let db = ClassDb::singleton(); - let a = db.class_set_property(manual, "name".into(), Variant::from("hello")); - let b = db.class_set_property(refc, "value".into(), Variant::from(-123)); + let a = db.class_set_property(manual, "name".into(), &Variant::from("hello")); + let b = db.class_set_property(refc, "value".into(), &Variant::from(-123)); (a, b) }); } @@ -27,8 +27,8 @@ fn object_arg_owned() { fn object_arg_borrowed() { with_objects(|manual, refc| { let db = ClassDb::singleton(); - let a = db.class_set_property(&manual, "name".into(), Variant::from("hello")); - let b = db.class_set_property(&refc, "value".into(), Variant::from(-123)); + let a = db.class_set_property(&manual, "name".into(), &Variant::from("hello")); + let b = db.class_set_property(&refc, "value".into(), &Variant::from(-123)); (a, b) }); } @@ -37,8 +37,8 @@ fn object_arg_borrowed() { fn object_arg_borrowed_mut() { with_objects(|mut manual, mut refc| { let db = ClassDb::singleton(); - let a = db.class_set_property(&mut manual, "name".into(), Variant::from("hello")); - let b = db.class_set_property(&mut refc, "value".into(), Variant::from(-123)); + let a = db.class_set_property(&mut manual, "name".into(), &Variant::from("hello")); + let b = db.class_set_property(&mut refc, "value".into(), &Variant::from(-123)); (a, b) }); } @@ -47,8 +47,8 @@ fn object_arg_borrowed_mut() { fn object_arg_option_owned() { with_objects(|manual, refc| { let db = ClassDb::singleton(); - let a = db.class_set_property(Some(manual), "name".into(), Variant::from("hello")); - let b = db.class_set_property(Some(refc), "value".into(), Variant::from(-123)); + let a = db.class_set_property(Some(manual), "name".into(), &Variant::from("hello")); + let b = db.class_set_property(Some(refc), "value".into(), &Variant::from(-123)); (a, b) }); } @@ -57,8 +57,8 @@ fn object_arg_option_owned() { fn object_arg_option_borrowed() { with_objects(|manual, refc| { let db = ClassDb::singleton(); - let a = db.class_set_property(Some(&manual), "name".into(), Variant::from("hello")); - let b = db.class_set_property(Some(&refc), "value".into(), Variant::from(-123)); + let a = db.class_set_property(Some(&manual), "name".into(), &Variant::from("hello")); + let b = db.class_set_property(Some(&refc), "value".into(), &Variant::from(-123)); (a, b) }); } @@ -67,8 +67,8 @@ fn object_arg_option_borrowed() { fn object_arg_option_borrowed_mut() { with_objects(|mut manual, mut refc| { let db = ClassDb::singleton(); - let a = db.class_set_property(Some(&mut manual), "name".into(), Variant::from("hello")); - let b = db.class_set_property(Some(&mut refc), "value".into(), Variant::from(-123)); + let a = db.class_set_property(Some(&mut manual), "name".into(), &Variant::from("hello")); + let b = db.class_set_property(Some(&mut refc), "value".into(), &Variant::from(-123)); (a, b) }); } @@ -80,10 +80,10 @@ fn object_arg_option_none() { // Will emit errors but should not crash. let db = ClassDb::singleton(); - let error = db.class_set_property(manual, "name".into(), Variant::from("hello")); + let error = db.class_set_property(manual, "name".into(), &Variant::from("hello")); assert_eq!(error, global::Error::ERR_UNAVAILABLE); - let error = db.class_set_property(refc, "value".into(), Variant::from(-123)); + let error = db.class_set_property(refc, "value".into(), &Variant::from(-123)); assert_eq!(error, global::Error::ERR_UNAVAILABLE); } @@ -91,10 +91,10 @@ fn object_arg_option_none() { fn object_arg_null_arg() { // Will emit errors but should not crash. let db = ClassDb::singleton(); - let error = db.class_set_property(Gd::null_arg(), "name".into(), Variant::from("hello")); + let error = db.class_set_property(Gd::null_arg(), "name".into(), &Variant::from("hello")); assert_eq!(error, global::Error::ERR_UNAVAILABLE); - let error = db.class_set_property(Gd::null_arg(), "value".into(), Variant::from(-123)); + let error = db.class_set_property(Gd::null_arg(), "value".into(), &Variant::from(-123)); assert_eq!(error, global::Error::ERR_UNAVAILABLE); } diff --git a/itest/rust/src/object_tests/onready_test.rs b/itest/rust/src/object_tests/onready_test.rs index 587201b42..4907f0eff 100644 --- a/itest/rust/src/object_tests/onready_test.rs +++ b/itest/rust/src/object_tests/onready_test.rs @@ -133,8 +133,8 @@ fn onready_property_access() { let mut obj = OnReadyWithImpl::create(true); obj.notify(NodeNotification::READY); - obj.set("auto".into(), 33.to_variant()); - obj.set("manual".into(), 44.to_variant()); + obj.set("auto".into(), &33.to_variant()); + obj.set("manual".into(), &44.to_variant()); { let obj = obj.bind(); diff --git a/itest/rust/src/object_tests/virtual_methods_test.rs b/itest/rust/src/object_tests/virtual_methods_test.rs index 26359d465..e9f17bbed 100644 --- a/itest/rust/src/object_tests/virtual_methods_test.rs +++ b/itest/rust/src/object_tests/virtual_methods_test.rs @@ -609,7 +609,7 @@ fn test_get_called() { } #[itest] -fn test_get_returns_correct() { +fn test_get_returns() { let mut obj = GetTest::new_gd(); { @@ -626,23 +626,23 @@ fn test_get_returns_correct() { fn test_set_called() { let mut obj = SetTest::new_gd(); assert!(!obj.bind().set_called); - obj.set("inexistent_property".into(), Variant::nil()); + obj.set("inexistent_property".into(), &Variant::nil()); assert!(obj.bind().set_called); let mut obj = SetTest::new_gd(); assert!(!obj.bind().set_called); - obj.set("settable".into(), 20.to_variant()); + obj.set("settable".into(), &20.to_variant()); assert!(obj.bind().set_called); } #[itest] -fn test_set_sets_correct() { +fn test_set_sets() { let mut obj = SetTest::new_gd(); assert_eq!(obj.bind().always_set_to_100, i64::default()); assert_eq!(obj.bind().settable, i64::default()); - obj.set("always_set_to_100".into(), "hello".to_variant()); - obj.set("settable".into(), 500.to_variant()); + obj.set("always_set_to_100".into(), &"hello".to_variant()); + obj.set("settable".into(), &500.to_variant()); assert_eq!(obj.bind().always_set_to_100, 100); assert_eq!(obj.bind().settable, 500); } diff --git a/itest/rust/src/register_tests/func_test.rs b/itest/rust/src/register_tests/func_test.rs index b321a5e1f..ada4a259a 100644 --- a/itest/rust/src/register_tests/func_test.rs +++ b/itest/rust/src/register_tests/func_test.rs @@ -196,7 +196,8 @@ impl GdSelfObj { "update_internal_signal".into(), &[new_internal.to_variant()], ); - return this.bind().internal_value; + + this.bind().internal_value } #[func(gd_self)] diff --git a/itest/rust/src/register_tests/func_virtual_test.rs b/itest/rust/src/register_tests/func_virtual_test.rs index bbcae380c..7b0d64446 100644 --- a/itest/rust/src/register_tests/func_virtual_test.rs +++ b/itest/rust/src/register_tests/func_virtual_test.rs @@ -56,7 +56,7 @@ fn func_virtual() { assert_eq!(object.bind().greet_lang(72), GString::from("Rust#72")); // With script: "GDScript". - object.set_script(make_script().to_variant()); + object.set_script(&make_script().to_variant()); assert_eq!(object.bind().greet_lang(72), GString::from("GDScript#72")); // Dynamic call: "GDScript". @@ -74,7 +74,7 @@ fn func_virtual_renamed() { ); // With script: "GDScript". - object.set_script(make_script().to_variant()); + object.set_script(&make_script().to_variant()); assert_eq!( object.bind().gl2("Hello".into()), GString::from("Hello GDScript") @@ -95,7 +95,7 @@ fn func_virtual_gd_self() { ); // With script: "GDScript". - object.set_script(make_script().to_variant()); + object.set_script(&make_script().to_variant()); assert_eq!( VirtualScriptCalls::greet_lang3(object.clone(), "Hoi".into()), GString::from("Hoi GDScript") @@ -109,7 +109,7 @@ fn func_virtual_gd_self() { #[itest] fn func_virtual_stateful() { let mut object = VirtualScriptCalls::new_gd(); - object.set_script(make_script().to_variant()); + object.set_script(&make_script().to_variant()); let variant = Vector3i::new(1, 2, 2).to_variant(); object.bind_mut().set_thing(variant.clone());