Wrap VRC dependencies and VRCAvatarDescriptors #504
Conversation
| { | ||
| } | ||
|
|
||
| #endif |
There was a problem hiding this comment.
MEMO: should this go to ndmf? https://github.com/anatawa12/AvatarOptimizer/pull/627/files
There was a problem hiding this comment.
AvatarTagComponent was originally used as a workaround for the component whitelist prior to the introduction of IEditorOnly. It's unfortunately part of the public API at this point, but I don't want to recommend its use in newer tools.
|
Thanks for the PR! Sorry - I probably won't have time to review this properly until next week. I'll take a look when I find the time... |
Editor/Animation/AnimatorCombiner.cs
Outdated
| @@ -277,13 +281,15 @@ private void AdjustBehavior(StateMachineBehaviour behavior) | |||
| { | |||
| switch (behavior) | |||
| { | |||
| #if MA_VRCSDK3_AVATARS | |||
There was a problem hiding this comment.
Move this outside the switch for now
There was a problem hiding this comment.
Should I rather wrap entire AdjustBehavior() method and calls to it, or keep the AdjustBehavior() method empty?
There was a problem hiding this comment.
Leave the method empty for now, I think it's reasonable to keep the method call itself to avoid spreading #ifs all over the codebase
Editor/AnimatorMerger.cs
Outdated
| @@ -256,13 +260,15 @@ private void AdjustBehavior(StateMachineBehaviour behavior) | |||
| { | |||
| switch (behavior) | |||
| { | |||
| #if MA_VRCSDK3_AVATARS | |||
There was a problem hiding this comment.
Same here (hm, these classes seem duplicated...)
| @@ -1,4 +1,6 @@ | |||
| using System; | |||
| #if MA_VRCSDK3_AVATARS | |||
There was a problem hiding this comment.
I wonder if it would be better to move the menu processing (or rather, the VRChat-specific stuff) to a separate asmdef and make it conditionally built...
There was a problem hiding this comment.
I think breaking up asmdef would deserve a separate PR beforeafter this one. I am not certain enough how to move assets into folders, though. (at least I can try... everything VRCSDK under Runtime/VRChat and Editor/VRChat (or another name) I suppose?)
There was a problem hiding this comment.
Couldn't try because extracting VRChat asmdef required extracting PluginDefinitions asmdef, too much to include in this PR
There was a problem hiding this comment.
Fair enough. We can work on that in a followup (in any case, I don't think we can move components...)
Editor/Util.cs
Outdated
| using Object = UnityEngine.Object; | ||
|
|
||
| namespace nadena.dev.modular_avatar.core.editor | ||
| { | ||
|
|
||
| #if MA_VRCSDK3_AVATARS | ||
| internal class CleanupTempAssets : IVRCSDKPostprocessAvatarCallback |
There was a problem hiding this comment.
This is dead code and can be deleted.
There was a problem hiding this comment.
That would be another task before this one.
|
Sorry for how long the reviews took - and thanks! |
This is similar to bdunderscore/modular-avatar#504 and anatawa12/AvatarOptimizer#609 which are motivated by supporting non-VRChat avatars. There are no immediate plans on my end to add non-VRChat avatar support yet, but I intend to pull in code from these repositories where appropriate, and this would make syncing with upstream easier.
Related issue: #478
This PR wraps everything VRCSDK specific inside
#if MA_VRCSDK3_AVATARSand remove anyVRCAvatarDescriptorusage outside them (some leftover of #482).Note: Does not get rid of VRCSDK-specific runtime components (like
ModularAvatarPBBlockerorModularAvatarParameters) yet. It is more strategic than technical change in Runtime only, so I think it deserves a separate PR.Note: VRCSDK specific MA components in prefabs would be Missing. They are supposed to be removed on processing avatar.