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

QueryCombinationIter should not implement ExactSizeIterator #5846

Closed
nicopap opened this issue Sep 1, 2022 · 1 comment
Closed

QueryCombinationIter should not implement ExactSizeIterator #5846

nicopap opened this issue Sep 1, 2022 · 1 comment
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior C-Code-Quality A section of code that is hard to understand or change D-Trivial Nice and easy! A great choice to get started with Bevy M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide

Comments

@nicopap
Copy link
Contributor

nicopap commented Sep 1, 2022

Bevy version

QueryCombinationIter can very easily have a length greater than usize::MAX, therefore shouldn't implement ExactSizeIterator.

Know length iterators with potential greater-than-usize::MAX length in std::iter and itertools do not implement ExactSizeIterator, because the value of len cannot be accurate if the length exceeds usize. (iter::Repeat, iter::Intersperse, itertools::Product, itertools::Combinations do not implement ExactSizeIterator)

@nicopap nicopap added C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Sep 1, 2022
@alice-i-cecile alice-i-cecile added the D-Trivial Nice and easy! A great choice to get started with Bevy label Sep 1, 2022
@ottah
Copy link
Contributor

ottah commented Sep 6, 2022

I've implemented this fix in #5895. I'm new to rust/bevy so any help with improving the PR will be appreciated!

@bors bors bot closed this as completed in 45e5eb1 Oct 24, 2022
james7132 pushed a commit to james7132/bevy that referenced this issue Oct 28, 2022
# Objective

- `QueryCombinationIter` can have sizes greater than `usize::MAX`.
- Fixes bevyengine#5846 

## Solution

- Only the implementation of `ExactSizeIterator` has been removed. Instead of using `query_combination.len()`, you can use `query_combination.size_hint().0` to get the same value as before.

---

## Migration Guide

- Switch to using other methods of getting the length.
Pietrek14 pushed a commit to Pietrek14/bevy that referenced this issue Dec 17, 2022
# Objective

- `QueryCombinationIter` can have sizes greater than `usize::MAX`.
- Fixes bevyengine#5846 

## Solution

- Only the implementation of `ExactSizeIterator` has been removed. Instead of using `query_combination.len()`, you can use `query_combination.size_hint().0` to get the same value as before.

---

## Migration Guide

- Switch to using other methods of getting the length.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this issue Feb 1, 2023
# Objective

- `QueryCombinationIter` can have sizes greater than `usize::MAX`.
- Fixes bevyengine#5846 

## Solution

- Only the implementation of `ExactSizeIterator` has been removed. Instead of using `query_combination.len()`, you can use `query_combination.size_hint().0` to get the same value as before.

---

## Migration Guide

- Switch to using other methods of getting the length.
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 C-Code-Quality A section of code that is hard to understand or change D-Trivial Nice and easy! A great choice to get started with Bevy M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants