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 any_for_filter to built-in run conditions #17326

Closed
Jaso333 opened this issue Jan 12, 2025 · 0 comments · Fixed by #17327
Closed

Add any_for_filter to built-in run conditions #17326

Jaso333 opened this issue Jan 12, 2025 · 0 comments · Fixed by #17327
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!

Comments

@Jaso333
Copy link
Contributor

Jaso333 commented Jan 12, 2025

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

Currently, the only built-in run condition like this is any_with_component. However, this very quickly loses value on the systems that depend on a combination of components including the provided component type. Compounding any_with_component doesn't address this as it doesn't look at archetypes like a filter does.

Take the following system for example:

fn update(mut enemies: Query<(&EnemyMovement, &mut Transform)>, time: Res<Time>) {
    for (movement, mut transform) in enemies.iter_mut() {
        transform.translation.y -= time.delta_secs() * movement.speed;
    }
}

This is a very basic system, but the built-in run conditions do not provide means to limit the scheduling of this system. If I were to use any_with_component::<EnemyMovement>, this system will still run if there are EnemyMovement components that exist without Transform, and therefore time would be wasted scheduling this system for execution in that frame. Instead, I've used the suggested condition:

app.add_systems(
  Update,
  update.run_if(any_for_filter::<(With<EnemyMovement>, With<Transform>)>),
);

What solution would you like?

I'm using this in every project in order to appropriately limit my scheduled systems:

pub fn any_for_filter<F: QueryFilter>(entities: Query<(), F>) -> bool {
    !entities.is_empty()
}

What alternative(s) have you considered?

No alternatives.

Additional context

N/A

@Jaso333 Jaso333 added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Jan 12, 2025
@BenjaminBrienen BenjaminBrienen added A-ECS Entities, components, systems, and events S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes and removed S-Needs-Triage This issue needs to be labelled labels Jan 12, 2025
@BenjaminBrienen BenjaminBrienen added D-Straightforward Simple bug fixes and API improvements, docs, test and examples and removed D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes labels Jan 12, 2025
github-merge-queue bot pushed a commit that referenced this issue Jan 14, 2025
# Objective

resolves #17326.

## Solution

Simply added the suggested run condition.

## Testing

A self-explanatory run condition. Fully verified by the operation of
`QueryFilter` in a system.
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-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants