From 22402df922251bfe5ab974d9c071a60d5575e59c Mon Sep 17 00:00:00 2001 From: B_head Date: Sun, 6 Mar 2022 01:46:27 +0900 Subject: [PATCH 01/12] Refactoring export attribute - Enable to use Path syntax. - Remove abstractions that inhibit expansion. - Remove code that only accepts [export = "wrong"]. - Add error message. --- gdnative-derive/src/methods.rs | 141 +++++++++++++++------------------ 1 file changed, 64 insertions(+), 77 deletions(-) diff --git a/gdnative-derive/src/methods.rs b/gdnative-derive/src/methods.rs index 730ff2f22..bcc4a1082 100644 --- a/gdnative-derive/src/methods.rs +++ b/gdnative-derive/src/methods.rs @@ -198,120 +198,107 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { if let Some("export") = last_seg.as_deref() { let _export_args = export_args.get_or_insert_with(ExportArgs::default); - if !attr.tokens.is_empty() { - use syn::{Meta, MetaNameValue, NestedMeta}; - - let meta = match attr.parse_meta() { - Ok(val) => val, + use syn::{punctuated::Punctuated, Lit, Meta, NestedMeta}; + let nested_meta_iter = match attr.parse_meta() { Err(err) => { errors.push(err); return false; } - }; - - let pairs: Vec<_> = match meta { - Meta::List(list) => list - .nested - .into_pairs() - .filter_map(|p| { - let span = p.span(); - match p.into_value() { - NestedMeta::Meta(Meta::NameValue(pair)) => { - Some(pair) - } - unexpected => { - let msg = format!( - "unexpected argument in list: {}", - unexpected.into_token_stream() - ); + Ok(Meta::NameValue(name_value)) => { + let span = name_value.span(); + let msg = "NameValue syntax is not valid"; errors.push(syn::Error::new(span, msg)); - None + return false; } + Ok(Meta::Path(_)) => { + Punctuated::::new().into_iter() } - }) - .collect(), - Meta::NameValue(pair) => vec![pair], - meta => { - let span = meta.span(); - let msg = format!( - "unexpected attribute argument: {}", - meta.into_token_stream() - ); + Ok(Meta::List(list)) => list.nested.into_iter(), + }; + for nested_meta in nested_meta_iter { + let (path, lit) = match &nested_meta { + NestedMeta::Lit(param) => { + let span = param.span(); + let msg = "Literal item is not valid"; errors.push(syn::Error::new(span, msg)); - return false; + continue; } - }; - - for MetaNameValue { path, lit, .. } in pairs { - let last = match path.segments.last() { - Some(val) => val, - None => { - errors.push(syn::Error::new( - path.span(), - "the path should not be empty", - )); - return false; + NestedMeta::Meta(param) => match param { + Meta::List(list) => { + let span = list.span(); + let msg = "List item is not valid"; + errors.push(syn::Error::new(span, msg)); + continue; } + Meta::Path(path) => (path, None), + Meta::NameValue(name_value) => { + (&name_value.path, Some(&name_value.lit)) + } + }, }; - let path = last.ident.to_string(); - - // Match optional export arguments - match path.as_str() { + if path.is_ident("rpc") { // rpc mode - "rpc" => { - let value = if let syn::Lit::Str(lit_str) = lit { - lit_str.value() - } else { + match lit { + None => { errors.push(syn::Error::new( - last.span(), - "unexpected type for rpc value, expected Str", + nested_meta.span(), + "name parameter requires string value", )); - return false; - }; - + } + Some(Lit::Str(str)) => { + let value = str.value(); if let Some(mode) = RpcMode::parse(value.as_str()) { if rpc.replace(mode).is_some() { errors.push(syn::Error::new( - last.span(), + nested_meta.span(), "rpc mode was set more than once", )); - return false; } } else { errors.push(syn::Error::new( - last.span(), + nested_meta.span(), format!("unexpected value for rpc: {}", value), )); - return false; } } + _ => { + errors.push(syn::Error::new( + nested_meta.span(), + "unexpected type for rpc value, expected string", + )); + } + } + } else if path.is_ident("name") { // name override - "name" => { - let value = if let syn::Lit::Str(lit_str) = lit { - lit_str.value() - } else { + match lit { + None => { errors.push(syn::Error::new( - last.span(), - "unexpected type for name value, expected Str", + nested_meta.span(), + "name parameter requires string value", )); - return false; - }; - - if name_override.replace(value).is_some() { + } + Some(Lit::Str(str)) => { + if name_override.replace(str.value()).is_some() { errors.push(syn::Error::new( - last.span(), + nested_meta.span(), "name was set more than once", )); - return false; } } _ => { - let msg = - format!("unknown option for export: `{}`", path); - errors.push(syn::Error::new(last.span(), msg)); - return false; + errors.push(syn::Error::new( + nested_meta.span(), + "unexpected type for name value, expected string", + )); } } + } + } else { + let msg = format!( + "unknown option for export: `{}`", + path.to_token_stream() + ); + errors.push(syn::Error::new(nested_meta.span(), msg)); } } return false; From 552b7a81bba0864bd25973e54cccc9e22d2c92df Mon Sep 17 00:00:00 2001 From: B_head Date: Sun, 6 Mar 2022 01:46:51 +0900 Subject: [PATCH 02/12] Add deref_return parameter for export argument https://github.com/godot-rust/godot-rust/pull/870 --- gdnative-core/src/export/macros.rs | 32 +++++++++++++++++++++++++++++- gdnative-derive/src/methods.rs | 19 ++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/gdnative-core/src/export/macros.rs b/gdnative-core/src/export/macros.rs index 92b3bea0f..5fc1708e9 100644 --- a/gdnative-core/src/export/macros.rs +++ b/gdnative-core/src/export/macros.rs @@ -11,11 +11,23 @@ macro_rules! godot_wrap_method_parameter_count { } } +#[doc(hidden)] +#[macro_export] +macro_rules! godot_wrap_method_if_deref { + (true, $ret:expr) => { + std::ops::Deref::deref(&$ret) + }; + (false, $ret:expr) => { + $ret + }; +} + #[doc(hidden)] #[macro_export] macro_rules! godot_wrap_method_inner { ( $type_name:ty, + $is_deref_return:ident, $map_method:ident, fn $method_name:ident( $self:ident, @@ -56,7 +68,9 @@ macro_rules! godot_wrap_method_inner { $($pname,)* $($opt_pname,)* ); - gdnative::core_types::OwnedToVariant::owned_to_variant(ret) + gdnative::core_types::OwnedToVariant::owned_to_variant( + $crate::godot_wrap_method_if_deref!($is_deref_return, ret) + ) } }) .unwrap_or_else(|err| { @@ -83,6 +97,7 @@ macro_rules! godot_wrap_method { // mutable ( $type_name:ty, + $is_deref_return:ident, fn $method_name:ident( &mut $self:ident, $owner:ident : $owner_ty:ty @@ -93,6 +108,7 @@ macro_rules! godot_wrap_method { ) => { $crate::godot_wrap_method_inner!( $type_name, + $is_deref_return, map_mut, fn $method_name( $self, @@ -105,6 +121,7 @@ macro_rules! godot_wrap_method { // immutable ( $type_name:ty, + $is_deref_return:ident, fn $method_name:ident( & $self:ident, $owner:ident : $owner_ty:ty @@ -115,6 +132,7 @@ macro_rules! godot_wrap_method { ) => { $crate::godot_wrap_method_inner!( $type_name, + $is_deref_return, map, fn $method_name( $self, @@ -127,6 +145,7 @@ macro_rules! godot_wrap_method { // owned ( $type_name:ty, + $is_deref_return:ident, fn $method_name:ident( mut $self:ident, $owner:ident : $owner_ty:ty @@ -137,6 +156,7 @@ macro_rules! godot_wrap_method { ) => { $crate::godot_wrap_method_inner!( $type_name, + $is_deref_return, map_owned, fn $method_name( $self, @@ -149,6 +169,7 @@ macro_rules! godot_wrap_method { // owned ( $type_name:ty, + $is_deref_return:ident, fn $method_name:ident( $self:ident, $owner:ident : $owner_ty:ty @@ -159,6 +180,7 @@ macro_rules! godot_wrap_method { ) => { $crate::godot_wrap_method_inner!( $type_name, + $is_deref_return, map_owned, fn $method_name( $self, @@ -171,6 +193,7 @@ macro_rules! godot_wrap_method { // mutable without return type ( $type_name:ty, + $is_deref_return:ident, fn $method_name:ident( &mut $self:ident, $owner:ident : $owner_ty:ty @@ -181,6 +204,7 @@ macro_rules! godot_wrap_method { ) => { $crate::godot_wrap_method!( $type_name, + $is_deref_return, fn $method_name( &mut $self, $owner: $owner_ty @@ -192,6 +216,7 @@ macro_rules! godot_wrap_method { // immutable without return type ( $type_name:ty, + $is_deref_return:ident, fn $method_name:ident( & $self:ident, $owner:ident : $owner_ty:ty @@ -202,6 +227,7 @@ macro_rules! godot_wrap_method { ) => { $crate::godot_wrap_method!( $type_name, + $is_deref_return, fn $method_name( & $self, $owner: $owner_ty @@ -213,6 +239,7 @@ macro_rules! godot_wrap_method { // owned without return type ( $type_name:ty, + $is_deref_return:ident, fn $method_name:ident( mut $self:ident, $owner:ident : $owner_ty:ty @@ -223,6 +250,7 @@ macro_rules! godot_wrap_method { ) => { $crate::godot_wrap_method!( $type_name, + $is_deref_return, fn $method_name( $self, $owner: $owner_ty @@ -234,6 +262,7 @@ macro_rules! godot_wrap_method { // owned without return type ( $type_name:ty, + $is_deref_return:ident, fn $method_name:ident( $self:ident, $owner:ident : $owner_ty:ty @@ -244,6 +273,7 @@ macro_rules! godot_wrap_method { ) => { $crate::godot_wrap_method!( $type_name, + $is_deref_return, fn $method_name( $self, $owner: $owner_ty diff --git a/gdnative-derive/src/methods.rs b/gdnative-derive/src/methods.rs index bcc4a1082..c4d264cff 100644 --- a/gdnative-derive/src/methods.rs +++ b/gdnative-derive/src/methods.rs @@ -66,6 +66,7 @@ pub(crate) struct ExportArgs { pub(crate) optional_args: Option, pub(crate) rpc_mode: RpcMode, pub(crate) name_override: Option, + pub(crate) is_deref_return: bool, } pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { @@ -116,6 +117,7 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { }; let rpc = args.rpc_mode; + let is_deref_return = args.is_deref_return; let args = sig.inputs.iter().enumerate().map(|(n, arg)| { let span = arg.span(); @@ -130,6 +132,7 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { { let method = ::gdnative::export::godot_wrap_method!( #class_name, + #is_deref_return, fn #name ( #( #args )* ) -> #ret_ty ); @@ -183,6 +186,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { let mut export_args = None; let mut rpc = None; let mut name_override = None; + let mut is_deref_return = false; let mut errors = vec![]; @@ -292,6 +296,20 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { )); } } + } else if path.is_ident("deref_return") { + // deref return value + if lit.is_some() { + errors.push(syn::Error::new( + nested_meta.span(), + "value for deref_return parameter is not valid", + )); + } else if is_deref_return { + errors.push(syn::Error::new( + nested_meta.span(), + "deref_return was apply more than once", + )); + } else { + is_deref_return = true; } } else { let msg = format!( @@ -350,6 +368,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { export_args.optional_args = optional_args; export_args.rpc_mode = rpc.unwrap_or(RpcMode::Disabled); export_args.name_override = name_override; + export_args.is_deref_return = is_deref_return; methods_to_export.push(ExportMethod { sig: method.sig.clone(), From c3f5fe84910bef6555cd4fd1ed2079a681ae5b2e Mon Sep 17 00:00:00 2001 From: B_head Date: Sun, 6 Mar 2022 09:07:31 +0900 Subject: [PATCH 03/12] Deprecated return by reference The ways of emit warnings is a terrible hack. This is because there is no way to emit warnings from macros in stable Rust. Follow these steps to emit warnings. - Detect whether reference types are used in derive_methods(). - Expand the call to deprecated_reference_return!() macro to user code. --- gdnative-core/src/export/macros.rs | 7 +++++ gdnative-core/src/export/mod.rs | 2 ++ gdnative-derive/src/lib.rs | 2 +- gdnative-derive/src/methods.rs | 42 +++++++++++++++++++----------- 4 files changed, 37 insertions(+), 16 deletions(-) diff --git a/gdnative-core/src/export/macros.rs b/gdnative-core/src/export/macros.rs index 5fc1708e9..8e5d76d46 100644 --- a/gdnative-core/src/export/macros.rs +++ b/gdnative-core/src/export/macros.rs @@ -22,6 +22,13 @@ macro_rules! godot_wrap_method_if_deref { }; } +#[doc(hidden)] +#[macro_export] +#[deprecated = "This function does not actually pass by reference to the Godot engine. You can clarify by writing #[export(deref_return)]."] +macro_rules! deprecated_reference_return { + () => {}; +} + #[doc(hidden)] #[macro_export] macro_rules! godot_wrap_method_inner { diff --git a/gdnative-core/src/export/mod.rs b/gdnative-core/src/export/mod.rs index 70fdd1437..a59619d7c 100644 --- a/gdnative-core/src/export/mod.rs +++ b/gdnative-core/src/export/mod.rs @@ -26,6 +26,8 @@ pub(crate) mod type_tag; pub mod user_data; +#[allow(deprecated)] +pub use crate::deprecated_reference_return; pub use crate::godot_wrap_method; pub use class::*; pub use class_builder::*; diff --git a/gdnative-derive/src/lib.rs b/gdnative-derive/src/lib.rs index 612a0df54..e37c3c006 100644 --- a/gdnative-derive/src/lib.rs +++ b/gdnative-derive/src/lib.rs @@ -50,7 +50,7 @@ mod variant; /// impl gdnative::export::NativeClassMethods for Foo { /// fn register(builder: &ClassBuilder) { /// use gdnative::export::*; -/// builder.method("foo", gdnative::export::godot_wrap_method!(Foo, fn foo(&self, _owner: &Reference, bar: i64) -> i64)) +/// builder.method("foo", gdnative::export::godot_wrap_method!(Foo, false, fn foo(&self, _owner: &Reference, bar: i64) -> i64)) /// .with_rpc_mode(RpcMode::Disabled) /// .done_stateless(); /// } diff --git a/gdnative-derive/src/methods.rs b/gdnative-derive/src/methods.rs index c4d264cff..1aa2e07a4 100644 --- a/gdnative-derive/src/methods.rs +++ b/gdnative-derive/src/methods.rs @@ -86,7 +86,7 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { let name = sig.ident; let name_string = args.name_override.unwrap_or_else(|| name.to_string()); let ret_span = sig.output.span(); - let ret_ty = match sig.output { + let ret_ty = match &sig.output { syn::ReturnType::Default => quote_spanned!(ret_span => ()), syn::ReturnType::Type(_, ty) => quote_spanned!( ret_span => #ty ), }; @@ -128,6 +128,16 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { } }); + let deprecated = if let syn::ReturnType::Type(_, ty) = &sig.output { + if !is_deref_return && matches!(**ty, syn::Type::Reference(_)) { + quote_spanned!(ret_span=> ::gdnative::export::deprecated_reference_return!();) + } else { + quote_spanned!(ret_span=>) + } + } else { + quote_spanned!(ret_span=>) + }; + quote_spanned!( sig_span=> { let method = ::gdnative::export::godot_wrap_method!( @@ -139,6 +149,8 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { #builder.method(#name_string, method) .with_rpc_mode(#rpc) .done_stateless(); + + #deprecated } ) }) @@ -204,19 +216,19 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { let _export_args = export_args.get_or_insert_with(ExportArgs::default); use syn::{punctuated::Punctuated, Lit, Meta, NestedMeta}; let nested_meta_iter = match attr.parse_meta() { - Err(err) => { - errors.push(err); - return false; - } + Err(err) => { + errors.push(err); + return false; + } Ok(Meta::NameValue(name_value)) => { let span = name_value.span(); let msg = "NameValue syntax is not valid"; - errors.push(syn::Error::new(span, msg)); + errors.push(syn::Error::new(span, msg)); return false; - } + } Ok(Meta::Path(_)) => { Punctuated::::new().into_iter() - } + } Ok(Meta::List(list)) => list.nested.into_iter(), }; for nested_meta in nested_meta_iter { @@ -239,15 +251,15 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { (&name_value.path, Some(&name_value.lit)) } }, - }; + }; if path.is_ident("rpc") { - // rpc mode + // rpc mode match lit { None => { - errors.push(syn::Error::new( + errors.push(syn::Error::new( nested_meta.span(), "name parameter requires string value", - )); + )); } Some(Lit::Str(str)) => { let value = str.value(); @@ -273,13 +285,13 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { } } } else if path.is_ident("name") { - // name override + // name override match lit { None => { - errors.push(syn::Error::new( + errors.push(syn::Error::new( nested_meta.span(), "name parameter requires string value", - )); + )); } Some(Lit::Str(str)) => { if name_override.replace(str.value()).is_some() { From badbdbefbafa501d55e6866cd2ed335c3d8e2dcc Mon Sep 17 00:00:00 2001 From: B_head Date: Wed, 9 Mar 2022 06:49:05 +0900 Subject: [PATCH 04/12] Refactoring: Clarify code responsibility - Separate method parameters and macro parameters. - Separate location of structure building. --- gdnative-derive/src/methods.rs | 40 ++++++++++++++-------------------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/gdnative-derive/src/methods.rs b/gdnative-derive/src/methods.rs index 1aa2e07a4..6671ac1d6 100644 --- a/gdnative-derive/src/methods.rs +++ b/gdnative-derive/src/methods.rs @@ -58,13 +58,13 @@ pub(crate) struct ClassMethodExport { #[derive(Clone, Eq, PartialEq, Hash, Debug)] pub(crate) struct ExportMethod { pub(crate) sig: Signature, - pub(crate) args: ExportArgs, + pub(crate) export_args: ExportArgs, + pub(crate) optional_args: Option, } #[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Default)] pub(crate) struct ExportArgs { - pub(crate) optional_args: Option, - pub(crate) rpc_mode: RpcMode, + pub(crate) rpc_mode: Option, pub(crate) name_override: Option, pub(crate) is_deref_return: bool, } @@ -80,11 +80,11 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { let methods = export .methods .into_iter() - .map(|ExportMethod { sig, args }| { + .map(|ExportMethod { sig, export_args , optional_args}| { let sig_span = sig.ident.span(); let name = sig.ident; - let name_string = args.name_override.unwrap_or_else(|| name.to_string()); + let name_string = export_args.name_override.unwrap_or_else(|| name.to_string()); let ret_span = sig.output.span(); let ret_ty = match &sig.output { syn::ReturnType::Default => quote_spanned!(ret_span => ()), @@ -101,7 +101,7 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { .to_compile_error(); } - let optional_args = match args.optional_args { + let optional_args = match optional_args { Some(count) => { let max_optional = arg_count - 2; // self and owner if count > max_optional { @@ -116,8 +116,8 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { None => 0, }; - let rpc = args.rpc_mode; - let is_deref_return = args.is_deref_return; + let rpc = export_args.rpc_mode.unwrap_or(RpcMode::Disabled); + let is_deref_return = export_args.is_deref_return; let args = sig.inputs.iter().enumerate().map(|(n, arg)| { let span = arg.span(); @@ -196,10 +196,6 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { let items = match func { ImplItem::Method(mut method) => { let mut export_args = None; - let mut rpc = None; - let mut name_override = None; - let mut is_deref_return = false; - let mut errors = vec![]; // only allow the "outer" style, aka #[thing] item. @@ -213,7 +209,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { .map(|i| i.ident.to_string()); if let Some("export") = last_seg.as_deref() { - let _export_args = export_args.get_or_insert_with(ExportArgs::default); + let mut _export_args = export_args.get_or_insert_with(ExportArgs::default); use syn::{punctuated::Punctuated, Lit, Meta, NestedMeta}; let nested_meta_iter = match attr.parse_meta() { Err(err) => { @@ -264,7 +260,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { Some(Lit::Str(str)) => { let value = str.value(); if let Some(mode) = RpcMode::parse(value.as_str()) { - if rpc.replace(mode).is_some() { + if _export_args.rpc_mode.replace(mode).is_some() { errors.push(syn::Error::new( nested_meta.span(), "rpc mode was set more than once", @@ -294,7 +290,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { )); } Some(Lit::Str(str)) => { - if name_override.replace(str.value()).is_some() { + if _export_args.name_override.replace(str.value()).is_some() { errors.push(syn::Error::new( nested_meta.span(), "name was set more than once", @@ -315,13 +311,13 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { nested_meta.span(), "value for deref_return parameter is not valid", )); - } else if is_deref_return { + } else if _export_args.is_deref_return { errors.push(syn::Error::new( nested_meta.span(), "deref_return was apply more than once", )); } else { - is_deref_return = true; + _export_args.is_deref_return = true; } } else { let msg = format!( @@ -338,7 +334,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { true }); - if let Some(mut export_args) = export_args.take() { + if let Some(export_args) = export_args.take() { let mut optional_args = None; for (n, arg) in method.sig.inputs.iter_mut().enumerate() { @@ -377,14 +373,10 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { } } - export_args.optional_args = optional_args; - export_args.rpc_mode = rpc.unwrap_or(RpcMode::Disabled); - export_args.name_override = name_override; - export_args.is_deref_return = is_deref_return; - methods_to_export.push(ExportMethod { sig: method.sig.clone(), - args: export_args, + export_args, + optional_args, }); } From 6d7c7348e377ae7909e6adcffaba0761b8461e09 Mon Sep 17 00:00:00 2001 From: B_head Date: Wed, 9 Mar 2022 09:01:12 +0900 Subject: [PATCH 05/12] Refactoring of export::macros.rs - Reduction of codes. - Normalize comma positions. --- gdnative-core/src/export/macros.rs | 189 +++++++++-------------------- 1 file changed, 54 insertions(+), 135 deletions(-) diff --git a/gdnative-core/src/export/macros.rs b/gdnative-core/src/export/macros.rs index 8e5d76d46..df11a9372 100644 --- a/gdnative-core/src/export/macros.rs +++ b/gdnative-core/src/export/macros.rs @@ -37,9 +37,9 @@ macro_rules! godot_wrap_method_inner { $is_deref_return:ident, $map_method:ident, fn $method_name:ident( - $self:ident, - $owner:ident : $owner_ty:ty - $(,$pname:ident : $pty:ty)* + $self:ident + , $owner:ident : $owner_ty:ty + $(, $pname:ident : $pty:ty)* $(, #[opt] $opt_pname:ident : $opt_pty:ty)* ) -> $retty:ty ) => { @@ -97,6 +97,17 @@ macro_rules! godot_wrap_method_inner { }; } +#[doc(hidden)] +#[macro_export] +macro_rules! godot_wrap_method_return_type { + () => { + () + }; + ($retty:ty) => { + $retty: ty + }; +} + /// Convenience macro to wrap an object's method into a function pointer /// that can be passed to the engine when registering a class. #[macro_export] @@ -106,23 +117,23 @@ macro_rules! godot_wrap_method { $type_name:ty, $is_deref_return:ident, fn $method_name:ident( - &mut $self:ident, - $owner:ident : $owner_ty:ty - $(,$pname:ident : $pty:ty)* - $(,#[opt] $opt_pname:ident : $opt_pty:ty)* + &mut $self:ident + , $owner:ident : $owner_ty:ty + $(, $pname:ident : $pty:ty)* + $(, #[opt] $opt_pname:ident : $opt_pty:ty)* $(,)? - ) -> $retty:ty + ) $(-> $retty:ty)? ) => { $crate::godot_wrap_method_inner!( $type_name, $is_deref_return, map_mut, fn $method_name( - $self, - $owner: $owner_ty - $(,$pname : $pty)* - $(,#[opt] $opt_pname : $opt_pty)* - ) -> $retty + $self + , $owner : $owner_ty + $(, $pname : $pty)* + $(, #[opt] $opt_pname : $opt_pty)* + ) -> godot_wrap_method_return_type!($($retty)?) ) }; // immutable @@ -130,23 +141,23 @@ macro_rules! godot_wrap_method { $type_name:ty, $is_deref_return:ident, fn $method_name:ident( - & $self:ident, - $owner:ident : $owner_ty:ty - $(,$pname:ident : $pty:ty)* - $(,#[opt] $opt_pname:ident : $opt_pty:ty)* + & $self:ident + , $owner:ident : $owner_ty:ty + $(, $pname:ident : $pty:ty)* + $(, #[opt] $opt_pname:ident : $opt_pty:ty)* $(,)? - ) -> $retty:ty + ) $(-> $retty:ty)? ) => { $crate::godot_wrap_method_inner!( $type_name, $is_deref_return, map, fn $method_name( - $self, - $owner: $owner_ty - $(,$pname : $pty)* - $(,#[opt] $opt_pname : $opt_pty)* - ) -> $retty + $self + , $owner : $owner_ty + $(, $pname : $pty)* + $(, #[opt] $opt_pname : $opt_pty)* + ) -> godot_wrap_method_return_type!($($retty)?) ) }; // owned @@ -154,23 +165,23 @@ macro_rules! godot_wrap_method { $type_name:ty, $is_deref_return:ident, fn $method_name:ident( - mut $self:ident, - $owner:ident : $owner_ty:ty - $(,$pname:ident : $pty:ty)* - $(,#[opt] $opt_pname:ident : $opt_pty:ty)* + mut $self:ident + , $owner:ident : $owner_ty:ty + $(, $pname:ident : $pty:ty)* + $(, #[opt] $opt_pname:ident : $opt_pty:ty)* $(,)? - ) -> $retty:ty + ) $(-> $retty:ty)? ) => { $crate::godot_wrap_method_inner!( $type_name, $is_deref_return, map_owned, fn $method_name( - $self, - $owner: $owner_ty - $(,$pname : $pty)* - $(,#[opt] $opt_pname : $opt_pty)* - ) -> $retty + $self + , $owner : $owner_ty + $(, $pname : $pty)* + $(, #[opt] $opt_pname : $opt_pty)* + ) -> godot_wrap_method_return_type!($($retty)?) ) }; // owned @@ -178,115 +189,23 @@ macro_rules! godot_wrap_method { $type_name:ty, $is_deref_return:ident, fn $method_name:ident( - $self:ident, - $owner:ident : $owner_ty:ty - $(,$pname:ident : $pty:ty)* - $(,#[opt] $opt_pname:ident : $opt_pty:ty)* + $self:ident + , $owner:ident : $owner_ty:ty + $(, $pname:ident : $pty:ty)* + $(, #[opt] $opt_pname:ident : $opt_pty:ty)* $(,)? - ) -> $retty:ty + ) $(-> $retty:ty)? ) => { $crate::godot_wrap_method_inner!( $type_name, $is_deref_return, map_owned, fn $method_name( - $self, - $owner: $owner_ty - $(,$pname : $pty)* - $(,#[opt] $opt_pname : $opt_pty)* - ) -> $retty - ) - }; - // mutable without return type - ( - $type_name:ty, - $is_deref_return:ident, - fn $method_name:ident( - &mut $self:ident, - $owner:ident : $owner_ty:ty - $(,$pname:ident : $pty:ty)* - $(,#[opt] $opt_pname:ident : $opt_pty:ty)* - $(,)? - ) - ) => { - $crate::godot_wrap_method!( - $type_name, - $is_deref_return, - fn $method_name( - &mut $self, - $owner: $owner_ty - $(,$pname : $pty)* - $(,#[opt] $opt_pname : $opt_pty)* - ) -> () - ) - }; - // immutable without return type - ( - $type_name:ty, - $is_deref_return:ident, - fn $method_name:ident( - & $self:ident, - $owner:ident : $owner_ty:ty - $(,$pname:ident : $pty:ty)* - $(,#[opt] $opt_pname:ident : $opt_pty:ty)* - $(,)? - ) - ) => { - $crate::godot_wrap_method!( - $type_name, - $is_deref_return, - fn $method_name( - & $self, - $owner: $owner_ty - $(,$pname : $pty)* - $(,#[opt] $opt_pname : $opt_pty)* - ) -> () - ) - }; - // owned without return type - ( - $type_name:ty, - $is_deref_return:ident, - fn $method_name:ident( - mut $self:ident, - $owner:ident : $owner_ty:ty - $(,$pname:ident : $pty:ty)* - $(,#[opt] $opt_pname:ident : $opt_pty:ty)* - $(,)? - ) - ) => { - $crate::godot_wrap_method!( - $type_name, - $is_deref_return, - fn $method_name( - $self, - $owner: $owner_ty - $(,$pname : $pty)* - $(,#[opt] $opt_pname : $opt_pty)* - ) -> () - ) - }; - // owned without return type - ( - $type_name:ty, - $is_deref_return:ident, - fn $method_name:ident( - $self:ident, - $owner:ident : $owner_ty:ty - $(,$pname:ident : $pty:ty)* - $(,#[opt] $opt_pname:ident : $opt_pty:ty)* - $(,)? - ) - ) => { - $crate::godot_wrap_method!( - $type_name, - $is_deref_return, - fn $method_name( - $self, - $owner: $owner_ty - $(,$pname : $pty)* - $(,#[opt] $opt_pname : $opt_pty)* - ) -> () + $self + , $owner : $owner_ty + $(, $pname : $pty)* + $(, #[opt] $opt_pname : $opt_pty)* + ) -> godot_wrap_method_return_type!($($retty)?) ) }; } From a6d479d976ae95f9e01ced3c12abbb696fd30c47 Mon Sep 17 00:00:00 2001 From: B_head Date: Sat, 30 Apr 2022 19:37:56 +0900 Subject: [PATCH 06/12] Add #[method] and #[base] attribute https://github.com/godot-rust/godot-rust/pull/872 --- gdnative-core/src/export/macros.rs | 36 +++++++++---- gdnative-derive/src/lib.rs | 2 +- gdnative-derive/src/methods.rs | 83 ++++++++++++++++++++++++------ 3 files changed, 93 insertions(+), 28 deletions(-) diff --git a/gdnative-core/src/export/macros.rs b/gdnative-core/src/export/macros.rs index df11a9372..083cd9e40 100644 --- a/gdnative-core/src/export/macros.rs +++ b/gdnative-core/src/export/macros.rs @@ -22,6 +22,12 @@ macro_rules! godot_wrap_method_if_deref { }; } +// The ways of emit warnings is a terrible hack. +// This is because there is no way to emit warnings from macros in stable Rust. +// +// Follow these steps to emit warnings. +// - Detect whether reference types are used in gdnative-derive::methods::derive_methods(). +// - Expand the call to the deprecated_reference_return!() macro to user code. #[doc(hidden)] #[macro_export] #[deprecated = "This function does not actually pass by reference to the Godot engine. You can clarify by writing #[export(deref_return)]."] @@ -29,6 +35,14 @@ macro_rules! deprecated_reference_return { () => {}; } +#[doc(hidden)] +#[macro_export] +macro_rules! godot_wrap_method_void { + ($ident:ident, $void:tt) => { + $ident + }; +} + #[doc(hidden)] #[macro_export] macro_rules! godot_wrap_method_inner { @@ -38,7 +52,7 @@ macro_rules! godot_wrap_method_inner { $map_method:ident, fn $method_name:ident( $self:ident - , $owner:ident : $owner_ty:ty + $(, #[base] $base:ident : $base_ty:ty)? $(, $pname:ident : $pty:ty)* $(, #[opt] $opt_pname:ident : $opt_pty:ty)* ) -> $retty:ty @@ -67,11 +81,11 @@ macro_rules! godot_wrap_method_inner { Args { $($pname,)* $($opt_pname,)* }: Args, ) -> $crate::core_types::Variant { this - .$map_method(|__rust_val, $owner| { + .$map_method(|__rust_val, __base| { #[allow(unused_unsafe)] unsafe { let ret = __rust_val.$method_name( - OwnerArg::from_safe_ref($owner), + $(OwnerArg::from_safe_ref($crate::godot_wrap_method_void!(__base,$base)),)? $($pname,)* $($opt_pname,)* ); @@ -118,7 +132,7 @@ macro_rules! godot_wrap_method { $is_deref_return:ident, fn $method_name:ident( &mut $self:ident - , $owner:ident : $owner_ty:ty + $(, #[base] $base:ident : $base_ty:ty)? $(, $pname:ident : $pty:ty)* $(, #[opt] $opt_pname:ident : $opt_pty:ty)* $(,)? @@ -130,7 +144,7 @@ macro_rules! godot_wrap_method { map_mut, fn $method_name( $self - , $owner : $owner_ty + $(, #[base] $base : $base_ty)? $(, $pname : $pty)* $(, #[opt] $opt_pname : $opt_pty)* ) -> godot_wrap_method_return_type!($($retty)?) @@ -142,7 +156,7 @@ macro_rules! godot_wrap_method { $is_deref_return:ident, fn $method_name:ident( & $self:ident - , $owner:ident : $owner_ty:ty + $(, #[base] $base:ident : $base_ty:ty)? $(, $pname:ident : $pty:ty)* $(, #[opt] $opt_pname:ident : $opt_pty:ty)* $(,)? @@ -154,7 +168,7 @@ macro_rules! godot_wrap_method { map, fn $method_name( $self - , $owner : $owner_ty + $(, #[base] $base : $base_ty)? $(, $pname : $pty)* $(, #[opt] $opt_pname : $opt_pty)* ) -> godot_wrap_method_return_type!($($retty)?) @@ -166,7 +180,7 @@ macro_rules! godot_wrap_method { $is_deref_return:ident, fn $method_name:ident( mut $self:ident - , $owner:ident : $owner_ty:ty + $(, #[base] $base:ident : $base_ty:ty)? $(, $pname:ident : $pty:ty)* $(, #[opt] $opt_pname:ident : $opt_pty:ty)* $(,)? @@ -178,7 +192,7 @@ macro_rules! godot_wrap_method { map_owned, fn $method_name( $self - , $owner : $owner_ty + $(, #[base] $base : $base_ty)? $(, $pname : $pty)* $(, #[opt] $opt_pname : $opt_pty)* ) -> godot_wrap_method_return_type!($($retty)?) @@ -190,7 +204,7 @@ macro_rules! godot_wrap_method { $is_deref_return:ident, fn $method_name:ident( $self:ident - , $owner:ident : $owner_ty:ty + $(, #[base] $base:ident : $base_ty:ty)? $(, $pname:ident : $pty:ty)* $(, #[opt] $opt_pname:ident : $opt_pty:ty)* $(,)? @@ -202,7 +216,7 @@ macro_rules! godot_wrap_method { map_owned, fn $method_name( $self - , $owner : $owner_ty + $(, #[base] $base : $base_ty)? $(, $pname : $pty)* $(, #[opt] $opt_pname : $opt_pty)* ) -> godot_wrap_method_return_type!($($retty)?) diff --git a/gdnative-derive/src/lib.rs b/gdnative-derive/src/lib.rs index e37c3c006..c3b2e3a7d 100644 --- a/gdnative-derive/src/lib.rs +++ b/gdnative-derive/src/lib.rs @@ -50,7 +50,7 @@ mod variant; /// impl gdnative::export::NativeClassMethods for Foo { /// fn register(builder: &ClassBuilder) { /// use gdnative::export::*; -/// builder.method("foo", gdnative::export::godot_wrap_method!(Foo, false, fn foo(&self, _owner: &Reference, bar: i64) -> i64)) +/// builder.method("foo", gdnative::export::godot_wrap_method!(Foo, false, fn foo(&self, #[base] _owner: &Reference, bar: i64) -> i64)) /// .with_rpc_mode(RpcMode::Disabled) /// .done_stateless(); /// } diff --git a/gdnative-derive/src/methods.rs b/gdnative-derive/src/methods.rs index 6671ac1d6..84470e1ed 100644 --- a/gdnative-derive/src/methods.rs +++ b/gdnative-derive/src/methods.rs @@ -60,10 +60,12 @@ pub(crate) struct ExportMethod { pub(crate) sig: Signature, pub(crate) export_args: ExportArgs, pub(crate) optional_args: Option, + pub(crate) exist_base_arg: bool, } #[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Default)] pub(crate) struct ExportArgs { + pub(crate) is_old_syntax: bool, pub(crate) rpc_mode: Option, pub(crate) name_override: Option, pub(crate) is_deref_return: bool, @@ -80,7 +82,7 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { let methods = export .methods .into_iter() - .map(|ExportMethod { sig, export_args , optional_args}| { + .map(|ExportMethod { sig, export_args, optional_args, exist_base_arg}| { let sig_span = sig.ident.span(); let name = sig.ident; @@ -93,10 +95,18 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { let arg_count = sig.inputs.len(); - if arg_count < 2 { + if arg_count == 0 { return syn::Error::new( sig_span, - "exported methods must take self and owner as arguments", + "exported methods must take self parameter", + ) + .to_compile_error(); + } + + if export_args.is_old_syntax && !exist_base_arg { + return syn::Error::new( + sig_span, + "exported methods must take second parameter", ) .to_compile_error(); } @@ -106,7 +116,7 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { let max_optional = arg_count - 2; // self and owner if count > max_optional { let message = format!( - "there can be at most {} optional arguments, got {}", + "there can be at most {} optional parameters, got {}", max_optional, count, ); return syn::Error::new(sig_span, message).to_compile_error(); @@ -121,13 +131,17 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { let args = sig.inputs.iter().enumerate().map(|(n, arg)| { let span = arg.span(); - if n < arg_count - optional_args { + if exist_base_arg && n == 1 { + quote_spanned!(span => #[base] #arg ,) + } + else if n < arg_count - optional_args { quote_spanned!(span => #arg ,) } else { quote_spanned!(span => #[opt] #arg ,) } }); + // See gdnative-core::export::deprecated_reference_return!() let deprecated = if let syn::ReturnType::Type(_, ty) = &sig.output { if !is_deref_return && matches!(**ty, syn::Type::Reference(_)) { quote_spanned!(ret_span=> ::gdnative::export::deprecated_reference_return!();) @@ -208,9 +222,23 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { .last() .map(|i| i.ident.to_string()); - if let Some("export") = last_seg.as_deref() { - let mut _export_args = export_args.get_or_insert_with(ExportArgs::default); + let (is_export, is_old_syntax) = if let Some("export") = last_seg.as_deref() + { + (true, true) + } else if let Some("method") = last_seg.as_deref() { + (true, false) + } else { + (false, false) + }; + + if is_export { use syn::{punctuated::Punctuated, Lit, Meta, NestedMeta}; + let mut export_args = + export_args.get_or_insert_with(ExportArgs::default); + export_args.is_old_syntax = is_old_syntax; + + // Codes like #[macro(path, name = "value")] are accepted. + // Codes like #[path], #[name = "value"] or #[macro("lit")] are not accepted. let nested_meta_iter = match attr.parse_meta() { Err(err) => { errors.push(err); @@ -260,7 +288,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { Some(Lit::Str(str)) => { let value = str.value(); if let Some(mode) = RpcMode::parse(value.as_str()) { - if _export_args.rpc_mode.replace(mode).is_some() { + if export_args.rpc_mode.replace(mode).is_some() { errors.push(syn::Error::new( nested_meta.span(), "rpc mode was set more than once", @@ -290,7 +318,11 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { )); } Some(Lit::Str(str)) => { - if _export_args.name_override.replace(str.value()).is_some() { + if export_args + .name_override + .replace(str.value()) + .is_some() + { errors.push(syn::Error::new( nested_meta.span(), "name was set more than once", @@ -311,13 +343,13 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { nested_meta.span(), "value for deref_return parameter is not valid", )); - } else if _export_args.is_deref_return { + } else if export_args.is_deref_return { errors.push(syn::Error::new( nested_meta.span(), "deref_return was apply more than once", )); } else { - _export_args.is_deref_return = true; + export_args.is_deref_return = true; } } else { let msg = format!( @@ -336,6 +368,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { if let Some(export_args) = export_args.take() { let mut optional_args = None; + let mut exist_base_arg = false; for (n, arg) in method.sig.inputs.iter_mut().enumerate() { let attrs = match arg { @@ -344,32 +377,49 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { }; let mut is_optional = false; + let mut is_base = false; attrs.retain(|attr| { if attr.path.is_ident("opt") { is_optional = true; false + } else if attr.path.is_ident("base") { + is_base = true; + false } else { true } }); + // In the old syntax, the second parameter is always the base parameter. + if export_args.is_old_syntax && n == 1 { + is_base = true; + } + if is_optional { if n < 2 { errors.push(syn::Error::new( arg.span(), "self or owner cannot be optional", )); - continue; + } else { + *optional_args.get_or_insert(0) += 1; } - - *optional_args.get_or_insert(0) += 1; } else if optional_args.is_some() { errors.push(syn::Error::new( arg.span(), "cannot add required parameters after optional ones", )); - continue; + } + + if is_base { + exist_base_arg = true; + if n != 1 { + errors.push(syn::Error::new( + arg.span(), + "base must be the second parameter.", + )); + } } } @@ -377,6 +427,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { sig: method.sig.clone(), export_args, optional_args, + exist_base_arg, }); } @@ -423,7 +474,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { continue; } - // remove "mut" from arguments. + // remove "mut" from parameters. // give every wildcard a (hopefully) unique name. method .sig From 7214f93d474bc26f9bfb43aa78df993859a309d9 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sun, 8 May 2022 14:37:41 +0200 Subject: [PATCH 07/12] Warn when using old #[export] syntax --- gdnative-core/src/export/macros.rs | 7 +++++++ gdnative-core/src/export/mod.rs | 4 ++-- gdnative-derive/src/methods.rs | 11 +++++++++-- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/gdnative-core/src/export/macros.rs b/gdnative-core/src/export/macros.rs index 083cd9e40..01f89e339 100644 --- a/gdnative-core/src/export/macros.rs +++ b/gdnative-core/src/export/macros.rs @@ -35,6 +35,13 @@ macro_rules! deprecated_reference_return { () => {}; } +#[doc(hidden)] +#[macro_export] +#[deprecated = "#[export] is deprecated and will be removed in the next major version. Use #[godot] instead."] +macro_rules! deprecated_export_syntax { + () => {}; +} + #[doc(hidden)] #[macro_export] macro_rules! godot_wrap_method_void { diff --git a/gdnative-core/src/export/mod.rs b/gdnative-core/src/export/mod.rs index a59619d7c..848c8d7c0 100644 --- a/gdnative-core/src/export/mod.rs +++ b/gdnative-core/src/export/mod.rs @@ -26,9 +26,9 @@ pub(crate) mod type_tag; pub mod user_data; -#[allow(deprecated)] -pub use crate::deprecated_reference_return; pub use crate::godot_wrap_method; +#[allow(deprecated)] +pub use crate::{deprecated_export_syntax, deprecated_reference_return}; pub use class::*; pub use class_builder::*; pub use method::*; diff --git a/gdnative-derive/src/methods.rs b/gdnative-derive/src/methods.rs index 84470e1ed..d7b8d9ac2 100644 --- a/gdnative-derive/src/methods.rs +++ b/gdnative-derive/src/methods.rs @@ -141,8 +141,14 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { } }); + let warn_deprecated_export = if export_args.is_old_syntax { + Some(quote_spanned!(ret_span=> ::gdnative::export::deprecated_export_syntax!();)) + } else { + None + }; + // See gdnative-core::export::deprecated_reference_return!() - let deprecated = if let syn::ReturnType::Type(_, ty) = &sig.output { + let warn_deprecated_ref_return = if let syn::ReturnType::Type(_, ty) = &sig.output { if !is_deref_return && matches!(**ty, syn::Type::Reference(_)) { quote_spanned!(ret_span=> ::gdnative::export::deprecated_reference_return!();) } else { @@ -164,7 +170,8 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { .with_rpc_mode(#rpc) .done_stateless(); - #deprecated + #warn_deprecated_export + #warn_deprecated_ref_return } ) }) From a63f64325f64b6c530f018b3eb931ac9c6ac3bd6 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sun, 8 May 2022 14:43:44 +0200 Subject: [PATCH 08/12] Improve error messages --- gdnative-derive/src/methods.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/gdnative-derive/src/methods.rs b/gdnative-derive/src/methods.rs index d7b8d9ac2..feba100f1 100644 --- a/gdnative-derive/src/methods.rs +++ b/gdnative-derive/src/methods.rs @@ -98,7 +98,7 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { if arg_count == 0 { return syn::Error::new( sig_span, - "exported methods must take self parameter", + "#[godot] exported methods must take self parameter", ) .to_compile_error(); } @@ -106,7 +106,7 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { if export_args.is_old_syntax && !exist_base_arg { return syn::Error::new( sig_span, - "exported methods must take second parameter", + "deprecated #[export] methods must take second parameter (base/owner)", ) .to_compile_error(); } @@ -289,7 +289,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { None => { errors.push(syn::Error::new( nested_meta.span(), - "name parameter requires string value", + "`rpc` parameter requires string value", )); } Some(Lit::Str(str)) => { @@ -298,20 +298,20 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { if export_args.rpc_mode.replace(mode).is_some() { errors.push(syn::Error::new( nested_meta.span(), - "rpc mode was set more than once", + "`rpc` mode was set more than once", )); } } else { errors.push(syn::Error::new( nested_meta.span(), - format!("unexpected value for rpc: {}", value), + format!("unexpected value for `rpc`: {}", value), )); } } _ => { errors.push(syn::Error::new( nested_meta.span(), - "unexpected type for rpc value, expected string", + "unexpected type for `rpc` value, expected string", )); } } @@ -321,7 +321,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { None => { errors.push(syn::Error::new( nested_meta.span(), - "name parameter requires string value", + "`name` parameter requires string value", )); } Some(Lit::Str(str)) => { @@ -332,14 +332,14 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { { errors.push(syn::Error::new( nested_meta.span(), - "name was set more than once", + "`name` was set more than once", )); } } _ => { errors.push(syn::Error::new( nested_meta.span(), - "unexpected type for name value, expected string", + "unexpected type for `name` value, expected string", )); } } @@ -348,19 +348,19 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { if lit.is_some() { errors.push(syn::Error::new( nested_meta.span(), - "value for deref_return parameter is not valid", + "`deref_return` does not take any values", )); } else if export_args.is_deref_return { errors.push(syn::Error::new( nested_meta.span(), - "deref_return was apply more than once", + "`deref_return` was set more than once", )); } else { export_args.is_deref_return = true; } } else { let msg = format!( - "unknown option for export: `{}`", + "unknown option for #[export]: `{}`", path.to_token_stream() ); errors.push(syn::Error::new(nested_meta.span(), msg)); @@ -407,7 +407,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { if n < 2 { errors.push(syn::Error::new( arg.span(), - "self or owner cannot be optional", + "self or base cannot be optional", )); } else { *optional_args.get_or_insert(0) += 1; From 51064168aec99880fd24b321689d6d1461fad66a Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sun, 8 May 2022 15:24:33 +0200 Subject: [PATCH 09/12] Rename #[method] -> #[godot], document new attributes --- gdnative-core/src/export/macros.rs | 3 +- gdnative-derive/src/lib.rs | 143 +++++++++++++++++------------ gdnative-derive/src/methods.rs | 7 +- 3 files changed, 93 insertions(+), 60 deletions(-) diff --git a/gdnative-core/src/export/macros.rs b/gdnative-core/src/export/macros.rs index 01f89e339..a6df805e5 100644 --- a/gdnative-core/src/export/macros.rs +++ b/gdnative-core/src/export/macros.rs @@ -37,7 +37,8 @@ macro_rules! deprecated_reference_return { #[doc(hidden)] #[macro_export] -#[deprecated = "#[export] is deprecated and will be removed in the next major version. Use #[godot] instead."] +#[deprecated = "\n#[export] is deprecated and will be removed in godot-rust 0.11. Use #[godot] instead.\n\ + For more information, see https://godot-rust.github.io/docs/gdnative/derive/derive.NativeClass.html."] macro_rules! deprecated_export_syntax { () => {}; } diff --git a/gdnative-derive/src/lib.rs b/gdnative-derive/src/lib.rs index c3b2e3a7d..d6f3ed455 100644 --- a/gdnative-derive/src/lib.rs +++ b/gdnative-derive/src/lib.rs @@ -205,13 +205,13 @@ pub fn profiled(meta: TokenStream, input: TokenStream) -> TokenStream { /// /// - `path = "my_category/my_property_name"` /// -/// Puts the property under the `my_category` category and renames it to -/// `my_property_name` in the inspector and for GDScript. +/// Puts the property under the `my_category` category and renames it to +/// `my_property_name` in the inspector and for GDScript. /// /// - `default = 42.0` /// -/// Sets the default value *in the inspector* for this property. The setter is *not* -/// guaranteed to be called by the engine with the value. +/// Sets the default value *in the inspector* for this property. The setter is *not* +/// guaranteed to be called by the engine with the value. /// /// - `before_get` / `after_get` / `before_set` / `after_set` `= "Self::hook_method"` /// @@ -220,22 +220,22 @@ pub fn profiled(meta: TokenStream, input: TokenStream) -> TokenStream { /// /// - `get` / `get_ref` / `set` /// -/// Configure getter/setter for property. All of them can accept a path to specify a custom -/// property accessor. For example, `#[property(get = "Self::my_getter")]` will use -/// `Self::my_getter` as the getter. +/// Configure getter/setter for property. All of them can accept a path to specify a custom +/// property accessor. For example, `#[property(get = "Self::my_getter")]` will use +/// `Self::my_getter` as the getter. /// -/// The difference of `get` and `get_ref` is that `get` will register the getter with -/// `with_getter` function, which means your getter should return an owned value `T`, but -/// `get_ref` use `with_ref_getter` to register getter. In this case, your custom getter -/// should return a shared reference `&T`. +/// The difference of `get` and `get_ref` is that `get` will register the getter with +/// `with_getter` function, which means your getter should return an owned value `T`, but +/// `get_ref` use `with_ref_getter` to register getter. In this case, your custom getter +/// should return a shared reference `&T`. /// -/// Situations with custom getters/setters and no backing fields require the use of the -/// type [`Property`][gdnative::export::Property]. Consult its documentation for -/// a deeper elaboration of property exporting. +/// Situations with custom getters/setters and no backing fields require the use of the +/// type [`Property`][gdnative::export::Property]. Consult its documentation for +/// a deeper elaboration of property exporting. /// /// - `no_editor` /// -/// Hides the property from the editor. Does not prevent it from being sent over network or saved in storage. +/// Hides the property from the editor. Does not prevent it from being sent over network or saved in storage. /// /// ### `#[methods]` /// Adds the necessary information to a an `impl` block to register the properties and methods with Godot. @@ -244,102 +244,139 @@ pub fn profiled(meta: TokenStream, input: TokenStream) -> TokenStream { /// /// For additional details about how `#[methods]` expands, please refer to [gdnative::methods](macro@methods) /// -/// ### `#[export]` +/// ### `#[godot]` /// Registers the attributed function signature to be used by Godot. +/// +/// This attribute was formerly called `#[export]`, but is not directly related to the concept of +/// [exporting](https://docs.godotengine.org/en/stable/tutorials/export/exporting_basics.html) in GDScript. +/// /// A valid function signature must have: /// - `&self` or `&mut self` as its first parameter -/// - `&T` or `TRef` where T refers to the type declared in `#[inherit(T)]` attribute as it's second parameter; this is typically called the `owner`. -/// - Optionally, any number of additional parameters, which must have the type `Variant` or must implement the `FromVariant` trait. `FromVariant` is implemented for most common types. +/// - Optionally, `&T` or `TRef` where T refers to the type declared in `#[inherit(T)]` attribute as it's second parameter; +/// this is typically called the _base_. The parameter must be attributed with `#[base]`. +/// - Any number of required parameters, which must have the type `Variant` or must implement the `FromVariant` trait. +/// `FromVariant` is implemented for most common types. +/// - Any number of optional parameters annotated with `#[opt]`. Same rules as for required parameters apply. +/// Optional parameters must appear at the end of the parameter list. /// - Return values must implement the `OwnedToVariant` trait (automatically implemented by `ToVariant`) /// or be a `Variant` type. /// /// ```ignore -/// #[export] -/// fn foo(&self, owner: &Reference); +/// // No access to base parameter +/// #[godot] +/// fn foo(&self); +/// +/// // Access base parameter as &T +/// #[godot] +/// fn foo(&self, #[base] base: &Reference); +/// +/// // Access base parameter as TRef +/// #[godot] +/// fn foo(&self, #[base] base: TRef); /// ``` -/// **Note**: Marking a function with `#[export]` does not have any effect unless inside an `impl` block that has the `#[methods]` attribute. /// -/// Possible arguments for this attribute are +/// **Note**: Marking a function with `#[godot]` does not have any effect unless inside an `impl` block that has the `#[methods]` attribute. +/// +/// Possible arguments for this attribute are: +/// +/// - `name = "overridden_function_name"` /// -/// - `name` = "overridden_function_name" +/// Overrides the function name as the method name to be registered in Godot. /// -/// Overrides the function name as the method name to be registered in Godot. +/// - `rpc = "selected_rpc"` /// -/// - `rpc` = "selected_rpc" -/// - "selected_rpc" must be one of the following values, which refer to the associated [Multiplayer API RPC Mode](https://docs.godotengine.org/en/stable/classes/class_multiplayerapi.html?highlight=RPC#enumerations) -/// - "disabled" - `RPCMode::RPC_MODE_DISABLED` -/// - "remote" - `RPCMode::RPC_MODE_REMOTE` -/// - "remote_sync" - `RPCMode::RPC_MODE_REMOTE_SYMC` -/// - "master" - `RPCMode::RPC_MODE_MASTER` -/// - "puppet" - `RPCMode::RPC_MODE_PUPPET` -/// - "master_sync" - `RPCMode::RPC_MODE_MASTERSYNC` -/// - "puppet_sync" - `RPCMode::RPC_MODE_PUPPETSYNC` +/// `"selected_rpc"` must be one of the following values, which refer to the associated [Multiplayer API RPC Mode](https://docs.godotengine.org/en/stable/classes/class_multiplayerapi.html?highlight=RPC#enumerations). +/// See also the Rust type [`export::RpcMode`]. +/// - `"disabled"` +/// - `"remote"` +/// - `"remote_sync"` +/// - `"master"` +/// - `"master_sync"` +/// - `"puppet"` +/// - `"puppet_sync"` +/// +/// This enables you to set the [Multiplayer API RPC Mode](https://docs.godotengine.org/en/stable/classes/class_multiplayerapi.html?highlight=RPC#enumerations) for the function. +/// Refer to [Godot's Remote Procedure documentation](https://docs.godotengine.org/en/stable/tutorials/networking/high_level_multiplayer.html#rpc) for more details. +/// +/// - `deref_return` +/// +/// Allows you to return a type using its `Deref` representation. This can avoid extra intermediate copies for larger objects, by explicitly +/// returning a reference (or in general, a type that dereferences to something that can be exported). +/// +/// For example: +/// ```ignore +/// #[godot(deref_return)] +/// fn get_numbers(&self) -> std::cell::Ref> { +/// // Assume self.cell is std::cell::RefCell> +/// self.cell.borrow() +/// } +/// ``` /// -/// This enables you to set the [Multiplayer API RPC Mode](https://docs.godotengine.org/en/stable/classes/class_multiplayerapi.html?highlight=RPC#enumerations) for the function. /// -/// Refer to [Godot's Remote Procedure documentation](https://docs.godotengine.org/en/stable/tutorials/networking/high_level_multiplayer.html#rpc) for more details. /// #### `Node` virtual functions /// /// This is a list of common Godot virtual functions that are automatically called via [notifications](https://docs.godotengine.org/en/stable/classes/class_object.html#class-object-method-notification). /// +/// It is assumed that every method is exported via `#[godot]` attribute. The parameter `#[base] base: &Node` can be omitted if you don't need it. +/// /// ```ignore -/// fn _ready(&self, owner: &Node); +/// fn _ready(&self, #[base] base: &Node); /// ``` /// Called when both the node and its children have entered the scene tree. /// _See [Godot docs](https://docs.godotengine.org/en/stable/classes/class_node.html#class-node-method-ready) for more information._ ///

/// /// ```ignore -/// fn _enter_tree(&self, owner: &Node); +/// fn _enter_tree(&self, #[base] base: &Node); /// ``` /// Called when the node enters the scene tree. /// _See [Godot docs](https://docs.godotengine.org/en/stable/classes/class_node.html#class-node-method-enter-tree) for more information._ ///

/// /// ```ignore -/// fn _exit_tree(&self, owner: &Node); +/// fn _exit_tree(&self, #[base] base: &Node); /// ``` /// Called when the node is removed from the scene tree. /// _See [Godot docs](https://docs.godotengine.org/en/stable/classes/class_node.html#class-node-method-exit-tree) for more information._ ///

/// /// ```ignore -/// fn _get_configuration_warning(&self, owner: &Node) -> GodotString; +/// fn _get_configuration_warning(&self, #[base] base: &Node) -> GodotString; /// ``` /// The string returned from this method is displayed as a warning in the Scene Dock if the script that overrides it is a tool script. /// _See [Godot docs](https://docs.godotengine.org/en/stable/classes/class_node.html#class-node-method-get-configuration-warning) for more information._ ///

/// /// ```ignore -/// fn _process(&mut self, owner: &Node, delta: f64); +/// fn _process(&mut self, #[base] base: &Node, delta: f64); /// ``` /// Called during processing step of the main loop. /// _See [Godot docs](https://docs.godotengine.org/en/stable/classes/class_node.html#class-node-method-process) for more information._ ///

/// /// ```ignore -/// fn _physics_process(&self, owner: &Node, delta: f64); +/// fn _physics_process(&self, #[base] base: &Node, delta: f64); /// ``` /// Called during physics update, with a fixed timestamp. /// _See [Godot docs](https://docs.godotengine.org/en/stable/classes/class_node.html#class-node-method-physics-process) for more information._ ///

/// /// ```ignore -/// fn _input(&self, owner: &Node, event: Ref); +/// fn _input(&self, #[base] base: &Node, event: Ref); /// ``` /// Called when there is an input event. /// _See [Godot docs](https://docs.godotengine.org/en/stable/classes/class_node.html#class-node-method-input) for more information._ ///

/// /// ```ignore -/// fn _unhandled_input(&self, owner: &Node, event: Ref); +/// fn _unhandled_input(&self, #[base] base: &Node, event: Ref); /// ``` /// Called when an `InputEvent` hasn't been consumed by `_input()` or any GUI. /// _See [Godot docs](https://docs.godotengine.org/en/stable/classes/class_node.html#class-node-method-unhandled-input) for more information._ ///

/// /// ```ignore -/// fn _unhandled_key_input (&self, owner: &Node, event: Ref); +/// fn _unhandled_key_input (&self, #[base] base: &Node, event: Ref); /// ``` /// Called when an `InputEventKey` hasn't been consumed by `_input()` or any GUI. /// _See [Godot docs](https://docs.godotengine.org/en/stable/classes/class_node.html#class-node-method-unhandled-key-input) for more information._ @@ -350,43 +387,35 @@ pub fn profiled(meta: TokenStream, input: TokenStream) -> TokenStream { /// This is a list of common Godot virtual functions that are automatically called via [notifications](https://docs.godotengine.org/en/stable/classes/class_object.html#class-object-method-notification). /// /// ```ignore -/// fn _clips_input(&self, owner: &Control) -> bool; +/// fn _clips_input(&self, #[base] base: &Control) -> bool; /// ``` /// Returns whether `_gui_input()` should not be called for children controls outside this control's rectangle. /// _See [Godot docs](https://docs.godotengine.org/en/stable/classes/class_control.html#class-control-method-clips-input) for more information._ ///

/// /// ```ignore -/// fn _get_minimum_size(&self, owner: &Control) -> Vector2; +/// fn _get_minimum_size(&self, #[base] base: &Control) -> Vector2; /// ``` /// Returns the minimum size for this control. /// _See [Godot docs](https://docs.godotengine.org/en/stable/classes/class_control.html#class-control-method-get-minimum-size) for more information._ ///

/// /// ```ignore -/// fn _gui_input(&self, owner: &Control, event: Ref); +/// fn _gui_input(&self, #[base] base: &Control, event: Ref); /// ``` /// Use this method to process and accept inputs on UI elements. /// _See [Godot docs](https://docs.godotengine.org/en/stable/classes/class_control.html#class-control-method-gui-input) for more information._ ///

/// /// ```ignore -/// fn _make_custom_tooltip(&self, owner: &Control, for_text: String) -> Ref; +/// fn _make_custom_tooltip(&self, #[base] base: &Control, for_text: String) -> Ref; /// ``` /// Returns a `Control` node that should be used as a tooltip instead of the default one. /// _See [Godot docs](https://docs.godotengine.org/en/stable/classes/class_control.html#class-control-method-make-custom-tooltip) for more information._ ///

#[proc_macro_derive( NativeClass, - attributes( - inherit, - export, - opt, - user_data, - property, - register_with, - no_constructor - ) + attributes(inherit, register_with, no_constructor, user_data, property) )] pub fn derive_native_class(input: TokenStream) -> TokenStream { // Converting the proc_macro::TokenStream into non proc_macro types so that tests diff --git a/gdnative-derive/src/methods.rs b/gdnative-derive/src/methods.rs index feba100f1..6ac346148 100644 --- a/gdnative-derive/src/methods.rs +++ b/gdnative-derive/src/methods.rs @@ -232,7 +232,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { let (is_export, is_old_syntax) = if let Some("export") = last_seg.as_deref() { (true, true) - } else if let Some("method") = last_seg.as_deref() { + } else if let Some("godot") = last_seg.as_deref() { (true, false) } else { (false, false) @@ -304,7 +304,10 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { } else { errors.push(syn::Error::new( nested_meta.span(), - format!("unexpected value for `rpc`: {}", value), + format!( + "unexpected value for `rpc`: {}", + value + ), )); } } From 1e9d4b3c7bda11e8e820b9454366d2e90a212a86 Mon Sep 17 00:00:00 2001 From: B_head Date: Tue, 10 May 2022 19:07:34 +0900 Subject: [PATCH 10/12] Fix: Display the proper macro name in error --- gdnative-derive/src/methods.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/gdnative-derive/src/methods.rs b/gdnative-derive/src/methods.rs index 6ac346148..d93983d82 100644 --- a/gdnative-derive/src/methods.rs +++ b/gdnative-derive/src/methods.rs @@ -229,14 +229,14 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { .last() .map(|i| i.ident.to_string()); - let (is_export, is_old_syntax) = if let Some("export") = last_seg.as_deref() - { - (true, true) - } else if let Some("godot") = last_seg.as_deref() { - (true, false) - } else { - (false, false) - }; + let (is_export, is_old_syntax, macro_name) = + if let Some("export") = last_seg.as_deref() { + (true, true, "export") + } else if let Some("godot") = last_seg.as_deref() { + (true, false, "godot") + } else { + (false, false, "unknown") + }; if is_export { use syn::{punctuated::Punctuated, Lit, Meta, NestedMeta}; @@ -363,7 +363,8 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { } } else { let msg = format!( - "unknown option for #[export]: `{}`", + "unknown option for #[{}]: `{}`", + macro_name, path.to_token_stream() ); errors.push(syn::Error::new(nested_meta.span(), msg)); From c2e34f4c77048611715d0a44d094a61ae217c15f Mon Sep 17 00:00:00 2001 From: B_head Date: Tue, 10 May 2022 19:08:27 +0900 Subject: [PATCH 11/12] Allow deprecated APIs in test codes --- test/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/src/lib.rs b/test/src/lib.rs index de734fa7d..97e6b4069 100644 --- a/test/src/lib.rs +++ b/test/src/lib.rs @@ -1,4 +1,5 @@ #![allow(clippy::blacklisted_name)] +#![allow(deprecated)] use gdnative::prelude::*; From e175059cda5da7da158934b0703f27471e352acf Mon Sep 17 00:00:00 2001 From: B_head Date: Tue, 10 May 2022 19:40:34 +0900 Subject: [PATCH 12/12] Replace `#[export]` in the example codes --- examples/dodge-the-creeps/src/hud.rs | 16 ++++++++-------- examples/dodge-the-creeps/src/main_scene.rs | 20 ++++++++++---------- examples/dodge-the-creeps/src/mob.rs | 12 ++++++------ examples/dodge-the-creeps/src/player.rs | 16 ++++++++-------- examples/hello-world/src/lib.rs | 4 ++-- examples/native-plugin/src/lib.rs | 16 ++++++++-------- examples/resource/src/lib.rs | 4 ++-- examples/rpc/src/client.rs | 12 ++++++------ examples/rpc/src/server.rs | 8 ++++---- examples/scene-create/src/lib.rs | 12 ++++++------ examples/signals/src/lib.rs | 16 ++++++++-------- examples/spinning-cube/src/lib.rs | 8 ++++---- gdnative/tests/ui/derive_pass.rs | 4 ++-- 13 files changed, 74 insertions(+), 74 deletions(-) diff --git a/examples/dodge-the-creeps/src/hud.rs b/examples/dodge-the-creeps/src/hud.rs index 0a8194828..b08957bd2 100644 --- a/examples/dodge-the-creeps/src/hud.rs +++ b/examples/dodge-the-creeps/src/hud.rs @@ -16,8 +16,8 @@ impl Hud { Hud } - #[export] - pub fn show_message(&self, owner: &CanvasLayer, text: String) { + #[godot] + pub fn show_message(&self, #[base] owner: &CanvasLayer, text: String) { let message_label = unsafe { owner.get_node_as::