Skip to content

Conversation

@fluffiac
Copy link
Contributor

Objective

Allow required component default values to be provided in-line.

#[derive(Component)]
#[require(
    FocusPolicy(block_focus_policy)
)]
struct SomeComponent;

fn block_focus_policy() -> FocusPolicy {
    FocusPolicy::Block
}

May now be expressed as:

#[derive(Component)]
#[require(
    FocusPolicy(|| FocusPolicy::Block)
)]
struct SomeComponent;

Solution

Modified the #[require] proc macro to accept a closure.

Testing

Tested using my branch as a dependency, and switching between the inline closure syntax and function syntax for a bunch of different components.

@github-actions
Copy link
Contributor

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-ECS Entities, components, systems, and events M-Release-Note Work that should be called out in the blog due to impact X-Contentious There are nontrivial implications that should be thought through S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Sep 17, 2024
@ItsDoot ItsDoot added C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples D-Macros Code that generates Rust code and removed C-Feature A new feature, making something new possible M-Release-Note Work that should be called out in the blog due to impact X-Contentious There are nontrivial implications that should be thought through S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Sep 17, 2024
@alice-i-cecile alice-i-cecile requested a review from cart September 17, 2024 19:01
@ItsDoot ItsDoot added S-Needs-Review Needs reviewer attention (from anyone!) to move forward M-Release-Note Work that should be called out in the blog due to impact labels Sep 17, 2024
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

I quite like this, but this needs user facing docs demonstrating that this is possible.

@ItsDoot ItsDoot added the X-Contentious There are nontrivial implications that should be thought through label Sep 17, 2024
@alice-i-cecile alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Sep 18, 2024
Copy link
Contributor

@atornity atornity left a comment

Choose a reason for hiding this comment

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

neat

@alice-i-cecile
Copy link
Member

@fluffiac do you have time to add docs, or would you like a hand finishing this off?

@fluffiac
Copy link
Contributor Author

fluffiac commented Oct 4, 2024

@alice-i-cecile thank you for the reminder. kind of forgot 😅

@alice-i-cecile alice-i-cecile removed the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label Oct 4, 2024
@alice-i-cecile alice-i-cecile added the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label Oct 4, 2024
@alice-i-cecile alice-i-cecile added this to the 0.15 milestone Oct 4, 2024
@alice-i-cecile alice-i-cecile requested a review from Jondolf October 4, 2024 00:32
@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 4, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Oct 4, 2024
@alice-i-cecile
Copy link
Member

Fantastic; thanks for getting back to this!

Merged via the queue into bevyengine:main with commit f0704cf Oct 4, 2024
28 checks passed
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Oct 4, 2024
…e#15269)

# Objective

Allow required component default values to be provided in-line.

```rust
#[derive(Component)]
#[require(
    FocusPolicy(block_focus_policy)
)]
struct SomeComponent;

fn block_focus_policy() -> FocusPolicy {
    FocusPolicy::Block
}
```

May now be expressed as:

```rust
#[derive(Component)]
#[require(
    FocusPolicy(|| FocusPolicy::Block)
)]
struct SomeComponent;
```

## Solution

Modified the #[require] proc macro to accept a closure. 

## Testing

Tested using my branch as a dependency, and switching between the inline
closure syntax and function syntax for a bunch of different components.
robtfm pushed a commit to robtfm/bevy that referenced this pull request Oct 4, 2024
…e#15269)

# Objective

Allow required component default values to be provided in-line.

```rust
#[derive(Component)]
#[require(
    FocusPolicy(block_focus_policy)
)]
struct SomeComponent;

fn block_focus_policy() -> FocusPolicy {
    FocusPolicy::Block
}
```

May now be expressed as:

```rust
#[derive(Component)]
#[require(
    FocusPolicy(|| FocusPolicy::Block)
)]
struct SomeComponent;
```

## Solution

Modified the #[require] proc macro to accept a closure. 

## Testing

Tested using my branch as a dependency, and switching between the inline
closure syntax and function syntax for a bunch of different components.
@alice-i-cecile
Copy link
Member

Thank you to everyone involved with the authoring or reviewing of this PR! This work is relatively important and needs release notes! Head over to bevyengine/bevy-website#1714 if you'd like to help out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Macros Code that generates Rust code D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Release-Note Work that should be called out in the blog due to impact S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Contentious There are nontrivial implications that should be thought through

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants