Improve query API docs by Nilirad · Pull Request #4989 · bevyengine/bevy

Nilirad · 2022-06-11T16:21:09Z

Objective

Query docs need a refresh and they look quite inconsistent.
WorldQuery docs had some info about the derive macro that should not belong there.
Closes Make a note of the algorithmic complexity of each of the query operations #4742.

Solution

Documentation has been changed for the following items:

Query (item level and methods)
QueryCombinationIter
WorldQuery trait
WorldQuery macro

`Query`

Introduction has been redone.
“Renamed” type parameters to reflect their purpose.
Examples for declaration idioms have been simplified and reordered.
“Query result” terminology has been changed to “query item”, which is more in line with the underlying types and with general terminology.
Query methods: structure has been made consistent, old notes about query being able to access certain methods based on mutability have been updated and centralized to the item level.
A tables has been added to navigate the safe methods.
A performance section has been added, notably with a table with big O computational complexity.

`QueryCombinationIter`

Moved stuff about QueryCombinationIter from Query methods to the item itself.

`WorldQuery` trait

Introduction has been tidied up
Macro usage moved to macro item, with a link in the original place.

`WorldQuery` macro

Removed foobar terminology in favor of more recognizable identifiers (ComponentA, my_system). Examples with concrete identifiers are used where appropriate.
Removed “Ignored fields” section on WorldQuery macro.
Examples split by use case.

Additional notes

I had to add bevy_ecs as a dev-dependency of bevy_ecs_macros to make doctests compile. Discussion on this is appreciated. I'm putting S-Controversial for now until a collective decision can be made. Solved.
Query and WorldQuery macro idioms should be in the book, but in the meantime they can stay here.
~~I don't know what the computational complexity for many_iter and many_for_each_mut is, so I added a placeholder for now.~~ Solved.
Consider removing the #[world_query(ignore)] feature, as it is no longer needed.
Please review by checking the rendered docs (cargo doc -p bevy_ecs --no-deps --open).
Suggestions on improving examples by making them simpler or more concrete (where applicable) are appreciated.

Todo list:

Resolve the dev-dependency dilemma
Write the computational complexity for many_iter and many_for_each_mut

Fixes The iter_many_mut documentation is confused #5506

crates/bevy_ecs/src/system/query.rs

tim-blackbird · 2022-06-11T20:09:50Z

many_iter and many_for_each_mut essentially amount to a bunch of Query::get's with slightly reduced overhead.
Since Query::get is O(1) the complexity should be O(n) where n is the number of entities provided.

crates/bevy_ecs/src/system/query.rs

alice-i-cecile

