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

Allow the creation of Query directly from the World #3887

Open
alice-i-cecile opened this issue Feb 7, 2022 · 0 comments
Open

Allow the creation of Query directly from the World #3887

alice-i-cecile opened this issue Feb 7, 2022 · 0 comments
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use

Comments

@alice-i-cecile
Copy link
Member

alice-i-cecile commented Feb 7, 2022

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

Querying the world directly always the creation of QueryState, rather than returning a Query that can be worked with directly.

let query_state = world.query::<(&Foo, &mut Bar), With<A>>();
for (foo, bar) in query_state.iter(&world){
  assert!(foo > bar);
}

This is directly relevant to:

  1. Exclusive systems. We could cache this state in a Local, but this is rarely done in practice.
  2. Commands. Under the current commands model, there is nowhere to cache this state.
  3. Integration tests. There is effectively no point caching this state, both because the queries are often not repeated, and because we are not in a performance-constrained setting.

The existing approach is confusing to beginners, heavy on boilerplate and directly exposes end users to QueryState, which should largely be engine-internal.

What solution would you like?

Create two core methods on World, replacing the current World::query and World::query_filtered:

  1. World::query: returns a stateless Query directly from the world. This is used in commands and integration tests.
  2. World::query_state: returns a QueryState. This is used in engine internals, and exclusive systems.

In order to get this to work we need to:

  1. Allow Query to store a &QueryState or a new InternalQueryState value, rather than just a QueryState.
  2. Tweak the initialization methods.
  3. Warn when Added or Changed are used with a InternalQueryState.

The example above becomes the much more direct and familiar:

for (foo, bar) in world.query::<(&Foo, &mut Bar), With<A>>().iter(){
  assert!(foo > bar);
}

What alternative(s) have you considered?

  1. Use and store a QueryState when writing integration tests and custom commands. Very boilerplate heavy and confusing to new users.
  2. Write and use one-shot systems (One-shot systems via Commands for scripting-like logic #2192) for integration testing. Rather boilerplate-heavy, has some serious open questions and virtually all of the same issues around statelessness.
  3. Write helper methods (in the engine, or in end user test code) that wrap this boilerplate. This doesn't save much work, and seriously reduces clarity and directness, especially as the methods proliferate. This was the approach taken in Utility functions for integration testing #3839.

Additional context

Closely related to #3774.

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed C-Feature A new feature, making something new possible labels Feb 7, 2022
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

No branches or pull requests

1 participant