ImNodeFlow icon indicating copy to clipboard operation
ImNodeFlow copied to clipboard

Dynamic Pins Crash

Open avlec opened this issue 1 year ago • 5 comments
trafficstars

Was trying out the following examples and they seem to crash when connecting pins to other objects with a Pin UID not found error.

ImNodeFlow.inl:189: const T& ImFlow::BaseNode::getInVal(const U&) [with T = double; U = std::__cxx11::basic_string<char>]: Assertion `it != m_ins.end() && "Pin UID not found!"' failed.

Using the following Node definition

struct Test : ImFlow::BaseNode
{
  void draw()
  {
    ImGui::Text("test");
    showIN<double>("INPUT", 0.0, ImFlow::ConnectionFilter::SameType());
    showOUT<double>("OUTPUT", [this](){ return getInVal<double>("INPUT"); });
  }
};

I noticed this while also trying out the notion of wrapping lambdas up as nodes, which I would also imagine should work. And it does render but has the same crash.

INF.addLambda([](ImFlow::BaseNode* self){
  ImGui::Text("lambda");
  self->showIN<double>("INPUT", 0.0, ImFlow::ConnectionFilter::SameType());
  self->showOUT<double>("OUTPUT", [self](){ return self->getInVal<double>("INPUT"); });
}, {0,0});

// with this added to ImNodeFlow class
template <typename L, typename B = BaseNode>
struct NodeWrapper : B, L
{
    NodeWrapper(L&& l): BaseNode(), L(std::forward<L>(l)) {}
    void draw() { L::operator()(this); }
};

template<typename L>
std::shared_ptr<NodeWrapper<L>> addLambda(L&& lambda, const ImVec2& pos)
{
    return addNode<NodeWrapper<L>>(pos, std::forward<L>(lambda));
}

avlec avatar Aug 05 '24 15:08 avlec

That's interesting... My initial guess would be some error caused by the latest updates to the recursion blacklist code. Sadly I won't be able to investigate it myself until September. As per the lambda, Im not sure the bug is exactly the same but let's hope so

Fattorino avatar Aug 05 '24 20:08 Fattorino

Yeah it seems like they'd be broken for the same reasons. Inheriting from lambdas is something that's actually used for a trick with std::visit that's outlined on the cppreference website as the overloaded template. So it should work. I more placed that bit of code in here because I stumbled across this bug because I was doing that. And I was doing that to prove that it'd be easy to add a convenience wrapper to add simple nodes to the editor without having to write a class.

avlec avatar Aug 06 '24 03:08 avlec

I just remembered, due to the dynamic nature of this type of pins, the value from an input is returned directly when calling showIN()

This was a design decision I made at the time, because I think using the same getInVal() function mixes two different things. Also searching by UID for something that won't always exist seems not the best implementation when it comes to using the feature.

But I'd like to hear some feedback and different opinions

Fattorino avatar Sep 16 '24 11:09 Fattorino

I think that this should work because I tried it and others certainly will. With the obvious caveat that if the pin didn't render on the current draw loop before referencing it would be invalid.

avlec avatar Sep 16 '24 17:09 avlec

I think I'll still keep them separate, creating a method called getDynamicInVal()

Fattorino avatar Sep 16 '24 18:09 Fattorino

closing as not ideal solution, I prefer the intended way to get a value from a dynamic pin

Fattorino avatar Jan 04 '25 12:01 Fattorino