Skip to content

Support optional arguments #192

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
1 commit merged into from Mar 25, 2020
Merged

Support optional arguments #192

1 commit merged into from Mar 25, 2020

Conversation

ghost
Copy link

@ghost ghost commented Aug 31, 2019

As @karroffel mentioned that a new cargo version is coming soon, I think it might be a good time to move this out of draft status, so the feature might get into the release!

Implements optional arguments in both godot_wrap_method and the methods procedural macro. Closes #185.

Explanation

This patch allows declaring optional arguments with the following syntax:

With godot_wrap_method:

builder.add_method("some_method", godot_wrap_method!(
    SomeType,
    fn some_method(&self, owner: Node, foo: i64, #[opt] bar: Option<String>, #[opt] baz: Variant) -> ()
));

With the procedural macro:

#[methods]
impl SomeType {
    #[export]
    fn some_method(&self, owner: Node, foo: i64, #[opt] bar: Option<String>, #[opt] baz: Variant) -> () {
        let bar = bar.unwrap_or_else(|| "DefaultValue".to_owned());
        // ... do things with bar ...
    }
}

Default values for optional arguments are obtained with Default. The behavior regarding Option<T> is consistent with its FromVariant implementation, so it should be easy to convert an existing function into one with optional arguments.

Future possibilities

VarArgs methods

With attribute argument parsing in place, it should be easy to later extend the syntax to allow VarArgs methods. Sample:

#[methods]
impl SomeType {
    /// foo is required, bar and baz are optional, and rest contains the rest of arguments if any remains
    #[export]
    fn some_method(&self, owner: Node, foo: i64, #[opt] bar: Option<String>, #[opt] baz: Variant, #[var] rest: VariantArray) -> () {
        // - snip -
    }
}

@ghost ghost changed the title [WIP] Support optional arguments in godot_wrap_method macro Support optional arguments Sep 23, 2019
@ghost ghost marked this pull request as ready for review September 23, 2019 15:28
@ghost
Copy link
Author

ghost commented Sep 23, 2019

Added procedural macro implementation and ready for review now!

@karroffel karroffel added feature Adds functionality to the library c: export Component: export (mod export, derive) labels Sep 24, 2019
@karroffel
Copy link
Member

This looks fine all in all, I am not too convinced about the attribute. I would say we wait until after the next release and give this some more testing?

@ghost
Copy link
Author

ghost commented Sep 28, 2019

Ok. That's reasonable.

@ghost
Copy link
Author

ghost commented Jan 10, 2020

Rebased on #274, will probably switch to the alternative syntax now that parameter attributes are stable for a few versions. Using a shorter attribute like #[opt] might help reduce the noise.

@ghost ghost changed the title Support optional arguments [WIP] Support optional arguments Jan 10, 2020
Default parameter values are obtained with `Default`. Use with `Option<T>`
possible, and consistent with `FromVariant` behavior.

Closes #185.
@ghost ghost changed the title [WIP] Support optional arguments Support optional arguments Mar 23, 2020
@ghost
Copy link
Author

ghost commented Mar 23, 2020

Updated the PR to use the argument attribute syntax, with the attribute #[opt].

@karroffel Could you review again?

Copy link
Member

@karroffel karroffel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Nice work!

@ghost ghost merged commit 40eb039 into godot-rust:master Mar 25, 2020
@ghost ghost deleted the feature/default-args branch March 25, 2020 12:14
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: export Component: export (mod export, derive) feature Adds functionality to the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible to have methods with default values?
1 participant