Skip to content
This repository has been archived by the owner on Oct 31, 2023. It is now read-only.

Pulled calculation logic out of NodeEditor.cs #90

Closed
wants to merge 1 commit into from
Closed

Pulled calculation logic out of NodeEditor.cs #90

wants to merge 1 commit into from

Conversation

Barsonax
Copy link
Contributor

The calculation logic is now in its own class that implements a interface called INodeCalculator. Currently the INodeCalculator field is assigned in the ReInit method but later on this may change.

… This class also implements a new interface called INodeCalculator.
@Seneral
Copy link
Owner

Seneral commented Aug 11, 2016

Cool, thank you.
From looking at it, what about all the other references to NodeEditor.RecalculateAll and more importantly NodeEditor.RecalculateFrom (most nodes, for example)?
While where at it why not get away from the term calculation when it's alot more than that (or, atleast planned). I've previously named it traversal algorithm because it's super generic but I guess that sounds weird here. But anyway I don't want to make the impression it's just for calculating values as the initial calculation algorithm;)
Any suggestions?
Also, we probably should think about the other usages this is actually made for and extend the interface with more functions that future traversal algorithms may need...


Some ideas and notes what this could be used for to consider:

  1. A statemachine: RecalculateAll/From would not be needed at all, as changing a node would not trigger the traversal algorithm obviously. Instead, we need an expanded API for script-triggered traversal (again, trying to be as generic as possible)
  2. Similar to above, a Dialogue system, which would need even closer code-integration. Imagine the traversal algorithm would ultimately need to wait for the respond of the player.
  3. Just for data visualization or wrappers: The functions named RecalculateAll/From would be used for live-serialization/updating of the data.
  4. Also: Functions like OnCanvasLoaded/OnBeforeCanvasSave, Awake, etc. would be ideal for addtional saving stuff and also subscribing to additional events like OnMoveNode, OnAddConnections - you name it.
    You see this could be a very important factor of flexibility for this framework:) A lot that can be done here - I hope this gave you an idea. I bet there are even more use cases that would benefit from this:)
    Btw it's just me writing out my thoughts and ideas for anyone to discuss, so don't be confused;)

@Barsonax
Copy link
Contributor Author

Barsonax commented Aug 11, 2016

This field is 'replacing' the logic that was first in the NodeEditor.cs:
public static INodeCalculator Calculator;

All references to the calculation logic are using that field now. Else that would have meant I broke the Node Editor :P.

I currently assign this field in the ReInit method since I don't think there is any other place yet to initialize it. Later on you could easily make this more dynamic though. Or maybe even remove that field from NodeEditor.cs altogether and put it somewhere else.

A while back I actually made a simple dialogue system using your framework. I did it in a different way though. I made a dialog node that holds the dialogdata and had a event for when this dialog started. When I started the game I simply searched all the dialog nodes and subscribed to this event. That was pretty much it.

However I did end up making all kind of different events for different kind of nodes (I had a node that triggered when you entered a area for instance or when certain objects where destroyed etc.). So I do think a more generic solution would be cleaner (or maybe you only need a few different events to handle everything?). I did not change anything in the framework to do this (well except for a simple generic helper function called GetNodesOfType() in NodeCanvas to easily get the nodes but this is just luxury).

Now iam thinking about this would something like a value changed event not solve most of these problems?

@Seneral
Copy link
Owner

Seneral commented Aug 11, 2016

Yes I understand that, but nodes still reference NodeEditor.RecalulateFrom and not NodeEditor.Calculator.RecalculateFrom, right?

Anyway, I think in the long run the calculator would be specified on a canvas(-type) basis. How exactly it is done however, I don't know. But I like your proposal of using events rather than funcitons... So instead of calling RecalculateFrom it then would simply be OnNodeChanged or something similar like you said.
Then we would not need the functions but rather only some 'structural' functions like OnCanvasLoaded/OnBeforeCanvasSave, Awake, etc as mentioned above and then some easy-to-access events. Maybe we should even go with NodeEditorEvents, even though I'm uncertain because we'd need to make that canvas-specific if you know what I mean.

Thanks for your input, I feel this is going in the right direction:)

@Barsonax
Copy link
Contributor Author

Barsonax commented Aug 11, 2016

They reference NodeEditor.Calculator.RecalculateFrom. Like I said I was not sure where to put the INodeCalculator field so I left that in the NodeEditor.cs till we find a better spot. Putting it on NodeCanvas level does sound like a good idea. For this to work you would have to write a menu obviously to choose the type of canvas that you want but for now you could just put in the default logic.

While we are talking about UI. Iam currently researching if winforms can be a complete replacement for the unity GUI system. Iam doing this basically because I hate the GUI system. Its good for simple stuff but you get loads of code bloat once it gets complex.

I recently discovered that winforms actually work inside of unity if you add the correct dll. I don't know if you know what winforms is but this makes it waaay easier to make UI. Instead of writing your UI in code you actually have a visual designer tool. Note I am just looking into this so iam not saying you should switch to winforms now XD.

You could have a OnInputChanged event in your nodebase class. These would be automatically subscribed based on what inputs you define.

Btw here is the code of my 'DialogNode':

    /// <summary>
    /// Triggers a dialog and provides it with data
    /// </summary>
    [System.Serializable]
    [Node(false, "Standard/Quests/DialogNode")]
    public class DialogNode : Node
    {
        public const string ID = "DialogNode";
        public override string GetID { get { return ID; } }
        public bool InputVal1;
        public bool OutputVal1;
        private bool _triggered;
        public delegate void DialogEventHandler(object sender, DialogEventArgs e);
        public event DialogEventHandler StartDialog;
        public bool Pause;
        [SerializeField]
        private DialogData _dialogData;

        [SerializeField]
        private string _dialogDataPath;

        public DialogData DialogData
        {
            get { return _dialogData ?? (_dialogData = ResourceManager.LoadResource<DialogData>(_dialogDataPath)); }
            set
            {
                _dialogData = value;
                _dialogDataPath = AssetDatabase.GetAssetPath(_dialogData);
            }
        }

        public override Node Create(Vector2 pos)
        {
            var node = CreateInstance<DialogNode>();

            node.name = "Dialog Node";
            node.rect = new Rect(pos.x, pos.y, 225, 100);

            node.CreateInput("Input 1", "System.Boolean");
            node.CreateOutput("Output 1", "System.Boolean");
            return node;
        }
#if UNITY_EDITOR
        protected internal override void NodeGUI()
        {
            if (Inputs[0].connection != null) RTEditorGUI.TextField(GUIContent.none, InputVal1.ToString());
            else GUILayout.Label(Inputs[0].name);
            InputKnob(0);
            EditorGUI.BeginChangeCheck();
            _dialogData = EditorGUILayout.ObjectField("DialogData", _dialogData, typeof(DialogData), false) as DialogData;
            if (EditorGUI.EndChangeCheck())
            {
                Debug.Log("Onchange");
                _dialogDataPath = AssetDatabase.GetAssetPath(_dialogData);
            }
            Pause = UnityEditor.EditorGUILayout.Toggle(new GUIContent("Pause", "If true then the dialog will pause the game"), Pause);
            Outputs[0].DisplayLayout();
            if (GUI.changed)
                NodeEditor.RecalculateFrom(this);
        }
#endif
        public override bool Calculate()
        {
            if (Inputs[0].connection != null)
                InputVal1 = Inputs[0].connection.GetValue<bool>();
            if (InputVal1 && StartDialog != null && _triggered == false)
            {
                StartDialog(this, new DialogEventArgs(this, DialogData, Pause));
                _triggered = true;
            }
            Outputs[0].SetValue<bool>(OutputVal1);
            return true;
        }
    }

    public class DialogEventArgs : EventArgs
    {
        public DialogNode DialogNode;
        public DialogData DialogData;

        /// <summary>
        /// Should the game pause?
        /// </summary>
        public bool Pause;

        public DialogEventArgs(DialogNode dialogNode, DialogData dialogData, bool pause)
        {
            DialogData = dialogData;
            Pause = pause;
            DialogNode = dialogNode;
        }
    }
}

@Seneral
Copy link
Owner

Seneral commented Aug 11, 2016

Yes I actually worked with winforms or rather WPF but there's one problem with it, even if it would work better and would be cleaner in the end: This project should serve as a basis everyone should be able to work on and understand. 'Forcing' them to using Winforms is not smart IMO because it is totally different than the standard ImGUI and others that may be unfamilar with XAML would have a hard time actually using and extending the framework to their needs.
Even though it sounds like a good idea, I'm sure this is not an option for this framework:/

Regarding your node, I can take a closer look tomorrow... I'll try to think of some good structure we could use:)

@Seneral
Copy link
Owner

Seneral commented Aug 11, 2016

Just as an addition - I've previously thought about having something like a state-machine and calculation on the same canvas. Sounds weird, but could make for some really versatile canvases... Of course this would now be way harder, if not impossible. Again, I need to think about it;) Though any suggestions are welcome!

@Barsonax
Copy link
Contributor Author

Winforms is not WPF. Those are 2 different things. WPF does not even work in mono actually. But just like I said I was just looking into this and got excited :P.

Being able to combine statemachines with the normal nodes sounds very flexible. I think this is possible. We should have a base class for the nodecanvas. This will contain the basic functionality. Then you can make different kind of nodecanvases. The default one will be the one we have now.

For the statemachine canvas I think there should be a CurrentState field on the canvas itself. When you update the state you start calculating from the CurrentState node. Basically you now look through all connections to check if something leads to another state node. The first time you find a new state you make this the currentstate (or priority based system? This would be slower though and don't see a true benefit from it). Next time everything repeats again.

Combining this with any kind of other nodes should be possible ofcourse but I think the input of the state nodes should still be a Boolean. I don't see how this could be different unless you want fuzzy logic?

Now you also want a hierarchical statemachine so this means each state can be a statemachine on its own. When the algorithm runs it will first check at the highest level and work its way down.

@Seneral
Copy link
Owner

Seneral commented Aug 13, 2016

Well yes but WPF is kind of similar in that it is based on a XML-file to describe the UI which is then drawn, or am I wrong? Anyhow it would still be a different system than Unity's and I would avoid that with reasons I already said:)

So we do have similar ideas of this system:) the specific 'Traversal algorithms' will be able to have properties how they like and may operate how they like:)
But I don't know what you mean with fetching the current node... and you can't compare it to the current system because it is totally not a state system;) Now, you need to consider ALL nodes that don't have an input to start calculation from them. In a state machine, it would probably require a special node that serves as a start node and possibly others - that algorithm would also be responsible for keeping such characteristics of the canvas.

Also, for a state machine I think we shouldn't make use of the current connection system, it is simply not built for this kind of use. I did made a prototype branch with a statemachine implemented in an old version of this framework, maybe you can take a look at it, it shows the basic ideas I had. Basically, you have normal transitions seperate from the connections, with conditions.
This is where a different concept I noted in #70 comes in - imagine if Node.cs would be partial, then a seperate file that comes with the optional state system plugin would be able to extend the node to have transitions;)

Combining states and normal nodes does allow for a lot more complex state machines and I do like the idea of state nodes having a normal connection output for normal nodes to attach. How this could be implemented is kind of easy, too: The canvas could have two 'traversal algorithms' running at the same time, which would work fine as they only listen to callbacks. So when the statemachine traverses to a new state, it could trigger a NodeChanged event (equivalent to the current RecalculateFrom) and the calculation algorithm would catch this event and react accordingly.
So in theory thanks to this event-based approach multiple traversal algorithms could act next to each other;)

On an additonal node for hierarchial statemachines - assuming subcanvases are handles by the hosting nodes it would require only a reference to that canvas to let it calculate in the calculation loop of the parent canvas:)

@Barsonax
Copy link
Contributor Author

Winforms is completely based on the code you write. There is no xml unlike WPF. The way its rendered is also completely different under the hood. They may look the same but these 2 systems are totally different.

Ill take a look at that statemachine branch later.

@Seneral
Copy link
Owner

Seneral commented Aug 20, 2016

Planning on improving the WIP docs and uploading them over this weekend, but after that we should make a seperate branch first, don't you think?
Don't exactly know how well it turns out and how hard it would be to implement but I don't expect anything too crazy... But I really like the concept!

@Barsonax
Copy link
Contributor Author

Been a bit busy lately (personal projects :P and a nasty unity bug) so I did not have the time yet to look at it yet. I will also be going on vacation soon.

@Seneral
Copy link
Owner

Seneral commented Sep 1, 2016

No problem, me to in a month and have been relatively busy because school started again. Maybe I'll do pull requests to your branch from time to time to expand on this and so we can put this together in one pull request:)

@Seneral
Copy link
Owner

Seneral commented Sep 5, 2016

Ok, merged it manually into develop (not master) with 723f3b9
Unfortunately that means it is not synced, sorry about that!
I'm continuing to work on it, first by refactoring the calculation system to only use callbacks for most flexibility;)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants