Seneral / Node_Editor_Framework

A flexible and modular Node Editor Framework for creating node based displays and editors in Unity
https://nodeeditor.seneral.dev
MIT License
2k stars 415 forks source link

Is there any reason not to provide a virtual Node Node.Duplicate() for nodes to duplicate them self? #164

Open No3371 opened 6 years ago

No3371 commented 6 years ago

Currently it seems the duplication is implemented in NodeEditorInputControls.cs, which only create a new empty Node, therefore is not handy when custom nodes contains data. Will it brokes anything to let a Node duplicate themself and fill in desired data?

Seneral commented 6 years ago

Yes duplicate is pretty useless atm indeed. There were plans to automatically transfer values (nothing built in to do that, as far as I know). Kind of left it where it was though. Manually filling that data seems kind of bad to me, too, but if you need that, should be an easy addition. But from the frameworks perspective, will likely not add that, since it promotes additional 'filler' code - unless there's no other way...

No3371 commented 6 years ago

It have to be implemented with Reflection, I guess? To get all field name without a (To be made) [IgnoreDuplicate] attribute and copy the value into the fields in the new node, is this a correct way to go if I am going to add that to the framework?

SlimeQ commented 6 years ago

I implemented this, just by using a virtual function and overriding it per-node. Not super extensible, but works when I need to have data duplication for a node

SlimeQ commented 5 years ago

My project has diverged pretty drastically from this codebase at this point, but I have a system where I save out all the important variables of each node to a separate object called NodeData. I mark fields with a custom attribute [SavedField] and then use reflection to automatically find important fields and store them in NodeData.

This initially was just to help with serialization but it's also allowed me to do a complete runtime undo/redo system AND easily duplicate nodes with their data intact.

@Seneral if you're interested in the details of such a system I'd be happy to help. My implementation is far from perfect (and in many cases dependent on my app) but it's been very effective.

Seneral commented 4 years ago

Sounds really cool and somewhat similar to my thoughts on this matter. Glad it works for you! But since I only thought of using it for duplication I didn't get to implement it so far.

Ultimately I would choose (with reflection) all members that are expected to be serialised by unitys rules in the implementing class, minus those in the base Node class, and do those manually (which includes connections and knobs and such). Then directly copy those value into the duplicated node.

As for a runtime undo/redo system, would probably use the same action-based system that I implemented just now, but as a wrapper around UndoPro and a small undo backend (literally just lists of actions) to handle both in one go. As for the GUI undos (e.g. specific node values) I'd probably use the duplication data similar to what you do. Could probably store them within a action-based undo as well, so no huge data backend needed at all. So I would probably not choose to make a separate node data or even attributes, but keep it as little code as possible, but generic for all use cases.

Atleast those were the plans if I ever get would come around to do them. If there's interest I might implement it, but as it currently stands I neither have the time nor reason to do such a big thing for rarely used features (duplicate/runtime undo).