Skip to content

Use bundle wrappers to select the insert mode #19768

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.

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Pascualex
Copy link
Contributor

@Pascualex Pascualex commented Jun 21, 2025

Objective

Allow bundles to define their insert mode in a flexible but explicit way. More specifically this is a potential solution to #19715 and an alternative to #19726 as presented here.

This is a first step in that direction an doesn't provide a merging solution, it focuses on aligning existing InsertMode APIs to the proposed direction.

Solution

The current insert APIs use a separate parameter to control the insert mode for the whole bundle:

fn insert(bundle: impl Bundle, mode: InsertMode) { ... }

The proposal changes this to allow each bundle to define its own insert mode. To keep things sane and flexible most bundles wouldn't actually use that capacity and instead would simply inherit the insert mode from their outer bundle (if one exists). To support that, we update the dynamic bundle trait:

// old
pub trait DynamicBundle {
    fn get_components(self, func: &mut impl FnMut(StorageType, OwningPtr<'_>));
}
// new
pub trait DynamicBundle {
    fn get_components(
        self,
        insert_mode: InsertMode,
        func: &mut impl FnMut(StorageType, InsertMode, OwningPtr<'_>),
    );
}

The idea is that as get_components gets recursively called, the insert mode flows from outer bundles to inner bundles:

// tuples
impl<B1: Bundle, B2: Bundle> DynamicBundle for (B1, B2) {
    fn get_components(
        self,
        insert_mode: InsertMode,
        func: &mut impl FnMut(StorageType, InsertMode, OwningPtr<'_>),
    ) {
        let (b1, b2) = self;
        b1.get_components(insert_mode, &mut *func);
        b2.get_components(insert_mode, &mut *func);
    }
}
// components
impl<C: Component> DynamicBundle for C {
    fn get_components(
        self,
        insert_mode: InsertMode,
        func: &mut impl FnMut(StorageType, InsertMode, OwningPtr<'_>),
    ) {
        OwningPtr::make(self, |ptr| func(C::STORAGE_TYPE, insert_mode, ptr));
    }
}

Then, we can define new bundle types that wrap an inner bundle and override the insert mode:

pub struct IgnoreIfCollides<B: Bundle>(pub B);
unsafe impl<B: Bundle> Bundle for IgnoreIfCollides<B> {
    // simply call inner bundle methods
}
impl<B: Bundle> DynamicBundle for IgnoreIfCollides<B> {
    fn get_components(
        self,
        _insert_mode: InsertMode, // ignore provided insert mode
        func: &mut impl FnMut(StorageType, InsertMode, OwningPtr<'_>),
    ) {
        self.0.get_components(InsertMode::Keep, func);
    }
}

Testing

  • Did you test these changes? If so, how?
    WIP. For now my main concern are component hooks, the current structure is not designed to support multiple insert modes in a single insert call. For now I've left the hooks always enabled as if all components used InsertMode::Replace but that obviously is not acceptable as a final solution.
  • Are there any parts that need more testing?
  • How can other people (reviewers) test your changes? Is there anything specific they need to know?
  • If relevant, what platforms did you test these changes on, and are there any important ones you can't test?

Showcase

These changes in action would look like this:

// old
commands
    .entity(entity)
    .insert(ComponentA)
    .insert_if_new(ComponentB);
// new
commands
    .entity(entity)
    .insert((
        ComponentA,
        IgnoreIfCollides(ComponentB),
    ));

We could deprecate insert_if_new(bundle) or keep it as a shorthand for insert(IgnoreIfCollides(bundle)).

Future work

These changes would pave the way for a future MergeIfCollides, which we would use in SpawnRelated:

pub trait SpawnRelated: RelationshipTarget {
    fn spawn<L: SpawnableList<Self::Relationship>>(
        list: L,
    ) -> MergeIfCollides<SpawnRelatedBundle<Self::Relationship, L>>;

    fn spawn_one<B: Bundle>(bundle: B) -> MergeIfCollides<SpawnOneRelated<Self::Relationship, B>>;
}

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events X-Controversial There is active debate or serious implications around merging this PR S-Needs-Review Needs reviewer attention (from anyone!) to move forward C-Feature A new feature, making something new possible labels Jun 21, 2025
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-Feature A new feature, making something new possible S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants