bevy
bevy copied to clipboard
Improve query API docs
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 #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
fromQuery
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 ofbevy_ecs_macros
to make doctests compile. Discussion on this is appreciated. I'm puttingS-Controversial
for now until a collective decision can be made.~~ Solved. -
Query
andWorldQuery
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
andmany_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:
- [x] Resolve the dev-dependency dilemma
- [x] Write the computational complexity for
many_iter
andmany_for_each_mut
- Fixes #5506
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.
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.
Squashed because rebasing in multiple steps is a headache.
I preserved co-authors in the commit message.
Rebased. I also pushed new commits. The first of the new commits is 5b064a2.
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 Iterator
ish type to point to docs in the methods that create them. QueryIter
and QueryManyIter
seem to follow that pattern as well.
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
Iterator
ish type to point to docs in the methods that create them.QueryIter
andQueryManyIter
seem to follow that pattern as well.
I did that because having put that on methods caused duplication of information, showing that the description is mislocated. This poses docs at a serious risk of remaining outdated in case of changes. This is probably not a piece of information that is going to change, but it's always better to deduplicate documentation and to describe at the nearest point to the implementation.
If you think it could reduce it's visibility, it's much better to link to the iterator instead of reverting the change.
I had to rebase after #5205 got merged, but I don't have enough expertise about WorldQuery
to fully understand what happened here. So the docs of the WorldQuery
needs some expert eyes, especially because there is no more FetchState
and now docs fail to build without warnings. I tried my best but I'm not able to go further alone.
More info on the commit notes.
Ping @BoxyUwU for docs review following your WorldQuery
changes.
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
Thanks!!
I split this into multiple PRs, since it's difficult to find people to review it. Hopefully they are more digestible.
- #5739
- #5740
- #5741
- #5742