mirror of
https://github.com/bevyengine/bevy
synced 2024-09-20 14:32:06 +00:00
399fd23797
# Objective Currently, a query iterator can be collected into a `Vec` and sorted, but this can be quite unwieldy, especially when many `Component`s are involved. The `itertools` crate helps somewhat, but the need to write a closure over all of `QueryData` can sometimes hurt ergonomics, anywhere from slightly to strongly. A key extraction function only partially helps, as `sort_by_key` does not allow returning non-`Copy` data. `sort_by` does not suffer from the `Copy` restriction, but now the user has to write out a `cmp` function over two `QueryData::Item`s when it could have just been handled by the `Ord` impl for the key. `sort` requires the entire `Iterator` Item to be `Ord`, which is rarely usable without manual helper functionality. If the user wants to hide away unused components with a `..` range, they need to track item tuple order across their function. Mutable `QueryData` can also introduce further complexity. Additionally, sometimes users solely include `Component`s /`Entity` to guarantee iteration order. For a user to write a function to abstract away repeated sorts over various `QueryData` types they use would require reaching for the `all_tuples!` macro, and continue tracking tuple order afterwards. Fixes https://github.com/bevyengine/bevy/issues/1470. ## Solution Custom sort methods on `QueryIter`, which take a query lens as a generic argument, like `transmute_lens` in `Query`. This allows users to choose what part of their queries they pass to their sort function calls, serving as a kind of "key extraction function" before the sort call. F.e. allowing users to implement `Ord` for a Component, then call `query.iter().sort::<OrdComponent>()` This works independent of mutability in `QueryData`, `QueryData` tuple order, or the underlying `iter/iter_mut` call. Non-`Copy` components could also be used this way, an internal `Arc<usize>` being an example. If `Ord` impls on components do not suffice, other sort methods can be used. Notably useful when combined with `EntityRef` or `EntityMut`. Another boon from using underlying `transmute` functionality, is that with the [allowed transmutes](http://dev-docs.bevyengine.org/bevy/ecs/prelude/struct.Query.html#allowed-transmutes), it is possible to sort a `Query` with `Entity` even if it wasn't included in the original `Query`. The additional generic parameter on the methods other than `sort` and `sort_unstable` currently cannot be removed due to Rust limitations, however their types can be inferred. The new methods do not conflict with the `itertools` sort methods, as those use the "sorted" prefix. This is implemented barely touching existing code. That change to existing code being that `QueryIter` now holds on to the reference to `UnsafeWorldCell` that is used to initialize it. A lens query is constructed with `Entity` attached at the end, sorted, and turned into an iterator. The iterator maps away the lens query, leaving only an iterator of `Entity`, which is used by `QuerySortedIter` to retrieve the actual items. `QuerySortedIter` resembles a combination of `QueryManyIter` and `QueryIter`, but it uses an entity list that is guaranteed to contain unique entities, and implements `ExactSizeIterator`, `DoubleEndedIterator`, `FusedIterator` regardless of mutability or filter kind (archetypal/non-archetypal). The sort methods are not allowed to be called after `next`, and will panic otherwise. This is checked using `QueryIterationCursor` state, which is unique on initialization. Empty queries are an exception to this, as they do not return any item in the first place. That is because tracking how many iterations have already passed would require regressing either normal query iteration a slight bit, or sorted iteration by a lot. Besides, that would not be the intended use of these methods. ## Testing To ensure that `next` being called before `sort` results in a panic, I added some tests. I also test that empty `QueryIter`s do not exhibit this restriction. The query sorts test checks for equivalence to the underlying sorts. This change requires that `Query<(Entity, Entity)>` remains legal, if that is not already guaranteed, which is also ensured by the aforementioned test. ## Next Steps Implement the set of sort methods for `QueryManyIter` as well. - This will mostly work the same, other than needing to return a new `QuerySortedManyIter` to account for iteration over lists of entities that are not guaranteed to be unique. This new query iterator will need a bit of internal restructuring to allow for double-ended mutable iteration, while not regressing read-only iteration. The implementations for each pair of - `sort`, `sort_unstable`, - `sort_by`, sort_unstable_by, - `sort_by_key,` `sort_by_cached_key` are the same aside from the panic message and the sort call, so they could be merged with an inner function. That would require the use of higher-ranked trait bounds on `WorldQuery::Item<'1>`, and is unclear to me whether it is currently doable. Iteration in QuerySortedIter might have space for improvement. When sorting by `Entity`, an `(Entity, Entity)` lens `QueryData` is constructed, is that worth remedying? When table sorts are implemented, a fast path could be introduced to these sort methods. ## Future Possibilities Implementing `Ord` for EntityLocation might be useful. Some papercuts in ergonomics can be improved by future Rust features: - The additional generic parameter aside from the query lens can be removed once this feature is stable: `Fn -> impl Trait` (`impl Trait` in `Fn` trait return position) - With type parameter defaults, the query lens generic can be defaulted to `QueryData::Item`, allowing the sort methods to look and behave like `slice::sort` when no query lens is specified. - With TAIT, the iterator generic on `QuerySortedIter` and thus the huge visible `impl Iterator` type in the sort function signatures can be removed. - With specialization, the bound on `L` could be relaxed to `QueryData` when the underlying iterator is mutable. ## Changelog Added `sort`, `sort_unstable`, `sort_by`, `sort_unstable_by`, `sort_by_key`, `sort_by_cached_key` to `QueryIter`. |
||
|---|---|---|
| .. | ||
| bevy_a11y | ||
| bevy_animation | ||
| bevy_app | ||
| bevy_asset | ||
| bevy_audio | ||
| bevy_color | ||
| bevy_core | ||
| bevy_core_pipeline | ||
| bevy_derive | ||
| bevy_dev_tools | ||
| bevy_diagnostic | ||
| bevy_dylib | ||
| bevy_dynamic_plugin | ||
| bevy_ecs | ||
| bevy_encase_derive | ||
| bevy_gilrs | ||
| bevy_gizmos | ||
| bevy_gltf | ||
| bevy_hierarchy | ||
| bevy_input | ||
| bevy_internal | ||
| bevy_log | ||
| bevy_macro_utils | ||
| bevy_math | ||
| bevy_mikktspace | ||
| bevy_pbr | ||
| bevy_ptr | ||
| bevy_reflect | ||
| bevy_render | ||
| bevy_scene | ||
| bevy_sprite | ||
| bevy_state | ||
| bevy_tasks | ||
| bevy_text | ||
| bevy_time | ||
| bevy_transform | ||
| bevy_ui | ||
| bevy_utils | ||
| bevy_window | ||
| bevy_winit | ||