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 more query join types (left, right, outer join) #13633

Open
alice-i-cecile opened this issue Jun 2, 2024 · 4 comments
Open

Add more query join types (left, right, outer join) #13633

alice-i-cecile opened this issue Jun 2, 2024 · 4 comments
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! X-Contentious There are nontrivial implications that should be thought through

Comments

@alice-i-cecile
Copy link
Member

What problem does this solve or what need does it fill?

The query join added in #11535 is useful, but only covers one of four common types of queries.

As discussed in #1658, these are:

  1. Inner join: matches all entities that are found in both queries.
  2. Outer join: matches all entities that are found in either query.
  3. Left join: matches all entities that are found in the first query.
  4. Right join: matches all entity

The join added in the PR above is an "inner join", which is useful and straightforward to implement.
However, this isn't always the desired behavior.

We should learn from the extensive history of database and implement this foundational functionality.

What solution would you like?

Add the other join type methods, being clear that the base join is an inner join.

Data that is missing should be filled with None, and the fields that could be missing should be converted to their Option equivalents.

What alternative(s) have you considered?

Maybe these join types aren't useful in practice in game dev and can just be omitted.

I had hoped that Query::join could take an extra parameter, the JoinType enum. However, I don't think that this is feasible, as the type returned varies based on the value based in. Perhaps a const generic would work, but that's blocked on rust-lang/rust#95174 for now.

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-ECS Entities, components, systems, and events D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! X-Uncontroversial This work is generally agreed upon labels Jun 2, 2024
@Adamkob12
Copy link
Contributor

Adamkob12 commented Jun 2, 2024

I like the idea of joining queries on component values, e.g:
join (&Enemy, &Transform) and (&Player, &Transform) on distance(enemy.transform, player.transform) < 30.0
which would in turn yield ((&Enemy, &Transform), (&Player, &Transform)).

But I'm not sure on how useful joining queries on entities is. Correct me if I'm wrong, but it seems like even the example given for Query::join is longer, slower and more complicated than what it would be if Query::join wasn't used at all.

Is there an example where joining queries on entities provides some utility that can't be or is much harder to achieve right now?

@alice-i-cecile alice-i-cecile added X-Contentious There are nontrivial implications that should be thought through and removed X-Uncontroversial This work is generally agreed upon labels Jun 2, 2024
@alice-i-cecile
Copy link
Member Author

Much like query transmutes, the value comes in much more complex use cases. Scripting APIs and helper methods are relatively common applications, but it's really hard to show the value in out of context snippets.

@alice-i-cecile
Copy link
Member Author

For outer joins, @ChristopherBiscardi had a nice example where a user can recombine mutually exclusive queries without a ParamSet:

fn burn(
    all_burned: Query<&Burned>,
    all_grass_type: Query<Entity, With<&GrassType>>
    all_non_grass_type: Query<Entity, Without<&GrassType>>
) {
    let critical_burn_effect: QueryLens<&Burned, Entity> = all_burned.join(all_grass_type);
    for (burn, entity) in &critical_burn_effect {
        // dmg = 2.
    }

    let burn_effect: QueryLens<&Burned, Entity> = all_burned.join(all_non_grass_type);
    for (burn, entity) in &burn_effect {
        // dmg = 1
    }
}

@alice-i-cecile
Copy link
Member Author

We could consider using query.union, query.intersection and so on for the naming, and base it off of set operations.

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-Feature A new feature, making something new possible D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

No branches or pull requests

2 participants