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

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

Closed
wants to merge 2 commits into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c37c85e
doc
tim-blackbird Sep 13, 2022
522ff10
docdoc
tim-blackbird Sep 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion crates/bevy_ecs/src/query/iter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,10 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Iterator for QueryIter<'w, 's, Q, F>
// This is correct as [`QueryIter`] always returns `None` once exhausted.
impl<'w, 's, Q: WorldQuery, F: WorldQuery> FusedIterator for QueryIter<'w, 's, Q, F> {}

/// An [`Iterator`] over [`Query`](crate::system::Query) results of a list of [`Entity`]s.
/// An [`Iterator`] over the query items generated from an iterator of [`Entity`]s.
///
/// Items are returned in the order of the provided iterator.
/// Entities that don't match the query are skipped.
///
/// This struct is created by the [`Query::iter_many`](crate::system::Query::iter_many) and [`Query::iter_many_mut`](crate::system::Query::iter_many_mut) methods.
pub struct QueryManyIter<'w, 's, Q: WorldQuery, F: WorldQuery, I: Iterator>
Expand Down
14 changes: 10 additions & 4 deletions crates/bevy_ecs/src/query/state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -594,11 +594,14 @@ impl<Q: WorldQuery, F: WorldQuery> QueryState<Q, F> {
}
}

/// Returns an [`Iterator`] over the query results of a list of [`Entity`]'s.
/// Returns an [`Iterator`] over the read-only query items generated from an [`Entity`] list.
///
/// This can only return immutable data (mutable data will be cast to an immutable form).
/// See [`Self::iter_many_mut`] for queries that contain at least one mutable component.
/// Items are returned in the order of the list of entities.
/// Entities that don't match the query are skipped.
Copy link
Contributor

Choose a reason for hiding this comment

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

What about linking to QueryManyIter instead of duplicating text? That would remove the risk of divergent evolution.

Copy link
Contributor

@rparrett rparrett Sep 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I agree, but didn't we discuss/do the opposite in #4989?

Copy link
Contributor

Choose a reason for hiding this comment

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

I really think not. I always try my best to keep API docs in their place and avoid duplicating them.

Copy link
Contributor

Choose a reason for hiding this comment

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

My bad, I misremembered how that conversation went down. I still wish we could stick to a single convention with regard to whether the meat of the docs live on the iterator type or the method, but as long as everything's linked up it's not worth fussing about.

Copy link
Contributor

Choose a reason for hiding this comment

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

The rule of thumb is to document as close as possible to the implementation, so the iterator. The problem may be that the closest item is not very visible. In this case, good linking practices help a lot.

///
/// # See also
///
/// - [`iter_many_mut`](Self::iter_many_mut) to get mutable query items.
#[inline]
pub fn iter_many<'w, 's, EntityList: IntoIterator>(
&'s mut self,
Expand All @@ -620,7 +623,10 @@ impl<Q: WorldQuery, F: WorldQuery> QueryState<Q, F> {
}
}

/// Returns an iterator over the query results of a list of [`Entity`]'s.
/// Returns an iterator over the query items generated from an [`Entity`] list.
///
/// Items are returned in the order of the list of entities.
/// Entities that don't match the query are skipped.
#[inline]
pub fn iter_many_mut<'w, 's, EntityList: IntoIterator>(
&'s mut self,
Expand Down
8 changes: 7 additions & 1 deletion crates/bevy_ecs/src/system/query.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,9 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> {

/// Returns an [`Iterator`] over the read-only query items generated from an [`Entity`] list.
///
/// Items are returned in the order of the list of entities.
/// Entities that don't match the query are skipped.
///
/// # Example
///
/// ```
Expand Down Expand Up @@ -512,7 +515,10 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> {
}
}

/// Returns an [`Iterator`] over the query items generated from an [`Entity`] list.
/// Returns an iterator over the query items generated from an [`Entity`] list.
///
/// Items are returned in the order of the list of entities.
/// Entities that don't match the query are skipped.
///
/// # Examples
///
Expand Down