This patch adds additional socket shapes for use in pynodes. They have been adjusted to be visually the same size as the default Circle. This was requested by Jacques Lucke author of Animation Nodes after a chat on BA.
Details
Details
- Reviewers
Campbell Barton (campbellbarton) Julian Eisel (Severin) Bastien Montagne (mont29) - Group Reviewers
Nodes
Diff Detail
Diff Detail
- Repository
- rB Blender
- Branch
- arcpatch-D2814
- Build Status
Buildable 801 Build 801: arc lint + arc unit
Event Timeline
Comment Actions
Here is why Animation Nodes would benefit from this patch:
AN has many different data types. Most types have two "subtypes": single element and list.
Currently this information is encoded in the socket transparency: opaque means single element, and semitransparent means list.
So far this works quite well however since AN v2.0 a lot of nodes support vectorization. That means that some sockets allow single element and list input. Vectorized sockets currently have a 0.8 transparency.
So in total I currently have 3 different transparency values for different socket of the same type. Users can only see these differences when looking very closely, so a better solution is needed.
Being able to give vectorized sockets a different shape would make them much easier to identify.
I understand that most node systems might not need this but I believe that AN and other node systems can make good use of this patch in the future.
Comment Actions
Looks good, minor changes requested.
| source/blender/editors/space_node/node_draw.c | ||
|---|---|---|
| 661–692 | Mixing different shapes into the same arrays seems unnecessarily confusing. Why not have 3x arrays: shape_cirlce[][2] = {...};
shape_diamond[][2] = {...};
shape_square[][2] = {...};Then pass each as a single argument & length: node_socket_shape_draw(..., shape_diamond, ARRAY_SIZE(shape_diamond)); | |
| source/blender/makesdna/DNA_node_types.h | ||
| 114 | Why not make it a char ? | |
| source/blender/makesrna/intern/rna_nodetree.c | ||
|---|---|---|
| 7006 | Suggest call it draw_shape - to make it clear this is only for display (DNA also). | |
Comment Actions
Changes as requested by @Campbell Barton (campbellbarton).
API changed from shape to draw_shape.
e.g. self.newInput("Generic", "CIRCLE", "data").draw_shape = "CIRCLE"
Comment Actions
Minor change I missed last time otherwise LGTM.
| source/blender/makesdna/DNA_node_types.h | ||
|---|---|---|
| 148 | Best to make this zero, otherwise Python API won't properly be able to access the values, even if they draw. | |
Comment Actions
I have mixed feelings here, and we should have the UI team pitching it.
From the python perspective I see why people would want this kind of flexibility.
But I'm a bit wary of adding a new "language" element in the UI that is not used in Blender itself. Although the original patch wants to use this to show "list" sockets. What to prevent another python scripter to use this to something else altogether?
With colors at least Blender defines an initial guideline (yellow = color, blue = vector, green = closure), and scripts extrapolate from it.
In this case I'm afraid that having no guidelines will lead to a very non-intuitive user experience.
Comment Actions
@Dalai Felinto (dfelinto) I understand your concerns. However, I think that python developers who are able to develop a pynodes system, that will be used by more than just a few people, are able to use this wisely themselves.
Furthermore, node systems are used to archieve flexibility (in many areas, not just animation/materials/compositing/...), I don't see why the node editor itself should not be flexible just because Blender does not need this.
Comment Actions
Patch in itself LGTM
@Dalai Felinto (dfelinto) I see your point, but… then you can argue about that for pretty much our whole UI code - people can also do very unintuitive UI and give poor UX with mere placement of buttons, not to mention adding menus or panels in random places, doing crazy things in custom UILists, etc. Thing is, you'll always find ways to misuse a tool/API/feature, so I agree with @Jacques Lucke (JacquesLucke) here, can’t really see how that new possibility could hurt in itself.
And would think our potential future “object nodes” (generic naming :P ) could make use of some orthogonal way to coloring to convey further information about sockets, even if this is not currently needed for our 'official' node systems?
Comment Actions
Animation nodes show why a color based 'language' has its limits here. The range of colors the human eye/brain can see is simply not enough if much information has to be communicated through colors. From what I know we're talking about more than 60 different node socket types, so a need for 60+ different colors.
Also, when talking about using colors as language, we should always keep in mind that there are color blind people. IMHO something really important to keep in mind.
Animation nodes is admittedly an extreme example (and the only one that would benefit from this patch I guess), but I'd say it's still a valid one. It is one of the most used add-ons out there and adds an entirely new dimension to Blender. I'd also expect that a future object/modifier node system would hit the same limits of color language.
I'm in general a fan of guidelines, but I don't think we need any for this change. There are so many ways to do stupid things from .py... and the pynode system is rarely used by other add-ons.
So I'd say this change is totally reasonable.
Had a quick glance over code, seems fine too (just one minor suggestion).
Edit: Well, Bastien was faster than I and commented with similar points :)
| source/blender/editors/space_node/node_draw.c | ||
|---|---|---|
| 698 | Why not use ARRAY_SIZE like Campbell suggested? Makes things less error prone when changing array size. | |
Comment Actions
If we add a more advanced node system the concept of a list socket is probably something we'd need. Perhaps two concentric circles could better communicate "list", though that might be difficult to distinguish at a glance? In some other systems I've also seen this type of socket indicated in the name, like "vertex[]".
I don't have a strong opinion about this though.
Comment Actions
@Campbell Barton (campbellbarton) thank you for the code review and commit
Should this diff be closed or abandoned?
Comment Actions
Note that node socket drawing works differently in 2.8x, I had to drop these changes when merging.
@Charlie Jolly (charlie) - would you be able to look into getting this working in 2.8x?
Comment Actions
Hi @Campbell Barton (campbellbarton), I've added a patch for 2.8. See https://developer.blender.org/D2829. It wasn't as straightforward as I'd hoped. 😄