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

[Merged by Bors] - Update Query methods documentation #5742

Closed
wants to merge 27 commits into from
Closed

[Merged by Bors] - Update Query methods documentation #5742

wants to merge 27 commits into from

Conversation

Nilirad
Copy link
Contributor

@Nilirad Nilirad commented Aug 19, 2022

Objective

Solution

Nilirad and others added 25 commits August 19, 2022 12:33
@Nilirad
Squash and rebase
1f81535 
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com>
@Nilirad
State that query creation has a time-constant cost
a04c319
@Nilirad
Table vs sparse set query iteratino performance
e403f9e
@Nilirad
Reference Query from WorldQuery
b0304e4
@Nilirad
Remove WorldQuery repetition in second paragraph
7be57c9
@Nilirad
Add note about for_each vs iter performance
8195f83
@Nilirad
Remove implementation detail from performance section
c7b2343
@Nilirad
Reword for_each performance
2a37c0e
@Nilirad
Remove for_each performance remarks from method descriptions
91c0b9d
@Nilirad
Add cross-links between iter and for_each methods
8ffe098
@Nilirad @james7132
Apply @james7132 suggestion on for_each performance
9166cd1 
Co-authored-by: James Liu <contact@jamessliu.com>
@Nilirad
Break doc comments at clause or sentence boundaries
ea313b8
@Nilirad
Split long line
bbf8a1e
@Nilirad
Migrate many_for_each_mut → iter_many_mut
890624c
@Nilirad
Fix iter_many_mut description
68df415
@Nilirad
Update to_readonly description
4aa7e20 
This will reflect the added context in this PR
that is aware of what query items are
and that they can be read-only or not.
@Nilirad
Ref-style link revision
d5372e9 
Moved all links to the bottom of the doc comment.
Links have been alphabetically ordered.

Rebase notes:

- After bevyengine#5205 got merged,
  the “Safety” section of `WorldQuery` has changed.
  I listed the mentioned methods for a better layout.
  I also removed the `WorldQuery::` prefix for readability.
- I don't have enough knowledge of the previous and current state of `WorldQuery` types,
  so the documentation may be outdated somewhere.
  `WorldQuery` needs another review round.
@Nilirad
Move `WorldQuery safety section up
74cf66c
@Nilirad @rparrett
Apply suggestions from code review
01ff486 
Co-authored-by: Rob Parrett <robparrett@gmail.com>
@Nilirad @BoxyUwU
Apply diff-based suggestions
fa78f7c 
Co-authored-by: Boxy <supbscripter@gmail.com>
@Nilirad
Remove duplicated and outdated “Safety” section
1ad2328
@Nilirad
Add link to State associated type
6a9c560
@Nilirad
Make clippy happy
684dff6
@Nilirad
Checkout other files from main
d18ea9b
@Nilirad
Remove Query struct docs
f7f663d
This was referenced Aug 19, 2022
Closed
Closed
@Nilirad Nilirad added C-Docs An addition or correction to our documentation A-ECS Entities, components, systems, and events labels Aug 19, 2022
Weibye
Weibye approved these changes Aug 27, 2022
View reviewed changes
Copy link
Contributor

@Weibye Weibye left a comment

Choose a reason for hiding this comment

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

This reads more clear and consistent, good work :)

crates/bevy_ecs/src/system/query.rs Show resolved Hide resolved
crates/bevy_ecs/src/system/query.rs Outdated Show resolved Hide resolved
crates/bevy_ecs/src/system/query.rs Outdated Show resolved Hide resolved
TimJentzsch
TimJentzsch reviewed Aug 27, 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
Nilirad and others added 2 commits August 27, 2022 18:32
@Nilirad @TimJentzsch
All or nothing :)
cb6b3a1 
Co-authored-by: TimJentzsch <tim-jentzsch@gmx.de>
@Nilirad @Weibye
Emphasize negation word
206abd4 
Co-authored-by: Andreas Weibye <13300393+Weibye@users.noreply.github.com>
TimJentzsch
TimJentzsch approved these changes Aug 27, 2022
View reviewed changes
Copy link
Contributor

@TimJentzsch TimJentzsch left a comment

Choose a reason for hiding this comment

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

Nicely done :)

@Weibye Weibye 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 Aug 27, 2022
bors bot pushed a commit that referenced this pull request Aug 30, 2022
@Nilirad
Add documentation to QueryCombinationIter (#5739)
5597fc5 
# 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)
@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Sep 2, 2022
@Nilirad
Update Query methods documentation (#5742)
47ecff4 
# 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.
@bors
Copy link
Contributor

bors bot commented Sep 2, 2022

Timed out.

@alice-i-cecile
Copy link
Member

bors retry

bors bot pushed a commit that referenced this pull request Sep 2, 2022
@Nilirad
Update Query methods documentation (#5742)
dfeb63e 
# 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.
@bors
Copy link
Contributor

bors bot commented Sep 2, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Update Query methods documentation [Merged by Bors] - Update Query methods documentation Sep 2, 2022
@bors bors bot closed this Sep 2, 2022
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
@Nilirad @james7132
Add documentation to QueryCombinationIter (bevyengine#5739)
148aa1c 
# 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)
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
@Nilirad @james7132
Update Query methods documentation (bevyengine#5742)
ceff825 
# 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.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
@Nilirad @ItsDoot
Add documentation to QueryCombinationIter (bevyengine#5739)
06b2731 
# 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)
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
@Nilirad @ItsDoot
Update Query methods documentation (bevyengine#5742)
df93083 
# 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.
@james7132 james7132 mentioned this pull request Feb 20, 2023
Closed
@Nilirad Nilirad deleted the Query-methods branch December 5, 2023 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TimJentzsch TimJentzsch TimJentzsch approved these changes

@Weibye Weibye Weibye approved these changes

Assignees
No one assigned
Labels
A-ECS Entities, components, systems, and events C-Docs An addition or correction to our documentation 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.

The iter_many_mut documentation is confused
4 participants
@Nilirad @alice-i-cecile @Weibye @TimJentzsch