Wouterdek/NodeNetwork

Node Nesting

FabianNitsche opened this issue · 6 comments

First of all: Thank you for this amazing library!

I am trying structure my nodes, because some inputs and outputs are connected. So I was thinking of nesting nodes. I looked around and came up with using the leading or trailing style to do so (inspired by #21):
grafik

But then I loose the connection funtionality of the nested node (it does not work right). Am I adressing this wrong? Is there a better way of grouping inputs and outputs?

I came up with a different grouping.

grafik

It may also be an alternative for #63

I am new to a lot of concepts used for this library, but if you like I could create a pull request to discuss?

I have a branch in my fork, that has the changes. It now also supports a group hierachy. It is not polished yet, but I think it adheres to your coding style:

https://github.com/FabianNitsche/NodeNetwork/tree/feature/endpointGrouping

Thank you for your interest and effort! I can see endpoint grouping could be useful visually. However, I took a look at your fork and I'm hesitant to add this to the repo in its current form.

Unless I'm missing something, one could essentially already achieve the same effect with the current codebase. You could just add 2 inputs, one before and one after the group members, and hide their ports. With a little bit of styling, that would look identical to the image in your post with 0 extra code in the repo required. This keeps the main codebase lean. It is more flexible too. You could also implement trees, spacers, infopanels, searchable/filterable groups in this way. The abstractions of these structures could be viewmodels that operate on a set of endpoints in the node.

As a quick illustration of what I mean, I'm thinking of something like this:

class InputGroup
{
	IObservableList<NodeInputViewModel> VisibleEndpoints { get; }

	ISourceList<NodeInputViewModel> Inputs { get; } = new SourceList<NodeInputViewModel>();

	NodeInputViewModel GroupHeader { get; set; }
	NodeInputViewModel GroupFooter { get; set; }

	InputGroup()
	{
		// Here, you could create a DynamicData binding so that VisibleEndpoints is always equal to:
		// {GroupHeader, Input1, ..., InputN, GroupFooter}
		// GroupHeader and GroupFooter would be inputs with some visual styling and without a port.

		// Later, more logic could be added here to filter the inputs for more complex layouts
	}
}

class MyNodeViewModel : NodeViewModel
{
	MyNodeViewModel()
	{
		InputGroup myGroup = new InputGroup();
		myGroup.Inputs.Add(new NodeInputViewModel());

		// Here, some DynamicData binding magic would add all InputGroup.VisibleEndpoints members to this.Inputs
		// and update this relationship as VisibleEndpoints changes.
		// Depending on what is available in DynamicData, this would be the hard part and may require some changes in NodeView/NodeViewModel.
	}
}

I feel like this will scale better than trying to fit all possible structures into the NodeViewModel.

I acknowledge that this would be a useful abstraction though, and that it might be nice to make it available out-of-the-box to all users. If you abstracted it that way, I would support adding it to NodeNetworkToolkit.

Thank you for your input, and I completely understand your hesitation and agree with your arguments. I will investigate your proposal.

However, would this also work for grouping inputs and outputs together? As you can see in the "ForLoop" example there are the non-grouped trigger in- and outputs and there is the group of the index in- and outputs.

From the structure of the NodeView style it seemed to me, that inputs and outputs are two separate, independent lists.

Ah, I missed that, sorry. This makes more sense now.
I've had this thought for a while now that separating inputs and outputs might have been a design mistake.
In, for example, Autodesk 3dsmax, there are just endpoints which can have an input port, output port or both. Seperating these into separate classes has caused some code duplication and makes things like layout less flexible.
Unfortunately, fixing this is not easy and would break some core parts of the API. That's maybe something for v5.0.0 or so.
Until then, I guess a layout-based solution like this is a good compromise. Make a PR from your repo and we'll go from there.

Thanks for merging the PR. I think this can be closed now.