Implement Inspectable for HashMap

[Luminoth]

Would it be reasonable to have Inspectable implemented for HashMaps where the key and value are both inspectable, similar to how it's done for Vec and Tuples?

[jakobhellermann]

Yes, that would be reasonable. The code should be similar to to the Vec impl if you want to try implementing it.

[Luminoth]

Do you have any guidance on how to do the UI layout for it? That's the one decision point that I think will be the biggest sticking point for me.

[Luminoth]

This is where I'm at with it currently:

impl<K, V> Inspectable for HashMap<K, V>
where
    K: Inspectable + Clone + Eq + Hash + Default,
    V: Inspectable + Default,
{
    type Attributes = (
        <K as Inspectable>::Attributes,
        <V as Inspectable>::Attributes,
    );

    fn ui(&mut self, ui: &mut egui::Ui, options: Self::Attributes, context: &mut Context) -> bool {
        let mut changed = false;

        ui.vertical(|ui| {
            let mut to_delete = None;

            let len = self.len();
            for (i, (key, val)) in self.iter_mut().enumerate() {
                ui.horizontal(|ui| {
                    if utils::ui::label_button(ui, "✖", egui::Color32::RED) {
                        to_delete = Some(key.clone());
                    }
                    // TODO: this can't be mutable
                    //k.ui(ui, options.0.clone(), &mut context.with_id(i as u64));
                    // TODO: separator?
                    changed |= val.ui(ui, options.1.clone(), &mut context.with_id(i as u64));
                });

                if i != len - 1 {
                    ui.separator();
                }
            }

            ui.vertical_centered_justified(|ui| {
                if ui.button("+").clicked() {
                    // TODO: this isn't great because the key is always getting overwritten
                    // and we have no way of modifying it
                    self.insert(K::default(), V::default());
                    changed = true;
                }
            });

            if let Some(key) = to_delete {
                self.remove(&key);
                changed = true;
            }
        });

        changed
    }

    fn setup(app: &mut App) {
        K::setup(app);
        V::setup(app);
    }
}

The TODOs are the UI bit that I'm not entirely certain how best to lay out for this. Outside of that I think everything else is right.

[jakobhellermann]

Oh right, the keys aren't mutable when iterating.

You could do something similar to the to_delete field, where you record changes while iterating and actually perform them after the iter_mut loop.

Something like

let keys_to_be_changed = Vec::new();

for (key, val) in self.iter_mut() {
  let mut new_key = key.clone();
  if (new_key.ui(ui, ..) {
    keys_to_be_changed.push((key.clone(), new_key));
  }
  ...
}

for (old_key, new_key) in keys_to_be_changed {
  let val = self.remove(old_key).unwrap();
  self.insert(new_key, val);
}

should work. It's a bit heavy on clones, but since it's just for a debug view, I can't think of a better way right now and it is better than not supporting hashmaps at all, I think it is fine.

[jakobhellermann]

For the UI I agree that just sticking key and value in a ui.horizontal seems like a good option.

[jakobhellermann]

implemented in https://github.com/jakobhellermann/bevy-inspector-egui/pull/72