Deprecate FilteredResources and similair structs - #25331
Conversation
chescock
left a comment
There was a problem hiding this comment.
Yup, this makes sense! If we'd had resources-as-components two years ago then I would not have added FilteredResources in the first place, so it seems appropriate to remove it now that it's no longer necessary.
I left some notes on the migration guide, but I suspect anyone using FilteredResources is also using FilteredEntityRef and will understand how to migrate.
| fn resource_system(query: Query<()>) { | ||
| let resource_a: Ref<ResA> = query.single().unwrap(); |
There was a problem hiding this comment.
This doesn't compile, does it? I think you want Query<FilteredEntityRef> and then to use get<R> or get_by_id to get the resource back out.
| fn resource_system(query: Query<()>) { | |
| let resource_a: Ref<ResA> = query.single().unwrap(); | |
| fn resource_system(query: Query<FilteredEntityRef>) { | |
| let entity: FilteredEntityRef = query.single().unwrap(); // Or use `Single<FilteredEntityRef>` as a parameter! | |
| let resource_a: &A = entity.get::<A>().unwrap(); | |
| // Or with change tracking | |
| let resource_a: Ref<A> = entity.get_ref::<A>().unwrap(); | |
| // Or by ID | |
| let resource: Ptr = entity.get_by_id(component_id).unwrap(); | |
| let change_ticks: ComponentTicks = entity.get_change_ticks_by_id(component_id).unwrap(); |
| .build_system(resource_system); | ||
|
|
||
| fn resource_system(query: Query<()>) { | ||
| let resource_a: Ref<ResA> = query.single().unwrap(); |
There was a problem hiding this comment.
It might be worth including some guidance for uses of FilteredResource that read multiple resources. I think the simplest advice would be to create a separate Query for each resource. Users that need a variable number could use Vec<Query<FilteredEntityMut>>. (And the motivating case for this was a script runner that would already have such a Vec as a parameter for its other queries.)
Another option if we merge your components-as-entities PR would be to create a single query for resources that has all of the access and is accessed with query.get(entity)?.get_by_id(entity)?.
Or, hmm, in the meantime, would it make sense to impl SystemParam for &ResourceEntities, like we do for things like &Archetypes? Then users could do query.get(resource_entities.get(component_id)?)?.get_by_id(component_id)? if they need.
|
|
||
| So instead of a `FilteredResourcesParamBuilder` that provides a `FilteredResourcesBuilder`, which resolves to `FilteredResources`, we have a `QueryParamBuilder` that provides a `QueryBuilder` that resolves to a `Query`. The `Mut` variants also turn into `Query`, `QueryParam`, and `QueryParamBuilder`. | ||
| Most of the migration should be rather straightforward, but there are some specifics we need to clear up. | ||
| First, change detection was automatically included for `FilteredResources` and `FilteredResourcesMut`, which is now opt-in. You have to specify `Ref` and `Mut` in `QueryBuilder::data` if you want change detection. |
There was a problem hiding this comment.
Nope, you can use & and &mut! (And I'd consider them more idiomatic.) Access to change ticks is always included with access to the data.
Objective
While working on resources as entities (#22915), I've had to repeatedly make changes to the filtered resources structs. In conversation with @chescock, we found that all of the functionality of these structs can now be handled by
QueryBuilderandQueryParamBuilder.Solution
Deprecate
FilteredResources,FilteredResourcesMut,FilteredResourcesBuilder,FilteredResourcesMutBuilder,FilteredResourcesParamBuilder, andFilteredResourcesMutParamBuilderin favour of genericComponentmethods.Once removed, this should save about ~1k lines of code and some complexity.
Testing
All existing tests pass.
TODO
This will need both a very good migration guide, and documentation (where?) that makes users aware that they can do this with
QueryBuilderandQueryParamBuilder.