Skip to content
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

Label entities by the component bundles they have for clearer inspection #1958

Open
alice-i-cecile opened this issue Apr 18, 2021 · 13 comments
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged

Comments

@alice-i-cecile
Copy link
Member

What problem does this solve or what need does it fill?

When inspecting entities for debugging or as part of an editor, it can often be challenging to quickly get a a sense of what "kind" of entity it is.

Right now, you can either inspect it visually, or pore through its (long) list of components. This will become unwieldy when we need things like tooltip displays.

What solution would you like?

Create "entity labels", which are many-to-many and use the same type magic as e.g. system and stage labels.

These can be added to the AppBuilder with the label_entities(bundle: impl Bundle).

Each entity will have a "labels" field, which will show all relevant labels in alphabetical order (there should only be one in most cases). A label is relevant to an entity if it has all of the components in that bundle.

These will be almost entirely for display purposes. Under the hood, this can be computed and stored on a per archetype level.

Depending on their cost, we may want to only enable these during debug builds.

What alternative(s) have you considered?

Somehow designate a specific component as being "defining", or specify a priority of these.

Additional context

These will be used in close association with archetype invariants (#1481). We should ensure that the syntax and location of these are closely tied.

These bundles will commonly be reused for kinded entities; it may be correct to allow syntactic sugar to allow their specification via these entity labels.

In theory we could use query filters to search for these entities. I'm not entirely convinced by this plan, but it's not the worst idea.

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed S-Needs-Triage This issue needs to be labelled labels Apr 18, 2021
@mockersf
Copy link
Member

Using strings for label, we have the Labels component to be able to add labels to an entity, and the EntityLabels to query entities by label

@cart
Copy link
Member

cart commented Apr 19, 2021

Create "entity labels", which are many-to-many and use the same type magic as e.g. system and stage labels.
These will be almost entirely for display purposes. Under the hood, this can be computed and stored on a per archetype level.

Functionally this sounds identical to zero-sized marker components, which are cheap, queryable, integrate with scenes, and already have all relevant CRUD ops defined. It sounds like this is largely a categorization issue? Ex: maybe we need a way to promote specific component types to "labels" for things like editors?

@cart
Copy link
Member

cart commented Apr 19, 2021

Somehow designate a specific component as being "defining", or specify a priority of these.

lol you already covered this. i guess i glazed over it 😄

@cart
Copy link
Member

cart commented Apr 19, 2021

Maybe we could just track "bundle components" somewhere? Ex: we could just add is_bundle_component to ComponentInfo. I think I'd prefer something like that to creating a brand new "typed component label" concept because it builds on what is already there and doesn't require increasing the user-facing api surface.

@alice-i-cecile
Copy link
Member Author

maybe we need a way to promote specific component types to "labels" for things like editors?

Yeah, I'm leaning towards this as my preferred flavor of solution. I'm definitely in favor of trying to integrate this smoothly with minimal API changes. This isn't urgent or very impactful, but it could be a nice QoL feature once we have an editor.

Maybe we could just track "bundle components" somewhere?

What if we had an Identifier trait that we could add to components? Or a #[identifier] marker that you could stick onto your fields in a bundle like we do with #[bundle]?

@cart
Copy link
Member

cart commented Apr 19, 2021

What if we had an Identifier trait that we could add to components? Or a #[identifier] marker that you could stick onto your fields in a bundle like we do with #[bundle]?

My problem with this is that it adds a new "Identifier" concept, which I'm not convinced we need. Bundles seem like a pretty reasonable "identifier" as-is. They denote that the entity was created for a specific purpose. Yes, there can be multiple bundles per entity, but we'd almost certainly want to support multiple "identifiers" anyway.

@alice-i-cecile
Copy link
Member Author

alice-i-cecile commented Apr 19, 2021

we'd almost certainly want to support multiple "identifiers" anyway.

Definitely agree.

Bundles seem like a pretty reasonable "identifier" as-is. They denote that the entity was created for a specific purpose.

I think that's fair, but I'm not sure what information you'd extract from them to display. The bundle name, and then all of your identifiers are *Bundle? I guess we could strip that off automatically actually...

@alice-i-cecile
Copy link
Member Author

I think you'd want to track this at the entity level, and set a special "bundle labels" field or something when you use .spawn_bundle or .insert_bundle

@cart
Copy link
Member

cart commented Apr 19, 2021

I think you'd want to track this at the entity level, and set a special "bundle labels" field or something when you use .spawn_bundle or .insert_bundle

Yeah I think this should be entity level. If someone happens to spawn the same components a bundle has, I don't think that should be "auto labeled" with that bundle. I personally think inserting a special "bundle component" is the best way to do this.

@concave-sphere
Copy link

I think that's fair, but I'm not sure what information you'd extract from them to display. The bundle name, and then all of your identifiers are *Bundle? I guess we could strip that off automatically actually...

That brings up something I've wondered about: why do bundles have to be named *Bundle? I feel like for most of the bundles I create, either it's clear from context, or they're in a well labeled module (like foo::entities::Blah). And it gets even more redundant when I write commands.spawn_bundle(foo::entities::BlahBundle) -- I just wrote 3 times on that line that it's a bundle. It seems like commands.spawn_bundle(foo::Blah) would have gotten the point across clearly enough.

But I still feel the need to add the suffix. I think this might be a subconscious attempt to avoid accidentally using a Bundle as a Component. Maybe the suffix could be dropped if Component was opt-in (#1843)?


That aside: I like the idea of using Bundle to label things. It makes a Bundle more meaningful than just a pile of components that happen to go together, and means that users get more features automatically by using the system the way it was designed.

@cart
Copy link
Member

cart commented Apr 20, 2021

why do bundles have to be named *Bundle?

Its largely an issue of namespacing. If we have a Sprite component and a Sprite bundle, it makes using those types confusing and error prone. As a general principle, I try to treat Bevy as a global namespace (even though this isn't literally true). This ensures that types can be used side-by-side (and discussed in documentation and tutorials) without needing to specify bevy_sprite::component::Sprite vs bevy_sprite::bundle::Sprite. With a "global namespace" Sprite is always just one thing (in this case: a component).

Theres also the secondary issue of "using bundles as components and vice-versa", which might be solved by #1843, but even with that I'm still very much in favor of the *Bundle suffix.

It makes a Bundle more meaningful than just a pile of components that happen to go together, and means that users get more features automatically by using the system the way it was designed.

Agreed. I've been planning on using Bundles for a number of things for awhile now:

  1. Better scene editor workflows (ex:all registered bundles show up in visual scene editors (like Godot Nodes) and have friendly icons to help visually indicate their purpose)
  2. Using bundles in scenes to make them resilient to new component additions and removals (ex: when a new component is added to a Bundle, old scenes implicitly pick it up).

@concave-sphere
Copy link

Its largely an issue of namespacing. If we have a Sprite component and a Sprite bundle, it makes using those types confusing and error prone.

Hmm, I'd be interested in discussing this further, but I don't want to derail this bug any more than I already have. And it may be partly my unfamiliarity with Rust community practices showing here: I've read through the API Guidelines, but they are pretty sparse compared to, eg, what I'm familiar with in Go land. On this question, for instance, there's is very complete guidance in a widely cited blog post and in Effective Go docs.

Is there a better forum to continue this discussion, or any community docs that you think would be a helpful reference? (or if you don't think it's worth the time, I'll drop it)

@alice-i-cecile
Copy link
Member Author

@concave-sphere #1247 would be an appropriate place to discuss this, or a seperate issue would be fine if you'd prefer :)

@alice-i-cecile alice-i-cecile added the S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged label Dec 12, 2021
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 C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged
Projects
None yet
Development

No branches or pull requests

4 participants