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

Script Node #26

Open
7 of 8 tasks
ly29 opened this issue Feb 6, 2017 · 67 comments
Open
7 of 8 tasks

Script Node #26

ly29 opened this issue Feb 6, 2017 · 67 comments

Comments

@ly29
Copy link
Contributor

ly29 commented Feb 6, 2017

script_baby
So just started working on it.

Barely there but functional in the sense that you can load a script and it works.

Todo: (or what does not work)

  • Reset
  • Editing
  • Reload
  • UI
  • Reload on open
  • Reload from TextEditor
  • First line of text must be empty
  • Text file must be valid python identifier (in other words allow file.py and similar things)
@ly29 ly29 mentioned this issue Feb 6, 2017
32 tasks
@ly29
Copy link
Contributor Author

ly29 commented Feb 6, 2017

So a very basic, somewhat rough version working, no custom drawing or custom properties but as a basic thing for testing and developing node functions it works.

@ly29
Copy link
Contributor Author

ly29 commented Feb 6, 2017

Basic syntax as follows, use @node_script to designate the function you want to test. No extra options allowed right now. The function should behave exactly as full node, including load on start up as long as the text file exist.

@node_script
def add(a: Int = 0, b: Int = 1) -> Number
    return a + b

Caveats

  • The text file must have a name that is valid python module identifier.
  • First line of text file must be empty
  • Very rough ui
  • No special drawing
  • No custom properties
  • no multi mode
  • etc.

@ly29 ly29 mentioned this issue Feb 6, 2017
@ly29
Copy link
Contributor Author

ly29 commented Feb 6, 2017

This should speed up development speed

@ly29
Copy link
Contributor Author

ly29 commented Feb 6, 2017

Simple example

skarmavbild 2017-02-06 kl 10 53 05

Corrected example

import numpy as np

@node_script
def add(verts: Vertices = None, scale : Float = 1.0) -> Vertices:
    s= scale[0]
    return verts * np.array([s, s, s, 0])

@portnov
Copy link

portnov commented Feb 6, 2017

s = scale[0]

looks weird :)

@ly29
Copy link
Contributor Author

ly29 commented Feb 6, 2017

Yeah, there is a reason for that, sadly and it is result of an inconsistency between data and sockets.

Since we are using numpy I decided to wrap the socket data to np.array([value]) to make it consistent with incoming data from another node, which has a side effect in this case.

@ly29
Copy link
Contributor Author

ly29 commented Feb 6, 2017

Since if we would drop a list of then different scales into the node we should get 10 different list of data.

@portnov
Copy link

portnov commented Feb 6, 2017

In this case we will get a problem with specific nodes needing vectorization again. In fact, in your example node author have to make a decision of whether to get [0] or iterate by the array or...?
I think this decision must be moved into node settings or core code.

@zeffii
Copy link
Contributor

zeffii commented Feb 6, 2017

import numpy as np
import bmesh

def rxdata_from_bmesh(bm):
    v = np.array([v.co[:] for v in bm.verts])
    # e = np.array([[i.index for i in e.verts] for e in bm.edges])
    # p = np.array([[i.index for i in p.verts] for p in bm.faces])
    return v #, e #, p    

@node_script
def sn_icosphere(subdiv: Int = 2, diam: Float = 1.0) -> Vertices:
    bm = bmesh.new()
    bmesh.ops.create_icosphere(bm, subdivisions=4, diameter=diam, calc_uvs=False)
    return rxdata_from_bmesh(bm)

@ly29
Copy link
Contributor Author

ly29 commented Feb 6, 2017

While I agree that my example is deeply flawed there is no clean solution possible for matching every possible case.

I was somewhat hurried this morning and didn't make the choice how the scale should be understood, if a scaling each set of vertices separately or if a the scale should be understood per vertex. Both are possible understandings, and it is and issue to be resolved but not an impossible problem with the base system, but a choice that has to be made, and clearly shown.

@zeffii
Copy link
Contributor

zeffii commented Feb 6, 2017

i like to have control over how to deal with nested input, that wasn't a weakness of 0.59 imo..

@ly29
Copy link
Contributor Author

ly29 commented Feb 6, 2017

The node should have control, but ideally by declaring what type of data it wants.

@ly29
Copy link
Contributor Author

ly29 commented Feb 6, 2017

Anyway that is more of type issue and not a script node issue

@portnov
Copy link

portnov commented Feb 6, 2017

Imho the most flexible way would be to move this matching logic into node settings (common for all nodes). Maybe node could declare what kind of matching it wants by default. But the user could redefine it.

@ly29
Copy link
Contributor Author

ly29 commented Feb 6, 2017

the matching is outside of the node, the question is of what to match.
that is what the type system tries to solve, there are some problems there admittedly, but they are in fact not that big or impossible to solve.

@portnov
Copy link

portnov commented Feb 6, 2017

the question is of what to match.

My proposal is to move it to node settings. In current sverchok we have "list match" node; imho it will be more usefull and probably better from performance side to have the same settings in each node.

