Making the Node Editor Framework more modular and flexible

[Seneral]

As we make the Node Editor Framework more general and add features like groups that not everyone might need, it also get's more complex and filled with potential useless features, depending on the needs.

I've the rough idea of modular system, that will make a lot of features modular and removable. Examples are the grouping, an additional XML save format (future) #43, the (currently default) forward calculation system, the statemachine behaviour (in developement) #34, or additional general nodes like the action-, expression- and conversion nodes (Branch)

That modular system would not be a part of the software code itself, but a general concept of using pre-processor options to integrate these modules.
Partial classes could be the main key to this. Each module could then add variables and functionality to the Node Editor without messing with the standard code.
Unfortunately, adding functionality to existing functions would not be possible.
Most of this is only theory so far, but it may help with the complexity and amount of features that are planned/implemented.

[Barsonax]

Wouldnt this be possible with a interface based approach? Might require some refactoring/moving of the code to make this work but you probably have to do this anyway.

For instance you can have a INodeCalculation interface that provides the logic for calculating the nodes. Then you just have to insert the correct class depending on what kind of calculation you want. You could even switch out a class with a different class at runtime.

I have done this with a pathfinder. All i have to do is insert the class that has some kind of algorithm (A_, Theta_, Jump Point or whatever). The logic is different but the interface is not.

Sure it might require some thinking to make this actually work but in my opinion this would result in a nice structured and flexible codebase which ppl could even extend or modify if they wanted to by implementing the interfaces while it also forces you to think about your design.

I would try to stay away from using partial classes for this as these are not really intended to do this. Their main usage is in automatic code generation like the visual studio designer so that auto generated code wont clutter up your code.

[Seneral]

@Barsonax Thanks for the ideas! Yes, for the calculation I had the exact same thoughts, I think this is the way to go. Also a reason why I did not yet made much effort of moving calculation code out of NodeEditor.cs (where it usually does not belong) because I had this in my head all the time...

@ChicK00o developed something amazing with 5c5ddc4 that kinda goes in this direction, where there are different types of canvas types. Binding them with different calculation ways would be really powerful:) With this already existing as the base, it would be way easier to handle other stuff mentioned in the OP like the statemachine:)
Again, something I'd love to develop further. As I said in a different post, it's time that is lacking:(

[Barsonax]

I know what you mean with not having enough time :P. Having full time job, doing some programming as hobby and still enjoying life just eats time.

I could take a look at it and put the calculation code in a separate class that implements a interface as first step to achieving this.

[Barsonax]

I made a pull request in which i pulled out the calculator logic from NodeEditor.cs into its own class that implements a interface. #90

[Seneral]

Initial idea of partial classes implemented in 1641989

Enhancement of the calculation system as discussed is still in progress though:)

[Roni1993]

I'm not sure if partial classes are a good idea for what you´re trying to do. Afaik partial classes are meant to be used as a structuering tool if your classes are getting big and you can't divide them into several meaningful classes or you need to work with multiple files for a class.

I think Barsonax' approch is more appropriate as it's how people are used to extend existing libaries/modules. I would actually go a little further and would adive a rework of the internals of the NodeEditor. There several small quirks that are kind of odd. Though as of now i'm not familiar enough with the code to make concrete suggestions.

[Seneral]

Sorry for responding so late. Made up a response in my mind and thought about it - but forgot to answer:O

I think it's a mix of both. I do think partial classes are powerful, when used correctly, because they allow changing some basic structural stuff of the framework without touching the framework code (so you can update without caring for your local changes).
But nevertheless, for some core functionalities, like the calculation algorithms, input system, nodes, connection types & canvas types, etc. you're right of course. That's what will really make the framework flexible;)

I do have ideas for this but I don't have much time to implement them:(
Had exams the last two weeks, a few days rest with visitations where I'll be able to work a bit, then a week school trip, 5 days rest to work, a week holiday... kinda busy but I'll find time to explore some ideas;)

[Nopey]

