Skip to content

Conversation

SkiFire13
Copy link
Contributor

Objective

Solution

  • Instead of accumulating the code for each method in a different Vec, accumulate only the names of non-ignored fields and their types, then use quote to generate the code for each of them in the method body.
  • To fix the bug, change the code populating the BundleFieldKind to push only one of them per-field (previously each #[bundle(ignore)] resulted in pushing twice, once for the correct BundleFieldKind::Ignore and then again unconditionally for BundleFieldKind::Component)

Testing

  • Added a regression test for the bug that was fixed

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels Jun 20, 2025
@alice-i-cecile alice-i-cecile added this to the 0.17 milestone Jun 20, 2025
@alice-i-cecile alice-i-cecile added the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label Jun 20, 2025
Copy link
Contributor

@ElliottjPierce ElliottjPierce left a comment

Choose a reason for hiding this comment

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

Looks good to me! Much cleaner now IMO

@ElliottjPierce ElliottjPierce 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 Jun 20, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 20, 2025
@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-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels Jun 20, 2025
Merged via the queue into bevyengine:main with commit 35166d9 Jun 20, 2025
36 checks passed
Trashtalk217 pushed a commit to Trashtalk217/bevy that referenced this pull request Jul 10, 2025
# Objective

- Splitted off from  bevyengine#19491
- Make adding generated code to the `Bundle` derive macro easier
- Fix a bug when multiple fields are `#[bundle(ignore)]`

## Solution

- Instead of accumulating the code for each method in a different `Vec`,
accumulate only the names of non-ignored fields and their types, then
use `quote` to generate the code for each of them in the method body.
- To fix the bug, change the code populating the `BundleFieldKind` to
push only one of them per-field (previously each `#[bundle(ignore)]`
resulted in pushing twice, once for the correct
`BundleFieldKind::Ignore` and then again unconditionally for
`BundleFieldKind::Component`)

## Testing

- Added a regression test for the bug that was fixed
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-Bug An unexpected or incorrect behavior C-Code-Quality A section of code that is hard to understand or change D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Uncontroversial This work is generally agreed upon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants