Skip to content

Default parameters in Engine API + meta param/return types #322

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
merged 11 commits into from
Jul 1, 2023

Conversation

Bromeon
Copy link
Member

@Bromeon Bromeon commented Jun 29, 2023

Default parameters in Engine API

Example RenderingServer::environment_set_ambient_light with signature:

void environment_set_ambient_light(
    RID env,
    Color color,
    EnvironmentAmbientSource ambient=0,
    float energy=1.0,
    float sky_contibution=0.0, 
    EnvironmentReflectionSource reflection_source=0
)

Previous gdext syntax:

server.environment_set_ambient_light(rid, color, 0, 1.0, 0.0, 0)

Now:

server.environment_set_ambient_light(rid, color)

With default arguments:

// select what you like, skip out-of-order:
server.environment_set_ambient_light_ex(rid, color).energy(2.0).done()

// provide all:
server.environment_set_ambient_light_ex(rid, color)
    .ambient(0)
    .energy(1.0)
    .sky_contribution(0.0)
    .reflection_source(0)
    .done()

See also this commit for the real-world impact this change has, within gdext alone.


Mechanics

Methods accepting default parameters come in two flavors: method and method_ex.

The _ex overload returns an "extender", a builder object providing a fluent API to accept values overriding the defaults. The arguments can be provided in any order, and arbitrary ones can be skipped. This is different from C++ and GDScript, where you have to provide all default arguments in their positional order.

This effectively implements an API similar to the one discussed in godot-rust/gdnative#814 (comment), however in a non-generic way and without enforcing ordering. Theoretically it's possible to specify the same argument twice, but that's a risk I'm gladly taking if I can avoid type-state with its compile-time and complexity implications.


Other changes

This pull request comes with a few collateral features:

  • Support for meta fields in the JSON file.
    Parameter or return types can have an extra "meta" type which constrains the domain for non-GDScript languages. For example, a parameter int can come with meta field uint16. These are translated to Rust, so that the correct type is passed (here u16).

  • GDScript expression parser.
    Implementing default parameters means we have to understand syntax like Vector2(1, 2) which is provided as the default_value field in GDScript. This one is then translated to an equivalent Rust representation when populating the extender struct.
    This turned out to be much more complex than anticipated, due to things like 0 meaning "enum" rather than "int", or beautiful expressions like Array[RDPipelineSpecializationConstant]([]).

  • Refactor class_generator.rs.
    Quite a lot of functionality in the code generator was touched. Already in Refactor small parts of class_generator.rs #315, I added a domain representation for function arguments and parameters, distinct from the JSON models. The function to generate method definitions now accepts consolidated structs instead of a dozen parameters. I cleaned up a few other things on-the-fly. Possibly I'll do a few follow-up cleanups, but they can be done independently of this functionality.

  • Implement EngineEnum for Vector*Axis enums.
    This allows ordinal conversion from/to those types, just like other engine enums.

Bromeon added 11 commits June 28, 2023 22:23
Assume a function with 3 parameters, 2 of which have default values:

   fn do_sth(num: int, string: GodotString, flag: bool);
   //                  ^ default ""         ^ default false

So far, it had to be called by providing all arguments, default or not:

   do_sth(1, "".into(), false)

With this change, there are now two overloads provided, which allow calling as follows:

   do_sth(1)
   do_sth_ex(1).string("s".into()).flag(true).done()
   do_sth_ex(1).flag(true).done()

The last example shows that it's also possible to skip parameters.
The builder pattern is an emulation of named arguments in Rust.

This implementation requires declaration of a builder struct for each function that has default parameters.

Design choices regarding forward-compatibility (still an improvement over status quo, where everything except rename is breaking.
* New default parameters are added         -> yes
* Existing parameters get a default value  -> no
* A default parameter is renamed           -> no
Allows conversions from/to integers (for GDScript default value literals).
E.g. type "int" can have meta "uint32" to express that it is behaving like
u32, and only the u32 domain is used.
@Bromeon Bromeon added feature Adds functionality to the library c: engine Godot classes (nodes, resources, ...) labels Jun 29, 2023
@Bromeon
Copy link
Member Author

Bromeon commented Jun 29, 2023

Some default parameters are currently unimplemented! because we cannot represent optional parameters until #156 is addressed.

bors try

bors bot added a commit that referenced this pull request Jun 29, 2023
@GodotRust
Copy link

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-322

@bors
Copy link
Contributor

bors bot commented Jun 29, 2023

try

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@Bromeon
Copy link
Member Author

Bromeon commented Jul 1, 2023

bors r+

@bors
Copy link
Contributor

bors bot commented Jul 1, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 6396f8d into master Jul 1, 2023
@bors bors bot deleted the feature/default-params branch July 1, 2023 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: engine Godot classes (nodes, resources, ...) feature Adds functionality to the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants