Allow duplicate elements in egui_dnd

[CanisLupus]

Hi! First of all thanks for building these addons to egui!

On to the issue: I was getting really strange bugs using egui_dnd::dnd while dragging elements until I understood that it happened when two or more elements in given vector were the same. Then I saw that the code is identifying items by their ID in show. I'm not sure what the intention was but I feel this should be logged as a bug. :) Otherwise consider this a request. 😁

There is no reason that I can see to require unique elements, given that they can already be identified uniquely by their index. Am I missing some aspect of this, perhaps? I guess it might be harder to track things as they get reordered dynamically, but it should definitely be possible.

For some context, I was very unhappy with the recently added "higher level" drag and drop functions in egui and your version just worked perfectly out of the box, which was wonderful, but this issue sadly makes it impossible to use without modifying my data just for this.

Could this be fixed? It would make it a lot more flexible! It would also not require the items to be hashable.

Also, unrelated for the most part, but requiring egui_dnd::dnd itself to take an id_source also forces the user code to have to pass in a custom ID through a few levels of functions, instead of relying on the pre-existing egui features like ui.push_id(...) that the user may already be using, which will just make sure that things inside that scope won't conflict with others using a different ID.

/ Daniel

[lucasmerlin]

Hi! Requiring unique ids is just a lot more robust than relying on the item index. In my collaborative application for example multiple users can reorder items in the same list, so the item index may change while someone is dragging something.

If you can guarantee that the item index will not change while the user is dragging, it is possible to use the item index as id though, I've created a new example here to illustrate this.

There are some limitations to this though, as you will need to disable animations, since they rely on the item ids to remain the same when the drag ends. Also you may only update the item's index once the drag as ended, so you cannot show live updates.

---

Why does egui_dnd need a unique, stable id?

The id is used to store non-visual data like the currently dragged item. If I were to use ui.auto_id() (or whatever it's called), and a new widget is shown above the dnd list while the user is dragging, it would get a new id and loose it's data, causing the drag to be canceled. That's why I require a stable id. This is similar to e.g. CollapsingHeader, which also requires a stable id (by default generated from the title, which needs to be unique and static, unless you pass a different id_source).

I think egui's auto ids should only be used to store trivial, visual-only data, e.g. for animations.

[CanisLupus]

Thank you so much for the detailed reply and for creating the example!

I failed to see that nice trick of having a "temporary" struct implementing DragDropItem. That solved the problem nicely. The limitation of having no animations is completely fine with me.

Apparently the item IDs can't be the same across multiple egui_dnd::dnd instances either. However, I could tweak your solution to make all item IDs based off of the ID already passed in to dnd (using with on the ID to add their specific part).

If I can leave some feedback, seeing that this crate will keep using IDs for items for the reasons you outlined, is to try to make it clear to the user that duplicate items will have problems, even across multiple instances. I could see this being a common issue (but I have no proof 😄), and it takes a bit of time to figure out if you try to integrate this in an existing project as I did (especially if you start adding it to something close to a generic "list drawer"). :)

---

On the other topic, I didn't mean to say to use egui's auto IDs. I totally understand why it needs an ID. I was mostly wondering why it takes it by argument when the user can generate an ID scope using push_id, which is an egui solution for making elements unique, and supposedly well-known if the user is using egui. Buuut I wasn't aware that CollapsingHeader uses the same approach as yours, so thanks for bringing that to my attention as well!

Appreciate all your time. Thanks a million!

[lucasmerlin]

I've updated documentation to be more clear that the item id needs to be unique.

And yeah I guess push_id could work fine, but if someone doesn't know about egui's id internals (I still get confused about eguis ids sometimes) this could lead to weird bugs and would be difficult to debug, so by requiring a Id this makes the id explicit and prevents many issues people may encounter.

I'm also not very happy with the DragDropItem trait, since as you experienced, it's easy to mess up by creating duplicate items. Maybe I'll remove the impl<T: Hash> DragDropItem for T impl so people have to explicitly implement it for their type. And rename the function to unique_id, that should make it really difficult to mess up 😄

[CanisLupus]

I've updated documentation to be more clear that the item id needs to be unique.

Awesome! Thanks :)

And yeah I guess push_id could work fine, but if someone doesn't know about egui's id internals (I still get confused about eguis ids sometimes) this could lead to weird bugs and would be difficult to debug, so by requiring a Id this makes the id explicit and prevents many issues people may encounter.

Fair enough about egui's internals. It seems egui itself doesn't always defer to push_id and requires specific IDs sometimes as you noted, so I understand.

I'm also not very happy with the DragDropItem trait, since as you experienced, it's easy to mess up by creating duplicate items. Maybe I'll remove the impl DragDropItem for T impl so people have to explicitly implement it for their type. And rename the function to unique_id, that should make it really difficult to mess up 😄

That is true 😄 It might become a bit annoying to implement it yourself for each type you want to use, but maybe. :)