Read-only queries cannot be issued on a read-only &World

[alice-i-cecile]

Problem

Given a &World, I cannot issue a query that only reads from the world.

There are two possible methods on World for this: query, query_filtered, plus SystemState::new(). All of them require &mut World, as they could provide mutating type parameters.

Proposed Solutions

Quick-and-dirty

Make read-only variants of all three methods.

Ergonomic

Add a system_state and read_only_system_state method to World, which generates a SystemState and then immediately calls .get_mut / .get on it.

This allows users to do things like:

let query = world.read_only_system_state::<Query<(&Foo, &Bar), With<Bat>>>();

for (foo, bar) in query.iter(){

}

[alice-i-cecile]

Related to #2192: the system_state method would probably not cache the system state, but would be very ergonomic for working with the World compared to existing alternatives.

In many cases, it is not feasible to require that the users manually handle state caching.

We may be able to cache the system state, and then look it up by type signature later? That optimization should definitely wait until after Stageless though.

[TheRawMeatball]

Personally, I don't think allowing query initialization from &World is a good idea. This encourages losing state which is both a minor perf footgun and a major correctness footgun in case Changed filters are used.

In many cases, it is not feasible to require that the users manually handle state caching.

I'd like to see some examples here, because I personally think it's fairly easy.

Furthermore, this conflicts with other potential plans, namely registration hooks.

[alice-i-cecile]

This encourages losing state which is both a minor perf footgun and a major correctness footgun in case Changed filters are used

Hmm, I do see your point there with Changed filters. However, these can still be constructed and not stored from &mut World.

[TheRawMeatball]

Yes, but a footgun already existing isn't an excuse to make it more prevalent. We should encourage storing this state, not discourage it.

[JoJoJet]

Currently, WorldQuery::init_state requires &mut World because it has to initialize any components it uses. We could achieve read-only initialization by adding a try_init_state(&World) method that returns None if any of the components have not already been initialized.

However, at first glance I think I agree with Nile that this may not be necessary to include. You should be able to do read-only queries by just adding a Local<QueryState<>> system param and letting the engine worry about initializing it. This also has the bonus of caching the state instead of recreating it each time it's used.

[tychedelia]

Here's a use case I'm running into from Nannou: We wrap an UnsafeWorldCell with our Nannou App type, which provides friendly methods for interacting with World via our sketch API. For example, we provide a method that retrieves the Entity for the currently focused window.

A Nannou application is implemented as a series of lifecycle functions, some of which accept &App and some &mut App, i.e. provide only read-only or mutable access to world via the underlying UnsafeWorldCell. Many of our methods are effectively read-only, but need to construct queries in order to do things like count how many open windows there are.

I'm solving this via interior mutability right now, which is fine for us right now, but is still a little annoying.

[sisso]

Personally I find this feature useful if you use bevy ecs internally and expose as traditional api. That is specially useful you have game code that is used by traditional game engines (godot, unity3d, etc) through some wrapper.

Works well as far you can clone/copy your results. As soon you start to return references, the API usage become a mess.

On the example:

struct Api { world: World, .. }

impl Api { 
   fn update(&mut self, delta_time: f32)
   fn get_player(&self) -> &Player;
   fn get_grid(&self) -> &Grid<T>
}

If get_player and get_grid are &mut self, you must take player, clone whatever data you need, drop, them get grid

[tychedelia]

Personally I find this feature useful if you use bevy ecs internally and expose as traditional api.

Yup, this is exactly the same issue we are running into.

[ethereumdegen]

For my usecases, I have found that I would really like (need) something like

let singleton_entity: Option<Entity>  = world.find_singleton::<T: Component>()  ;

which is able to use immutable world to return an entity as long as only a single entity has the component (T).

Then, the following line of code [common usecase] could be like

let component_state = world.get::<T>(singleton_entity)

I need this and I dont think it would be 'illegal' , since you can already do this with a resource. This essentially allows me to use a singleton component like a resource that is tied to an entity.

The fact that I can only query for entities based on components with mut world right now is not enough for me. I dont have access to mut world for what I am doing. [ i want to be able to find (read) those ui nodes ( by singleton component) inside of World access for all of my fancy reactive components (like my component that sets UI visibility based on a custom struct that implements a trait function that takes in World and outputs the visibility state for each frame -- allowing for declarative computation of visibility ) ]

I can understand why world.query needs mut world but if you are expecting to only do a .single() on the query, it for sure should not need mut world but this isnt an option at the moment .

This is a more constrained 'ask' compared to the OP suggestion and so may be safer to implement..? but still useful.

[hymm]

It should be possible to make a version of World::query that takes a &self instead of &mut self now that get_state was added for query transmutes. It would just need to return None or an error if the components aren't initialized.

[alice-i-cecile]

Currently, WorldQuery::init_state requires &mut World because it has to initialize any components it uses

Why is that the case for Query, but not for World::get, or the EntityRef equivalent? Surely this needs to be initialized the same way in both cases.