Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sockets refactoring #3507

Merged
merged 21 commits into from
Sep 8, 2020
Merged

Sockets refactoring #3507

merged 21 commits into from
Sep 8, 2020

Conversation

Durman
Copy link
Collaborator

@Durman Durman commented Sep 2, 2020

Addressed problem description

#2772 #3483

Preflight checklist

Put an x letter in each brackets when you're done this item:

  • Code changes complete.
  • Code documentation complete.
  • Manual testing done.
  • Ready for merge.

this field should be overridden by other sockets
which would like to use quick links to nodes
which are most often used with them.
In this way it will be easier for other developers to understand
how to add quick links to their new socket types.
just add `bl_idname` of a node which should be quick linked.
core/sockets.py Show resolved Hide resolved
I did not find in `self.node.rna_type` any node properties

fad9759
@Durman
Copy link
Collaborator Author

Durman commented Sep 3, 2020

I'm eager to delete this code because sockets should not know about node types.

sverchok/core/sockets.py

Lines 227 to 231 in c69efa6

if self.bl_idname == 'SvStringsSocket':
if node.bl_idname in {'SvScriptNodeLite', 'SvScriptNode'}:
if not self.is_output and not self.is_linked and self.prop_type:
layout.prop(node, self.prop_type, index=self.prop_index, text=self.name)
return

Sockets should grant API in general and any (node) which would like to use sockets should use them via API and don't inject their parts of code into sockets classes.

It looks like I have a solution which can be useful not only for SNL nodes but most likely it will require MK2 version. I'll have a look what can be done.

make SNL node to use socket properties instead of its own
remove SNL code from socket module
avoiding socket initialization infinite recursion
and remove unused parts of code
now it more like node properties are exported
fix effected jsons
remove unused socket properties
Move out SNL node code from sockets module
and move responsibility of deep coping data into policy classes
@portnov
Copy link
Collaborator

portnov commented Sep 5, 2020

I'd say the next candidate for refactoring should be deprecation of fullList method (and replacing it with repeat_last_for_length). The problem with fullList is that it modifies the list in-place, and as far as I understand wide usage of this method has very big share of responsibility of why do we have to do deepcopy each time when transferring data in sockets. If we get rid of fullList, it seems to me (but still is to be confirmed) there will remain only a few places where we modify input data in-place; when we fix those, we will be able to disable deepcopy at last....
well, dreams... :)

@Durman
Copy link
Collaborator Author

Durman commented Sep 5, 2020

I'm usually using cycle from itertools to full fill data.
Also if this approach #3437 will be wider used it won't be the problem.

@portnov
Copy link
Collaborator

portnov commented Sep 5, 2020

For new nodes yes, but fullList is widely used in existing nodes, including list-manipulation nodes...

@Durman
Copy link
Collaborator Author

Durman commented Sep 6, 2020

Using node properties in socket and its own looks redundant. I think it would be possible to use only socket properties and custom draw feature sometimes. Most likely why ptop_name was created is to ability to add custom update function to property. At the present we could force an update function of socket property to call node update function if needed with next code:

sverchok/data_structure.py

Lines 920 to 939 in 979172d

def update_with_kwargs(update_function, **kwargs):
"""
You can wrap property update function for adding extra key arguments to it, like this:
def update_prop(self, context, extra_arg=None):
print(extra_arg)
node_prop_name: bpy.props.BoolProperty(update=update_with_kwargs(update_prop, extra_arg='node_prop_name'))
"""
# https://docs.python.org/3/library/functools.html#functools.partial
@wraps(update_function)
def handel_update_call(node, context):
update_function(node, context, **handel_update_call.extra_args)
handel_update_call.extra_args = dict()
for attr_name, data in kwargs.items():
handel_update_call.extra_args[attr_name] = data
return handel_update_call

This code will let to put extra arguments to update function for example a name of updated property.

class Socket

    def prop_update(self, context, prop_name=None):
         if hasattr(self.node, update_socket_prop):
             self.node.update_socket_prop(self, context, prop_name):
        else:
            updateNode()

    value: bpy.props.FloatProperty(update=update_with_kwargs(prop_update, 'value'))


class Node:

    def update_socket_prop(self, socket, context, prop_name):
        if prop_name == 'Values':
            ...
        elif prop_name == 'Another prop':
           ...

memorizing number of objects into socket field
@Durman
Copy link
Collaborator Author

Durman commented Sep 6, 2020

API of new socket creating will be next after the PR:

class Socket:
    color = (1,1,1,1)  # color of new socket (only mandatory attribute)
    quick_link_to_node = 'GenVectorsNode'  # id name of a node which will be connected by quick link 
    default_property: FloatProperty(update=update_from_socket)  # if use_prop is True and socket is input and socket is unconnected this property will be shown
    default_conversion_name = 'FieldImplicitConversionPolicy'  # if sockets need another default conversion

    def draw_proerty(): ...  # if socket property has advance UI like Vector or Color sockets have now

In most cases it will be enough.
All properties can be Python type or Blender property. Blender property will support individual values per instance. Python values are like class variables.

It is available now to use default property for Strings socket. It supports float and int types but other types can easily be added.

class Node:
    def sv_init():
        sock = self.inputs.new('SvStringsSocket', 'Numbers')
        sock.use_prop = True
        sock.default_property_type = 'int'
        scok.default_int_value = 10

Draw performance is better now it will be noticeable after 100 - 200 nodes in a tree.

@Durman Durman merged commit ff90949 into master Sep 8, 2020
@portnov
Copy link
Collaborator

portnov commented Sep 9, 2020

@Durman Please check, didn't this break existing blend files? I have a blend file where values of SvVerticesSocket's "built-in" properties (those with p.use_prop=True) are gone, but i'm not sure, maybe it is not related...

Well, actually i'm already sure. Here is the broken file.
solid_tests_15.zip
It opens fine with 85f6eaf, but on master it has a red node.

@Durman
Copy link
Collaborator Author

Durman commented Sep 9, 2020

Yes in did, now I realize it. I think I know how this can be fixed. I will open new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants