Query::get_many should not check for duplicates

[chescock]

Objective

Restore the behavior of Query::get_many prior to #15858.

When passed duplicate Entitys, get_many is supposed to return results for all of them, since read-only queries don't alias. However, #15858 merged the implementation with get_many_mut and caused it to return QueryEntityError::AliasedMutability.

Solution

Introduce a new Query::get_many_readonly method that consumes the Query like get_many_inner, but that is constrained to D: ReadOnlyQueryData so that it can skip the aliasing check. Implement Query::get_many in terms of that new method. Add a test, and a comment explaining why it doesn't match the pattern of the other &self methods.

[Carter0]

Code looks good and I like the test! Strange functionality though. Why would anyone want this?

[chescock]

Why would anyone want this?

I think the advantage of get_many over iter_many is that you can destructure an array with pattern matching, which you can't do with iterators. So you can write things like

if let Ok([player_data, enemy_data]) = query.get_many([player_id, enemy_id])

[Carter0]

Why would anyone want this?

I think the advantage of get_many over iter_many is that you can destructure an array with pattern matching, which you can't do with iterators. So you can write things like

if let Ok([player_data, enemy_data]) = query.get_many([player_id, enemy_id])

Yeah, but wouldn't you always want to check for duplicates? Like having ([player_id, player_id]) would be weird even if you can't alter the player data cause it's read-only.

[chescock]

Yeah, but wouldn't you always want to check for duplicates? Like having ([player_id, player_id]) would be weird even if you can't alter the player data cause it's read-only.

I think the goal there is just performance. Doing the check isn't free. We need it for mutable data for soundness, but for read-only data we can skip it.

But I haven't actually used this method myself, so I may not be the best person to justify it :).

[Carter0]

Yeah, but wouldn't you always want to check for duplicates? Like having ([player_id, player_id]) would be weird even if you can't alter the player data cause it's read-only.

I think the goal there is just performance. Doing the check isn't free. We need it for mutable data for soundness, but for read-only data we can skip it.

But I haven't actually used this method myself, so I may not be the best person to justify it :).

yoki doki makes sense