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

[Merged by Bors] - Allow iter combinations on queries with filters #3656

Conversation

harudagondi
Copy link
Member

Objective

Solution

  • Derived Copy on all *Fetch<T> structs, and manually implemented Clone to allow the test to pass (.count() does not work on QueryCombinationIter when Clone is derived)

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Jan 13, 2022
@harudagondi harudagondi force-pushed the allow-iter-combinations-on-queries-with-filters branch from f832dee to 3f26e35 Compare January 13, 2022 10:12
@harudagondi harudagondi deleted the allow-iter-combinations-on-queries-with-filters branch January 13, 2022 10:13
@harudagondi harudagondi restored the allow-iter-combinations-on-queries-with-filters branch January 13, 2022 10:14
@harudagondi harudagondi reopened this Jan 13, 2022
crates/bevy_ecs/src/query/filter.rs Outdated Show resolved Hide resolved
crates/bevy_ecs/src/query/filter.rs Outdated Show resolved Hide resolved
crates/bevy_ecs/src/query/filter.rs Outdated Show resolved Hide resolved
crates/bevy_ecs/src/query/filter.rs Outdated Show resolved Hide resolved
crates/bevy_ecs/src/query/state.rs Outdated Show resolved Hide resolved
@harudagondi
Copy link
Member Author

harudagondi commented Jan 13, 2022

Okay, I honestly do not know how to remove commit 3f26e35 from there, since I am very much inexperienced with git and I don't know how it got there in the first place. Should I just make a new pull request instead, with the appropriate commits?

Update: Just found out how to remove the commit using git rebase.

@harudagondi harudagondi force-pushed the allow-iter-combinations-on-queries-with-filters branch from 93bd4d7 to d6052f7 Compare January 13, 2022 16:52
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior and removed S-Needs-Triage This issue needs to be labelled labels Jan 13, 2022
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks for contributing; the fix looks good now and the test is very comprehensive.

@alice-i-cecile alice-i-cecile 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 Jan 13, 2022
@alice-i-cecile alice-i-cecile added this to the Bevy 0.6.1 milestone Jan 15, 2022
crates/bevy_ecs/src/query/mod.rs Outdated Show resolved Hide resolved
@alice-i-cecile
Copy link
Member

BTW @harudagondi, you can press "Resolve conversation" on the comments that have been addressed. It helps keep the thread clean for reviewers :)

@alice-i-cecile alice-i-cecile removed this from the Bevy 0.6.1 milestone Feb 10, 2022
@alice-i-cecile alice-i-cecile added this to the Bevy 0.7 milestone Feb 24, 2022
@cart
Copy link
Member

cart commented Apr 8, 2022

bors r+

bors bot pushed a commit that referenced this pull request Apr 8, 2022
# Objective

- Previously, `iter_combinations()` does not work on queries that have filters.
- Fixes #3651

## Solution

- Derived Copy on all `*Fetch<T>` structs, and manually implemented `Clone` to allow the test to pass (`.count()` does not work on `QueryCombinationIter` when `Clone` is derived)


Co-authored-by: Carter Anderson <mcanders1@gmail.com>
@bors bors bot changed the title Allow iter combinations on queries with filters [Merged by Bors] - Allow iter combinations on queries with filters Apr 8, 2022
@bors bors bot closed this Apr 8, 2022
aevyrie pushed a commit to aevyrie/bevy that referenced this pull request Jun 7, 2022
# Objective

- Previously, `iter_combinations()` does not work on queries that have filters.
- Fixes bevyengine#3651

## Solution

- Derived Copy on all `*Fetch<T>` structs, and manually implemented `Clone` to allow the test to pass (`.count()` does not work on `QueryCombinationIter` when `Clone` is derived)


Co-authored-by: Carter Anderson <mcanders1@gmail.com>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

- Previously, `iter_combinations()` does not work on queries that have filters.
- Fixes bevyengine#3651

## Solution

- Derived Copy on all `*Fetch<T>` structs, and manually implemented `Clone` to allow the test to pass (`.count()` does not work on `QueryCombinationIter` when `Clone` is derived)


Co-authored-by: Carter Anderson <mcanders1@gmail.com>
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-Bug An unexpected or incorrect behavior 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
Development

Successfully merging this pull request may close these issues.

Compiler error when using iter_combinations with filtered query
5 participants