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

World::inspect_entity should return an iterator, not Vec #13933

Closed
ItsDoot opened this issue Jun 19, 2024 · 0 comments · Fixed by #13934
Closed

World::inspect_entity should return an iterator, not Vec #13933

ItsDoot opened this issue Jun 19, 2024 · 0 comments · Fixed by #13934
Labels
A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples X-Uncontroversial This work is generally agreed upon

Comments

@ItsDoot
Copy link
Contributor

ItsDoot commented Jun 19, 2024

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

It's pretty uncommon for us to return Vecs in the API, and Iterators are generally more useful (since you can just collect them into a Vec immediately anyways). The current code even converts it from an iterator at the end:

bevy/crates/bevy_ecs/src/world/mod.rs

Lines 498 to 502 in e34ecf2

archetype
.components()
.filter_map(|id| self.components().get_info(id))
.collect()
}

What solution would you like?

Make World::inspect_entity return an impl Iterator<Item=&ComponentInfo> instead of a Vec<&ComponentInfo>.
@ItsDoot ItsDoot added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels Jun 19, 2024
@alice-i-cecile alice-i-cecile added C-Code-Quality A section of code that is hard to understand or change S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! X-Uncontroversial This work is generally agreed upon labels Jun 19, 2024
@ItsDoot ItsDoot mentioned this issue Jun 19, 2024
Merged
@ItsDoot ItsDoot removed the S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! label Jun 19, 2024
github-merge-queue bot pushed a commit that referenced this issue Jun 19, 2024
@ItsDoot
Change World::inspect_entity to return an Iterator instead of Vec (#1…
ee2487a 
…3934)

# Objective

Fixes #13933.

## Solution

Changed the return type.

## Testing

Fixed and reused the pre-existing tests for `inspect_entity`.

---

## Migration Guide

- `World::inspect_entity` now returns an `Iterator` instead of a `Vec`.
If you need a `Vec`, immediately collect the iterator:
`world.inspect_entity(entity).collect<Vec<_>>()`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples X-Uncontroversial This work is generally agreed upon
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Change World::inspect_entity to return an Iterator instead of Vec ItsDoot/bevy
2 participants
@alice-i-cecile @ItsDoot