Marker component discoverability

[alice-i-cecile]

Problem

Marker components (zero-sized structs used only for filtering) are very useful and natural. However, they have a few problems that make them hard to use in the engine. From @cart in #3796:

1. Marker components have a "visual editor" problem. With a standard "display current entity components" approach, marker components aren't discoverable. You need to know that a given entity supports them. With the current bool approach, Visibility would always show up on supported entities in the editor with a toggle-able is_visible widget.
2. There's also the general "discoverability" problem: marker components that aren't part of the default bundle will have no direct correlation to the entities that support them. Ex: the NotShadowReceiver marker component suffers from this problem.

Potential Solutions

1. Don't use marker components in the engine (or don't use them for core elements).
   i. Query filters are faster than manually checking values. This is important for e.g. core rendering algorithms.
   ii. Query filters are less blocking than manually checking values.
   iii. Query filters are clearer than manually checking values, as the information is encoded in the function signature.
   iv. The issues raised above will also apply to end user games, so this is not really a solution.
2. Marker components "enable behavior" by convention: ShadowCaster, not NotShadowCaster.
   i. Improves discoverability: most relevant behaviors will be enabled by default.
   ii. Improves clarity by avoiding double-negative reasoning.
   iii. Will very slightly increase overhead in cases where the behavior is a default.
   iv. Does not solve visual editor problem, as there's no way to tie entities back to their original bundles.
   v. Does not fully solve discoverability problem, as not all interesting behavior is on-by-default.
3. Bundles with optional components (Bundles with optional components #2157).
   i. Solves the general discoverability problem, but only for plugin/engine components.
   ii. Does not solve the visual editor problem, since we don't know which bundles an entity is made up of.
   iii. May have non-trivial performance overhead for spawning entities with optional bundle fields.
   iv. Almost certainly requires Make a dynamically applicable version of Bundle #3694 to be merged.
4. Tool to detect which bundles an entity contains (Label entities by the component bundles they have for clearer inspection #1958).
   i. More powerful with 2 and 3, solving their visual editor problem.
   ii. Store this information at entity creation time, or compare the components to a registered list of bundles.
5. Leverage archetype invariants (Archetype Invariants #1481) and inspect "related components" at the component level.
   i. Hard to display in the docs.rs docs, unless by some miracle we can do this all at a type level.
   ii. Provides a clear, general mechanism for discovery in both cases.
   iii. Discoverability is user-extensible.
6. Create a tool that shows which other components a component is found with in queries.
   i. Like 5, but way easier and automatic.

[alice-i-cecile]

So, personal preferences time.

1. I really dislike this; it just kicks the ball down the road and costs us perf.
2. This should be done regardless for clarity and consistency reasons.
3. It's cool, and make sense, but I'm really nervous about the perf costs. It also doesn't fully solve the problem for large games.
4. I think I like this. I much prefer the "compare to a list of registered bundles" approach, since it's more robust.
5. I think this is a powerful, general solution, but it'll take some 🧪 to make sure it's feasible.
6. Strictly better than 5, because it's dramatically simpler and automatic.

In general, I'm nervous about the more bundle-centric solutions: it pushes away from the core appeal of the ECS (a fully compositional set of components), and is generally less robust.

[alice-i-cecile]

As an extra benefit: marker components allow users to use addition / removal detection on the components, rather than having to track the previous state (change detection alone is not sufficient, since the value could have been mutated without changing).

[MiniaczQ]

Based on my reasoning:

1. Boilerplate and performance hit.

2. Doesn't make sense in some situations. Would enemy entities hold Player(false) and player Enemy(false) if they use the same bundle?

3. I never associate markers with bundles. They are more context driven, as in, how their entities get processed. Rarely do I consider them as part of data.

4. It benefits from 2 and 3, so same problems.

5. I'm not too knowledgeable about the topic, but archetypes could help guiding the user where markers can be used and where not?

6. I think the best way to present the usefulness of a marker is to show which components it associates with in queries and how lack/existence of it can change the systems that are used. I know it's not easy to present, but that's what I think I'd find helpful.

[alice-i-cecile]

@MiniaczQ your solution is totally the way to go. Thanks!

Concrete strategy:

1. Create a Schedule::related_components::<C>() method, which examines all queries in a schedule, and then returns any components that are found in the same query (ideally with the name / system id / query signature of the system where it's found).
2. Create a method for this on App, which (eventually) examines this across all schedules.
3. Advertise this method in the Component docs and book.
4. Expose this via CLI functionality.
5. Expose that CLI functionality in the visual editor when examining components.

[cart]

1. Yup not a real option
2. ZSTs (at least with table storage) won't take up extra space in the table, so there wouldn't be more overhead there (other than the one time cost of initializing the archetype, which is small and amortizes to zero anyway). That being said, something like Visible would probably want to live in a Sparse Set for faster add + removes. And yeah not really a solution to the problems at hand.
3. My opinion on "optional components in bundles" hasn't changed. Thats a pretty fundamental change to what a bundle is conceptually. It would likely have a perf cost / would have very real implications for the archetype graph.
4. On its own this doesn't resolve the discoverability problem. It would need to be paired with metadata like "Bundle X entities support the following marker components".
5. This feels like overloading archetype invariants (which establish constraints on entity component compositions). Ex: Entities with A must have B. Whereas this is more of an "Entities with A can have B". Thats not a constraint to enforce, it is "metadata".
6. This really doesn't feel like an actual solve. Its might be a useful debugging tool, but there are a number of reasons this doesn't feel like the right answer:
   a. hinges on a Query pairing two components exists in the codebase, which may or may not be true for a given context, especially during development
   b. isn't explicit: people reading code won't be able to immediately construct the list of components that fit this constraint in their heads. they'd need to scan the entire codebase to be sure
   c. probably has "false positives": just because two components show up in the same query, doesn't mean they should be "paired" in editor experiences.
   d. requires additional tooling, further complicating the development process

My take is that this should probably just be additional "metadata" attached to either Bundles or collections of components. Here are some more proposals:

Proposal 7: Additional Bundle metadata

#[derive(Bundle)]
#[bundle_supports(NotShadowReceiver)]
struct MeshBundle {
  mesh: Handle<Mesh>,
  global_transform: GlobalTransform
}

// bundle trait would be extended to include Bundle::get_supported_components()

Pros: simple. self-documenting. relevant information is paired directly with other bundle components + easily discoverable
Cons: only works for Bundles with derives (maybe fine). requires correlating entities back to bundles / making bundles a "first class" part of an entity (which we might want to do for the editor anyway ... we've discussed this at length already but we haven't yet come to a conclusion).

Proposal 8: Decoupled Component Composition Metadata

app.add_supported_component_composition::<NoShadowReceiver, (Handle<Mesh>, GlobalTransform)>()

Pros: self-documenting (for people reading code), decentralized (downstream users can define new supported compositions, can add their components to upstream "bundles"), doesn't require expanding the the Bundle scope like (7) ... if an entity has the right component composition, it will "support" the given component.
Cons: decentralized (harder to know where a composition is defined / what the total list of supported compositions is)

Proposal 9: Compositions defined on Components

#[derive(Component)]
#[supported_composition(Handle<Mesh>, GlobalTransform)]
struct NotShadowReceiver;

Pros: self-documenting, centralized + discoverable (all supported compositions are easily discoverable in one place)
Cons: supported compositions must be defined upstream

In order of preference: (8), (7), (9).
(8) is the most flexible and fits the current paradigm the best. But it comes at the cost of discoverability in docs / people browsing the codebase. (7) is the most "discoverable" and self documenting but it requires expanding the scope of Bundles and probably would have issues with supporting "downstream" components (although i guess we could add an api for that like world.add_supported_bundle::<NotShadowReceiver, MyBundle>().

[MiniaczQ]

My feedback on those:

7. Seems to bind markers to bundles again, not a big fan for earlier mentioned reasons.
8. Seems better since it's no longer about bundles. Decentralization isn't necessarily a good thing, it makes it harder to find, so not as explicit.
9. Probably the best one. Touch more work than 6, but definitely much more explicit and visible from code.

I'm guessing the provided components are the minimal set, so actual queries can take more, right?

[cart]

I'm guessing the provided components are the minimal set, so actual queries can take more, right?

Yup!

I guess its also worth calling out that (9) and (8) are compatible with each other. We could encourage (9) generally, but support (8) if we find cases that need it.

[MiniaczQ]

Would it be possible to use both 8 and 9? As in, centralize the definition around the marker upstream, but still allow for extensions downstream.

[alice-i-cecile]

Proposal 10: Extensible Compositions defined on Components

#[derive(Component)]
#[supported_composition(Handle<Mesh>, GlobalTransform)]
struct NotShadowReceiver;

app.add_supported_component_composition::<NoShadowReceiver, (FancyShadowConfig)>()

A hybrid of 8 and 9, enabling non-upstream extensibility without sacrificing the benefits of proposal 9.
The complete list won't always be available by looking at the source, but at least most of it will be.

Current preferences

In order:

• 6: Fully automatic, very powerful. Requires external tooling.
• 10: Centrally located, but still extensible. Must be manually maintained in both directions.
• 9: Like 10, but not extensible.
• 8: Like 10, but messy and doesn't show up in the docs or at the struct definition.
• 4: elevates bundles to first-class citizen, but at least it's automatic.
• 7: Elevates bundles to first-class citizens, worse than 9.
• 5: overloads usage, doesn't really solve the problem
• 3: Doesn't solve the problem, and bundle structs with optional components are a bad, expensive pattern
• 1: doesn't solve the problem

I think we should do 2, but not to solve this problem.

[ManevilleF]

I like 7 the most, the objective is not to make optional marker components part of bundles but to have an indication of which components are "conceptually" working together.
The issue is that we would then need to correlate correctly both the derive attributes and the query archetypes

[cart]

I do want to stress that there is a reasonable chance that bundles will be elevated to first class citizens / expand in scope to accommodate prefabs (ex: add lifecycle hooks, omit default component values in scenes, ensure bundles stored in older versions of scenes support components added to the bundle in newer versions). Very much TBD, but relevant nonetheless. It might cease to be a "con".

[cart]

But I still think (10) has my vote in the interim.

[alice-i-cecile]

Sounds good. We can move forward with 10, and toss in 6 as a separate feature (I think it's very useful regardless, but solves a slightly different problem).

[aevyrie]

Coupling components together seems like it would make composing components from third party plugins difficult. It would also need rules like rust traits, only the implementer of a component type or a relationship can specify a relationship, otherwise you end up with potential conflicts. I'm not a huge fan of using Bundles to inform relationships; I find myself re-implementing my own version of bevy-native bundles (e.g. PbrBundle) because I need to swap out a single component to change the functionality of an entity. Using bundles tends to make component composition more difficult, and I'd prefer not to give them more "power" than they already have.

Alternatively, this relationship metadata could be coming from systems - that's where these relationships are used, right? Documenting relationships is redundant information: relationships are already specified by Querys. More specifically, system queries define what entity archetypes are used together to do useful things. That's the purpose of documenting this, right? It answers the question "what components can I compose, and what functionality do they add?"

Extending this line of thought, we could specifically document ZST components by looking at what components are being queried with With<> or Without<>, these filters are almost exclusively used with ZST marker components.

[MiniaczQ]

Some more thoughts on proposal 8 and all the ones that rely on it.
If possible, it'd be better to use only 1 method for defining compositions.
Having both means someone can mix them upstream, which is not intended.

If possible it'd be good to make use of newtype pattern, so that downstream can 'inherit' the original marker.
Then additional composition derives can also happen. (from proposal 9)
This will also solve the issue of multiple downstream crates affecting the original marker.

[alice-i-cecile]

Extending this line of thought, we could specifically document ZST components by looking at what components are being queried with With<> or Without<>, these filters are almost exclusively used with ZST marker components.

Proposal 11

Implement proposal 6, but then further refine the list to reduce false positives.

In the editor, split all component lists into ZST and non-ZST sublists.

Discussion

Alternatively, this relationship metadata could be coming from systems - that's where these relationships are used, right? Documenting relationships is redundant information: relationships are already specified by Querys. More specifically, system queries define what entity archetypes are used together to do useful things. That's the purpose of documenting this, right? It answers the question "what components can I compose, and what functionality do they add?"

I very much agree with this.

When attempting to discover "what else can I do with this component", what I want is "here is a list of active systems that act on this component", and "here is a list of components used in those systems". As a doc writer, I'm going to be frustrated by constantly trying to keep this information in sync. As a user, I'll be frustrated when this information is wrong: either as a false positive or false negative, due to out-of-date docs or inaccurate assumptions about the state of my app.

Ideally we'd have IDE and rustdoc support for this, but that would require a lot of external support.

I think proposal 11 achieves the same goals as your proposal @aevyrie. ZSTs and non-ZSTs serve two very different roles in the Bevy ecosystem. Splitting the list in a consistent, collapsible fashion will allow me to quickly parse "what does this component do", and "which components can I add / remove to toggle behavior".

When examining the associated components for a ZST, we can skim over the ZST sublist and vice-versa.

[mcobzarenco]

Just to echo @MiniaczQ as well as @aevyrie

One problem with 10 is that it provides two different ways of doing the same thing.

1. #[supported_composition(...)] needs to be kept in sync with the systems, consequence of "Documenting relationships is redundant information: relationships are already specified by Querys."
2. app.add_supported_component_composition also does, but at least it is called close to the place the systems are added to the app
3. they shouldn't be mixed

Only supporting add_supported_component_composition seems preferable, i.e. 8. This has poorer docs maybe, but could be a reasonable compromise

[MiniaczQ]

I fail to understand how defining it at app level is more preferable than defining it at the marker itself. It's not like you see system parameters there. 😕

[alice-i-cecile]

If possible, it'd be better to use only 1 method for defining compositions.
Having both means someone can mix them upstream, which is not intended.

I agree with that, everything else being equal. The ergonomics of 8 are worse than 9 though, and because it's runtime you're forced to rely on an external tool anyways.

[alice-i-cecile]

If possible it'd be good to make use of newtype pattern, so that downstream can 'inherit' the original marker.
Then additional composition derives can also happen. (from proposal 9)
This will also solve the issue of multiple downstream crates affecting the original marker.

Doesn't work unfortunately. If we have

struct Upsteam;
struct Downstream(Upstream);

Query<&Transform, With<Upstream>> won't detect the Downstream component and vice versa.

[cart]

One more thing thats worth pointing out: while this is important to solve for the editor, I don't think we need to prioritize an implementation until then. Once we reach agreement on a plan, I think that can unblock adopting the marker component pattern more widely in Bevy crates.

[cart]

We also might want to implement both (11) and (10). I'm worried about noise / false positives from (11). And manually filtering out noise could result in a lot of code noise, in itself. But given that the information is already there, the impl shouldn't be too hard. (10) is also relatively trivial to implement.

Once we have an editor, we could try enabling (11) and (10) in a complicated code base and evaluate the "supported component" experiences.

[cart]

That plan by itself is satisfying enough for me. We have two workable solutions and we can compare them in practice when the time comes.

[cart]

In the interim, we could just throw "supported marker component" docs into relevant bundles.

[mcobzarenco]

I fail to understand how defining it at app level is more preferable than defining it at the marker itself. It's not like you see system parameters there.

It makes component composition dynamic and something that can be configured. E.g. if it's defined on the marker with an attribute, there would be no obvious way to opt out. As this feature is for an editor, seems useful to limit what components can be added on a custom bundle. If instead RenderPlugin would do

app.add_supported_component_composition::<NoShadowReceiver, (Handle<Mesh>, GlobalTransform)>()

that could be configured depending on needs.

Say we have "enable behavior" by convention: ShadowCaster, not NotShadowCaster. Imagine a hypothetical skybox implemented with a cube and a specialized material with a shader that translates the camera in the center of the cube. E.g.

#[derive(Component, Bundle)]
pub struct SkyboxBundle {
    pub material: Handle<SkyboxMaterial>,
    pub mesh: Handle<Mesh>,
    pub visible: Visible,
    pub transform: Transform,
    ...
}

I am thinking that ideally it should not be an option to add ShadowCaster to it in the editor as it doesn't make sense semantically, even if there exists a system for a Mesh + ShadowCaster.

[MiniaczQ]

Say we have "enable behavior" by convention: ShadowCaster, not NotShadowCaster. Imagine a hypothetical skybox implemented with a cube and a specialized material with a shader that translates the camera in the center of the cube. E.g.

#[derive(Component, Bundle)]
pub struct SkyboxBundle {
    pub material: Handle<SkyboxMaterial>,
    pub mesh: Handle<Mesh>,
    pub visible: Visible,
    pub transform: Transform,
    ...
}

This is based entirely on my understanding, correct me if I got it wrong.

I don't think this was ever about limiting the ways markers can be applied?
The main idea was to extract information from queries, about where markers can be used, to something more centralized.
And since that would now be the main source of knowledge, we need to enforce it. Not the other way around.

Another thing is, we abstracted from bundles. The composition rules provide the minimal set of components that need to exist in a query, for a marker to be applicable. There is no blacklisting specific bundles despite them satisfying the condition.

The 'enable behavior' seems irrelevant, so I'm skipping over it.

Also not super relevant, but I don't think a bundle should be deriving from Component.

[mcobzarenco]

Another thing is, we abstracted from bundles. The composition rules provide the minimal set of components that need to exist in a query, for a marker to be applicable. There is no blacklisting specific bundles despite them satisfying the condition.

Yes, my apologies for not having a clearer example -- the fact that it's a bundle is irrelevant. Only that there is an entity that has body Handle<Mesh> and GlobalTransform. Which will mean there's a compatible composition with ShadowCaster and hypothetically add the option to enable ShadowCaster in an editor.

A solution which would allow controlling what compatible compositions are exposed for a particular entity, I thought it'd be easier when avoiding attributes on a type vs. defining at runtime what's allowed.

The 'enable behavior' seems irrelevant, so I'm skipping over it.

That was just to make the example work, e.g. otherwise SkyboxBundle would have NotShadowCaster and there would be no relationship -- not relevant to the point.

Also, not super relevant, but I don't think a bundle should be deriving from Component.

Oops, indeed 👍 -- left as I refactored the example

[MiniaczQ]

A solution which would allow controlling what compatible compositions are exposed for a particular entity, I thought it'd be easier when avoiding attributes on a type vs. defining at runtime what's allowed.

This would make entities store additional metadata and needlessly restrict the usability.
I don't think I can agree with that.

[mcobzarenco]

This would make entities store additional metadata and needlessly restrict the usability.
I don't think I can agree with that.

Right, there would need to be some way to differentiate at the runtime between the entities, I see what you mean, makes sense

[superdump]

My thinking on this isn’t so clear but while considering 6 I got some feeling that it misses something. In the renderer we have disjoint queries on views that have semantics about meshes. We’re discussing for example a Visible (or similar) marker component on entities with meshes and that impacts other queries about views (VisibleEntities). My gut felt like while you may be able group markers on entities according to how they are queried, is there something missing in general about the intent or consequences of the marker component as a system practically ties multiple queries together? And then we have systems which do multiple queries which feel like they should be handled in disjoint systems but for reasons (like needing knowledge about when some component is added/removed on some entity to take its own decisions about what to do and splitting systems would mean Commands syncs being necessary between them which is not so easy currently), they are handled in the same system.

I think what I’m getting at is that, as a user, when looking at an entity and seeing availability of a market component that was discovered due to being part of a query, I know very little about the effects or consequences of enabling/disabling it. It could easily be noisy and create usability problems in that it’s information overload or the purpose of the market component is misunderstood and they toggle it and unintentionally break stuff. And documenting this at the market component definition site may not be possible as the extensibility through custom systems can as all kinds of side effects that are completely unpredictable.

I’m not totally clear whether I have a legitimate point here but hopefully someone who understands better can judge that. :)

[superdump]

We also might want to implement both (11) and (10). I'm worried about noise / false positives from (11). And manually filtering out noise could result in a lot of code noise, in itself. But given that the information is already there, the impl shouldn't be too hard. (10) is also relatively trivial to implement.

Once we have an editor, we could try enabling (11) and (10) in a complicated code base and evaluate the "supported component" experiences.

In an editor or code IDE you could present curated results from 10 ahead of ‘searched’ results from 11. Results from 10 could be bold and from 11 not. Etc

[SamPruden]

If I've understood this issue correctly, it's about how marker components are "suggested" by the editor when composing entities. I'll be contrarian and say that I think this is actually fundamentally the wrong approach. Components should - mostly - not be surfaced in the editor.

On a larger team, it's important to separate the responsibilities of engineers and designers. Level design and engineering work should be able to be iterated on simultaneously and asynchronously with minimal breakage to each other. The editor is used to specify the intended behaviour, with little care for how that behaviour is implemented. It specifically does not want to be tied to the underlying ECS data layout, for two reasons.

1. The data layout that is good for engineering and performance may be significantly different from that which is convenient for a designer. ECS components are designed according to how they're used in transformations, but designers want things organised by concept and convenience. Neither of these experiences should need to be compromised for the other.
2. Engineers should be free to change the ECS data layout without breaking compatibility with scenes that have already been composed in the editor.

I'll throw my own proposal onto the pile.

1. Introduce "Authors". An Author is a logical grouping of editable data that the engineers expose to the editor. This is a stable interface - changing the data layout of an author is a breaking change for an already designed scene. Designers and the editor care about Authors, not components. They're an abstraction layer over the underlying ECS data.
2. Engineers are responsible for interpreting Author data into ECS data. They can change the ECS data layout as much as they want, as long as they update the code that interprets the Authors. A single Author can be interpreted into multiple and variable numbers of components, and perhaps even multiple entities.
3. There's some default widget that provides editor UI for authors based on their fields. A CustomWidget trait can be applied to Authors to customise their UI if desired.

In this setup, there's no need for marker components to be surfaced in any particular way. To continue from the examples, a Mesh Author would simply include a shadow_caster: bool field, and the editor would expose that. How that gets interpreted into ECS data is a matter for the engineers, and can be seamlessly changed halfway through production without designers even noticing.

Notably, this is the same conclusion that Unity have recently reached. As their CTO, Joachim Ante said:

The concept of runtime data and editing data being the same is simply a bad idea. It took me 14 years to figure that out (Sorry it took so long...)

---

This is relevant to this discussion, but perhaps it's also its own discussion. I can create a separate issue or discussion for it if desired.

[cart]

Definitely worth bringing up! I do think how runtime components map to "editor representation" and "scene representation" is worth discussing (and I think your proposal does deserve its own discussion).

From a certain perspective, I think its similar to the other suggestions, as it involves correlating entities of a certain "type" to other "potentially available" components. Therefore I don't think making a final call on that has bearing on whether or not we start using marker components today (as your suggestion also solves the problem at hand).

We now have even more solid paths forward :)

[SamPruden]

I've created a dedicated and more detailed issue for my authors proposal. #3877

[alice-i-cecile]

Closing in favor of #14946, which should solve this well.