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

Add methods for joining queries with lists. #5075

Closed
wants to merge 7 commits into from

Conversation

tim-blackbird
Copy link
Contributor

@tim-blackbird tim-blackbird commented Jun 22, 2022

Objective

This is similar in nature to #4879.

Add new new methods iter_join_map(_mut) to reduce the need to use Query::get.

These methods, compared to Query::get:

  1. Have reduced overhead.
  2. Do not require a match; Reduced rightward drift.
  3. Improved ergonomics (for the immutable variants, at least).

The name iter_join_map was chosen as the operation resembles an inner join on Entity between a Query and an arbitrary list that is mapped to Entity via the supplied fn.

Examples

#[derive(Component)]
struct Health {
    value: f32
}

#[derive(Component)]
struct DamageEvent {
    target: Entity,
    damage: f32,
}

fn system(
    mut damage_events: EventReader<DamageEvent>,
    health_query: Query<&Health>,
) {
    for (health, event) in
        health_query.iter_join_map(damage_events.iter(), |event| event.target)
    {
        println!("Entity has {} health and will take {} damage!", health.value, event.damage);
    }
}

fn system(
    mut damage_events: EventReader<DamageEvent>,
    mut health_query: Query<&mut Health>,
) {
    let mut join = health_query.iter_join_map_mut(damage_events.iter(), |event| event.target);
    while let Some((mut health, event)) = join.fetch_next() {
        health.value -= event.damage;
    }
}
The same systems without this PR.
fn system(
    child_query: Query<(&Parent, &Health)>,
    parent_query: Query<&Health>,
) {
    for event in damage_events.iter() {
        if let Ok(health) = health_query.get(event.target) {
            println!("Entity has {} health and will take {} damage!", health.value, event.damage);
        }
    }
}

fn system(
    mut damage_events: EventReader<DamageEvent>,
    mut health_query: Query<&mut Health>,
) {
    for event in damage_events.iter() {
        if let Ok(mut health) = health_query.get_mut(event.target) {
            health.value -= event.damage;
        }
    }
}
More examples.
fn system(
    child_query: Query<(&Parent, &Health)>,
    parent_query: Query<&Health>,
) {
    for (parent_health, (_, health)) in
        parent_query.iter_join_map(&child_query, |(parent, _)| parent.0)
    {
        println!(
            "This entity's health is {}, and its parent's health is {}.",
            health.0, parent_health.0
        );
    }
}

fn system(parent_query: Query<(&Children, &Health)>, child_query: Query<&Health>) {
    let parent_iter = parent_query
        .iter()
        .flat_map(|(children, health)| children.iter().map(move |c| (*c, health)));
    
    for (child_health, (_, parent_health)) in
        child_query.iter_join_map(parent_iter, |(parent, _)| *parent)
    {
        println!(
            "This child entity's health is {}, and its parent's health is {}.",
            child_health.0, parent_health.0
        );
    }
}

If we had lending iterators :'( the mutable variant would simply look like this:

for (mut health, event) in
    health_query.iter_join_map_mut(damage_events.iter(), |event| event.target)
{
    health.value -= event.damage;
}

Note for reviewers

You may notice that QueryJoinMapIter and QueryManyIter are practically identical.
I tried to re-use QueryJoinMapIter with a hard-coded closure for map_f for iter_many, but in the return type it needs a concrete type for MapFn.
It seems to be impossible to use a function as a concrete type.

Follow up

  • It would be easy to add a right join as well. Outer join and left join will be trickier.
  • Add benchmarks for iter_many and iter_join_map.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Jun 22, 2022
@alice-i-cecile
Copy link
Member

alice-i-cecile commented Jun 22, 2022

Does this fully solve #1658?

@tim-blackbird
Copy link
Contributor Author

Does this fully solve #1658?

Not quite, #1658 mentions a join of two queries that produces a new Query which I do think is possible, but not for me haha.
I do think this PR makes iterating over 'relations' nicer, so it improves the situation a bit.

@BoxyUwU BoxyUwU self-requested a review July 1, 2022 16:16
@BoxyUwU BoxyUwU added the P-Unsound A bug that results in undefined compiler behavior label Jul 1, 2022
@tim-blackbird tim-blackbird marked this pull request as draft July 3, 2022 18:29
@tim-blackbird
Copy link
Contributor Author

Converting to draft because I'd like to base this on #5170 which will fix the unsoundness instead.
I'll rework this and split out the changes to iter_many.

@alice-i-cecile alice-i-cecile added the S-Blocked This cannot move forward until something else changes label Jul 3, 2022
@tim-blackbird tim-blackbird marked this pull request as ready for review August 3, 2022 13:19
tim-blackbird and others added 2 commits September 2, 2022 12:25
Co-authored-by: Tristan Guichaoua <33934311+tguichaoua@users.noreply.github.com>
@BoxyUwU BoxyUwU removed S-Blocked This cannot move forward until something else changes P-Unsound A bug that results in undefined compiler behavior labels Sep 2, 2022
@tim-blackbird tim-blackbird deleted the query-iter-join branch September 14, 2023 13:00
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.

4 participants