lucasmerlin / hello_egui

A collection of useful crates for egui
https://lucasmerlin.github.io/hello_egui/
MIT License
272 stars 24 forks source link

[egui_dnd] Two elements with same content create conflicts #25

Closed SpeckyYT closed 2 months ago

SpeckyYT commented 3 months ago

I was implementing egui_dnd for a hotkey system which requires changing the order for precedence.

Here's what it looks like when I don't do anything: image

Here's what it looks like when I try to move one of the two elements with same content: image

Also, small unrelated question, is it possible to remove an element with egui_dnd?

lucasmerlin commented 3 months ago

Do you have three separate dnd areas? Are they sharing the same ID?

is it possible to remove an element with egui_dnd?

Do you mean by dragging it somewhere outside or on a delete area or something like that? Not possible right now, but that would be a nice addition.

SpeckyYT commented 3 months ago

Do you have three separate dnd areas? Are they sharing the same ID?

Sorry for not explaining it well enough. There is only 1 dnd area, where every line uses ui.horizontal() and represents a separate value. I do not assign any IDs by myself in code.

The following is the code I wrote:

// Definitions:
// self.quick_keys == Vec<(egui::Key, usize, usize)> => (key, from, to)

Dnd::new(ui, "dnd_quick_keys").show_vec(&mut self.quick_keys, |ui, (key, from, to), handle, _state| {
    ui.horizontal(|ui| {
        handle.ui(ui, |ui| {
            key_input(ui, key); // uses approx `ui.text_edit_singleline(&mut key.to_string())`
            integer_input(ui, from); // uses `DragValue::new(from)`
            integer_input(ui, to); // uses `DragValue::new(to)`
        });
        if ui.button("-").clicked() {
            // self.quick_keys.remove(state.index);
        }
    });
});

I tried to use ui.push_id((state.index)), but nothing changes and it still creates conflicts.

Dnd::new(ui, "dnd_quick_keys").show_vec(&mut self.quick_keys, |ui, (key, from, to), handle, _state| {
    ui.push_id((*key, *from, *to, _state.index), |ui| {
      // ui horizontal, handle ui, etc
    });
});

I also tried to change the content entirely depending if you drag it or not, but still nothing, still conflicts

if state.dragged {
    ui.label(format!("{:?}", key));
} else {
    key_input(ui, key);
    integer_input(ui, from);
    integer_input(ui, to);
    if ui.button("-").clicked() {
        // self.quick_keys.remove(state.index);
    }
}

I also tried to remove the ui.horizontal entirely, but no, still conflicts.

Not sure if I'm doing something wrong.

Here's the video of what happens when two elements are the same:

https://github.com/lucasmerlin/hello_egui/assets/40666126/5bc683b3-ef35-4c94-9163-7c975174c0ec

And here's what happens if they aren't equal anymore:

https://github.com/lucasmerlin/hello_egui/assets/40666126/ffb2f8aa-c269-4f73-8fc1-c0c9ed967531

Do you mean by dragging it somewhere outside or on a delete area or something like that?

I was simply thinking of a button to remove it, but I don't think that's possible because the vector is already taken as a &mut [T] reference, so I can't simply remove it inside of the dnd. I guess I'll have to try to find a different solution for it.

EDIT: I found a decent solution, I made it so that when you input and store the Delete key, it gets removed on the next iteration, that's quite a good solution in my case.

lucasmerlin commented 3 months ago

Ah, I think I know what's going on. Every item in the list needs to be unique. Since your vec is defined as Vec<(egui::Key, usize, usize)>, if there e.g. are two (A, 1, 1) items, they will have a conflict. You could try creating a struct like

Item {
  id: usize,
  key: Key,
  from: usize,
  to: usize,
}

and then implement DragDropItem for this, just returning Id::new(self.id). And then when adding a new item, you could e.g. iter the vec to get the highest existing id and add 1.

I guess egui_dnd's documentation isn't very clear here, I'll try to improve it.

For deletion, I usually just do:

// (Pseudocode, this won't compile)
let mut delete = None;
dnd(items, |ui, item| {
  if ui.button("Delete").clicked() {
    delete = item.id;
  }
})
if let Some(delete_item) = delete {
  items.retain(|&x| x.id != delete_item);
}
SpeckyYT commented 3 months ago

Is there any reason why the values are forced to be unique? I thought that this crate would store the index by itself behind the scenes, so that it never conflicts.

If me storing the index is the only solution, then I'll probably do that. 👍

lucasmerlin commented 3 months ago

Is there any reason why the values are forced to be unique? I thought that this crate would store the index by itself behind the scenes, so that it never conflicts.

In the beginning I tried to work with the index only but it turned out too fragile. When the list order changes or a item is inserted (maybe because a update was received from the server) while the user is dragging, the currently dragged item would suddenly change.

SpeckyYT commented 3 months ago

Ok, I fixed the issue by not allowing duplicates. For me this issue is completed, but if you want, you can close it now, or whenever you update the documentation. Thanks for the help.

Edit: I clicked the wrong button lmao