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.

Sign up for GitHub

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

feat: support merging humanoid bones with PBs in limited cases #1429

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: support merging humanoid bones with PBs in limited cases #1429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 43 additions & 14 deletions Editor/MergeArmatureHook.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ internal class
.PathMappings;

private HashSet<Transform> humanoidBones = new HashSet<Transform>();
private HashSet<Transform> mergedObjects = new HashSet<Transform>();
private readonly HashSet<Transform> prunePBsObjects = new();
private HashSet<Transform> thisPassAdded = new HashSet<Transform>();

internal void OnPreprocessAvatar(ndmf.BuildContext context, GameObject avatarGameObject)
Expand Down Expand Up @@ -217,7 +217,7 @@ private void MergeArmatureWithReporting(ModularAvatarMergeArmature config)

BuildReport.ReportingObject(config, () =>
{
mergedObjects.Clear();
prunePBsObjects.Clear();
thisPassAdded.Clear();
MergeArmature(config, target);
#if MA_VRCSDK3_AVATARS
Expand Down Expand Up @@ -303,7 +303,8 @@ private void MergeArmature(ModularAvatarMergeArmature mergeArmature, GameObject
private void RecursiveMerge(ModularAvatarMergeArmature config,
GameObject src,
GameObject newParent,
bool zipMerge)
bool zipMerge
)
{
if (src == newParent)
{
Expand All @@ -313,10 +314,9 @@ private void RecursiveMerge(ModularAvatarMergeArmature config,

if (zipMerge)
{
mergedObjects.Add(src.transform);
thisPassAdded.Add(src.transform);
}

bool retain = HasAdditionalComponents(src) || !zipMerge;
zipMerge = zipMerge && src.GetComponent<IConstraint>() == null;

Expand Down Expand Up @@ -367,10 +367,26 @@ private void RecursiveMerge(ModularAvatarMergeArmature config,
src.name = src.name + "$" + Guid.NewGuid();
}

src.GetOrAddComponent<ModularAvatarPBBlocker>();
mergedSrcBone = src;

if (zipMerge)
HashSet<Transform> childPhysBonesBlockedSet = null;

#if MA_VRCSDK3_AVATARS
src.GetOrAddComponent<ModularAvatarPBBlocker>();

if (physBoneByRootBone.TryGetValue(src.transform, out var pb)
&& !NotAffectedByPhysBoneOrSimilarChainsAsTarget(src.transform, newParent.transform))
{
childPhysBonesBlockedSet = new HashSet<Transform>(pb.ignoreTransforms);
}
else if (zipMerge)
{
prunePBsObjects.Add(src.transform);
}
#endif

// If we're zipping, and the current object is not being used for PBs, we can remove it later.
if (zipMerge && childPhysBonesBlockedSet == null)
{
PathMappings.MarkTransformLookthrough(src);
BoneDatabase.AddMergedBone(src.transform);
Expand All @@ -384,6 +400,8 @@ private void RecursiveMerge(ModularAvatarMergeArmature config,

if (zipMerge)
{
var reportedHumanoidBoneError = false;

foreach (Transform child in children)
{
if (child.GetComponent <ModularAvatarMergeArmature>() != null)
Expand All @@ -407,15 +425,26 @@ private void RecursiveMerge(ModularAvatarMergeArmature config,
// Also zip merge when it seems to have been copied from avatar side by checking the dinstance.
if (targetObject != null)
{
if (NotAffectedByPhysBoneOrSimilarChainsAsTarget(child, targetObject))
if (childPhysBonesBlockedSet != null
&& !childPhysBonesBlockedSet.Contains(child)
&& !child.TryGetComponent<ModularAvatarPBBlocker>(out _))
{
childNewParent = targetObject.gameObject;
shouldZip = true;
// This object is potentially impacted by the parent's physbones; is it humanoid?
if (!reportedHumanoidBoneError && humanoidBones.Contains(targetObject.transform))
{
// If so, fail the build, as we won't properly apply this to humanoid children.
BuildReport.LogFatal(
"error.merge_armature.physbone_on_humanoid_bone", new string[0], config);
reportedHumanoidBoneError = true;
}

// Don't move this child object
continue;
}
else if (humanoidBones.Contains(targetObject))
else
{
BuildReport.LogFatal(
"error.merge_armature.physbone_on_humanoid_bone", new string[0], config);
childNewParent = targetObject.gameObject;
shouldZip = true;
}
}
}
Expand Down Expand Up @@ -468,7 +497,7 @@ Transform FindOriginalParent(Transform merged)
*/
private void PruneDuplicatePhysBones()
{
foreach (var obj in mergedObjects)
foreach (var obj in prunePBsObjects)
{
if (obj.GetComponent<VRCPhysBone>() == null) continue;
var baseObj = FindOriginalParent(obj);
Expand Down
83 changes: 83 additions & 0 deletions UnitTests~/DuplicatePBStripping/DuplicatePBStripping.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
#if MA_VRCSDK3_AVATARS

using nadena.dev.modular_avatar.animation;
using nadena.dev.modular_avatar.core.editor;
using nadena.dev.ndmf;
using NUnit.Framework;
using UnityEngine;
using VRC.SDK3.Dynamics.PhysBone.Components;
using AvatarProcessor = nadena.dev.modular_avatar.core.editor.AvatarProcessor;

namespace modular_avatar_tests.DuplicatePBStripping
{
Expand Down Expand Up @@ -66,6 +70,85 @@ public void StripsExtraPBones_far()
// They should not be merged to preserve intentionally attached PhysBone, which is not copied from the avatar.
Assert.AreEqual(2, prefab.GetComponentsInChildren<VRCPhysBone>().Length);
}

[Test]
public void AcceptsHumanoidPB_OnTipBones()
{
var prefab = CreatePrefab("DuplicatePBStripping_HumanoidTip.prefab");
AvatarProcessor.ProcessAvatar(prefab);

var head = prefab.transform.Find("Armature/Hips/Spine/Chest/Neck/Head");
Transform subHead = null;

foreach (Transform t in head)
{
if (t.gameObject.name.StartsWith("Head$"))
{
subHead = t;
break;
}
}

Assert.NotNull(subHead);

Assert.AreEqual(1, subHead.childCount);
Assert.IsTrue(subHead.TryGetComponent<VRCPhysBone>(out _));
}

[Test]
public void RejectsHumanoidPB_OnInnerBones()
{
var prefab = CreatePrefab("DuplicatePBStripping_HumanoidInner.prefab");

var context = CreateContext(prefab);

// TODO - port to new animation API
context.ActivateExtensionContext<ModularAvatarContext>();
context.ActivateExtensionContext<AnimationServicesContext>();
var errors = ErrorReport.CaptureErrors(() => new MergeArmatureHook().OnPreprocessAvatar(context, prefab));
Assert.AreEqual(1, errors.Count);

var error = errors[0];
Assert.AreEqual("error.merge_armature.physbone_on_humanoid_bone", ((SimpleError)error.TheError).TitleKey);
}

[Test]
public void AcceptsHumanoidPB_OnInnerBones_WithPBIgnores()
{
var prefab = CreatePrefab("DuplicatePBStripping_HumanoidInner_Ignored.prefab");

AssertInnerBones(prefab);
}

[Test]
public void AcceptsHumanoidPB_OnInnerBones_WithPBBlocker()
{
var prefab = CreatePrefab("DuplicatePBStripping_HumanoidInner_PBBlocker.prefab");

AssertInnerBones(prefab);
}

private static void AssertInnerBones(GameObject prefab)
{
var errors = ErrorReport.CaptureErrors(() => AvatarProcessor.ProcessAvatar(prefab));
Assert.AreEqual(0, errors.Count);

var hips = prefab.transform.Find("Armature/Hips");
Transform subHips = null;
foreach (Transform t in hips)
{
if (t.gameObject.name.StartsWith("Hips"))
{
subHips = t;
break;
}
}

Assert.NotNull(subHips);
Assert.AreEqual(1, subHips.childCount);
Assert.IsTrue(subHips.GetChild(0).name.StartsWith("New Child$"));
Assert.IsTrue(subHips.TryGetComponent<VRCPhysBone>(out _));
}
}
}

Expand Down
Loading
Loading