Here is how I think the framework could be structured:
Node Editor class uses utility functions on other objects for things like click detection, permission to make a connection, drawing, etc. (it does a few things right now it probably shouldn't be doing like directly accessing the nodes rect to for click detection, instead of asking the node if it hit it, and the entire Calculation region)
An abstract Node Class, which the people use to create their nodes. Would have a list of Knobs

A single Knob class, that is mainly just a container for the list of KnobModifiers.
Would have methods for things like bool canConnect(knob from), void draw(position?)
Would know what side of the node it is on.
Would have a list of connected knobs, and a getter for convenience that returns the first connected knob, null if there are none, and raises a warning or exception if there are multiple.

an abstract KnobModifier, mainly for allowing and denying connections.

• bool canConnect(knob from) - called from Knob
• void draw() - would draw things like the input vs output indicator, maybe KMType would draw the knob background?

Some Builtin Knob Modifiers, with temporary names. naming policy could be improved.

• KMDirectional
• KMNoRecursion (extend KMDirectional? we should probably have a discussion about that)
• KMConnectionLimit
• KMCalculate (Would add directional and if its an input knob, a connection limit.)
• KMType? (Would limit to knobs with a matching KMType, such as float -> float)
  And of course any the user needs to implement for their project

For Dynamic Input/Output (issue #91), the Node could implement an event handler that increases the number of knobs.

For multiple input connections and restricting output (issue #74), the Node could simply use different knob modifiers, as the directionality and the connectionLimit are independent.

For State machine support the Nodes could use a KMDirectional, KMConnectionLimit on the output, and something to make the conditions. Possibly use a monobehaviour with a reference to the canvas, and a variable for the current node, and call some functions on your node (through an interface you made all the state machine nodes extend), where that functions does all the logic for that state, including switching to the next state.

For Node Areas/Groups (issue #41): we could either implement it monolithically or one of the modular ways.

• Monolithically: adding a special case to the code that controls creation of connections.
• Modularly: for connection creation, either have a way that any knobmod can override others with true (might get confusing, with special rules..) or write all the knobmod so that if they use the other knob's information at all, they look for a knobmod of the same type, and if there is none, they don't block. (might get confusing, and result in unintended connections)

For Serialization (issue #43), we could serialize the nodes in the canvas with their knobs & knobmods attached, but with a [Nonserialized] on the list of connections, and then go through and serialize all the connections as a big list of knob identifiers that can be reconstructed.

one problem I can see right away is that it might be ugly to try and implement coloring the connections in this model, unless the knob mods get a chance to color the connection.

This is just my view on the issue, so please tell me your thoughts and ideas before I start implementing this in about a week 😅

[Seneral]

Thanks for your opinion, that's alot! :)
I'll try to leave my opinion on it step by step...

First paragraph: Seems we agree here;) The calculation has already been lifted of NodeEditor.cs in the develop branch and especially in #109 , and the whole region 'Space Transformations' (including click detection) is also planned to be moved to more apropriate places.
About the node class, isn't it already like this? Or do you mean something else?

Regarding the Knob concept: I don't think I fully understand it yet, but it appears as if you only want ONE Knob class and define it's properties (how it behaves) through a KnobModifier? That would mean we remove InputKnob and OutputKnob as we know right now, and instead we make the whole 'Connection Knob concept' weakly typed, as far as I understand, against the language specifications.
Don't think that is needed, when we can use the same concept as with the Node and extend the NodeKnob class, or not? Also, the NodeKnob as it is right now is not only for connecting but could be used for anything else that sticks on a side of the node;)
But maybe we can make it a bit like your idea, just by extending a class instead of adding modifiers to it. Like, we have a ConnectionKnob extending from NodeKnob that can connect to multiple ConnectionKnob and then child classes InputKnob and OutputKnob restrict this further by extending methods that you are listing for KnobModifier.

As to your statemachine idea, as far as I understand the MonoBehaviour is responsible for traversing the state machine? Then, again referring to #109 , this could soon be easily solved by extending NodeCanvasTraversal and NodeCanvas to easily set rules for statemachine canvases and how to traverse them. Also, MonoBehaviours are only applicable at runtime, not in the editor...

Concerning node groups in #41 , we already solved that in a seperate branch that I yet forgot to merge (stupid me) ;) I'll try to merge it soon.

Serialization is not really related to the framework structure directly I think, problem there is more whether we make the property saving generic or hardcoded, and to what degree... Can be implemented with basically every framework structure;)

[Nopey]

Thanks for the reply!
About that node.cs thing, You're right. it is already abstract, and I just forgot when I was writing that. Sorry about the confusion.
The Knobs. Yes, I was thinking about having a sealed Knob class, with a handful of knob modifiers to make it do what it wants, but with strongly typed connections. The type would be tested in a knob modifier, more specifically, one of the above KMType or KMCalculate.
The reason I thought knob modifiers rather then knobs is because of the following inheritance problem
we have a ConnectionKnob, and either a DirectionalKnob (with a flag specifying direction) or Input and Output knobs. We then want to implement a limit on the number of connections, and we either extend ConnectionKnob or the DirectionalKnobs, or both. If both, we end up with 2-3 knobs that are all limiting of the number of inputs, with the entire class copy pasted, and lots of temptation to implement it monolithically directly inside connectionKnob.(which may or may not be a good idea)
I agree that compromise is the best between my suggested over usage of modifiers, and the monolithic way it is now.
With the statemachine, extending the canvas is the way to go, now that I read more into it.
Node groups, oh Node groups, oh how did we ever live without you? (checking out that branch right after I finish replying to this)
Serialization is off topic, probably shouldn't have mentioned it.

[Seneral]

Ah, now I got you! :)
So you want KnobModifiers to be stack-able to achieve a final behaviour?
Sounds pretty good to me, although there are still about the same amount of problems this way... (f.E. the coloring).
Also, the redundancy problem with inheritance isn't as major, because a sub class could only overwrite what it needs to be different, e.g. doing what KnobModifiers is currently for: canConnect ()

My suggestion when taking the current inheritance route:

• A base NodeKnob class as we have right now for general use
• A ConnectionKnob child class that handles all the connection to other ConnectionKnobs and drawing aswell as a limiting type that defines the color (but can be skipped if not needed)
• Classes extending ConnectionKnob defined behaviour by overwriting canConnect () and other methods if needed (also can overwrite color)
  Which is basically what we have right now, BUT connection weakly typed (moved to ConnectionKnobs). Still, it's possible to adress specific knob behaviours strongly-typed.

My suggestion when taking KnobModifier route:

• A base NodeKnob class as we have right now for general use
• A ConnectionKnob child class that handles all the connection to other ConnectionKnobs and drawing
• ConnectionKnob holds KnobModifiers, which are defining limiting rules for connection compability depending on, e.g., amount of possible connections, type compability, ... and can overwrite the color if desired (for e.g. types)
• ConnectionKnob can still be extended, no reason to limit that, right?
  Which is basically the same but KnobModifiers defining the behaviour instead of subclasses, which forces the behaviour to be weakly-typed.

Note that I'm probably using the terms 'strong-typed' and 'weak-typed' wrong here, I hope you know what I mean... Basically, in comparision when checking if a knob can accept more than one inputs: weakly-typed: nodeKnob.hasModifier (typeof(ConnectionLimit)); and strong-typed: nodeKnob is NodeInput;

Additionally, you were naming a KMNoRecursion Knob modifier - That's something we have to discuss. Where should it be defined whether recursion is allowed or not? In the Node itself, in the Knob as you are proposing, or in the actual traversal algorithm that in the end needs to handle this?

[Nopey]

Quick question, Why are InputKnob and OutputKnob seperate classes? The only reason I can see is the assymetry of the calculation and connection limits. A single class with a flag for which direction it is could simplify the code if we end up rolling with knob inheritance without any knob modifiers.

[Seneral]

Yep, the redundancy of the code in these classes is one problem right now. With suggestion number one (inheritance route) the only difference would then be the overwritten method canConnect (). The advantage of that is that it's strongly-typed and NodeInput can only receive one connection and only from NodeOutput.
When taking that route, you'd be able to change this bevaviour by extending ConnectionKnob with canConnect () specifying your new behaviour.

[Nopey]

I've begun implementation of this (latest commit as of writing this is (5a56c5)
Theres something wrong with the serialization taking place when saving to an asset file, because going into playmode (causing an assembly reload) doesn't break anything, but saving and loading a canvas does.
The main thing that seems to break is the connections and connectionrules Lists, on the ConnectionKnob.
I Suspect either I broke EditorSaveManager, or some other code that is run when saving to an asset that isn't lastsession.asset
Could you help out with the serialization? Its fine if you don't.

The CalculationGraph is completely disabled ATM, and will need to be reimplemented. I have held off on this because of the broken serialization.
There are also a few decisions that should be discussed, such as the creation of connections from inputs. I have currently allowed creation of connections from inputs, but the original behaviour was to only allow creation of connections from outputs. To replicate the original behaviour, simply change true to !isInput in CR_Directional.CanStartConnecting() (its marked with a TODO)
GUI images Can be colored using GUI.color, instead of tinting the texture, although I have no idea how big an impact on performance using GUI.color brings.
Might as well add a GIF

I Changed the Knob image to a different image, but that can (and probably should) be set back to the original.

[Seneral]

Looking good so far! Will take a closer look at it soon, once I improved the modularity update #109 and reworked the cache system for #36 ... probably tomorrow!
Sure can help you with serialization, it's simple to add new ScriptableObjects once you got it;)

Btw, let's continue that in #74 ;)

[Seneral]

The basic idea behind this has been implemented in commit 22ba05d:)
Next thing to make more modular is the knobs and connection system, in #74
For further suggestions regarding general modularity or flexibility, please create a new issue!