sverchok icon indicating copy to clipboard operation
sverchok copied to clipboard

fix #4723

Open satabol opened this issue 2 years ago • 9 comments

fix #4723

now:

image

satabol avatar Nov 14 '22 20:11 satabol

Probably it's better to renames sockets instead (not easier though). Because such changes can potentially break layouts.

Durman avatar Nov 15 '22 04:11 Durman

@Durman But mode Edges->Is Border has an order socket functions like Faces->Is Border. The orders are same now. May be keep the order in Faces->Is Border is better?

satabol avatar Nov 15 '22 06:11 satabol

Without creating new version of the node I don't see too much choice but to rename sockets (even if order of names will be inconsistent with names in other modes). Or we can accept your proposal and hope that it wont break any layouts (I usually use Mesh Filter for the operation).

Durman avatar Nov 16 '22 04:11 Durman

@Durman Sorry. I did second commit accidental ))). I use Mesh Filter too. May be anybody saw that error and did not using it at all? When I was writing docs for that node I did not see that moment too and used mask for examples. I have not strong opinion for that situation.

satabol avatar Nov 16 '22 22:11 satabol

@Durman

  1. New version of node get no effect because problem inside external function and not in the node. I test if apply changes in code this will effect immediately after reload sverchok without any glitches in existing nodes. image

  2. Rename sockets is not good idea because sockets names defined externally bot not inside node. https://github.com/nortikin/sverchok/blob/e0f300e27cf98dc8ba80d353420458c432bbab02/utils/modules/polygon_utils.py#L488 So if create new version of node and rename sockets in this line then this will affect for both nodes - old and new nodes.

So I propose swap params in this pull request.

satabol avatar Aug 24 '23 19:08 satabol

If understand correctly your fix will swap socket outputs. Output of the first socket will go to the second one and vice versa. It's not good because can break existing layouts potentially. What if to rename outputs instead?

-'Is Boundary':        (70, 'vp', '',   '',  pols_is_boundary,      'sss', 'Mask, Boundary, Interior', 'Is the face boundary'),
+'Is Boundary':        (70, 'vp', '',   '',  pols_is_boundary,      'sss', 'Mask, Interior, Boundary', 'Is the face boundary'),

Durman avatar Aug 25 '23 17:08 Durman

@Durman Rename outputs (2) will harm any case - for old and new nodes. So swap outputs (1) is less evil. )

satabol avatar Aug 26 '23 07:08 satabol

Probably you don't understand something or me. The problem what I see is that when you swap sockets (socket 1 outputs data 2 and socket 2 outputs data 1) they start to output different data in old layouts. It means that in old layouts the node and all dependent nodes output one result. But when you open the layout after the fix the node and all dependent nodes will return another result. Ideally we want trees output the same data for all Sverchok versions. Renaming sockets does not have such effect. Socket names are just information for users.

Durman avatar Aug 27 '23 13:08 Durman

@Durman You are right. It is not good idea to rename sockets. But you can't rename sockets now. The core of problem is these sockets output names defined externally of analyzer node. This is very strange! Other nodes has no that feature. It is senseless to create new version of analyzer node. If you want new version of analyzer node you have to define new version of param "faces_modes_dict" and other params too (these params defined externally of analyzer node module). It is very hard to support that code in the future. Or if to place dependency params and functions for this node from "utils\modules\polygon_utils.py" to the "component_analyzer.py" then it has a sense to create a new version of analyzer node. But now one cannot create a new version "analyzer node" at all. ?

satabol avatar Aug 27 '23 15:08 satabol