Add a method to order the query by specifing an order of entities

[SarthakSingh31]

Objective

Fixes #14393

Solution

Add a way to easily specify the order in which you want to get the items from a query iterator by specifying the order of entities it should fetch the items in.

Showcase

use rand::{seq::SliceRandom, thread_rng};

fn system_1(query: Query<&PartValue>) {
    let mut rng = thread_rng();
    let part_values: Vec<&PartValue> = query
        .iter()
        .with_order_of_entities::<(),_>(|items| {
            items.shuffle(&mut rng);
            items.into_iter().map(|item| item.1)
        })
        .collect();
}

[Victoronz]

Do you have more use cases that would benefit from this API in mind?

I struggle to think of anything that would not be covered by the current sorted_by_cached_key method + general iterator architecture.
See the implementation of #14407 for how the sorted_by_cached_key approach would work for randomization.
Not exposing the internal entity list to the user means we do not need to validate after, which is more performant and simpler for the user.
Thus, if a reorder can be expressed with sorted_by_cached_key, then use of with_order_of_entities should be discouraged.

The current implementation of QuerySortedIter is in essence a special case of QueryManyIter, where the entity list is the same set of entities as queried by QueryIter but in a different order. This means not only does every entity have to be present in the original QueryIter, but also that there can be no removals or duplications.
Each combination of these properties should be its own iterator type (some of which already exist, some of which might be added in the future).

This also means that each with_order_of_entities method for the different query iterators would have to manually check each of these properties the iterator wants to guarantee, worsening the performance and user complexity penalty.

(In the current commit, this new method checks whether the reordered entity list is a subset of the original, but neither checks for duplicates, or the "no removals" property)

[SarthakSingh31]

doesn't randomize as implemented in #14407 always return the items in the same order?

After thinking through a bit, I have to agree that sorted_by_cached_key does provide a very powerful way to specify order of elements. If I did want to sort them randomly I could do query.iter().sorted_by_cached_key(|_| rng.gen::<usize>()). The only downside is that the time complexity is O(nlogn) when we can get O(n).

We could add a randomize based of with_order_of_entities_unchecked that runs in O(n) and takes in a impl Rng. Then we could remove the with_order_of_entities.

do you think there is any value in adding with_order_of_entities_unchecked to ease the implementation of the other sort/order methods? Otherwise I will close this pull request. We can make with_order_of_entities_unchecked a private helper if its useful.

I do think it is a little unintuitive to use sorted_by_cached_key to get items in a random order. Maybe we should add an example or something.

[Victoronz]

doesn't randomize as implemented in #14407 always return the items in the same order?

What #14407 did was use the same machinery that hashers use to create different hashes, RandomState. Each RandomState::new() call is seeded randomly (Look at the implementation to see how, it uses random sources underlying user machines can provide). This was then used to create a hasher, which hashed each entity to create a random distribution.

After thinking through a bit, I have to agree that sorted_by_cached_key does provide a very powerful way to specify order of elements. If I did want to sort them randomly I could do query.iter().sorted_by_cached_key(|_| rng.gen::<usize>()). The only downside is that the time complexity is O(nlogn) when we can get O(n).

We could add a randomize based of with_order_of_entities_unchecked that runs in O(n) and takes in a impl Rng. Then we could remove the with_order_of_entities.

What "random" should mean depends heavily on the use case, and different use cases have different trade-offs.
See Alice's comment on #14407 to see the reasoning for not adding this kind of convenience method.
I'd imagine this is similar to the reason why there aren't any randomize-like methods for std-lib collections and iterators.

do you think there is any value in adding with_order_of_entities_unchecked to ease the implementation of the other sort/order methods? Otherwise I will close this pull request. We can make with_order_of_entities_unchecked a private helper if its useful.

While there is cleanup that could be done around here, I don't think this approach is the way to go.
I am also still actively working in this area, and do intend to do some cleanup when I have a better idea of how the query iterator APIs shake out.

I do think it is a little unintuitive to use sorted_by_cached_key to get items in a random order. Maybe we should add an example or something.

Same conclusion we came to in #14407! There are a few non-obvious uses for these sorts I know of, and I'd like to record them in an example.