Disjoint QueryData access

[vil-mo]

Objective

Fixes #14518

Solution

Added QueryData implementer similarly to how ParamSet is implemented

Testing

2 doc tests and 3 tests that are adapted forms of the same tests for ParamSet

Showcase

Adds DataSet that imlements QueryData and allows disjoint access to it's members, similarly to how ParamSet works.
Mainly would be useful with in generic contexts where you need QueryData, but can't guarantee it won't conflict with other QueryData. Or with complex custom QueryData implementers that conflict with each other.
This will compile even if A and B conflict:

fn iterate_generic_data<A: QueryData, B: QueryData>(mut query: Query<DataSet<(A, B)>>) {
    for mut set in query.iter_mut() {
        let a = set.d0();
        let b = set.d1();
    }
}

[chescock]

Suggested change

	// SAFETY: deferes to soundness of `#data: WorldQuery` impl
	// SAFETY: defers to soundness of `#data: WorldQuery` impl

[chescock]

The current_access is separate for each #data, right? That might be more clear if you use a separate variable for each one instead of sharing the same variable.

Suggested change

	let mut current_access;
	#(
	// Updating empty [`FilteredAccess`] and then extending passed access.
	// This is done to avoid conflicts with other members of the set.
	current_access = FilteredAccess::default();
	#(
	// Updating empty [`FilteredAccess`] and then extending passed access.
	// This is done to avoid conflicts with other members of the set.
	let mut current_access = FilteredAccess::default();

[alice-i-cecile]

@chescock @hymm can I get your opinions on this?

[chescock]

It's definitely more niche than ParamSet, but I don't see a simpler way to solve the use case in the linked issue. There could theoretically be cases with overlapping #[derive(QueryData)] queries where it could help, as well.

I believe it was sound when it was written, since it ensures only one subquery is alive at a time and declares all the access from each.

It will need an update in light of #16843, though. It's now valid for a QueryData to request mutable access to a resource in init_fetch, and it would be unsound if multiple subqueries did that. I think that can be resolved by adding the resource access from each subquery to the main access before checking the next one.

Also, this uses a proc macro, which was consistent with ParamSet when written, but #16847 changed that to be a macro_rules macro. It would be nice to change this to macro_rules as well, but that doesn't need to block it.

[hymm]

If we had real variadics, I would say just merge this. As it is though this adds a fair amount of codegen for a niche feature, so I'm a little hesitant. This is probably only really useful in generic contexts. Overall I do lean towards merging this, since the alternatives are a lot more awkward to write.

We could limit this to 4 items like ParamSet was originally until a user asks for more.

[hymm]

Could you add a should_panic test with the Query<DataSet> as the first system parameter too. The conflict checking can be order dependent.

[hymm]

Could you add a should_panic test with the conflict in the second item of the DataSet?

[hymm]

The docs should make it clear that the query will fetch entitieis that match the union of all the QueryData's in the DataSet.

[alice-i-cecile]

I'm a little reluctant to merge this, given the level of code gen for a niche feature and the absolute headache that ParamSet has been. I'm open to being convinced however.