design proposition for components

[wougzy]

So i tried to implement some design for components (i use them a lot in my rigging scripts)

At the beggining i wanted to make a standalone class that would use cmdx. But the more I went further, the more I needed to modify the existing classes. that's why i also tried to implement them directly in cmdx

My biggest concern was the need to update how encode and ObjectSet should work with components. actually it's only designed to work with nodes. and i found it very tricky to work with deformers for example

Have a look at my last commit here https://github.com/wougzy/cmdx/commit/1d4aa2dc0d44cea127108652dbd1dceee7ed5a51

Check first how encode was updated. I made it more flexible but at the price of a very small check, so i'm not entirely sure about it. I also added a encodeList function that is faster than map(encode) (only if the list exceed 5 elements) and is more likely to work with components

I noticed how you had to deal with the legacy of MFnSet too. I modified that a bit to be able to use the api2 Fn. but i'm not sure about it. I should make some profiling to see if we have some performance benefits of using api2 when possible. in any case i made the ObjectSet compliant with components now (and even plugs)

see for example how easy it is to add vertex to some deformerSet :

dfm = cmdx.encode('cluster1')
points = cmdx.encode('pCube1.vtx[3]')
dfm.deformerSet.add(points)

print dfm.deformerSet.member().elements

i also added some deformer and shape to be able to cast components more easily. it's still very basic and there's probably a lot to do to implement the most useful functions of them. for now i kept it to the minimum before bringing more.

i liked the way you can smuggle data cache into Node. i took advantage of it by caching dagpath or iterators already built in Node instances.

[mottosso]

Thanks for this, it looks interesting!

About .encode(), spontaneously I feel we should use a different function for components, rather than complicate encode. Maybe..

cmdx.component("pCube1.ctx[3]")

Or, maybe piggyback on Mesh and..

pcube1 = cmdx.encode("pCubeShape1")
points = pcube1.vertex(3)

Or even..

pcube1 = cmdx.encode("pCubeShape1")
points = pcube1.vtx[3]

Generally I would want to avoid having to format and parse strings like "pCube1.vtx[3]". I know it's what cmds and MEL takes natively, but I can't see a good reason to stick with it.

I noticed how you had to deal with the legacy of MFnSet too.

Yes, that has to do with compatibility with Maya 2015.. but, that was a while, and I think it would be feasible at this point to consider dropping support for 2015-2017 and focus on 2018+. It would enable us to trim some of these things, which I think should really help clean up some of that _legacy = True branching you've got there.

[wougzy]

