mottosso / cmdx

Fast and persistent subset of maya.cmds
https://mottosso.com/cmdx
BSD 2-Clause "Simplified" License
193 stars 36 forks source link

cmdx.listRelatives always returns None #20

Open vonbehr opened 4 years ago

vonbehr commented 4 years ago

Steps to reproduce:

  1. Create cube primitive.
  2. run `import cmdx import maya.cmds as cmds

print cmdx.listRelatives("pCube1") print cmds.listRelatives("pCube1")`

cmds.listRelatives returns [u'pCubeShape1'] while cmdx.listRelatives("pCube1") returns None

Example from the command reference also returns false and not true.

cmdx 0.4.6 Maya 2020.2 Windows 10

mottosso commented 4 years ago

Thanks for reporting this @fvbehr, are you able to spot what's going on?

vonbehr commented 4 years ago

No, I am afraid that debugging this is beyond my skill level.

mottosso commented 4 years ago

For the time being, you can do:

list(cmdx.encode("pCube1").descendents())

For hierarchy, or any of the other options here.

mottosso commented 4 years ago

Could it be that for cmds.listRelatives the shapes=True argument defaults to True? If you pass shapes=True does it return values as you expect? It's False here in cmdx, apparently. It should act like Maya's cmdx.listRelatives however, so if this is the case then that's a bug.

vonbehr commented 4 years ago

Testsetup:

cmdx.listRelatives("pCube1") returns None cmds.listRelatives("pCube1") returns [u'pCubeShape1']

cmdx.listRelatives("pCube1", shapes=True) returns None cmds.listRelatives("pCube1", shapes=True) returns [u'pCubeShape1']

cmdx.listRelatives("pCube1", shapes=False) returns None cmds.listRelatives("pCube1", shapes=False) returns [u'pCubeShape1']

cmdx.listRelatives("group1", shapes=True) returns None cmds.listRelatives("group1", shapes=True) returns None

cmdx.listRelatives("group1") returns None cmds.listRelatives("group1") returns [u'pCube1']

cmdx.listRelatives("group1", allDescendents=True) returns None cmds.listRelatives("group1", allDescendents=True) returns [u'pCubeShape1', u'pCube1']

It looks like cmdx always returns None. No mater what flags are used.

vonbehr commented 4 years ago

Could it be that for cmds.listRelatives the shapes=True argument defaults to True?

The shapes=True flag only returns shape nodes. It defaults to False and then returns all nodes, including shapes.

mottosso commented 4 years ago

I think I see the problem.

It's expecting a cmdx.DagNode whereas we are passing a string. Yes, I think we should either add support for automatically converting strings to cmdx objects, or you can..

cmdx.listRelatives(cmdx.encode("pCube1"))

Normally, all commands from cmdx return cmdx objects, like cmdx.createNode and even cmdx.listRelatives. But it's true, it would be convenient to mimic Maya's cmds more closely here. If so, it would be a matter of adding this to listRelatives.

if isinstance(node, str):
  node = encode(node)

That would handle the conversion automatically and internally. What do you think?

vonbehr commented 4 years ago

I would prefer if it would mimic Maya's cmds.

I added the if statement to listRelatives.

cmdx.listRelatives("pCube1") returns None while cmdx.listRelatives("pCube1", shapes=True) returns [|group1|pCube1|pCubeShape1], the whole hierarchy cmdx.listRelatives("pCube1", children=True) returns [ ] and cmdx.listRelatives("group1", children=True) returns [|group1|pCube1]

So its not returning the same as cmds.

What values can I pass as the type argument? It expects an int. All values except 0 seem to return an empty list.

mottosso commented 4 years ago

It's expecting an cmdx type, like cmdx.kJoint. It's one of these, alternatively from the Maya API directly for more choice, here e.g. om.MFn.kJoint.

The reason for not taking a string like cmds is performance.

returns [|group1|pCube1|pCubeShape1], the whole hierarchy

