Nelarius / imnodes

A small, dependency-free node editor for dear imgui
MIT License
1.97k stars 241 forks source link

Dragging a link disconnects already connected input pins on snap #123

Open smilediver opened 2 years ago

smilediver commented 2 years ago

This is probably an unintended consequence of ImNodesAttributeFlags_EnableLinkCreationOnSnap implementation. I think it should not report a new link in IsLinkCreated if the input pin is already connected.

Auburn commented 2 years ago

IsLinkCreated has an optional bool* created_from_snap argument that you can use to avoid this. If the link is created_from_snap and you already have a connection to that attribute simply do nothing

smilediver commented 2 years ago

Thanks for the advise, and I already made this workaround.

I did realize now that in some cases people might have graphs with multiple links to the input pin, so this behavior is not far fetched. Now I'm thinking that maybe there should be something like ImNodesAttributeFlags_SingleLink flag to help with the support for the single link pins (which may be the most common case) to ease the setup. Setting that flag, could disable IsLinkCreated for snapping to already connected single link pins.

Auburn commented 2 years ago

In my use case I only allow single links to an input, but if a new link is dragged onto an already linked attribute it simply deletes the existing link. I think this is a better UX design for a user.

I could see a use case for SingleLink stopping new links being started from an attribute with an existing link though. I did actually start to implement this yesterday but scraped it due to the above reasons

smilediver commented 2 years ago

I think you're mixing snapping when dragging a link, with actually making a connection (ie releasing the mouse button). I agree that releasing a mouse button on a single link pin should delete old link and make a connection (that's what I do too), but if you're simply dragging a link around and it snaps to input pins then it should not make an actual connection, and just snap visually.

Auburn commented 2 years ago

That is already possible using the method I explained in my first comment

Auburn commented 2 years ago

See the interaction here: https://github.com/Nelarius/imnodes/issues/41#issuecomment-647132113

smilediver commented 2 years ago

I know, and that's what I already did. I've probably not explained myself properly. :) What I'm arguing is, IMHO, it's worth to add support for this directly to the library, instead of pushing this down to users to recreate this on their own.

I think the ultimate goal for the API could be that you setup node/link calls, then you setup some generic link creation/destruction code, and then control the behavior of the library by setting various flags. And if you want flexibility, you don't set any flags, and manage everything by yourself, like you can do it now.