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

Callable is now passed by-ref #944

Merged
merged 3 commits into from
Nov 5, 2024
Merged

Callable is now passed by-ref #944

merged 3 commits into from
Nov 5, 2024

Conversation

Bromeon
Copy link
Member

@Bromeon Bromeon commented Nov 5, 2024

Changes APIs to accept &Callable instead of Callable.
Same for &Signal but it doesn't seem to be used in parameter position.

This is mostly for consistency with #900, #906 and #940.
Now, all non-Copy builtins are actually passed by-ref.

I'm also aware that this may cause a minor inconvenience of an added & for ad-hoc callables (which are quite frequent), especially around connect(). I don't think it's a huge deal, but we can maybe consider making those APIs more flexible in the future, to accept both owned and borrowed callables.

This PR comes with some smaller cleanups.

@Bromeon Bromeon added feature Adds functionality to the library c: core Core components c: engine Godot classes (nodes, resources, ...) labels Nov 5, 2024
@Bromeon Bromeon changed the title feature/callable-by-ref Callable is now passed by-ref Nov 5, 2024
@Bromeon Bromeon added the breaking-change Requires SemVer bump label Nov 5, 2024
@GodotRust
Copy link

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

@Bromeon Bromeon added this pull request to the merge queue Nov 5, 2024
@GodotRust
Copy link

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

Merged via the queue into master with commit 0d1f4d1 Nov 5, 2024
15 checks passed
@Bromeon Bromeon deleted the feature/callable-by-ref branch November 5, 2024 21:15
@Bromeon Bromeon added this to the 0.2 milestone Nov 9, 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