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.

Sign up for GitHub

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

Jump to bottom

Implement methods to list all components and all resources #3012

Closed
wants to merge 6 commits into from
Closed

Implement methods to list all components and all resources #3012

wants to merge 6 commits into from

Conversation

sseemayer
Copy link
Contributor

@sseemayer sseemayer commented Oct 23, 2021 •
edited
Loading

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

@sseemayer
impl<'c> IntoIterator for &'c Components
8a354c0 
This allows iteration over all `Component`s registered in a world by
implementing `IntoIterator` for the `Components` type.
@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Oct 23, 2021
@sseemayer sseemayer marked this pull request as draft October 23, 2021 14:52
@alice-i-cecile alice-i-cecile self-requested a review October 23, 2021 14:54
@sseemayer
Copy link
Contributor Author

I am realizing there is some more design discussion to be had before I can continue implementing this - see the issue associated with this PR

@NiklasEi NiklasEi added 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 Oct 23, 2021
@sseemayer
Rename Components to WorldData
e48cd26 
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`
@sseemayer sseemayer force-pushed the 3007-iterate-resources-and-components branch from 2200887 to e48cd26 Compare October 24, 2021 16:12
@sseemayer
Update WorldData test to also show Resource iteration
6fbe43f
@sseemayer
Implement Components and Resources system parameters
6a774cd 
These can be used in systems to get an overview over all available
Components and Resources.
@sseemayer sseemayer changed the title WIP: Implement methods to list all components and all resources Implement methods to list all components and all resources Oct 24, 2021
@sseemayer sseemayer marked this pull request as ready for review October 24, 2021 20:26
alice-i-cecile
alice-i-cecile approved these changes Oct 24, 2021
View reviewed changes
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 an outstanding first PR. I read the changes in full and have no complaints.

inodentry
inodentry approved these changes Oct 31, 2021
View reviewed changes
Copy link
Contributor

@inodentry inodentry left a comment

Choose a reason for hiding this comment

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

This all looks good to me.

I really like the "component" -> "data" rename. It makes so much more sense this way. I previously found it confusing that bevy ecs uses the term "components" to refer both to the user-facing concept and api (distinct from "resources") and the internal backing storage for all data (including resources). They are separate things and deserve distinct names.

And, being able to iterate "all components", "all resources", and "all data"/both, is obviously also very useful.

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it hacktoberfest-accepted and removed S-Needs-Review labels Oct 31, 2021
@alice-i-cecile alice-i-cecile added M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide and removed S-Needs-Migration-Guide labels Jan 20, 2022
@alice-i-cecile
Copy link
Member

@sseemayer I'm interested in merging this; can you please rebase?

@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 May 9, 2022
@KevinKeyser
Copy link

Would it be okay with either of you if I open a PR for this rebased on main? @alice-i-cecile @inodentry
This would be my first PR, but the work I put in already passes all the CI checks on my fork.

@alice-i-cecile
Copy link
Member

@KevinKeyser Yes please! That's the intent behind the Adopt-Me label :) And I've wanted this functionality for ages.

@KevinKeyser KevinKeyser mentioned this pull request Jun 7, 2022
Closed
4 tasks
@alice-i-cecile
Copy link
Member

Closing in favor of #4955.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MinerSebas MinerSebas MinerSebas left review comments

@inodentry inodentry inodentry approved these changes

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

Assignees
No one assigned
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Adopt-Me The original PR author has no intent to complete this work. Pick me up! S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Simple methods to list all components and all resources
6 participants
@sseemayer @alice-i-cecile @KevinKeyser @inodentry @MinerSebas @NiklasEi