Node group inputs and outputs have incorrect "name" property #95432

New Issue

Lloyd · 2022-02-02T12:18:28+01:00

Lloyd commented

2022-02-02 12:18:28 +01:00

System Information
Operating system: Linux-5.8.0-44-generic-x86_64-with-glibc2.31 64 Bits
Graphics card: Radeon RX 580 Series (POLARIS10, DRM 3.42.0, 5.8.0-44-generic, LLVM 12.0.1) AMD 4.6 (Core Profile) Mesa 21.2.0-devel

Blender Version
Broken: version: 2.93.7 3.01 3.1 3.2
Worked: (newest version of Blender that worked as expected)

Short description of error
When adding new Input or Output Sockets in the Group Node Properties Tab, the selected Input or Output Socket name is duplicated.
Two issues:

- the socket name is not unique (this can get problematic since entries in node.outputs are name based)
- design question: should the new name default to either "Input" or "Output"?
  Should Default to Input or Output. ( Only Defaults to Input or Output if there are no Sockets except the Default ones )

Exact steps for others to reproduce the error
In the Node Editor add Input or Output Sockets to a nodegroup in the sidebar, change the name and try adding more Inputs or Outputs, the System will assign the name of the selected sockets.

Seems like there are muliple functions handling new sockets

make_socket_interface [in affect when adding a new socket through the UI] gives a unique identifier (but copies the name exactly -- no uniqueness)
nodeAddSocket or make_socket take care of making the name unique as well

{F12842289 size=full}

{F12842292 size=full}

ref #91826 (just to show that unique names are important).

