search

ousnius / BodySlide-and-Outfit-Studio

BodySlide and Outfit Studio, a tool to convert, create, and customize outfits and bodies for Bethesda games.
GNU General Public License v3.0
286 stars 63 forks source link

Implemented Symmetrizer #434

Closed sts1skj closed 2 years ago

sts1skj commented 2 years ago
sts1skj commented 2 years ago

I didn't try to implement a triangle symmetrizer. Maybe I should have.

sts1skj commented 2 years ago

This symmetrizer does something so natural and intuitive that it can be described with a single English word: symmetrizer. But the implementation is horribly messy. This pull request is 1500 lines of code! It's so complex that my programming instincts are strongly suggesting that there's something I could have done to make this code simpler. But I'm not seeing it right now. Sigh.

sts1skj commented 2 years ago

I've decided I need to do a better job separating the symmetrization options for a bone pair.

sts1skj commented 2 years ago

I have an idea for a way to simplify this code a little bit: extract the mask handling from MatchSymmetricVertices and FindVertexAsymmetries. Maybe I'll try it tomorrow.

sts1skj commented 2 years ago

Though the last commit had a net increase of 7 code lines, it also breaks functionality up into more functions, which is longer but simpler, so this last commit is overall a significant simplification.

Should the symmetrizer code be split out of OutfitProject.{h,cpp} into a separate unit, say Symmetrizer.{h,cpp}? Should it be pulled out of the OutfitProject class and put into its own class? (I'm leaning towards "no" for both of these questions.)

ousnius commented 2 years ago

Thanks for the work on this and the additional cleanup + code comments.

Should the symmetrizer code be split out of OutfitProject.{h,cpp} into a separate unit, say Symmetrizer.{h,cpp}? Should it be pulled out of the OutfitProject class and put into its own class? (I'm leaning towards "no" for both of these questions.)

Right now, I'm also leaning towards no. The OutfitStudio and OutfitProject sources are very large and they definitely need to be split up into multiple sources at some point, but that's a bigger project and not important now.

I will be trying out the symmetrizer myself. Provided I don't find any issues, would you think this is ready to merge?

sts1skj commented 2 years ago

Yes, I think it's ready to merge, assuming you find no issues and don't see anything that could be cleaned up or improved.

ousnius commented 2 years ago

@sts1skj I've given it a quick try, the feature is great and I couldn't find any crash or bug. It only took me a short while to understand that the dialog shows checkboxes for asymmetric features only (and once they're symmetric they're gone).

This is probably the most useful for sliders and bone weights. Since it's a new separate feature/dialog and likely won't break anything existing, I will merge it.