Add get_at_mut to Map trait

[PROMETHIA-27]

What problem does this solve or what need does it fill?

Currently the solution to getting a mutable reference to the key/value at an index of a map is to get the key and value immutably, clone the key, then use that to index the map. This is unnecessarily inefficient and could easily be avoided.

What solution would you like?

A get_at_mut method that works like get_at, but returns mutable references.

I might PR this myself later if I remember.

[MrGVSV]

I assume this is for ordered maps? In which case, I think it makes sense to add these kinds of methods, but with a disclaimer that not all maps guarantee a particular order.

[PROMETHIA-27]

Technically this is for any map actually; which makes me realize we need .iter_mut() as well to do what I'm doing properly, as I'm just relying on unordered maps being stable.

[MrGVSV]

Oops I completely missed the fact that we already have Map::get_at (even though you mention it directly in the description).

[TheBlek]

Hello! I want to tackle this problem. I'm new here, so any help would be really appreciated @MrGVSV.

Here's what I've been able to gather so far:

• There seems to be only two implementers of this trait in bevy: utils::hashbrown::HashMap (actually implemented through a macro) and reflect::DynamicMap
• While it is possible to implement (&mut key, &mut value) for DynamicMap, however HashMap seems to provide only (& key, &mut value) iterator to use for an implementation analogous to get_at

So my question is:
Does Map::get_mut_at really need to provide mutable reference to a key? (I wonder if changing a key through such a reference could incur invalidation of HashMap state)

[MrGVSV]

Hello! I want to tackle this problem. I'm new here, so any help would be really appreciated @MrGVSV.

Awesome! Yeah feel free to leave questions in #reflection-dev on Discord if you have any. Always great to have new contributors!

Does Map::get_mut_at really need to provide mutable reference to a key? (I wonder if changing a key through such a reference could incur invalidation of HashMap state)

Good question! I don't think we should make the key mutable. I agree that that makes things a lot more complicated. On top of that, HashMap::iter_mut like you mention only provides (&K, &mut V). So we can use that as an indicator that the key should probably be immutable.

[djeedai]

Mutable key on a hash map is plain wrong. HashMap is correct here.