External components with #[derive(Component)]

@DJMcNab

With the introduction of an opt-in Component trait in #2254, external types can no longer be directly included as components.

In the standard scenario, we have 4 parties:

Bevy
the external crate (e.g. Rapier)
the interface crate (e.g. bevy_rapier)
the end user who just wants to make a game

Due to Rust's orphan rules, only Bevy or the external crate can implement Component for arbitrary external crate structs directly. The interface crate or end user can only bypass this using a wrapper type, and using a technique known as extension traits.

Users need to be able to use external types as components, in one form or another.

There are several options for this:

Bevy implements Component for external types. This is impossible, as this set is unknowable, and using overly broad blanket impls causes all of the issues that [Merged by Bors] - Implement and require #[derive(Component)] on all component structs #2254 solved.
The external crate implements Component for the relevant types, possibly hidden behind a feature flag. This is the solution used for serde and other ecosystem crates. This is ideal, but not always going to happen, as it is outside of the control of anyone who cares about Bevy.
The interface crate or end user writes a manual wrapper type around the external component. This prevents accidental conflicts between different uses of the same component, but is heavy on boilerplate and conceptual indirection.
The interface crate or end user writes an ExternalComponent<T, StorageType> struct and implements component on it. This avoids accidental conflicts and reduces boilerplate, but reduces the type safety of the API in some of the ways that [Merged by Bors] - Implement and require #[derive(Component)] on all component structs #2254 solved.
Bevy writes an ExternalComponent<T, StorageType> struct and implements component on it. This avoids ecosystem fragmentation, but has most of the same problems as 4. Extending this becomes increasingly onerous as we add more configuration to component types.

Given that 1 is impossible, and 2 is not enforceable, we are left with a choice between 3-5.
The tentative consensus from the active ECS contributors is that 3 is the least bad option, with the encouragement to use Deref and DerefMut on the wrapper types until delegation is one day (🤞🏽) added to Rust.

4 is a particularly bad solution due to the impact on type safety and difficulty extending it. 5 is less bad than 4 in some ways, as getting 4 correct is going to be frustrating for new users and its easy to screw up and make a completely unconfigurable wrapper type. However, by including an ExternalComponent type in Bevy itself, we are effectively endorsing that solution.

Thus we should:

clearly document how to include external component types in the documentation for both end users and plugin authors
investigate disabling the naive use of 4 by forbidding unconstrained generic derives of the Component trait (credit to @DJMcNab)
investigate publishing a tool / crate to more quickly allow users to wrap types in Bevy components (and resources, as the same issues exists for Reflect even without a Resource trait), credit to @BoxyUwU

I think reconsidering (1) is worth it, given that ecosystem compatibility wasn't a (major) part of the discussion in the RFC. I think we should weigh "ecosystem compatibility" / "everything is a component" vs the various reasons for adopting derive(Component). "Impossible" seems like too drastic of a framing. It isn't too late to revert imo.

I think reconsidering (1) is worth it, given that ecosystem compatibility wasn't a (major) part of the discussion in the RFC. I think we should weigh "ecosystem compatibility" / "everything is a component" vs the various reasons for adopting derive(Component). "Impossible" seems like too drastic of a framing. It isn't too late to revert imo.

Fair enough; while I disagree with doing so, reversion is an option. The "impossible" part of 1 was specifically for implementing Component for specific types, rather than a blanket impl. This is impossible because the set of possible components is unbounded and we have to append to an ever growing list, shipped to all users.

FYI, I don't think that reverting this change will actually solve the ecosystem problems. It only hides them in the simplest of cases.

Reflect is going to be needed by approximately every finished game, on almost all components and resources, either for saving, scenes or networking. End users and interface crates cannot implement this trait on external types, in exactly the same way as Component, so you're back to the current situation without any of the advantages of #2254.

This fact means that I actually think that we should go in the other direction, and unify things with #[derive(Resource)].
If you can't ever use raw external types in the ECS in finished games, forcing a wrapper from the beginning is the less harmful approach because it avoids a massive refactoring late into the project's life.

There's actually a sixth option: we get negative trait bounds into rustc and do a blanket impl ruling out the really awful user footguns like Bundle. Or, use specialization to do something similar.

This is probably my personal favorite on a lot of levels, but isn't happening for a good long while, no matter how much effort the Bevy community throws at the problem :(

About (1) "Bevy implements Component for external types"

We could setup a process where a library author can submit the components into Bevy. With an appropriate code owner file and directory structure, the author could "own" this part of the code even though its in Bevy repo

