-
-
Notifications
You must be signed in to change notification settings - Fork 4k
Not planned
Labels
A-ECSEntities, components, systems, and eventsEntities, components, systems, and eventsS-Needs-DesignThis issue requires design work to think about how it would best be accomplishedThis issue requires design work to think about how it would best be accomplishedX-ControversialThere is active debate or serious implications around merging this PRThere is active debate or serious implications around merging this PR
Description
As seen in #19613, bundle effects (#17521) have really pushed the limits of what our spawning API can do.
This is great, but the "bundle" terminology makes less and less sense. Originally, it meant a "bundle of components", intended to be inserted on a single entity. Now it means... something that can be spawned? But in some cases (like our querying API), it really is intended to solely mean a "bundle of components".
Interacts with #19491, which proposes changes to these traits.
janhohenheim and greym0uth
Metadata
Metadata
Assignees
Labels
A-ECSEntities, components, systems, and eventsEntities, components, systems, and eventsS-Needs-DesignThis issue requires design work to think about how it would best be accomplishedThis issue requires design work to think about how it would best be accomplishedX-ControversialThere is active debate or serious implications around merging this PRThere is active debate or serious implications around merging this PR
Type
Projects
Milestone
Relationships
Development
Select code repository
Activity
urben1680 commentedon Jun 13, 2025
I have the impression that
BundleEffect
is just a disguisedEntityCommand
and it would be more clean to replace the first with the latter. This would unify the concept and even allows fallible bundle effects.I wrote this in Discord yesterday:
BundleEffect
withEntityCommand
Bundle
to components-only againBundleCommand<T>
BundleCommand<BundleMarker>
forT: Bundle
BundleCommand<CommandMarker>
forT: EntityCommand
Then
SpawnRelatedBundle
andSpawnOneRelated
would just implementEntityCommand
and it would be very easy to inject custom commands along bundles.andriyDev commentedon Jun 13, 2025
Just wanted to bring up that
Spawnable
could be confusing since you can also insert them on an existing entity.I don't think that's more confusing though than
Bundle
today, so I'm all for it!Spawnable
is a great name!alice-i-cecile commentedon Jun 13, 2025
I'm nervous about merging commands and BundleEffect: we need to think carefully about how that would interact with #10154. I do see your point though!
janhohenheim commentedon Jun 13, 2025
Insertable
?Atlas16A commentedon Jun 13, 2025
I'm against the name change.
Similar to the issue of event being shared between regular events and observer triggers, bundles is used for both "set of components to insert on entity" and "set and heiarchy of entities to spawn from a function"
While spawnable makes sense for the second case, it doesn't make as much sense for the first case. Bundles still are used in the first case, especially for required components that have no good default and require manual initialization.
alice-i-cecile commentedon Jun 13, 2025
I agree that the trait is used too broadly: remove takes a
Bundle
, but I really don't think that it supports all of the fancy features that Spawn does for example.Ideally we keep
Bundle
around for the simple "set of components" concept, and give a better trait name to the more powerful form.SkiFire13 commentedon Jun 13, 2025
Note that after #19491
remove
will take aStaticBundle
, andBundle
will be used only for spawning and insertingcart commentedon Jun 13, 2025
I'm also against this name change. Bundle is not intended to be the "arbitrary templating pattern" and making it that would have negative consequences for the understand-ability and function of the system as a whole. See my comment here for rationale:
#19613 (comment)
cart commentedon Jun 13, 2025
Closing this based on my comment here: #19613 (comment)
Bundles should continue to be what they are currently (a collection of a specific set of components). BundleEffect in combination with hierarchy spawning does slightly blur the lines, but not in a fundamental way. And any use of BundleEffect that does something else should be discouraged.