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

Add a simple method to get a ComponentId for a given Component type #5060

Closed
alice-i-cecile opened this issue Jun 21, 2022 · 3 comments
Closed
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use

Comments

@alice-i-cecile
Copy link
Member

alice-i-cecile commented Jun 21, 2022

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

Being able to extract the ComponentId for a given component of type C is surprisingly unintuitive.

What solution would you like?

Create a public method on Component that mirrors Bundle::component_ids().

EDIT: see below; this isn't the right API. These methods should live on World and Components instead.

What alternative(s) have you considered?

  1. Use world.init_component::<C>(). This is unintuitive, but basically works.
  2. Convert the component type into a bundle, and call (C, )::component_ids(&mut components, &mut storages).iter().next().unwrap(). Which is more than a little cursed.

Additional context

Discovered while working on #1481 with @plof27.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Jun 21, 2022
@alice-i-cecile
Copy link
Member Author

This should be very easy, but that code is not exactly the most approachable, so no Good-First-Issue tag here.

@GarettCooper
Copy link
Contributor

I'm not clear on how this or the Bundle equivalent is supposed to be used, but I might be missing something.

I started implementing it by matching the parameters of Bundle::component_ids(), but when I went to add tests I couldn't find a way of obtaining a mut reference to a World's Components or Storages from outside of bevy_ecs. Is this intended for consumption entirely within the ECS crate, or have I just missed something? You could implement the same thing using &mut World of course, but with my limited knowledge of Bevy I believe that would limit its usage to exclusive systems or the initial setup which seems too restrictive for normal consumers of bevy_ecs (Though I don't know enough about real Bevy applications to know for sure).

Besides that, I feel like providing a Components instance to a function on the Component to extract the ComponentId is less intuitive than the other way around. Components.get_id(C::type_id()).unwrap() also has the advantage of not requiring mut Components (though that's obviously not an option if creating a ComponentId if one is already exists is a strict requirement).

@alice-i-cecile
Copy link
Member Author

Thanks for investigating @GarettCooper. I think your criticisms are very fair!

After discussing this with @DJMcNab and @james7132, I think we should add two methods:

  1. World::component_id::<C>() -> Option<ComponentId>. This should be read-only, and thus distinct from init_component.
  2. Components::component_id::<C>() -> Option<ComponentId>.

We should not add this as a method to Component, as the correct ComponentId can vary between worlds, and the provided ComponentId must be correct for any usage of this to not violate correctness.

As part of this PR, we should add basic docs to ComponentId, and point to these constructor methods (and the one on Bundle).

@alice-i-cecile alice-i-cecile changed the title Add a method to get a ComponentId from a Component Add a simple method to get a ComponentId for a given Component type Jun 21, 2022
@bors bors bot closed this as completed in fa56a5c Jun 25, 2022
inodentry pushed a commit to IyesGames/bevy that referenced this issue Aug 8, 2022
# Objective

- Simplify the process of obtaining a `ComponentId` instance corresponding to a `Component`.
- Resolves bevyengine#5060.

## Solution

- Add a `component_id::<T: Component>(&self)` function to both `World` and `Components` to retrieve the `ComponentId` associated with `T` from a immutable reference.

---

## Changelog

- Added `World::component_id::<C>()` and `Components::component_id::<C>()` to retrieve a `Component`'s corresponding `ComponentId` if it exists.
james7132 pushed a commit to james7132/bevy that referenced this issue Oct 28, 2022
# Objective

- Simplify the process of obtaining a `ComponentId` instance corresponding to a `Component`.
- Resolves bevyengine#5060.

## Solution

- Add a `component_id::<T: Component>(&self)` function to both `World` and `Components` to retrieve the `ComponentId` associated with `T` from a immutable reference.

---

## Changelog

- Added `World::component_id::<C>()` and `Components::component_id::<C>()` to retrieve a `Component`'s corresponding `ComponentId` if it exists.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this issue Feb 1, 2023
# Objective

- Simplify the process of obtaining a `ComponentId` instance corresponding to a `Component`.
- Resolves bevyengine#5060.

## Solution

- Add a `component_id::<T: Component>(&self)` function to both `World` and `Components` to retrieve the `ComponentId` associated with `T` from a immutable reference.

---

## Changelog

- Added `World::component_id::<C>()` and `Components::component_id::<C>()` to retrieve a `Component`'s corresponding `ComponentId` if it exists.
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-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants