zakarumych / egui-snarl

Customizable egui node-graph library
https://crates.io/crates/egui-snarl
MIT License
252 stars 25 forks source link

Popup `graph_menu` on pin drop #10

Closed kang-sw closed 6 months ago

kang-sw commented 7 months ago

Currently right clicking on the panel is the only way to add graph menu. It'd be nice to making graph menu open on dragged pin is dropped to empty space of panel. Since we can track from which node what pin is being dragged, it'd be possible to auto-connect newly created node from the menu with the node pin currently being dragged.

Pseudo code:

let pin = await viewer.drag
await pin.dropped

let new_node = await viewer.graph_menu(..)

// Iterate until we find matching pin

if pin.is_input {
    for output_pin in new_node.outputs {

        if viewer.try_connect(pin, output_pin) {
            return;
        }
    }
} else {
    for input_pin in new_node.inputs {
        if viewer.try_connect(input_pin, pin) {
            return;
        }
    }
}

// if no pin is matching, it's just okay ...
zakarumych commented 7 months ago

Yes, it'd be very useful. SnarlViewer should use separate method to populate that menu with pins that needs to be connected in arguments. It'll be able to use this context to, for example, list only nodes that may be connected. And it should perform connections inside this method directly.

Would you like to work on this?

kang-sw commented 7 months ago

Ah, yes. I'll try. What is your opinion for adding new trait method? I think it'd be useful to provide contextual informations such as, which node/pin is being connected.


Oh; seems your comment is changed😄. I agree with creating separate method.

kang-sw commented 7 months ago

And for this, it seems 'SnarlViewer::connect()' method does not return any result when invoked. This is mandatory for verifying automatic-connection validity.

Is it okay to change the signature to return Result or boolean? It will be breaking change; and if it's okay to return Result, what should the error type be?

link

    /// Asks the viewer to connect two pins.
    ///
    /// This is usually happens when user drags a wire from one node's output pin to another node's input pin or vice versa.
    /// By default this method connects the pins and returns `Ok(())`.
    #[inline]
    fn connect(&mut self, from: &OutPin, to: &InPin, snarl: &mut Snarl<T>) {
        snarl.connect(from.id, to.id);
    }

And it should perform connections inside this method directly.

If logic goes this way, it seems method signature won't need to be changed.

zakarumych commented 7 months ago

SnarlViewer::connect is command to add connection. Currently there's nothing to do if it fails, or intermediate note is added, or all nodes just vanish in result :)

Result in return can be added if there will be something to do about it.

UI commands should be self-contained. So one UI action should not cause multiple commands.