Dynamically Adding and Removing Pins

[avlec]

From preliminary testing despite the documentation on addIN and addOUT I seem to be able to dynamically add pins at runtime outside of the constructor. It seems like this just works and maybe there's some warrant to verifying this with a more extensive test as I just tested adding a single in/out to a node that was already "wired" up to other nodes.

I understand that probably the more conceptually correct way to do this would be by replacing the node with a new one with the right pin configurations. But there is no way to remove nodes from the view. And if that was added another problem would be that the connections on the existing node have to be re-routed to the new node. Which is why I think fully supporting adding and removing of IN/OUT pins outside of the constructor would be a preferred approach.

Would be interested to hear your thoughts about this.

[Fattorino]

Adding Pins at run time is perfectly safe and it's the intended way of dealing with nodes with a variable number of Pins. Pin deletion is missing because honestly, I forgot that would have been useful. Although, it is part of the short-term goals to rework parts of the Pin logic. Two things in particular: adding a custom renderer that can override the default one to allow more freedom on the visual appearance of the Pin while still handling its related logic internally (already completed); developing a second way of adding pins to a node so that addIN and addOUT can be used for static Pins and some other method for dynamics Pins.

To achieve something like this

. . .
if (something)
    showIN(. . .);
. . .

This will create a Pin that will last until the frame's end and then be destroyed. So it will only be visible when the condition is validated, no need for manual deletion. I think this is the simplest to use and most flexible solution (also it shouldn't be so hard to implement but I'll find out next week when my exam session is over and I'll implement this), but I'd also like some feedback.

[avlec]

Idea of locking in static pins with the constructor, and transitioning addIN and addOUT to dynamic. This is a kinda idea I have, only problem with it is we need m_inf initialized before the calls to withIN and withOUT. So maybe this needs some work, I'm not sure if you can get away with not having m_inf in the Pin or not. (e.g., delete the member and always use the ImNodeFlow context from BaseNode that contains the pin. Or you could make it a const ImNodeFlow& to the pointer in the BaseNode. You'd probably know what would work best, so take this idea for a spin (maybe after your exam session) but I'd enjoy to work with you on this.

namespace ImFlow {
  class BaseNode {
    // ...
    template <typename T>
    std::pair<PinUID, Pin> withIN(const std::string& name, T defReturn, ConnectionFilter filter)
    {
      PinUID h = std::hash<decltype(name)>{}(uid);
      return std::make_pair(h, std::make_shared<InPin<T>>(name, filter, this defReturn, m_inf));
      // WARNING: m_inf is a problem cause it's uninitialized
    }
    template <typename  T>
    std::pair<PinUID, Pin> withOUT(const std::string& name, std::function<T()> func, ConnectionFilter filter)
    {
      PinUID h = std::hash<decltype(name)>{}(uid);
      return std::make_pair(h, std::make_shared<OutPin<T>>(name, filter, this defReturn, m_inf));
      // WARNING: m_inf is a problem cause it's uninitialized
    }
    // ...
}

// usage example
struct X : ImFlow::BaseNode {
  explicit X(const std::string &name, ImVec2 pos, ImFlow::ImNodeFlow *inf)
  : BaseNode(name, pos, inf,
      {
        withIN<float>("IN1", 0.0f, ImFlow::ConnectionFilter_Float),
        // more inputs
      },
      { 
        withOUT<float>("OUT1", 0.0f, ImFlow::ConnectionFilter_Float)->,
        // more outputs
      })
  {}
// rest of class
};

[Fattorino]

First experimental test:

ImGui::Checkbox("Extra pin", &show);
if (show)
    ImGui::Text("PA: %.2f", showIN<float>("Test", 0.f));

https://github.com/Fattorino/ImNodeFlow/assets/90210751/5c3b117c-2562-4f11-b383-fc2efe963eb5

[Fattorino]

Just a warning, I'll be refactoring parts of the code related to Pins (as also mentioned in #7) and renaming getters and setters for consistency's sake. So you know what parts of the code I'm changing.

[Fattorino]

@avlec If you find the time to test the new temporary pins let me know what you think, it's still a beta but they seem pretty solid to me.

[Fattorino]

I'll ask here because idk where to ask. I'm working on a new style + more customizability, what do you think?

[avlec]

I think it looks good. Definitely something to be able to opt-in/out of. The different node types allow for a nice visual grouping. I've had some ideas realted to this that I can lay out I suppose.

• Saving of Node Positions to the ImGui.ini file so that instead of manually placing nodes it can load where they last were.
• Allowing Pins on Nodes to be rearranged visually

[Fattorino]

As per saving a "layout" I was thinking of providing helpers for importing and exporting but not actually automatically writing to any file.

And for the rearranging, I'm not sure how that would look or work practically. Can you explain what you have in mind with an example? Thanks

[avlec]

Might not be as immediately useful as some of the other improvements being worked on. It'd more just help with visual clutter from Link overlap there are a few different ways you can go with this.

1. Capability in the code to adjust the position of pins on the exterior of the node.
2. Capability in the code to add spacing.
3. Capability for the user to drag pins around the exterior of the node repositioning them through the UI. (Probably the least useful)

[Fattorino]

Some of these options already exist. SeePinStyle.extra.padding to adjust top and bottom spacing, and PinStyle.extra.socket_padding to adjust the space between the pin's text and connection point. Also, I'll not be adding extra options to focus on other features for now, but if you want to, I'll be sure to merge it.

[Fattorino]

After some more testing, I think dynamic pins are completed, so unless you have found something tomorrow I'll close this issue

[avlec]

I was going to check this out. I'm currently sick so probably won't be able to take a peek but from the code and demo it seems great to me.

[Fattorino]

Then I'm going to close it and if needed we'll reopen it later. Commits 0cbbf51, b687b31, and 0b8ed8b close this issue.