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

first pass of keying a plug with pair blend or anim layer #72

Merged
merged 5 commits into from
Feb 28, 2022

Conversation

monkeez
Copy link
Collaborator

@monkeez monkeez commented Feb 21, 2022

First pass fix of #59

This should work with plugs in animation layers too (it walks the connections to find the correct plug to apply animation to).

Couple of caveats:

  • If the plug is constrained but doesn't already have a pairBlend it will just disconnect and replace the constraint.
  • I had to add a bunch of animBlend types to a tuple at the top of the file because for some reason type checking against MFn.kBlendNodeBase doesn't seem to work? Maybe you know how to solve?

I also wanted to ask about how setting an attribute with a dictionary should work. Currently if you have animation on an attribute and you assign a dict it will add to the keys:

# node["tx"] has a key on frame 0 and 5
node["tx"] = {10: 5, 15: -5}
# node["tx"] now has keys on 0, 5, 10, and 15

I was wondering if it would make more sense for that operation to wipe all the keys and then instead we could put that adding functionality on the += operator instead.

Thanks 😄

Copy link
Owner

@mottosso mottosso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooo, nice. :)

I was wondering if it would make more sense for that operation to wipe all the keys

That is a good question. We are assigning keys to frames. I can see how it would make sense to consider the frames the keepers of those values, and that you assign to those frames. In that context, it wouldn't make sense to then remove other frames that you did not assign to.

On the other hand, if we look at the channel as a whole, then assigning to a whole channel would imply that we're overwriting whatever was already there. And, since we do have that += operator to work with, it could make sense to separate these two.

I would be up for this, it seems sensible (and simpler).

Maybe you know how to solve?

Hm, did you try MObject.hasFn(MFn.kBlendNodeBase)? It's what cmdx.Node.isA() uses and should handle inheritance. At the very least we should document which type each of those IDs are.

cmdx.py Outdated
>>> parent = createNode("transform")
>>> _ = cmds.parentConstraint(str(parent), str(node))
>>> animBlend = ls(type="animBlendNodeBase")[0]
>>> node["tx"].findAnimatedPlug().plug() == animBlend["inputA"].plug()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this better be is rather than ==, as == would not compare the plugs but the plug values.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried changing it to is and it didn't work for some reason. Shouldn't == work here as I'm using the .plug() method to grab the underlying MPlug?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, you are right. Initially, plugs were reused such that their physical Python instance was stored in a global dictionary, similar to nodes. But, I think that got removed in favour of stability.. Easy to get some nasty crashes when accessing a plug that isn't valid anymore.

So is probably isn't going to work.. I tried finding whether there was another way to spot their equivalence, and there is __hash__ to enable storage of plugs in a dictionary. They just look at plug.path() which is fair; if their paths are the same, then we can trust they are the same. So I would check against .path() instead.

@mottosso
Copy link
Owner

Also, stellar job on those docstrings and doctests. Could not have written it better myself.

@monkeez
Copy link
Collaborator Author

monkeez commented Feb 22, 2022

Hm, did you try MObject.hasFn(MFn.kBlendNodeBase)?

Yeah, it doesn't work and I have no idea why, always returns false.

After looking at it more I'm not sure how to handle the += vs = operator thing for adding/editing keys when it comes to changing mod.set_attr(). So I think for now I'll leave the functionality as is to keep this PR light. Could circle back later after thinking of a nicer solution.

@mottosso
Copy link
Owner

About the MFn types.. it looks like it doesn't actually support the base set? 🤔

This one tells you which function sets are supported; and I would have expected to find Base there, but it's not. In that case, let's at least replace the node IDs with om.MFn.kBlendNodeAdditiveScale etc so it's readable. They should both work with the call to isA and input(type=).

@monkeez
Copy link
Collaborator Author

monkeez commented Feb 27, 2022

I don't think I've got anything else to add to this one for now!

@mottosso mottosso merged commit 814c7d9 into mottosso:master Feb 28, 2022
@mottosso
Copy link
Owner

Bam! Nice 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

Successfully merging this pull request may close these issues.

2 participants