Skip to content

Commit

Permalink
Update codebase to use IntoIterator where possible. (#5269)
Browse files Browse the repository at this point in the history
Remove unnecessary calls to `iter()`/`iter_mut()`.
Mainly updates the use of queries in our code, docs, and examples.

```rust
// From
for _ in list.iter() {
for _ in list.iter_mut() {

// To
for _ in &list {
for _ in &mut list {
```

We already enable the pedantic lint [clippy::explicit_iter_loop](https://rust-lang.github.io/rust-clippy/stable/) inside of Bevy. However, this only warns for a few known types from the standard library.

## Note for reviewers
As you can see the additions and deletions are exactly equal.
Maybe give it a quick skim to check I didn't sneak in a crypto miner, but you don't have to torture yourself by reading every line.
I already experienced enough pain making this PR :) 


Co-authored-by: devil-ira <[email protected]>
  • Loading branch information
tim-blackbird and tim-blackbird committed Jul 11, 2022
1 parent 3203a85 commit 4847f7e
Show file tree
Hide file tree
Showing 95 changed files with 191 additions and 191 deletions.
2 changes: 1 addition & 1 deletion benches/benches/bevy_ecs/iteration/iter_simple_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ impl Benchmark {
}

fn query_system(mut query: Query<(&Velocity, &mut Position)>) {
for (velocity, mut position) in query.iter_mut() {
for (velocity, mut position) in &mut query {
position.0 += velocity.0;
}
}
Expand Down
4 changes: 2 additions & 2 deletions benches/benches/bevy_ecs/world/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ pub fn insert_commands(criterion: &mut Criterion) {

bencher.iter(|| {
let mut commands = Commands::new(&mut command_queue, &world);
for entity in entities.iter() {
for entity in &entities {
commands
.entity(*entity)
.insert_bundle((Matrix::default(), Vec3::default()));
Expand All @@ -112,7 +112,7 @@ pub fn insert_commands(criterion: &mut Criterion) {
bencher.iter(|| {
let mut commands = Commands::new(&mut command_queue, &world);
let mut values = Vec::with_capacity(entity_count);
for entity in entities.iter() {
for entity in &entities {
values.push((*entity, (Matrix::default(), Vec3::default())));
}
commands.insert_or_spawn_batch(values);
Expand Down
2 changes: 1 addition & 1 deletion benches/benches/bevy_reflect/struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ fn concrete_struct_field(criterion: &mut Criterion) {
.collect::<Vec<_>>();

bencher.iter(|| {
for name in field_names.iter() {
for name in &field_names {
s.field(black_box(name));
}
});
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_animation/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ pub fn animation_player(
mut transforms: Query<&mut Transform>,
children: Query<&Children>,
) {
for (entity, mut player) in animation_players.iter_mut() {
for (entity, mut player) in &mut animation_players {
if let Some(animation_clip) = animations.get(&player.animation_clip) {
// Continue if paused unless the `AnimationPlayer` was changed
// This allow the animation to still be updated if the player.elapsed field was manually updated in pause
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_asset/src/asset_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ impl AssetServer {
{
let mut loaders = self.server.loaders.write();
let loader_index = loaders.len();
for extension in loader.extensions().iter() {
for extension in loader.extensions() {
self.server
.extension_to_loader_index
.write()
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_core_pipeline/src/core_3d/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ pub fn prepare_core_3d_depth_textures(
>,
) {
let mut textures = HashMap::default();
for (entity, camera) in views_3d.iter() {
for (entity, camera) in &views_3d {
if let Some(physical_target_size) = camera.physical_target_size {
let cached_texture = textures
.entry(camera.target.clone())
Expand Down
10 changes: 5 additions & 5 deletions crates/bevy_ecs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ use bevy_ecs::prelude::*;
struct Position { x: f32, y: f32 }

fn print_position(query: Query<(Entity, &Position)>) {
for (entity, position) in query.iter() {
for (entity, position) in &query {
println!("Entity {:?} is at position: x {}, y {}", entity, position.x, position.y);
}
}
Expand Down Expand Up @@ -130,7 +130,7 @@ struct Velocity { x: f32, y: f32 }

// This system moves each entity with a Position and Velocity component
fn movement(mut query: Query<(&mut Position, &Velocity)>) {
for (mut position, velocity) in query.iter_mut() {
for (mut position, velocity) in &mut query {
position.x += velocity.x;
position.y += velocity.y;
}
Expand Down Expand Up @@ -176,7 +176,7 @@ struct Alive;
// Gets the Position component of all Entities with Player component and without the Alive
// component.
fn system(query: Query<&Position, (With<Player>, Without<Alive>)>) {
for position in query.iter() {
for position in &query {
}
}
```
Expand All @@ -197,13 +197,13 @@ struct Velocity { x: f32, y: f32 }

// Gets the Position component of all Entities whose Velocity has changed since the last run of the System
fn system_changed(query: Query<&Position, Changed<Velocity>>) {
for position in query.iter() {
for position in &query {
}
}

// Gets the Position component of all Entities that had a Velocity component added since the last run of the System
fn system_added(query: Query<&Position, Added<Velocity>>) {
for position in query.iter() {
for position in &query {
}
}
```
Expand Down
8 changes: 4 additions & 4 deletions crates/bevy_ecs/examples/change_detection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,24 +79,24 @@ fn print_changed_entities(
entity_with_added_component: Query<Entity, Added<Age>>,
entity_with_mutated_component: Query<(Entity, &Age), Changed<Age>>,
) {
for entity in entity_with_added_component.iter() {
for entity in &entity_with_added_component {
println!(" {:?} has it's first birthday!", entity);
}
for (entity, value) in entity_with_mutated_component.iter() {
for (entity, value) in &entity_with_mutated_component {
println!(" {:?} is now {:?} frames old", entity, value);
}
}

// This system iterates over all entities and increases their age in every frame
fn age_all_entities(mut entities: Query<&mut Age>) {
for mut age in entities.iter_mut() {
for mut age in &mut entities {
age.frames += 1;
}
}

// This system iterates over all entities in every frame and despawns entities older than 2 frames
fn remove_old_entities(mut commands: Commands, entities: Query<(Entity, &Age)>) {
for (entity, age) in entities.iter() {
for (entity, age) in &entities {
if age.frames > 2 {
println!(" despawning {:?} due to age > 2", entity);
commands.entity(entity).despawn();
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/macros/src/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream {
let mut field_idents = Vec::new();
let mut field_types = Vec::new();

for field in fields.iter() {
for field in fields {
let WorldQueryFieldInfo { is_ignored, attrs } = read_world_query_field_info(field);

let field_ident = field.ident.as_ref().unwrap().clone();
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/entity/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ use std::{
/// # struct Expired;
/// #
/// fn dispose_expired_food(mut commands: Commands, query: Query<Entity, With<Expired>>) {
/// for food_entity in query.iter() {
/// for food_entity in &query {
/// commands.entity(food_entity).despawn();
/// }
/// }
Expand Down
16 changes: 8 additions & 8 deletions crates/bevy_ecs/src/query/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ use std::{cell::UnsafeCell, marker::PhantomData};
/// }
///
/// fn my_system(query: Query<MyQuery>) {
/// for q in query.iter() {
/// for q in &query {
/// // Note the type of the returned item.
/// let q: MyQueryItem<'_> = q;
/// q.foo;
Expand Down Expand Up @@ -130,11 +130,11 @@ use std::{cell::UnsafeCell, marker::PhantomData};
///
/// fn my_system(mut health_query: Query<HealthQuery>) {
/// // Iterator's item is `HealthQueryReadOnlyItem`.
/// for health in health_query.iter() {
/// for health in &health_query {
/// println!("Total: {}", health.total());
/// }
/// // Iterator's item is `HealthQueryItem`.
/// for mut health in health_query.iter_mut() {
/// for mut health in &mut health_query {
/// health.damage(1.0);
/// println!("Total (mut): {}", health.total());
/// }
Expand All @@ -158,7 +158,7 @@ use std::{cell::UnsafeCell, marker::PhantomData};
/// }
///
/// fn my_system(mut my_query: Query<(FooReadOnly, FooReadOnly)>) {
/// for (i1, i2) in my_query.iter_mut() {
/// for (i1, i2) in &mut my_query {
/// let _: FooReadOnlyItem<'_> = i1;
/// let _: FooReadOnlyItem<'_> = i2;
/// }
Expand Down Expand Up @@ -254,7 +254,7 @@ use std::{cell::UnsafeCell, marker::PhantomData};
///
/// // You can also compose derived queries with regular ones in tuples.
/// fn my_system(query: Query<(&Foo, MyQuery, FooQuery)>) {
/// for (foo, my_query, foo_query) in query.iter() {
/// for (foo, my_query, foo_query) in &query {
/// foo; my_query; foo_query;
/// }
/// }
Expand All @@ -279,7 +279,7 @@ use std::{cell::UnsafeCell, marker::PhantomData};
/// }
///
/// fn my_system(query: Query<EmptyQuery>) {
/// for _ in query.iter() {}
/// for _ in &query {}
/// }
///
/// # bevy_ecs::system::assert_is_system(my_system);
Expand Down Expand Up @@ -311,7 +311,7 @@ use std::{cell::UnsafeCell, marker::PhantomData};
/// }
///
/// fn my_system(query: Query<Entity, MyFilter<Foo, Qux>>) {
/// for _ in query.iter() {}
/// for _ in &query {}
/// }
///
/// # bevy_ecs::system::assert_is_system(my_system);
Expand Down Expand Up @@ -1087,7 +1087,7 @@ unsafe impl<'w, T: Fetch<'w>> Fetch<'w> for OptionFetch<T> {
/// # struct Transform {};
/// #
/// fn print_moving_objects_system(query: Query<(&Name, ChangeTrackers<Transform>)>) {
/// for (name, tracker) in query.iter() {
/// for (name, tracker) in &query {
/// if tracker.is_changed() {
/// println!("Entity moved: {:?}", name);
/// } else {
Expand Down
10 changes: 5 additions & 5 deletions crates/bevy_ecs/src/query/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ use super::ReadOnlyWorldQuery;
/// # struct Name { name: &'static str };
/// #
/// fn compliment_entity_system(query: Query<&Name, With<IsBeautiful>>) {
/// for name in query.iter() {
/// for name in &query {
/// println!("{} is looking lovely today!", name.name);
/// }
/// }
Expand Down Expand Up @@ -177,7 +177,7 @@ impl<T> Copy for WithFetch<T> {}
/// # struct Name { name: &'static str };
/// #
/// fn no_permit_system(query: Query<&Name, Without<Permit>>) {
/// for name in query.iter() {
/// for name in &query{
/// println!("{} has no permit!", name.name);
/// }
/// }
Expand Down Expand Up @@ -324,7 +324,7 @@ impl<T> Copy for WithoutFetch<T> {}
/// # struct Style {};
/// #
/// fn print_cool_entity_system(query: Query<Entity, Or<(Changed<Color>, Changed<Style>)>>) {
/// for entity in query.iter() {
/// for entity in &query {
/// println!("Entity {:?} got a new style or color", entity);
/// }
/// }
Expand Down Expand Up @@ -685,7 +685,7 @@ impl_tick_filter!(
/// # struct Name {};
///
/// fn print_add_name_component(query: Query<&Name, Added<Name>>) {
/// for name in query.iter() {
/// for name in &query {
/// println!("Named entity created: {:?}", name)
/// }
/// }
Expand Down Expand Up @@ -725,7 +725,7 @@ impl_tick_filter!(
/// # struct Transform {};
///
/// fn print_moving_objects_system(query: Query<&Name, Changed<Transform>>) {
/// for name in query.iter() {
/// for name in &query {
/// println!("Entity Moved: {:?}", name);
/// }
/// }
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/query/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ impl<Q: WorldQuery, F: WorldQuery> QueryState<Q, F> {
///
/// let mut mutable_component_values = query_state.get_many_mut(&mut world, entities).unwrap();
///
/// for mut a in mutable_component_values.iter_mut(){
/// for mut a in &mut mutable_component_values {
/// a.0 += 5;
/// }
///
Expand Down Expand Up @@ -1074,7 +1074,7 @@ impl<Q: WorldQuery, F: WorldQuery> QueryState<Q, F> {

let tables = &world.storages.tables;

for entity in entity_list.into_iter() {
for entity in entity_list {
let location = match world.entities.get(*entity.borrow()) {
Some(location) => location,
None => continue,
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/schedule/executor_parallel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ impl ParallelSystemExecutor for ParallelExecutor {
self.should_run.grow(systems.len());

// Construct scheduling data for systems.
for container in systems.iter() {
for container in systems {
let dependencies_total = container.dependencies().len();
let system = container.system();
let (start_sender, start_receiver) = async_channel::bounded(1);
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/storage/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ impl Tables {
.from_key(component_ids)
.or_insert_with(|| {
let mut table = Table::with_capacity(0, component_ids.len());
for component_id in component_ids.iter() {
for component_id in component_ids {
table.add_column(components.get_info_unchecked(*component_id));
}
tables.push(table);
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/system/commands/parallel_scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ unsafe impl SystemParamState for ParallelCommandsState {
}

fn apply(&mut self, world: &mut World) {
for cq in self.thread_local_storage.iter_mut() {
for cq in &mut self.thread_local_storage {
cq.get_mut().apply(world);
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/system/exclusive_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ mod tests {
query: Query<Entity, With<Foo>>,
mut counter: ResMut<usize>,
) {
for entity in query.iter() {
for entity in &query {
*counter += 1;
commands.entity(entity).remove::<Foo>();
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/system/function_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ impl SystemMeta {
/// world.resource_scope(|world, mut cached_state: Mut<CachedSystemState>| {
/// let mut event_reader = cached_state.event_state.get_mut(world);
///
/// for events in event_reader.iter(){
/// for events in event_reader.iter() {
/// println!("Hello World!");
/// };
/// });
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_ecs/src/system/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
//! mut query: Query<(&Player, &mut Score)>,
//! mut round: ResMut<Round>,
//! ) {
//! for (player, mut score) in query.iter_mut() {
//! for (player, mut score) in &mut query {
//! if player.alive {
//! score.0 += round.0;
//! }
Expand Down Expand Up @@ -135,7 +135,7 @@ mod tests {
#[test]
fn simple_system() {
fn sys(query: Query<&A>) {
for a in query.iter() {
for a in &query {
println!("{:?}", a);
}
}
Expand Down Expand Up @@ -598,7 +598,7 @@ mod tests {
mut modified: ResMut<bool>,
) {
assert_eq!(query.iter().count(), 1, "entity exists");
for entity in query.iter() {
for entity in &query {
let location = entities.get(entity).unwrap();
let archetype = archetypes.get(location.archetype_id).unwrap();
let archetype_components = archetype.components().collect::<Vec<_>>();
Expand Down
Loading

0 comments on commit 4847f7e

Please sign in to comment.