Proof of Concept for Required Components: Require<T>

[Zeenobit]

Objective

An attempt to fix #7272

This is a proposal for a way to express bundle requirements using a bit of macro help.

Solution

My solution is to leverage the existing derive_bundle macro to provide a set of required component IDs for every Bundle.

Currently, the syntax looks like this:

#[derive(Component)]
struct A;

#[derive(Bundle, Default)]
struct B {
    #[bundle(require)]
    #[allow(unused)]
    a: Require<A>,
}

Require<T> is simply a PhantomData<T> which has a custom implementation of Bundle.
It is not inserted as a component. Instead, it is used to populate a list of ComponentId which is checked during BundleInfo construction:

for id in requires {
    if !component_ids.contains(&id) {
        panic!(
            "Bundle {bundle_type_name} requires missing component: {}",
            components.get_info_unchecked(id).name(),
        );
    }
}

This allows us to enforce Bundle dependencies by panicking.

There is currently a few issues I'm trying to resolve:

1. Ideally, user shouldn't have to specify #[bundle(require)]. If we can parse the type name containing Require<*>, I think we can make it less verbose.
2. I'm not sure how to get rid of #[allow(unused)]. Require is like PhantomData, but not. It should be unused, but rust doesn't know that, which causes warnings in user code. I'm open to ideas here.
3. I would like to give the user the option to insert a default if the requirement is missing, instead of always panicking.

I've included two tests with the initial implementation which demonstrates complete usage.

    #[test]
    #[should_panic]
    fn bundle_with_missing_require() {
        use super::bundle::Require;
        #[derive(Component)]
        struct A;

        #[derive(Bundle, Default)]
        struct B {
            #[bundle(require)]
            #[allow(unused)]
            a: Require<A>,
        }

        let mut world = World::default();

        world.spawn(B::default());
    }

    #[test]
    fn bundle_with_require() {
        use super::bundle::Require;
        #[derive(Component)]
        struct A;

        #[derive(Bundle, Default)]
        struct B {
            #[bundle(require)]
            #[allow(unused)]
            a: Require<A>,
        }

        let mut world = World::default();

        let entity = world.spawn((A, B::default()));

        assert!(entity.contains::<A>());
    }

Please note, Currently this PR is just a draft. I'm still new to internals of Bevy, and I'm interested in dialogue with more experienced developers. If there is any way to simplify things further, or if there are any edge cases I'm missing, I'm open to any and all feedback.

Personally, I think this is a major feature missing from Bevy, since it's impossible for bundles to overlap. This means if two bundles depend on a component existing on an entity, there is simply no way to enforce or express that prior to runtime. This is what motivated me to open this PR.

---

Changelog

Added

• Require<T> type to specify a required component within a bundle
• BundleFieldKind::Require to support #[bundle(require)]
• Bundle::requires() which does nothing on all internal Bundle implementations except for Require<T>.
• Tests: bundle_with_missing_require and bundle_with_require

Changed

• derive_bundle to implement requires() for deriving Bundles
• BundleInfo::new to check for missing required components

[alice-i-cecile]

I prefer #5121: I don't think that these should be defined at compile time (and there's quite a few critical relationships that can't be expressed here).

If these are compile time only it becomes very dangerous for library authors to set them.

[Zeenobit]

I prefer #5121: I don't think that these should be defined at compile time (and there's quite a few critical relationships that can't be expressed here).

If these are compile time only it becomes very dangerous for library authors to set them.

I'd love some counter examples for why having this check be at compile time is bad.
My rationale is because the bundle requirements are expressed exactly where the bundle is defined, the bundle creator controls what is required, which would increase safety for library authors, not decrease it.

Is it possible that we need both dynamic and compile time invariants...?

[alice-i-cecile]

Hmm, because this is tied to bundles specifically I think that moving it to compile time is basically okay. The concerns around compile time archetype invariants are:

• rules can't be removed (doesn't matter here, you can just make your own bundle type unless the component types are locked down)
• users can't add more rules (I agree that this makes sense as the owner of that bundle's decision)

More concerns though:

• I don't think this actually prevents bad state: users can selectively remove required components off of entities to break this invariant in a very surprising way.
• I'm nervous about the side effects and tech debt of hooking into BundleInfo here: this means you can never use a bundle of this type by itself in any current or future APIs (such as With filters, which would be logically valid). That seems overkill: we really just want to check this at spawning right?

Overall I still feel that we should review and finalize the old PR (which is effectively complete) rather than having two very similar features, one of which is substantially more limited.

[Zeenobit]

Regarding your concerns:

I don't think this actually prevents bad state: users can selectively remove required components off of entities to break this invariant in a very surprising way.

Agreed. I don't see a way around this unless we keep something in memory to keep track of removed components, and then that just gets messy. Personally, I'm ok with ignoring this on the basis of "we're all (mostly) adults here" 😉 , but I get why others may disagree.

I'm nervous about the side effects and tech debt of hooking into BundleInfo here: this means you can never use a bundle of this type by itself in any current or future APIs (such as With filters, which would be logically valid). That seems overkill: we really just want to check this at spawning right?

Again, to me, this is a feature, not a downside. 😁 I want to say "All entities with this bundle must have component X.", period. This gives me a full guarantee that no entity can spawn without my required component.

An example of this which motivated me to make this is I have several systems which all require their entities to have Name, mainly for debug purposes. I can't have each of the bundles of these systems have Name, because that overlaps. Instead, I could have all the bundles have Require<Name>, and I can safely assume all entities with this bundle will spawn with Name. This avoids my systems silently not running due to a query mismatch.

This is why I think we may benefit from having "Required Components" be a separate feature for more low level control, and reserve archetype invariants for higher level entity composition. i.e. Dynamic vs. Static requirements.

Regardless, if the other CL is near completion, I'll hold off working on this to see how that pans out. I'd like to keep this PR open for the time being though, if that's ok.

[alice-i-cecile]

Yep, feel free to keep this open and let's see what others think!