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".