We could setup a process where a library author can submit the components into Bevy. With an appropriate code owner file and directory structure, the author could "own" this part of the code even though its in Bevy repo

From an organizational perspective, I'm not sure that this solves any problems that approach 2 doesn't. If the crate doesn't care about Bevy, they won't bother adding to this list.

I guess you could have random unauthorized end users submitting this to Bevy, which gets around that. But that seems... very unpleasant and chaotic, and will cause huge issues if the crate owners themselves change their mind and want to integrate with Bevy in their own ways.

Fundamentally, I think that the ownership rules are there for a good reason.

If it's like alice is saying, I think 1 should be excluded from options, this restraint should be put in place as early as possible, as it will be much more difficult to change later.
Out of options 3-5, I think a modified 5 would be the best,
because once the rust delegate comes out, it will be the closest thing syntax-wise, hence it'll be easier to switch for users.
To prevent the issue of it being a 'standard' I propose making a crate just for it, which main goal is to become deprecated.

Meanwhile, it may be a good idea to bring more attention to the rust-lang delegate PR, problems like here are bound to happen elsewhere and delaying it only makes it worse.

A variant on proposal 4, proposed by @Alphamodder.
The following is paraphrased from those comments. Call it proposal 6.

Create a Bevy-endorsed ExternalComponent<T> type. This type is only necessary in query signatures and when adding components by value, but the references returned by queries would always be the real external type, avoiding all of the Deref/DerefMut compatibility shenanigans entirely.

pub trait IntoComponent {
    type Component = C;
    type StorageType;
    fn into(self) -> C;
}

impl<T> IntoComponent for T: Component {
    type Component = Self;
    const StorageType = T::Storage::STORAGE_TYPE;
    
    fn into(self) -> Self { self }
}

pub struct External<T>(pub T);

impl<T> IntoComponent for External<T> {
    type Component = T;
    
    // THIS IS AN IMPORTANT CAVEAT, SEE BELOW
    const StorageType = StorageType::Dynamic; 
    fn into(self) -> Self { self }
}

impl EntityMut {
    // all similar methods which take a component by value would require wrapping external components in External(value). Alternatively, we could have
    // methods like `insert_external(&mut self, value: T)` which move this boilerplate to the method signature.
    pub fn insert<T: IntoComponent>(&mut self, value: T) -> &mut Self { /* ... */ }
}

// Could achieve the same effect without changing the current T: Component bounds, but just having separate impls for `&External<T>`, `ReadFetch<External<T>>`, etc.
impl<T: IntoComponent> WorldQuery for &T {
    // contents remain unchanged (though bounds on ReadFetch are also altered to match, see below)
}

impl<'w, 's, T: IntoComponent> Fetch<'w, 's> for ReadFetch<T> {
    type Item = &'w T::Component;
    
    // basically the same
}

// ...the same kind of tweaked impls for &mut T, With<T>, Without<T>, etc.

The caveat mentioned above is that External<T> wouldn't be entirely zero-cost because of the introduction of the StorageType::Dynamic variant and a configuration sidechannel to configure how components with this variant are stored at runtime. This would incur an O(1) lookup somewhere when actually querying for external components, compared to the way all storage types are currently statically known as a const.

In #[derive(Bundle)] users could include external components in a Bundle with an #[external] attribute rather than the External<T> type. This way, the bundle could have a field of the real type and the wrapper automatically added in the generated trait impl, eliminating all boilerplate for the consumer of the bundle.

Removed from the 0.6 milestone because we have resolved to use #[derive(Component)], encourage upstream Component impls where possible, and encourage DerefMut / Deref wrapper Components when upstream support isn't possible. Still plenty to do to make this all easier on people (like potentially including Deref/DerefMut derives), so we shouldn't close this yet.

Closing this out. The migration seems to have gone relatively smoothly.

Navigation Menu

Repository navigation

External components with #[derive(Component)] #2966

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

External components with #[derive(Component)] #2966

Description

Activity

cart commented on Oct 15, 2021

alice-i-cecile commented on Oct 15, 2021

alice-i-cecile commented on Oct 15, 2021

mockersf commented on Oct 15, 2021

alice-i-cecile commented on Oct 15, 2021

MiniaczQ commented on Oct 15, 2021

alice-i-cecile commented on Oct 20, 2021

cart commented on Dec 21, 2021

alice-i-cecile commented on Apr 27, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions