Skip to content
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

String argument conversion + AsArg trait #940

Merged
merged 12 commits into from
Nov 5, 2024
Merged

Conversation

Bromeon
Copy link
Member

@Bromeon Bromeon commented Nov 3, 2024

This is a colossal PR, simply because the changes go very deep into code generation and touch all the foundations:

  • Add AsArg<T> trait for generic passing of string arguments.
  • Change codegen to use impl AsArg<T> for strings.
  • Refactor the very messy parameter/lifetime/extender signature generation.
  • Generalize AsArg<T> for generic argument passing inside Array<T> and Packed*Array.
  • Update hundreds of occurrences in itest, examples as well as higher-level internal code.

TLDR: instead of

godot_method("thing".into());
godot_method(gstring.clone());

you now write:

godot_method("thing");
godot_method(&gstring);

There are still some rough edges, but it's a mergeable start, and I'd like to iteratively improve it based on user feedback. Since the whole argument passing has been fundamentally reworked lately (object arguments in #800 + reference passing in #900 and #906), there is a lot that can be made more ergonomic and consistent in the future.

@Bromeon Bromeon added feature Adds functionality to the library c: core Core components c: engine Godot classes (nodes, resources, ...) labels Nov 3, 2024
@GodotRust
Copy link

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

@Bromeon Bromeon added the breaking-change Requires SemVer bump label Nov 4, 2024
@Bromeon Bromeon added this to the 0.2 milestone Nov 4, 2024
@Bromeon Bromeon added this pull request to the merge queue Nov 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 4, 2024
@Bromeon Bromeon added this pull request to the merge queue Nov 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 4, 2024
@Bromeon Bromeon added this pull request to the merge queue Nov 5, 2024
Merged via the queue into master with commit e86388f Nov 5, 2024
15 checks passed
@Bromeon Bromeon deleted the feature/string-args branch November 5, 2024 11:14
0x53A added a commit to 0x53A/gdext_coroutines that referenced this pull request Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Requires SemVer bump c: core Core components 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