**System Information** Operating system: Linux-5.8.0-44-generic-x86_64-with-glibc2.31 64 Bits Graphics card: Radeon RX 580 Series (POLARIS10, DRM 3.42.0, 5.8.0-44-generic, LLVM 12.0.1) AMD 4.6 (Core Profile) Mesa 21.2.0-devel **Blender Version** Broken: version: 2.93.7 3.01 3.1 3.2 Worked: (newest version of Blender that worked as expected) **Short description of error** When adding new Input or Output Sockets in the Group Node Properties Tab, the selected Input or Output Socket name is duplicated. Two issues: - - [x] the socket name is not unique (this can get problematic since entries in node.outputs are name based) - - [x] design question: should the new name default to either "Input" or "Output"? Should Default to Input or Output. ( Only Defaults to Input or Output if there are no Sockets except the Default ones ) **Exact steps for others to reproduce the error** In the Node Editor add Input or Output Sockets to a nodegroup in the sidebar, change the name and try adding more Inputs or Outputs, the System will assign the name of the selected sockets. Seems like there are muliple functions handling new sockets - `make_socket_interface` [in affect when adding a new socket through the UI] gives a unique identifier (but copies the name exactly -- no uniqueness) - `nodeAddSocket` or `make_socket` take care of making the name unique as well {[F12842289](https://archive.blender.org/developer/F12842289/image.png) size=full} {[F12842292](https://archive.blender.org/developer/F12842292/image.png) size=full} ref #91826 (just to show that unique names are important).

Lloyd commented

2022-02-02 12:18:28 +01:00

Added subscriber: @LloydAlmeida

Philipp Oeser commented

2022-02-02 16:21:55 +01:00

Added subscriber: @lichtwerk

Philipp Oeser commented

2022-02-02 16:21:55 +01:00

Changed status from 'Needs Triage' to: 'Confirmed'

Philipp Oeser commented

2022-02-02 16:21:55 +01:00

Hm, not sure if it should default to "Input" or "Output" code pretty much looks like it wants to duplicate the name.
What is more concerning (I think) is that these dont get a new unique name.

Seems like there are muliple functions handling new sockets

make_socket_interface [in affect when adding a new socket through the UI] gives a unique identifier (but copies the name exactly -- no uniqueness)
nodeAddSocket or make_socket take care of making the name unique as well

ref #91826 (just to show that unique names are important).

I will add my concern to the report description (and leave the question of defaulting to "Input" / "Output" as a design question).

Hm, not sure if it should default to "Input" or "Output" code pretty much looks like it wants to duplicate the name. What is more concerning (I think) is that these dont get a new **unique** name. Seems like there are muliple functions handling new sockets - `make_socket_interface` [in affect when adding a new socket through the UI] gives a unique identifier (but copies the name exactly -- no uniqueness) - `nodeAddSocket` or `make_socket` take care of making the name unique as well ref #91826 (just to show that unique names are important). I will add my concern to the report description (and leave the question of defaulting to "Input" / "Output" as a design question).

Philipp Oeser changed title from ~~Geometry Nodes - Selected Input or Output name is duplicated for the New Input.~~ to Nodes Groups: adding new inputs or outputs will duplicate previous names (exactly, no uniqueness)

2022-02-02 16:29:53 +01:00

Hans Goudey commented

2022-02-02 16:41:58 +01:00

Added subscriber: @HooglyBoogly

Hans Goudey commented

2022-02-02 16:41:58 +01:00

The node socket names aren't expected to be unique. That's why there's a separate identifier for each socket.

I don't think adding a .001 to names or something is helpful. Builtin nodes can reuse the same name for multiple sockets, so I think node groups should be able to as well.

When adding new Input or Output Sockets in the Group Node Properties Tab, the selected Input or Output Socket name is duplicated.

It's not just the name that's duplicated, but everything except the identifier. That's just because otherwise the new input/output is totally arbitrary.
I don't think defaulting to "Input" or "Output" would be an improvement.

The node socket names aren't expected to be unique. That's why there's a separate identifier for each socket. I don't think adding a `.001` to names or something is helpful. Builtin nodes can reuse the same name for multiple sockets, so I think node groups should be able to as well. > When adding new Input or Output Sockets in the Group Node Properties Tab, the selected Input or Output Socket name is duplicated. It's not just the name that's duplicated, but everything except the identifier. That's just because otherwise the new input/output is totally arbitrary. I don't think defaulting to "Input" or "Output" would be an improvement.

Lloyd commented

2022-02-02 16:58:57 +01:00

Shouldn't the Socket also be added to the end of the list.

Hans Goudey commented

2022-02-02 17:24:21 +01:00

Shouldn't the Socket also be added to the end of the list.

It uses the same behavior as just about every list in Blender, adding right after the active element. I think it's better to be consistent here.

>Shouldn't the Socket also be added to the end of the list. It uses the same behavior as just about every list in Blender, adding right after the active element. I think it's better to be consistent here.

Lloyd commented

2022-02-02 18:23:21 +01:00

Quite a few lists ( Shape Keys, UV Maps, Vertex Colors, Face Maps ) have the Value added to the end not the Active element. ( Which would be the desired result of Adding something ) I like the way the Attributes are added, the user can choose from the popup. With GN Inputs / Outputs I find I have to almost always reorder, change the Type and figure which was the added Socket especially if adding more than one at a time.

Hans Goudey commented

2022-02-02 18:28:19 +01:00

Okay, hmm, looks like I remembered incorrectly about the ordering, my bad.
A popup sounds reasonable, though not that different than just adjusting the type and name afterwards honestly.

I'm not convinced any of this behavior is a bug though, since it's working "as expected."

Okay, hmm, looks like I remembered incorrectly about the ordering, my bad. A popup sounds reasonable, though not that different than just adjusting the type and name afterwards honestly. I'm not convinced any of this behavior is a bug though, since it's working "as expected."

Philipp Oeser commented

2022-02-02 18:46:48 +01:00

In #95432#1300229, @HooglyBoogly wrote:
The node socket names aren't expected to be unique. That's why there's a separate identifier for each socket.

I don't think adding a .001 to names or something is helpful. Builtin nodes can reuse the same name for multiple sockets, so I think node groups should be able to as well.

Hm, how can we ever distinguish these in other Editors then?
{F12842289 size=full}

Or from python?
{F12842292 size=full}

> In #95432#1300229, @HooglyBoogly wrote: > The node socket names aren't expected to be unique. That's why there's a separate identifier for each socket. > > I don't think adding a `.001` to names or something is helpful. Builtin nodes can reuse the same name for multiple sockets, so I think node groups should be able to as well. Hm, how can we ever distinguish these in other Editors then? {[F12842289](https://archive.blender.org/developer/F12842289/image.png) size=full} Or from python? {[F12842292](https://archive.blender.org/developer/F12842292/image.png) size=full}

Philipp Oeser commented

2022-02-02 18:49:29 +01:00

In #95432#1300198, @lichtwerk wrote:
Seems like there are muliple functions handling new sockets

make_socket_interface [in affect when adding a new socket through the UI] gives a unique identifier (but copies the name exactly -- no uniqueness)

nodeAddSocket or make_socket take care of making the name unique as well

and why do the other functions ensure uniqueness?

> In #95432#1300198, @lichtwerk wrote: > Seems like there are muliple functions handling new sockets > - `make_socket_interface` [in affect when adding a new socket through the UI] gives a unique identifier (but copies the name exactly -- no uniqueness) > - `nodeAddSocket` or `make_socket` take care of making the name unique as well and why do the other functions ensure uniqueness?

Lloyd commented

2022-02-03 11:15:13 +01:00

I haven't used it much, but from what I could see, the Add Attribute in the Object Data Properties does a few checks. User cannot add the same name for a Domain+Data Type. Also gives a warning.

I haven't used it much, but from what I could see, the Add Attribute in the Object Data Properties does a few checks. User cannot add the same name for a Domain+Data Type. Also gives a warning. ![Attr.png](https://archive.blender.org/developer/F12843431/Attr.png)

Hans Goudey commented

2022-02-07 21:20:20 +01:00

Added subscriber: @JacquesLucke

Hans Goudey commented

2022-02-07 21:20:20 +01:00

and why do the other functions ensure uniqueness?

They ensure uniqueness in the identifier, not the name. Sockets have a unique identifier, but not necessarily a unique name, that's on purpose. If the sockets need to be distinguished in an animation editor, they should be renamed.

There's one thing in your screenshots that looks wrong to me though, it's the Python API for retrieving sockets. I think that should be using the socket identifier, rather than the name, that looks weird to me.

Or from python?
{F12842292 size=full}

@JacquesLucke, am I missing something, or do you agree? In rna_def_node_socket, it looks like RNA_def_struct_name_property should be moved to the "identifier" property to fix this.

By the way, make_socket_interface makes node group inputs and outputs. While those also use bNodeSocket for storage, that's something we want to change, and they are conceptually different than sockets.

> and why do the other functions ensure uniqueness? They ensure uniqueness in the identifier, not the name. Sockets have a unique identifier, but not necessarily a unique name, that's on purpose. If the sockets need to be distinguished in an animation editor, they should be renamed. There's one thing in your screenshots that looks wrong to me though, it's the Python API for retrieving sockets. I think that should be using the socket identifier, rather than the name, that looks weird to me. > Or from python? > {[F12842292](https://archive.blender.org/developer/F12842292/image.png) size=full} @JacquesLucke, am I missing something, or do you agree? In `rna_def_node_socket`, it looks like `RNA_def_struct_name_property` should be moved to the `"identifier"` property to fix this. --- By the way, `make_socket_interface` makes node group inputs and outputs. While those also use `bNodeSocket` for storage, that's something we want to change, and they are conceptually different than sockets.

Philipp Oeser commented

2022-02-07 21:29:29 +01:00

In #95432#1303428, @HooglyBoogly wrote:
They ensure uniqueness in the identifier, not the name.

Oops, sorry, yes, my bad

> In #95432#1303428, @HooglyBoogly wrote: > They ensure uniqueness in the identifier, not the name. Oops, sorry, yes, my bad

Jacques Lucke commented

2022-02-08 11:34:38 +01:00

am I missing something, or do you agree? In rna_def_node_socket, it looks like RNA_def_struct_name_property should be moved to the "identifier" property to fix this.

I agree with that, it's a bit unfortunate. It's not something we can easily change without breaking lots of stuff.

> am I missing something, or do you agree? In rna_def_node_socket, it looks like RNA_def_struct_name_property should be moved to the "identifier" property to fix this. I agree with that, it's a bit unfortunate. It's not something we can easily change without breaking lots of stuff.

Hans Goudey commented

2022-02-14 07:09:47 +01:00

Okay, right, too bad. It would be hard to find all the places that depend on this behavior.

I'll convert this to a known issue then.

Okay, right, too bad. It would be hard to find all the places that depend on this behavior. I'll convert this to a known issue then.

Hans Goudey changed title from ~~Nodes Groups: adding new inputs or outputs will duplicate previous names (exactly, no uniqueness)~~ to Node group inputs have incorrect "name" property

2022-02-14 07:10:05 +01:00

Hans Goudey changed title from ~~Node group inputs have incorrect "name" property~~ to Node group inputs and outputs have incorrect "name" property

2022-02-14 07:10:13 +01:00

Philipp Oeser removed the

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Node group inputs and outputs have incorrect "name" property #95432