Skip to content

Allow not emitting BundleFromComponents with Bundle derive macro #19249

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Allow not emitting BundleFromComponents with Bundle derive macro #19249

wants to merge 4 commits into from

Conversation

TheNeikos
Copy link
Contributor

Objective

Fixes #19136

Solution

  • Add a new container attribute which when set does not emit BundleFromComponents

Testing

  • Did you test these changes?

Yes, a new test was added.

  • Are there any parts that need more testing?

Since BundleFromComponents is unsafe I made extra sure that I did not misunderstand its purpose. As far as I can tell, not implementing it is ok.

  • How can other people (reviewers) test your changes? Is there anything specific they need to know?

Nope

  • If relevant, what platforms did you test these changes on, and are there any important ones you can't test?

I don't think the platform is relevant

One thing I am not sure about is how to document this? I'll gladly add it

@alice-i-cecile
Copy link
Member

Agreed that this needs docs. Maybe on the Bundle trait?

@TheNeikos
Copy link
Contributor Author

@nfagerlund would this solve your issue?

@TheNeikos
Copy link
Contributor Author

@alice-i-cecile I've added documentation!

I looked at the miri error, and it didn't seem related? It said something about: "fatal error: cross-interpreting doctests is not currently supported by Miri."

So I am not sure what that means

@TheNeikos TheNeikos force-pushed the feature/allow_bundle_without_extract branch from a8063d1 to 592934d Compare May 17, 2025 08:46
@NiklasEi
Copy link
Member

I looked at the miri error, and it didn't seem related? It said something about: "fatal error: cross-interpreting doctests is not currently supported by Miri."

Once #19253 is merged you should get green builds after a rebase

TheNeikos added 4 commits May 17, 2025 17:59
@TheNeikos
Add option to Bundle macro to not support from Components
6d7e58c 
Signed-off-by: Marcel Müller <neikos@neikos.email>
@TheNeikos
Extract constants and use them
bded83e 
Signed-off-by: Marcel Müller <neikos@neikos.email>
@TheNeikos
Refactor error handling to not bail and structure attributes
e980225 
Signed-off-by: Marcel Müller <neikos@neikos.email>
@TheNeikos
Add documentation to Bundle macro
e7aeb40 
Signed-off-by: Marcel Müller <neikos@neikos.email>
@TheNeikos TheNeikos force-pushed the feature/allow_bundle_without_extract branch from 592934d to e7aeb40 Compare May 17, 2025 15:59
@janhohenheim janhohenheim added S-Needs-Review Needs reviewer attention (from anyone!) to move forward A-ECS Entities, components, systems, and events labels May 17, 2025
alice-i-cecile
alice-i-cecile reviewed May 19, 2025
View reviewed changes
crates/bevy_ecs/src/bundle.rs
/// }
/// ```
///
/// Some fields may be bundles that do not implement
Copy link
Member

Choose a reason for hiding this comment

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

This needs a bit more context about why this might occur.

@alice-i-cecile alice-i-cecile added X-Uncontroversial This work is generally agreed upon D-Macros Code that generates Rust code D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes C-Bug An unexpected or incorrect behavior C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed C-Bug An unexpected or incorrect behavior labels May 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alice-i-cecile alice-i-cecile alice-i-cecile left review comments

Assignees
No one assigned
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-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Uncontroversial This work is generally agreed upon
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support fields with bundle effects in Bundle derive macro
4 participants
@TheNeikos @alice-i-cecile @NiklasEi @janhohenheim