setzer22 / egui_node_graph

Build your node graph applications in Rust, using egui
MIT License
715 stars 134 forks source link

Consider allowing properties to be both inputs and outputs at the same time #8

Closed francesco-cattoglio closed 2 years ago

francesco-cattoglio commented 2 years ago

Right now a Property can be either an Input or an Output, but not both. We should consider the option of allowing a property to be both an input and an output. This is used in the blueprint editor of UnrealEngine to declare the control flow of the script: Blueprint example To achieve this, we could unify InputParam and OutputParam structs, and instead differentiate between inputs and outputs in a new ParamKind enum (which would have many possible values, like InputOnly, OutputOnly, InputOutput, InputOrConstant(ValueType), etc).

setzer22 commented 2 years ago

What you're describing is the exact same data model I came up with when I started this. I ended up gravitating towards the current separate input/output system because this approach introduced several drawbacks. Here's the two main reasons I see in favor of the current design:

As I see it, inputs and outputs logically represent the "port" (the tiny little circle where connections attach to), not the slot, so it makes sense that there's two of them. In the UE screenshot you shared, you could think of the control flow "slot" a pair of (input, output) parameters, which happen to be rendered at the same height.

I think rendering a pair of input/output parameters at the same height can be seen as a presentation issue, and one that is easily solved. I don't think it's something that necessarily needs to be baked into the data model.

The way I was planning to address this is to allow storing additional information in a node that would give hints to the library to know exactly how parameters need to be drawn. Right now I'm choosing a very simple heuristic where I just sort the inputs on top, and the outputs at the bottom, but this is already undesirable in many cases, so together with this mechanism (which needs to happen anyway), we can introduce something that will let the user say "bundle this input and this output together at the same height". This system would also deprecate the show_inline flag, which would become just another hint. Think something like (very rough sketch):

// A simple node, two inputs and an output
ParamDrawOp::Input("v1")
ParamDrawOp::Input("v2")
ParamDrawOp::Output("result")
// Same, but the output is on top
ParamDrawOp::Output("result")
ParamDrawOp::Input("v1")
ParamDrawOp::Input("v2")
// Or in the middle
ParamDrawOp::Input("v1")
ParamDrawOp::Output("result")
ParamDrawOp::Input("v2")
// There's now a "control flow" pair of params, the OutputSameLine ensures the line is shared with the previous input
ParamDrawOp::Input("flow_in")
ParamDrawOp::OutputSameLine("flow_out")
ParamDrawOp::Output("result")
ParamDrawOp::Input("v1")
ParamDrawOp::Input("v2")
francesco-cattoglio commented 2 years ago

Perfect, thanks for the explanation of the design decision you made.

setzer22 commented 2 years ago

It's good to put all this in writing, so thanks for asking the right questions :)