Skip to content

Conversation

janis-bhm
Copy link
Contributor

This is an alternative solution to #20976

Objective

Users using MovingPtr bundles, or implementing bundle-related types that have to deal with MovingPtrs of bundles currently have to copy the bundle onto the stack before they can spawn or insert it, mostly negating the benefits from #20772.

Solution

By implementing Bundle for MovingPtr<B: Bundle>, bundles can be spawned with the existing API, including as parts of other bundles, like (R::from(entity), bundle_ptr).
In order to implement Bundle for any MovingPtr<'_, B: Bundle> we have to remove the 'static bound on Bundle, which is possible when using typeid to get the bundle's unique identifier, as suggested by @james7132.
Internally, typeid uses dynamic dispatch to go from a T: 'a to a T: 'static to call TypeId::of on, but it appears as if rustc is able to de-virtualise this call. This should, however, definitely be benchmarked before this PR is merged.

A number of users of Bundle require Bundle + 'static now, which means a surprisingly wide diff, including for upgrading users of bevy_ecs. Some of the examples have also revealed that could lead to confusing error messages and requires frequent use of use<> when using the fn() -> impl Bundle pattern to prevent rustc from pulling in unrelated lifetimes.
This could be simplified by renaming the bundle trait and providing a trait alias trait Bundle: BikeshedBundleName + 'static which retains the previous bounds on Bundle; in this case, only implementers of the Bundle trait would be directly impacted (see #21443 and #19491, which factors out some of Bundle into another trait, as a reference).

Alternatives

#21443

@janis-bhm janis-bhm added C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Benchmarking This set of changes needs performance benchmarking to double-check that they help X-Contentious There are nontrivial implications that should be thought through D-Unsafe Touches with unsafe code in some way S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 8, 2025
@james7132 james7132 added the M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Oct 8, 2025
Copy link
Contributor

github-actions bot commented Oct 8, 2025

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Please review the instructions for writing migration guides, then expand or revise the content in the migration guides directory to reflect your changes.

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-Usability A targeted quality-of-life change that makes Bevy easier to use D-Unsafe Touches with unsafe code in some way M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Needs-Benchmarking This set of changes needs performance benchmarking to double-check that they help S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Contentious There are nontrivial implications that should be thought through

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants