Page MenuHome

T101705: Don't create a lazy socket for dangling connections
ClosedPublic

Authored by Iliya Katueshenock (Moder) on Oct 9 2022, 2:42 PM.

Details

Summary

T101705: Don't create a lazy socket for dangling connections

In the commit db0ef8b00d3c there was a standardized behavior for reroute dangling nodes.
But that only took into account the connection. Lazy sockets in the constructor of multisocket
were still being created. This resulted in an error when looking up their standard
value, as there was no input. This patch adds dangling connection checking logic
to the multisocket constructor as well.

Also, this looks like a temporary workaround for now, as the dangling data can
be cached to avoid creating nodes of unused reroutes. But for now, it's enough
just to have a method to check for dangling.

Diff Detail

Repository
rB Blender

Event Timeline

Iliya Katueshenock (Moder) retitled this revision from WIP: T101705: Don't create a lazy socket for dangling connections to T101705: Don't create a lazy socket for dangling connections.Oct 9 2022, 4:33 PM
Iliya Katueshenock (Moder) edited the summary of this revision. (Show Details)
Jacques Lucke (JacquesLucke) added inline comments.
source/blender/blenkernel/BKE_node_runtime.hh
495 ↗(On Diff #56654)

Add an assert for this at the top of the function.

This revision is now accepted and ready to land.Oct 9 2022, 4:51 PM
Iliya Katueshenock (Moder) updated this revision to Diff 56656.EditedOct 9 2022, 5:05 PM
  • Add cycle assert
Iliya Katueshenock (Moder) marked an inline comment as done.Oct 9 2022, 5:15 PM
Hans Goudey (HooglyBoogly) added inline comments.
source/blender/blenkernel/BKE_node_runtime.hh
486–487 ↗(On Diff #56656)

I'm not sure this belongs as a method on bNodeSocket. First of all, it's only for reroute nodes, which makes it arbitrary whether it acts on the socket or the node, and makes it unclear when the function needs to be called.

Also, I'd generally expect these functions to have predictable (constant) runtime.

I'd think this makes more sense as a function in blenkernel like node_is_dangling_reroute.

I'm curious what Jacques thinks about that though.

source/blender/blenkernel/BKE_node_runtime.hh
486–487 ↗(On Diff #56656)

Yes, it's probably better to still be a node method. Although I still think who will be better at implementing this. In theory, it might be best for noodles?

Iliya Katueshenock (Moder) planned changes to this revision.Oct 12 2022, 7:58 PM
This revision was not accepted when it landed; it landed in state Changes Planned.Oct 17 2022, 12:01 PM
This revision was automatically updated to reflect the committed changes.