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

Implement methods to list all components and all resources (Rebase of PR #3012) #4955

Conversation

KevinKeyser
Copy link

Objective

Solution

  • Rename Components into WorldData to reflect that it stores metadata both on Components and Resources. Also rename ComponentId to DataId, ComponentInfo to DataInfo and ComponentDescriptor to DataDescriptor
  • implement IntoIterator for WorldData to iterate over all components and resources
  • create a new Components system descriptor and implement IntoIterator to iterate over all components
  • create a new Resources system descriptor and implement IntoIterator to iterate over all resources

Changelog

  • struct ComponentId -> struct DataId
  • struct Components -> struct WorldData
    • Components::components -> WorldData::data
  • struct ComponentInfo -> struct DataInfo
  • struct ComponentDescriptor -> struct DataDescriptor

sseemayer and others added 9 commits June 6, 2022 15:17
This allows iteration over all `Component`s registered in a world by
implementing `IntoIterator` for the `Components` type.
the Components struct was not just storing metadata on components, but
also on resources. WorldData is a new umbrella term for components and
resources.

also rename the following:

`ComponentId` -> `DataId`
`ComponentInfo` -> `DataInfo`
`ComponentDescriptor` -> `DataDescriptor`
These can be used in systems to get an overview over all available
Components and Resources.
@alice-i-cecile
Copy link
Member

My instinct is that we should merge #4809 first, which should simplify the implementation here significantly. @james7132, are you in agreement?

@jakobhellermann
Copy link
Contributor

For what it's worth you can iterate over all component infos already:

pub fn iter(&self) -> impl Iterator<Item = &ComponentInfo> + '_ {
(it will include both components and resources).

To find all resource component IDs you can just the components in the resource archetype, something like world.archetypes().resource().components().

@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 7, 2022
@targrub
Copy link
Contributor

targrub commented Oct 8, 2022

@KevinKeyser Is this a dead PR at this point?

Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the ComponentId -> DataId change: I think it's genuinely correct and clearer. Resources having a ComponentId is always baffling. However it makes this PR much harder to review, and should be its own PR.

@james7132 james7132 self-requested a review October 19, 2022 09:20
fn iterate_resources() {
let mut world = World::default();
world.insert_resource(Vec::<String>::new());
world.insert_resource(42usize);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These will have to be redone to account for the opt-in Resource trait.

}

#[test]
fn remove_tracking() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fn remove_tracking() {
fn removal_tracking() {

Just to reduce diff noise.

Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is quite nice: I really like how much clearer DataId and WorldData are. If you rebase this and get CI passing I'd be happy to see this merged basically as is.

@alice-i-cecile
Copy link
Member

@KevinKeyser I still really like this work, but at this point it's going to be easier to remake this PR than to solve merge conflicts. I'm going to Adopt-Me to this PR, but feel free to adopt it yourself if you'd like!

@alice-i-cecile alice-i-cecile added the S-Adopt-Me The original PR author has no intent to complete this work. Pick me up! label Sep 11, 2023
@Adamkob12
Copy link
Contributor

I'd like to pick this up soon.


/// The [`SystemParamState`] of [`Components`].
#[doc(hidden)]
pub struct ComponentsState;

// SAFE: no component value access
unsafe impl SystemParamState for ComponentsState {
unsafe impl<'a> SystemParamState for ComponentsState {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably trivial, but is this explicit lifetime required?

};
let component_id = if let Some(component_id) = self.data.get_resource_id(TypeId::of::<R>())
{
component_id
Copy link

@AustinHellerRepo AustinHellerRepo Mar 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be resource_id or data_id (as well as in other related functions)?

@AustinHellerRepo
Copy link

AustinHellerRepo commented Mar 30, 2024

I'm hoping that this PR will help permit me to more easily serialize specific components (when I only have the name) of an entity as well as overwrite the state of a component by deserializing a previous state's String into the component.
If this is now possible with these changes, would it be possible to get a unit test or example included in this PR?
If that is out of scope for these changes, any help with this task would be greatly appreciated. Thank you.

Edit: I can get a Ptr to the component, but it might be a type that contain pointers to the heap (with data that I also want to serialize). I'm hoping that there's a way to apply the TypeId and Ptr with serde (or by some other means) in such a way that I can get back all of the data pertaining to the component. I also need to be able to do the reverse to overwrite the component state.

@alice-i-cecile
Copy link
Member

@matiqo15, this seems really useful for bevyengine/rfcs#77. Perhaps we should re-adopt it and get the basic form merged?

@matiqo15
Copy link
Member

@matiqo15, this seems really useful for bevyengine/rfcs#77. Perhaps we should re-adopt it and get the basic form merged?

Agree, this could help a lot

github-merge-queue bot pushed a commit that referenced this pull request Apr 3, 2024
# Objective

- Closes #12019
- Related to #4955
- Useful for dev_tools and networking

## Solution

- Create `World::iter_resources()` and `World::iter_resources_mut()`

---------

Co-authored-by: Alice Cecile <[email protected]>
Co-authored-by: James Liu <[email protected]>
Co-authored-by: Pablo Reinhardt <[email protected]>
chompaa pushed a commit to chompaa/bevy that referenced this pull request Apr 5, 2024
# Objective

- Closes bevyengine#12019
- Related to bevyengine#4955
- Useful for dev_tools and networking

## Solution

- Create `World::iter_resources()` and `World::iter_resources_mut()`

---------

Co-authored-by: Alice Cecile <[email protected]>
Co-authored-by: James Liu <[email protected]>
Co-authored-by: Pablo Reinhardt <[email protected]>
@VitalyAnkh
Copy link
Contributor

@alice-i-cecile Now that #12829 has been merged, is this PR still necessary?

@alice-i-cecile
Copy link
Member

It would still be nice to have something like this for components, but I'm going to close this out regardless.

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 S-Adopt-Me The original PR author has no intent to complete this work. Pick me up!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simple methods to list all components and all resources
9 participants