Make QueryState::transmute&co validate the world of the &Components used

[SkiFire13]

Objective

• Fix Query::transmute/Query::transmute_filtered accept any &Components parameter and this is unsound #14629

Solution

• Make QueryState::transmute, QueryState::transmute_filtered, QueryState::join and QueryState::join_filtered take a impl Into<UnsafeWorldCell> instead of a &Components and validate their WorldId

Migration Guide

• QueryState::transmute, QueryState::transmute_filtered, QueryState::join and QueryState::join_filtered now take a impl Into<UnsafeWorldCell> instead of a &Components

[alice-i-cecile]

Nice clean design, good docs and a regression test! Can't ask for more.

[cart]

I think this design does work nicely, but I can't help but think that metadata() is just an arbitrary (and redundant) slice of UnsafeWorldCell. Rather than introduce a new name / concept, I'm thinking we should just add the missing method to UnsafeWorldCell and use that instead.

As a proposal, I've pushed that change so we can discuss. Somewhat controversially, it includes From<&World> for UnsafeWorldCell and changed the parameter in the relevant functions to accept Into<UnsafeWorldCell>, which allows us to just pass &World into them. Im pretty sure not including From<&World> impls for UnsafeWorldCell was an intentional design decision to make the conversion explicit, but given that it is completely safe to create an UnsafeWorldCell, and given that it is increasingly used in our APIs, I think adding this impl is both acceptable and desirable.

[alice-i-cecile]

given that it is completely safe to create an UnsafeWorldCell, and given that it is increasingly used in our APIs, I think adding this impl is both acceptable and desirable.

Agreed here.

Rather than introduce a new name / concept, I'm thinking we should just add the missing method to UnsafeWorldCell and use that instead.

I think this is a reasonable proposal: it's nice to keep things simple.

[SkiFire13]

My initial motivation for making it a separate struct was that I wanted it to be as easy to obtain and use as &Components, &Entities and similar. On the other hand UnsafeWorldCell sounds scary and doesn't implement SystemParam either, making it more difficult to obtain (you don't always have a &World available). In hindsight however this is required only when interfacing with QueryState (Query will use its internal UnsafeWorldCell), so maybe this isn't really needed?

[alice-i-cecile]

I like this new diff better :) @SkiFire13 if you're on board with this new plan / diff, let me know and I'll count it as a second approval.

[SkiFire13]

I like this new diff better :) @SkiFire13 if you're on board with this new plan / diff, let me know and I'll count it as a second approval.

Looks good to me.

[SkiFire13]

Updated the first comment and title to reflect cart's changes