Yes i agree for encode. my first guess was to add some flag to change behaviour (like encode("pCube1.vtx[3], component=True))

But at some point i ran into difficulties with using different ways to encode all types of path. especially when you have to encode the result of cmds.sets(q=1) and you have possibly components in the result. i noticed that when i queried members of the class ObjectSet but it was because of the version compatibility.

That's why i wrote encodeList. it is designed to encode lists of anything like om.MSelectionList and its iterator are used for. so i guess leaving encode to only nodes is acceptable. but you cannot really use it with map(encode) with cmds commands like ls to process their results. (while encodeList could do it)

I understand the point on avoiding to parse strings like "pCube1.vtx[0]" intentionnaly but we have to find a way to process what cmds commands returns when we have to use them

About querying Mesh to get components, i began to make the attribute vtx a property that return a full Component object. but i didn't like the way that getitem function would spawn a second Component object with a single index. But maybe i can try some optimization on it so we wouldn't have to cast 2 Components each time you get the vtx property.

when talking about performance you wouldn't want to iterate over components by casting each time a new component object with single index. there is no real need to get a list of single component object except maybe to get a flatten list of string of them. that's why i didn't go there.

for the moment this one is implemented already :

pcube1 = cmdx.encode("pCubeShape1")
points = pcube1.vertex(3)

i'll give a second thought about the getitem of Component. there's probably some optimization to do there

[wougzy]

i guess my sample would be better like this (:

dfm = cmdx.encode('cluster1')
obj = cmdx.encode('pCube1')
dfm.deformerSet.add(obj.shape().vertex(3))
dfm.deformerSet.add(obj.shape().vertices((5, 6)))

print dfm.deformerSet.member().elements

[mottosso]

I understand the point on avoiding to parse strings like "pCube1.vtx[0]" intentionnaly but we have to find a way to process what cmds commands returns when we have to use them

Oh yes that is true.. For that reason, I think you're right about parsing that from encode. It would be the gateway from cmds to cmdx.

but i didn't like the way that getitem function would spawn a second Component object with a single index

Hm, could you elaborate on this?

mesh.vtx[1:5] # Vertices 1 to 5
mesh.vtx[1, 3, 5] # Vertices 1, 3 and 5

But maybe i can try some optimization

Just a note, there is also premature optimisation lurking about. I wouldn't worry about performance just yet; make it right, then make it fast. In this case, we want it right but also readable before looking at how to make it fast.

dfm.deformerSet.add(obj.shape().vertices((5, 6))) dfm.deformerSet.member().elements

There are many things that hurt my eyes here. xD

1. Nested namespace, i.e. dfm.deformerSet.add
2. Attribute on function-call, i.e. obj.shape().vertices
3. Nested parentheses i.e. vertices((5, 6)))

I think the advantage of using vertices[] and/or vtx[] is that it's clear we're accessing elements, whereas () suggests there may be keyword arguments. Anything with () to my mind should also be verbs, something that does something, like rotate() or .move(). The bracket-syntax also ties in nicely with how attributes are accessed I think.

The deformerSet should definitely be a function, but maybe not a function of Node.. Could it be a flat function, e.g. cmdx.deformerSet(node)? Hm, I'm not terribly excited about the dedicated classes per node type, that's going down the PyMEL route and there's no bottom there. I would rather have fewer classes and more flat functions. I think.

So deformerSet, is that just a shortcut of getting hold of another node, the objectSet node holding the vertices? If so, I think it definitely makes sense to keep that as a flat function, e.g. cmdx.findDeformerSet(node).

[wougzy]

ok ok i admit it, this sample is totally unreadable (: it was just an example how convenient it could be to write lines but that do not demonstrate how you would have to use it

well first i recoded the property vtx again. i added a simple cache optimization and it is really fast to query a single component (up to 9µs compared to the original 7µs i had from .vertex())

you can check the last commit here https://github.com/wougzy/cmdx/commit/df210b9ed5b275c156352dc4249947ea40c471a8

now this same sample can be written like this:

dfm = cmdx.encode('cluster1')
obj = cmdx.encode('pCube1')

shp = obj.shape()
dfm_set = dfm.deformerSet()

dfm_set.add(shp.vtx[3])
dfm_set.add(shp.vtx[4, 5])

I also removed the property type of .deformerSet. I agree on that point, methods should be verbs that represent actions. But that's why i was confused here. I thought i could add this king of naming like i found in DagNode. I really like the way to get parent, children and shape of DagNode directly from it (kinda like pymel) but in other packages these methods are often named .getParent(), .getShape() etc.. I just wanted to stick to your naming convention (: So just to sum up i added .deformerSet to Deformer class just like .shape or .children were added to DagNode class.

[mottosso]

I just wanted to stick to your naming convention (

I think you're right. I've never used/seen deformerSet before, so it looks more alien than parent/child. But if that's more or less what it resembles, than it probably makes the most sense to other users/readers too. I'll let you be the authority on this one as the one using this.

I like the latest code example, I would be happy merging something like that. PR on the way? :)

[wougzy]

haha i hope i won't be the only one to use it!

About the PR, should I leave all the _legacy stuff as it is? for now it's still working with maya 2015. I believe a lot of studio are not using it anymore but we never know (and we do haha, but only for very old shows and I don't intend to use cmdx there)

I'd like to add more stuff too (like remaining component types of mesh) should i finish this before or this can wait?

[mottosso]

About the PR, should I leave all the _legacy stuff as it is?

Hm, it is ugly. How about we leave compatibility to wherever you use cmdx at currently, what is that 2016? Worst case, yes let's leave it and make a PR specifically to cut out old cruft.

I'd like to add more stuff too (like remaining component types of mesh) should i finish this before or this can wait?

Let's keep PRs as small as possible; better merge too often than too seldom.

[wougzy]

I can remove these. It only concerns Maya 2015 anyway. I think people who are still using this version only run legacy stuff. so probably no new development. So yeah let's bury that if you prefer that way (:

[monkeez]

Just throwing it out there that I'm all for dropping support for Maya 2015-2017 🙂