-
Notifications
You must be signed in to change notification settings - Fork 36
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
fix hang when adding nested compound attributes #38
Conversation
cmdx.py
Outdated
@@ -5258,6 +5258,7 @@ class Compound(_AbstractAttribute): | |||
Multi = None | |||
|
|||
def __init__(self, name, children=None, **kwargs): | |||
self.Fn = om.MFnCompoundAttribute() # see https://github.com/mottosso/cmdx/issues/5#issuecomment-717330454 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we make the class Fn = None
? One of these aren't necessary.
Also try and stick to 80 lines with if possible, which it can be here if we put the comment on the line above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, removed.
Actually I was wondering if we should keep declaring Fn
as currently (static for each attribute type), but always instantiate something like self.Fn = cls.Fn()
directly in _AbstractAttribute.__init__()
, but I'm not 100% sure this is strictly necessary outside for non-Compound attributes.
Also I'm not sure how to actually get to the type-specific Fn
from _AbstractAttribute
😄
Could we add just one test to this, to make sure it works on all Maya's, and that it won't break again in the future? |
I could, though last time I used tests wasn't very fun ;) Btw, I recently created wrappers for some APIs (eg playbackOptions), that are exposed as commands although to me they are much better suited as a class with properties (all it does is expose get/set accessors). |
Haha, I understand exactly. xD I've added a test just now.
Not for cmdx, that's more in the territory of PyMEL I think; i.e. all-encompassing mess. What's keeping you from using that? |
fixes #5