Page MenuHome

improve mocap addon hierarchy mapping guessing for child bones and for common alternatives
AbandonedPublic

Authored by Luke Miller (dodgyville) on May 16 2018, 5:35 AM.

Diff Detail

Event Timeline

Luke Miller (dodgyville) updated this revision to Diff 14637.

updated to current master (April 2019)

I think nobody will review this as it is now. The diff doesn't even have a proper description, it doesn't explain which problem exists in the first place, or how that problem is solved now. It also doesn't describe where these "common alternatives" come from and how common they really are.

I think nobody will review this as it is now. The diff doesn't even have a proper description, it doesn't explain which problem exists in the first place, or how that problem is solved now. It also doesn't describe where these "common alternatives" come from and how common they really are.

Thanks for taking a look at it.

It exists because when automating autorigging, rather than manually assigning bones (and there can be dozens) it it's extremely useful to prefill. This diff extends the existing nameMatch method which already tries to prefill similar names by adding more options.

For common conversions I looked at a bunch of different rigs I've used over the years (Epic, Mixamo, Perception Neuron among others).

Sybren A. Stüvel (sybren) requested changes to this revision.Fri, Aug 9, 12:54 PM

Overall I would recommend following PEP8 for formatting & naming, at least for new code.

mocap/mocap_tools.py
693

It may be a good idea to add type declarations, such as def findBoneSide(bone: str) -> str and add a docstring that explains what the function does.

This is especially important since the function is now doing more; it's not just returning "Left" or "Right", but can return all kinds of strings.

697

I would recommend making this a bit nicer and declaring such patterns in a place that's easier to extend. For example, declaring at the top of the file (including clear comments on what they are, how they are used, and how they can be extended).

699

There is a strange combination of "case-insensitive" (by calling .lower()) and "case-sensitive" (by matching on patterns before calling .lower()) going on in this code. If this is intentional, this should be clarified in comments. Otherwise (and I'm guessing this is the case) it may be better to just use bone = bone.lower() and do everything in lower case.

725

There is no need to call .lower() all the time, if you just make the strings lower-case in the first place.

734

Magical values need to be documented well. Better to use enums instead.

738

Why is this 'robust'?

This revision now requires changes to proceed.Fri, Aug 9, 12:54 PM
mocap/mocap_tools.py
708

My comment on the above list applies here as well. It's better to have this outside the function, in a place that allows for proper documentation.

Overall I would recommend following PEP8 for formatting & naming, at least for new code.

Hi Sybren, I really appreciate you taking the time to review this. Unfortunately it looks like mocap_tools is not being ported to 2.80 so this patch looks orphaned now. I have also discovered auto-rig pro and will probably use that for this project.