Skip to content

Split Bundle and StaticBundle#19761

Open
SkiFire13 wants to merge 29 commits into
bevyengine:mainfrom
SkiFire13:split-bundle
Open

Split Bundle and StaticBundle#19761
SkiFire13 wants to merge 29 commits into
bevyengine:mainfrom
SkiFire13:split-bundle

Conversation

@SkiFire13

Copy link
Copy Markdown
Contributor

Objective

Solution

  • Create a new StaticBundle type, mirroring Bundle for now
  • Switch all APIs that need to statically know the components in a Bundle (i.e. from its type) to use StaticBundle
  • Add &self parameters to Bundle to ensure all APIs that use it have a concrete instance to work with in the future.

Additional notes

For now this doesn't do anything functionally, as Bundle still requires the components to be statically fixed. However this opens the doors for changes to the Bundle trait that allow dynamic components without impacting APIs that actually rely on static components.

@ElliottjPierce ElliottjPierce left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me. The parallel contents of code for StaticBundle vs Bundle is kinda annoying, but I know that's changing in the next prs, so I'm not worried about it.

@ElliottjPierce ElliottjPierce added A-ECS Entities, components, systems, and events S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 21, 2025
@alice-i-cecile alice-i-cecile added C-Code-Quality A section of code that is hard to understand or change X-Contentious There are nontrivial implications that should be thought through labels Jun 22, 2025
Comment thread crates/bevy_ecs/src/bundle.rs Outdated
Comment thread crates/bevy_ecs/src/bundle.rs Outdated
Comment thread crates/bevy_ecs/src/bundle.rs Outdated
Comment thread release-content/migration-guides/static-bundle-split.md Outdated
Comment thread crates/bevy_ecs/src/bundle.rs Outdated
Comment thread crates/bevy_ecs/src/bundle.rs Outdated
Comment thread crates/bevy_ecs/src/bundle.rs Outdated

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some small notes on docs that I think we should clean up quick. I would really appreciate a paragraph or two of module docs laying out these traits and how they interconnect, but that's fine to leave to follow-up if you prefer.

More importantly, I'm not sure why we should keep a split between DynamicBundle and Bundle around after these changes. It looks like Bundle should now be object-safe, allowing us to merge them together.

I haven't done a deep dive on this area of the code in a while though, so please feel free to explain why we can't / shouldn't do that :)

@SkiFire13

Copy link
Copy Markdown
Contributor Author

More importantly, I'm not sure why we should keep a split between DynamicBundle and Bundle around after these changes. It looks like Bundle should now be object-safe, allowing us to merge them together.

Bundle is not object safe yet (it still has a bunch of generic functions without where Self: Sized) but this will change in a followup PR.

However even then it will be a bit of a hassle to remove DynamicBundle. In the unsplitted PR I limited myself to renaming it to ComponentsFromBundle to avoid confusion with dynamic bundles after trying to remove it. The issue is that the insert_by_ids method internally uses it with a struct wrapping some OwningPtrs, which is not Send + Sync + 'static, while Bundle requires them (but not DynamicBundle/ComponentsFromList!). Removing those requirements is possible but leads to ton of boilerplate elsewhere (e.g. all commands require them).

@cart cart moved this to Respond (With Priority) in @cart's attention Jun 23, 2025

@JaySpruce JaySpruce left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Bundles always felt kind of 'messy' to me, and this direction feels tidier.

@alice-i-cecile alice-i-cecile added X-Needs-SME This type of work requires an SME to approve it. S-Waiting-on-SME This is currently waiting for an SME to resolve something controversial and removed X-Contentious There are nontrivial implications that should be thought through S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jul 7, 2025
@cart cart moved this from Focus to Candidate in @cart's attention Jan 28, 2026
@cart cart closed this May 5, 2026
@github-project-automation github-project-automation Bot moved this from Candidate to Done in @cart's attention May 5, 2026
@cart cart reopened this May 5, 2026
@cart cart moved this from Done to Candidate in @cart's attention May 5, 2026
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-Code-Quality A section of code that is hard to understand or change S-Waiting-on-SME This is currently waiting for an SME to resolve something controversial X-Needs-SME This type of work requires an SME to approve it.

Projects

Status: Candidate

Development

Successfully merging this pull request may close these issues.

5 participants