It's actually returning a cmdx.DagNode which when you print it calls node.path(). Nodes in cmdx are absolute references that doesn't break when they move or change name. Try calling node.name() instead for the more familiar short path.

vonbehr commented 4 years ago

It's actually returning a cmdx.DagNode which when you print it calls node.path().

Right. I missed that. My bad. I think the long path is preferably anyway.

mottosso commented 4 years ago

If you'd like, it would be great with a PR containing the changes you'd like to see added, then we can chat about specifics from there and also ensure the tests are all happy (performance included).

vonbehr commented 4 years ago

Will do.

chelloiaco commented 9 months ago

yoink 🙂

chelloiaco commented 9 months ago

@mottosso another small change I think could help with this issue is to instead of __repr__ returning the path for the node, (which we are used to being a string), perhaps have it return the type of the object + the path. Something like

<cmdx.DagNode : |pCube1|pCubeShape1>

That way the user will know right off the bat that the returned value from a cmdx.listRelatives is not and cannot be treated like a string.

Of course, they could still get the self.path in string format if they str(node) it. What do you think? Would this be too disruptive, backwards compatibility-wise?

mottosso commented 9 months ago

Hm, I remember going back and forth on this. It's possible that if you pass a cmdx.DagNode to a maya.cmds command, that Maya would treat it as a string via repr. It's worth a shot though, the tests should cover any backwards compatibility issues. You can try this locally already, and run the tests locally. If those pass, I see no reason why not. Nobody would be surprised by repr returning too much information I think.

chelloiaco commented 9 months ago

It's possible that if you pass a cmdx.DagNode to a maya.cmds command, that Maya would treat it as a string via repr

That is a good point. It wouldn’t be ideal to have this functionality lost.

I’ll see what I can do and make a PR or just comment what I find out here.

chelloiaco commented 9 months ago

It's possible that if you pass a cmdx.DagNode to a maya.cmds command, that Maya would treat it as a string via repr

I tested the following here (on the master branch, with no changes) and got the following output:

node = cmdx.createNode("transform")  
cmds.ls(node)
# Error: TypeError: file <maya console> line 1: Object |transform1 is invalid # 

cmds.addAttr(node, ln='test')
# Error: TypeError: file <maya console> line 1: Object |transform1 is invalid # 

However, the following works, since the node does exist.

cmds.ls(str(node))
# Result: ['transform1'] # 

cmds.ls(repr(node))
# Result: ['transform1'] # 

cmds.addAttr(repr(node), ln='test')
cmds.objExists(repr(node) + '.test')
# Result: True # 

Does the same happen there on your end?

I can't tell exactly why this happens, since cmds should try to call the __str__ or __repr__ from whatever it's given. I tested this theory with a very simple class and it confirms:

class MyClassRepr(object):
    def __init__(self, name):
        self.name = name

    def __repr__(self):
        return self.name

class MyClassStr(object):
    def __init__(self, name):
        self.name = name

    def __str__(self):
        return self.name
cmdx.createNode('transform')
# Result: |transform1 # 

test_repr = MyClassRepr('transform1')
type(test_repr)
# Result: <class '__main__.MyClassRepr'> # 

test_str = MyClassStr('transform1')
type(test_str)
# Result: <class '__main__.MyClassStr'> # 

cmds.ls(test_repr)
# Result: ['transform1'] # 

cmds.ls(test_str)
# Result: ['transform1'] # 

Of course this is a lot simpler, but it is unclear to me why it wouldn't work with cmdx.DagNode (or cmdx.Node for that matter). Could this be a bug?

mottosso commented 9 months ago

That was interesting, I manage to figure out that this is what's causing it to fail.

class MyClassStr(object):
    def __init__(self, name):
        self.name = name

    def __str__(self):
        return self.name

    def __getitem__(self, key):
        return True

node = MyClassStr("multMatrix1")
cmds.ls(node)
# # TypeError: Object multMatrix1 is invalid

This is what we use to access["attributes"]. I tried finding a way around it, but there doesn't seem to be one? 🤔

With that in mind, it would be safe to expand on __repr__ to return more info.