I really like the consistency this adds, and the language changes are good. There's a few things to clean up. Not sure if / how we can resolve the dev-dependency problem; I'd probably just revert that and merge it without links for now :(

Nilirad · 2022-06-12T10:30:24Z

I moved back the macro docs to the WorldQuery trait and restored changes to bevy_ecs_macros from the main branch.

I also removed the S-Controversial label.

crates/bevy_ecs/src/query/fetch.rs

crates/bevy_ecs/src/query/iter.rs

crates/bevy_ecs/src/system/query.rs

Nilirad · 2022-06-16T08:16:01Z

Squashed because rebasing in multiple steps is a headache.

I preserved co-authors in the commit message.

crates/bevy_ecs/src/query/fetch.rs

crates/bevy_ecs/src/system/query.rs

Nilirad · 2022-07-31T15:43:28Z

Rebased. I also pushed new commits. The first of the new commits is 5b064a2.

rparrett

Don't have time to look this over this morning, but I did a quick pass with the spell-checker.

crates/bevy_ecs/src/system/query.rs

rparrett · 2022-08-04T20:16:55Z

Curious about the motivation behind

Moved stuff about QueryCombinationIter from Query methods to the item itself.

The general pattern in rust docs seems to be for the Iteratorish type to point to docs in the methods that create them. QueryIter and QueryManyIter seem to follow that pattern as well.

alice-i-cecile · 2022-08-08T17:58:28Z

Ping @BoxyUwU for docs review following your WorldQuery changes.

BoxyUwU · 2022-08-08T18:21:23Z

Generally what happend in that PR is that all of the traits were merged into WorldQuery. So anything talking about Fetch::foo or FetchState::foo now should talk about WorldQuery::foo

crates/bevy_ecs/src/query/fetch.rs

Co-authored-by: Boxy <supbscripter@gmail.com>

Nilirad · 2022-08-08T19:40:33Z

Thanks!!

Nilirad · 2022-08-19T12:01:36Z

I split this into multiple PRs, since it's difficult to find people to review it. Hopefully they are more digestible.

# Objective - Document `QueryCombinationIter` ## Solution - Describe the item, add usage and examples - Copy notes about the number of query items generated from the corresponding query methods (they will be removed in #5742 ([motivation])) ## Additional notes - Derived from #4989 [motivation]: #4989 (comment)

# Objective - Update docs to `WorldQuery` ## Solution - See #4989. This PR is derived from it, and limited to the `WorldQuery` item docs.

# Objective - Update `Query` docs with better terminology - add some performance remarks (Fixes #4742) ## Solution - See #4989. This PR is derived from it. It just includes changes to the `Query` struct docs.

# Objective - Increase consistency across documentation of `Query` methods. - Fixes #5506 ## Solution - See #4989. This PR is derived from it. It just includes changes to the `Query` methods' docs.

# Objective - Document `QueryCombinationIter` ## Solution - Describe the item, add usage and examples - Copy notes about the number of query items generated from the corresponding query methods (they will be removed in bevyengine#5742 ([motivation])) ## Additional notes - Derived from bevyengine#4989 [motivation]: bevyengine#4989 (comment)

# Objective - Update docs to `WorldQuery` ## Solution - See bevyengine#4989. This PR is derived from it, and limited to the `WorldQuery` item docs.

# Objective - Update `Query` docs with better terminology - add some performance remarks (Fixes bevyengine#4742) ## Solution - See bevyengine#4989. This PR is derived from it. It just includes changes to the `Query` struct docs.

# Objective - Increase consistency across documentation of `Query` methods. - Fixes bevyengine#5506 ## Solution - See bevyengine#4989. This PR is derived from it. It just includes changes to the `Query` methods' docs.

# Objective - Document `QueryCombinationIter` ## Solution - Describe the item, add usage and examples - Copy notes about the number of query items generated from the corresponding query methods (they will be removed in bevyengine#5742 ([motivation])) ## Additional notes - Derived from bevyengine#4989 [motivation]: bevyengine#4989 (comment)

# Objective - Update docs to `WorldQuery` ## Solution - See bevyengine#4989. This PR is derived from it, and limited to the `WorldQuery` item docs.

# Objective - Update `Query` docs with better terminology - add some performance remarks (Fixes bevyengine#4742) ## Solution - See bevyengine#4989. This PR is derived from it. It just includes changes to the `Query` struct docs.

# Objective - Increase consistency across documentation of `Query` methods. - Fixes bevyengine#5506 ## Solution - See bevyengine#4989. This PR is derived from it. It just includes changes to the `Query` methods' docs.

Nilirad added C-Docs An addition or correction to our documentation A-ECS Entities, components, systems, and events X-Needs-SME This type of work requires an SME to approve it. labels Jun 11, 2022

alice-i-cecile self-requested a review June 11, 2022 16:29

alice-i-cecile reviewed Jun 11, 2022

View reviewed changes

crates/bevy_ecs/src/system/query.rs Outdated Show resolved Hide resolved

alice-i-cecile reviewed Jun 11, 2022

View reviewed changes

crates/bevy_ecs/src/system/query.rs Outdated Show resolved Hide resolved

alice-i-cecile reviewed Jun 11, 2022

View reviewed changes

crates/bevy_ecs/src/system/query.rs Show resolved Hide resolved

alice-i-cecile reviewed Jun 11, 2022

View reviewed changes

Nilirad removed the X-Needs-SME This type of work requires an SME to approve it. label Jun 12, 2022

MinerSebas reviewed Jun 12, 2022

View reviewed changes

crates/bevy_ecs/src/query/fetch.rs Outdated Show resolved Hide resolved

crates/bevy_ecs/src/query/fetch.rs Outdated Show resolved Hide resolved

crates/bevy_ecs/src/query/iter.rs Outdated Show resolved Hide resolved

crates/bevy_ecs/src/system/query.rs Outdated Show resolved Hide resolved

Nilirad force-pushed the query_docs branch from 6ee8fb7 to 90e55d7 Compare June 16, 2022 08:13

james7132 requested changes Jun 20, 2022

View reviewed changes

james7132 requested changes Jul 2, 2022

View reviewed changes

crates/bevy_ecs/src/system/query.rs Outdated Show resolved Hide resolved

Nilirad commented Jul 2, 2022

View reviewed changes

crates/bevy_ecs/src/system/query.rs Show resolved Hide resolved

alice-i-cecile requested review from james7132 and mockersf July 2, 2022 11:34

james7132 approved these changes Jul 2, 2022

View reviewed changes

alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jul 2, 2022

james7132 removed the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jul 3, 2022

Nilirad force-pushed the query_docs branch from 6a9d885 to 812dec6 Compare July 14, 2022 06:52

Nilirad mentioned this pull request Jul 31, 2022

The iter_many_mut documentation is confused #5506

Closed

Nilirad force-pushed the query_docs branch from 812dec6 to b3fa3fc Compare July 31, 2022 15:40

rparrett reviewed Jul 31, 2022

View reviewed changes

crates/bevy_ecs/src/system/query.rs Outdated Show resolved Hide resolved

crates/bevy_ecs/src/system/query.rs Outdated Show resolved Hide resolved

alice-i-cecile self-requested a review July 31, 2022 20:40

nicopap added this to the Bevy 0.8.1 milestone Aug 1, 2022

BoxyUwU suggested changes Aug 8, 2022

View reviewed changes

crates/bevy_ecs/src/query/fetch.rs Show resolved Hide resolved

crates/bevy_ecs/src/query/fetch.rs Outdated Show resolved Hide resolved

crates/bevy_ecs/src/query/fetch.rs Outdated Show resolved Hide resolved

Nilirad and others added 4 commits August 8, 2022 21:16

Apply diff-based suggestions

b9d3cea

Co-authored-by: Boxy <supbscripter@gmail.com>

Remove duplicated and outdated “Safety” section

8950bfe

Add link to State associated type

a6b56df

Make clippy happy

706204f

cart removed this from the Bevy 0.8.1 milestone Aug 18, 2022

Nilirad closed this Aug 19, 2022

bors bot pushed a commit that referenced this pull request Sep 2, 2022

Improve WorldQuery docs (#5740)

59bf3c4

# Objective - Update docs to `WorldQuery` ## Solution - See #4989. This PR is derived from it, and limited to the `WorldQuery` item docs.

rparrett mentioned this pull request Sep 15, 2022

[Merged by Bors] - Clarify the behaviour of iter_many in the docs #5973

Closed

james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022

Improve WorldQuery docs (bevyengine#5740)

fcfcca0

# Objective - Update docs to `WorldQuery` ## Solution - See bevyengine#4989. This PR is derived from it, and limited to the `WorldQuery` item docs.

ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023

Improve WorldQuery docs (bevyengine#5740)

2306fee

# Objective - Update docs to `WorldQuery` ## Solution - See bevyengine#4989. This PR is derived from it, and limited to the `WorldQuery` item docs.

Nilirad deleted the query_docs branch December 5, 2023 18:05

Uh oh!

Comments

Conversation

Nilirad commented Jun 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Objective

Solution

Query

QueryCombinationIter

WorldQuery trait

WorldQuery macro

Additional notes

Todo list:

Uh oh!

Uh oh!

Uh oh!

tim-blackbird commented Jun 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

alice-i-cecile left a comment

Choose a reason for hiding this comment

Uh oh!

Nilirad commented Jun 12, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Nilirad commented Jun 16, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Nilirad commented Jul 31, 2022

Uh oh!

rparrett left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rparrett commented Aug 4, 2022

Uh oh!

alice-i-cecile commented Aug 8, 2022

Uh oh!

BoxyUwU commented Aug 8, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Nilirad commented Aug 8, 2022

Uh oh!

Nilirad commented Aug 19, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

Nilirad commented Jun 11, 2022 •

edited

Loading

`Query`

`QueryCombinationIter`

`WorldQuery` trait

`WorldQuery` macro

tim-blackbird commented Jun 11, 2022 •

edited

Loading