@zeffii
Copy link
Contributor

zeffii commented Feb 6, 2017

import numpy as np
import bmesh

def rxdata_from_bmesh(bm):
    v = np.array([v.co[:] for v in bm.verts])
    e = np.array([[i.index for i in e.verts] for e in bm.edges])
    # p = np.array([[i.index for i in p.verts] for p in bm.faces])
    return v, e # , p    

@node_script
def sn_icosphere(subdiv: Int = 2, diam: Float = 1.0) -> ([Vertices], [Edges]):
    bm = bmesh.new()
    bmesh.ops.create_icosphere(bm, subdivisions=4, diameter=diam, calc_uvs=False)
    return rxdata_from_bmesh(bm)   # even if this is wrapped in a list

gives an error

  File "svrx\nodes\output\mesh_out.py", line 37, in stop
    obj = make_bmesh_geometry(verts[:, :3], edges, faces,
IndexError: too many indices for array

being that all the faces are tris, that could be done with flat storage out of the box..

@ly29
Copy link
Contributor Author

ly29 commented Feb 6, 2017

There is converter for Sverchok style data in smesh in util.

@ly29
Copy link
Contributor Author

ly29 commented Feb 6, 2017

Hmm also vertices should be n,4 in shape. Will check in detail tomorrow. Not sure what goes wrong, I think that should work

@zeffii
Copy link
Contributor

zeffii commented Feb 6, 2017

it makes the vertices, but as soon as i try to connect Edges or Faces i get that error

@zeffii
Copy link
Contributor

zeffii commented Feb 6, 2017

oh I see, then rename it to SvPolygons ?

@ly29
Copy link
Contributor Author

ly29 commented Feb 6, 2017

Check the shapes of the arrays also

@zeffii
Copy link
Contributor

zeffii commented Feb 6, 2017

it makes sense to do a proper svrxdata_from_bm soon.

@ly29
Copy link
Contributor Author

ly29 commented Feb 6, 2017

Absolutely

@zeffii
Copy link
Contributor

zeffii commented Feb 6, 2017

i'll start reading the code closer now :)

@ly29
Copy link
Contributor Author

ly29 commented Feb 6, 2017

Ask if wonder anything

@ly29
Copy link
Contributor Author

ly29 commented Feb 6, 2017

Still many parts are a bit sketchy

@ly29
Copy link
Contributor Author

ly29 commented Feb 7, 2017

@zeffii
So rxdata_from_bm exists now

import bmesh
from svrx.util.geom import vectorize
from svrx.util.mesh import rxdata_from_bm

@vectorize
def make_icosphere(subdiv, diam):
    bm = bmesh.new()
    bmesh.ops.create_icosphere(bm, subdivisions=subdiv, diameter=diam, calc_uvs=False)
    return rxdata_from_bm(bm)

@node_script
def sn_icosphere(subdiv: Int = 2, diam: Float = 1.0) -> ([Vertices], [Edges], [Faces]):
    return list(make_icosphere(subdiv, diam))    

This is how a generator functions looks right now. As stated the problem is that there is no type that corresponds to a singular number but Float, Int corresponds to a list of numbers and the smallest part is an np.array with corresponding shape (n,) .

Is this to much complexity for the generator functions? It is a very common pattern to produce complexity and something that should be done more clearly. It could easily be wrapped into a decorator function. @vectorize basically does it already but use yield causing the return list(...) or we could create a type that actually corresponds to a singular number, which makes a lot of sense.

Many functions do not fall into the above generator pattern but need the whole list, in broadcastable format for many cases.

@ly29
Copy link
Contributor Author

ly29 commented Feb 7, 2017

btw: what's the deal with the requirement to have an empty line at the top of a sn?

Yes that goes together with injecting the standard header, however. Actually an extra ; there should fix that I see now. I wanted to keep the line number for errors.

https://github.com/Sverchok/Sverchok/blob/master/core/script.py#L49-L63

subdiv: IntP(max=5) = 2, ?

Something like that the syntax would become yes. UInt(max=5) = 2 would be nice

@zeffii
Copy link
Contributor

zeffii commented Feb 7, 2017

Still one would have confirm that numbers fall into the appropriate range inside of the node and/or the system.

yeah, first line of defence is nice to have -- (this may change)

start thinking about design docs for users:

  • When exposing variables to the node and its users here are a few tips
    • Figure out if your script's algorithm needs some min / max limits on certain properties.
    • If you know that values beyond a certain range will take very long to process, use a max on the socket, and test for the max inside your code. The socket's max only has effect on the value when the value comes from the slider.

@zeffii
Copy link
Contributor

zeffii commented Feb 7, 2017

I know it's early, but : https://github.com/Sverchok/Sverchok/wiki/ScriptNode-RX-cookbook

@zeffii
Copy link
Contributor

zeffii commented Feb 7, 2017

@ly29
Copy link
Contributor Author

ly29 commented Feb 7, 2017

Nice work!

@ly29 ly29 closed this as completed Feb 7, 2017
@ly29 ly29 reopened this Feb 7, 2017
@zeffii
Copy link
Contributor

zeffii commented Feb 7, 2017

Sn takes label from nodescript decorated function and not text-datablok name, i have mixed feelings about this

@ly29
Copy link
Contributor Author

ly29 commented Feb 7, 2017

Ideally the text name and function name should align, but to be clear perhaps the file name could be drawn in draw_buttons

@ly29
Copy link
Contributor Author

ly29 commented Feb 7, 2017

Added from script to node instructions in the wiki. That could easily be turned into a script...

@zeffii
Copy link
Contributor

zeffii commented Feb 7, 2017

let's be cheeky and disallow 'Text' as a filename :)

@zeffii
Copy link
Contributor

zeffii commented Feb 10, 2017

I think i'll temporarily add svrx trees to the sverchok operator in TextEditor. but eventually i'm sure we can add a kb combo that only triggers if a certain node tree is active/last used.

@zeffii
Copy link
Contributor

zeffii commented Feb 11, 2017

how about draw_buttons_ext gets a way to call some custom_draw function too, i'd like something like

def draw_buttons_ext(self, context, layout):
    layout_draw_weblink('SvRx Cookbook')   #
    layout_draw_simple_templates()    # maybe 10 scipts and a signature file with multiple templates.
    if self.custom_ext:
         self.custom_draw_buttons_ext(context, layout)

@ly29
Copy link
Contributor Author

ly29 commented Feb 11, 2017

I am for easily accessible and well commented templates/examples

@zeffii
Copy link
Contributor

zeffii commented Feb 11, 2017

right.. maybe add the link to the cookbook inside the signature templates file , no need to draw weblink in ext.

@ly29
Copy link
Contributor Author

ly29 commented Feb 11, 2017

Cook something together :)

@zeffii
Copy link
Contributor

zeffii commented Feb 11, 2017

ok.. for now the templates will have to drop the .py extention :)

@ly29
Copy link
Contributor Author

ly29 commented Feb 12, 2017

Dot py and basiclly any name should now via name mangling.
Unless you have file called say script.py and another called scriptpy but, honestly, just don't do that.

@ly29
Copy link
Contributor Author

ly29 commented Feb 21, 2017

Will add load error message using error module.

@ly29
Copy link
Contributor Author

ly29 commented Feb 22, 2017

syntaxerror

@ly29
Copy link
Contributor Author

ly29 commented Feb 22, 2017

Would be nice if one could highlight the error as well in the text editor

@zeffii
Copy link
Contributor

zeffii commented Feb 22, 2017

we can put the cursor on the line

# t is a reference to the text object.
t.current_line_index = line_index

and bpy.ops.text.select_line() is as far as i'm aware the only way to select text from python.

@zeffii
Copy link
Contributor

zeffii commented Feb 22, 2017

where's the emoticon for vomit.

@zeffii
Copy link
Contributor

zeffii commented Feb 22, 2017

i motion to resist doing that, and lobby instead for more TextEditor accessors in 2.8.

@ly29
Copy link
Contributor Author

ly29 commented Feb 22, 2017

where's the emoticon for vomit

🤢

i motion to resist doing that

👍

@zeffii
Copy link
Contributor

zeffii commented Feb 26, 2017

i'm missing something obvious here... but how do i make a controller script..

from svrx.typing import Int, ColorP
import bpy

@node_script
def set_color(edge_col: ColorP(default=(0.3, 0.2, 0.5)) = Required, num: Int = 2) -> Int:
    
    nodes = bpy.data.node_groups['NodeTree.002'].nodes
    
    nodes['View bm GL'].edge_color = edge_col
    nodes['View bm GL.001'].edge_color = edge_col
    nodes['View bm GL.002'].edge_color = edge_col
    
    return 2

here the output is not wanted.. and also the body doesn't get executed, also not sure why it's not accepting ColorP . a little bit lost

@ly29
Copy link
Contributor Author

ly29 commented Feb 26, 2017

Scriptnode doesn't accept properties yet, didn't want get into the whole creating classes thing there yet.

This works

import bpy

@node_script
def set_color(edge_col: Color() = Required):
    edge_col = edge_col[:3]
    nodes = bpy.data.node_groups['NodeTree.002'].nodes
    
    nodes['View bm GL'].edge_color = edge_col
    nodes['View bm GL.001'].edge_color = edge_col
    nodes['View bm GL.002'].edge_color = edge_col

script_control

@ly29
Copy link
Contributor Author

ly29 commented Feb 26, 2017

I doesn't stop them from existing but doesn't create them either

@ly29
Copy link
Contributor Author

ly29 commented Feb 26, 2017

Also nodes doesn't get evaluated unless they are connected.

one can have full node_func stateful in the text editor and they should appear under User in the menu. Just reference the file with a script node and it would work.

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

No branches or pull requests

3 participants