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.

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

Replace many_for_each_mut with iter_many_mut. #5313

Closed

Conversation

tim-blackbird
Copy link
Contributor

@tim-blackbird tim-blackbird commented Jul 13, 2022

Huh, that's wierd. How did I manage to close this with a force push. and why did the force push yeet everything...
:l

Objective

Replace many_for_each_mut with iter_many_mut using the same tricks to avoid aliased mutability that iter_combinations_mut uses.

Based on #5170.
View the changes for this PR here.

Why

many_for_each_mut is worse for a few reasons:

  1. The closure prevents the use of continue, break, and return behaves like a limited continue.
  2. rustfmt will crumple it and double the indentation when the line gets too long.
    query.many_for_each_mut(
        &entity_list,
        |(mut transform, velocity, mut component_c)| {
            // Double trouble.
        },
    );
  3. It is more surprising to have many_for_each_mut as a mutable counterpart to iter_many than iter_many_mut.
  4. It required a separate unsafe fn; more duplicate unsafe code to mess up.
  5. The iter_many_mut API matches the existing iter_combinations_mut API.

}
}
None
}

/// Get next result from the query
#[inline(always)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added #[inline(always)] to all next/fetch_next methods. Should I have?

@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use A-ECS Entities, components, systems, and events labels Jul 14, 2022
@tim-blackbird
Copy link
Contributor Author

GitHub is now refusing to update this PR with changes I'm making to this branch. rip.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants