Search Unity

Feature Request Make the OnConnect/OnDisconnect delegates in GraphView.Port accessible

Discussion in 'UI Toolkit' started by JoshuaMcKenzie, Dec 31, 2018.

  1. JoshuaMcKenzie

    JoshuaMcKenzie

    Joined:
    Jun 20, 2015
    Posts:
    916
    Currently, extending GraphView.Port implementation or even changing a Node's rules on how they show ports is very limited. I wanted to change the rules for a specific type of node to always show certain ports even when collapsed. And even though Node.RefreshPorts() is not virtual, its still for the most part doable. but I noticed that the port.OnConnect and port.OnDisconnect delegates were internal which complicates things.

    Then I looked at the implementation for Port.Connect(),.Disconnect(), and .DisconnectAll(). These functions actually invoke the said internal delegates, but the functions themselves have the virtual modifier. This presents a limitation on extending Ports because one that is overriding said functions can't invoke these delegates without being forced to call the base implementations. And in my case I also want to override said functions because m_connections used in these functions is a HashSet and some of my Ports would like to enforce an order on connected edges (the order of edges on a port would imply priority for nodes that would need that context).

    For now I'm to resorting to making duplicate delegates and some reflection (in case the internals are invoked, so they also invoke my delegates) just so I can add my own OnPortConnect handlers for my nodes, which feels really, really ugly. It just seems that OnConnect and OnDisconnect being made internal feels arbitrary. I'd like that either the delegates are made public or at least protected with some "AddListener/RemoveListener" interface.
     
    JoshuaMcKenzie, Dec 31, 2018
    #1

  2. uDamian

    uDamian

    Unity Technologies

    Joined:
    Dec 11, 2017
    Posts:
    1,231
    If you use GraphView.graphViewChanged delegate, you'll be notified of all new edge connections (and disconnections). This is why those other delegates are internal. Except in some very specific performance sensitive areas (like knowing when and node is being moved), all UI change notifications should be handled at the GraphView level with its global delegates.

    Furthermore, the implementation details of how the Node stores its connections should not affect how your model interprets those connections. If you want order to be important, just store your connections per node in your own way, updating this store whenever your GraphView.graphViewChanged delegate is called. Don't rely on the UI layer's storage. The UI only uses this storage for UI operations (like knowing which edges to delete when a node is deleted).

    Ideally, you have a completely separate data model that stores your graph of nodes, independent of the UI layer. You then use GraphView only to visually display and manage this separate data model.
     
    uDamian, Jan 8, 2019
    #2
    daleth90 and JoshuaMcKenzie like this.

  3. JoshuaMcKenzie

    JoshuaMcKenzie

    Joined:
    Jun 20, 2015
    Posts:
    916
    Thanks for the reply. That does make sense and I'm relieved to know that there is an entry point that my custom graphs can listen to when connecting, disconnecting ports.

    Yes, I do have a Model-Layer of my graph which indeed is tracking the order of their connections and a Middle Layer which serves as an adapter/controller between the two other Layers. Basically an MVC pattern. The affordance I was specifically trying to convey was communicate in the UI the order of the connections from a particular port, more as a reflection of the model layer, and less as the authoritative source. I have no expectation of the UI-layer being available at run-time so all the necessary info will be stored on the model layer.
     
    JoshuaMcKenzie, Jan 11, 2019
    #3
    uDamian likes this.