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

Rename #[godot] to #[method] #933

Merged
merged 1 commit into from
Aug 31, 2022
Merged

Conversation

Bromeon
Copy link
Member

@Bromeon Bromeon commented Aug 25, 2022

Follow-up to #872.

As mentioned there, the syntax was provisional and still subject to change. This PR renames #[godot] back to #[method].
Other changes, such as the #[base] parameter name, remain unaffected (still a bit conflicted about that too, but that's another story...)

Reasoning behind the name #[godot] was a potential greater unification of godot-rust attributes, which would allow to use #[godot] more widespread as the attribute, e.g.

#[derive(NativeClass)]
#[godot(inherit=Node, no_constructor)]
struct MyClass {
    #[godot(get)]
    my_property: i32,
}

#[godot_api]
impl MyClass {
    #[godot]
    fn my_method(&self, #[base] base: &Node) {...}
}

However, this comes with a bigger change. In order to avoid huge deprecations or breaking changes, I think it makes sense to not include this in the current minor version. It could be discussed for v0.11, or maybe only for GDExtension. Since having #[godot] in only one place would be "somewhere in between" the old and new naming, I'd rather find an identifier that fits the existing #[property] and #[methods] naming scheme -- and #[method] as suggested by the original author would fit relatively nicely.

Overall, I still believe "method" is not expressive and there is room for improvement, as well as for other names like "property" or so. But I don't want this to delay the whole 0.10.1 release further; we'd rather think about nicer naming another time, maybe when GDExtension is around.

This PR introduces a breaking change toward master, but not toward the last released version 0.10.0, so it can be included in 0.10.1.

@Bromeon Bromeon added c: export Component: export (mod export, derive) breaking-change Issues and PRs that are breaking to fix/merge. quality-of-life No new functionality, but improves ergonomics/internals labels Aug 25, 2022
@Bromeon Bromeon added this to the v0.10.1 milestone Aug 25, 2022
@Bromeon
Copy link
Member Author

Bromeon commented Aug 25, 2022

bors try

bors bot added a commit that referenced this pull request Aug 25, 2022
@bors
Copy link
Contributor

bors bot commented Aug 25, 2022

try

Build succeeded:

@Bromeon Bromeon mentioned this pull request Aug 25, 2022
4 tasks
@Bromeon
Copy link
Member Author

Bromeon commented Aug 30, 2022

bors r+

bors bot added a commit that referenced this pull request Aug 30, 2022
933: Rename `#[godot]` to `#[method]` r=Bromeon a=Bromeon

Follow-up to #872.

As mentioned there, the syntax was provisional and still subject to change. This PR renames `#[godot]` back to `#[method]`.
Other changes, such as the `#[base]` parameter name, remain unaffected (still a bit conflicted about that too, but that's another story...)

Reasoning behind the name `#[godot]` was a potential greater unification of godot-rust attributes, which would allow to use `#[godot]` more widespread as _the_ attribute, e.g.
```rs
#[derive(NativeClass)]
#[godot(inherit=Node, no_constructor)]
struct MyClass {
    #[godot(get)]
    my_property: i32,
}

#[godot_api]
impl MyClass {
    #[godot]
    fn my_method(&self, #[base] base: &Node) {...}
}
```

However, this comes with a bigger change. In order to avoid huge deprecations or breaking changes, I think it makes sense to not include this in the current minor version. It could be discussed for v0.11, or maybe only for GDExtension. Since having `#[godot]` in only one place would be "somewhere in between" the old and new naming, I'd rather find an identifier that fits the existing `#[property]` and `#[methods]` naming scheme -- and `#[method]` as suggested by the original author would fit relatively nicely. 

Overall, I still believe "method" is not expressive and there is room for improvement, as well as for other names like "property" or so. But I don't want this to delay the whole 0.10.1 release further; we'd rather think about nicer naming another time, maybe when GDExtension is around.

This PR introduces a breaking change toward `master`, but not toward the last released version `0.10.0`, so it can be included in `0.10.1`.

Co-authored-by: Jan Haller <[email protected]>
@bors
Copy link
Contributor

bors bot commented Aug 30, 2022

Build failed:

@Bromeon
Copy link
Member Author

Bromeon commented Aug 31, 2022

bors r+

@bors
Copy link
Contributor

bors bot commented Aug 31, 2022

Build succeeded:

@bors bors bot merged commit 29b89b0 into godot-rust:master Aug 31, 2022
@Bromeon Bromeon deleted the qol/export-syntax-2 branch August 31, 2022 16:56
bors bot added a commit that referenced this pull request Aug 31, 2022
935: Rename a few remaining occurrences of #[export] r=Bromeon a=Bromeon

Forgot to include this commit in #933.

bors r+

Co-authored-by: Jan Haller <[email protected]>
bors bot added a commit that referenced this pull request Aug 31, 2022
935: Rename a few remaining occurrences of #[export] r=Bromeon a=Bromeon

Forgot to include this commit in #933.

bors r+

Co-authored-by: Jan Haller <[email protected]>
bors bot added a commit that referenced this pull request Aug 31, 2022
935: Rename a few remaining occurrences of #[export] r=Bromeon a=Bromeon

Forgot to include this commit in #933.

bors r+

Co-authored-by: Jan Haller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Issues and PRs that are breaking to fix/merge. c: export Component: export (mod export, derive) quality-of-life No new functionality, but improves ergonomics/internals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant