Not all AnimModifiers are getting reverted when being removed from a batch of assets

Simply, the Animation Modifiers are not being reverted on all assets when removed from a batch of Anim Sequences. Only one of the Anim Sequences gets properly reverted.

I’ve fixed this in our local game instance like so (see sections marked “DROBINSON FIX”):

`FReply SRemoveAnimationModifierContentBrowserWindow::OnApply()
{
// Get user confirmation for action(s)
const bool bShouldRevert = FMessageDialog::Open(EAppMsgType::YesNo, LOCTEXT(“RemoveAndRevertPopupText”, “Should the Animation Modifiers be reverted before removing them?”), LOCTEXT(“RemoveAndRevertPopupTitle”, “Revert before Removing?”)) == EAppReturnType::Yes;
const bool bShouldRemove = FMessageDialog::Open(EAppMsgType::YesNo, LOCTEXT(“RemoveAnimModifiers”, “Are you sure you want to remove the anim modifiers from all the selected animation sequences?”), LOCTEXT(“RemoveAnimModifiersPopupTitle”, “Are you sure?”)) == EAppReturnType::Yes;

if (!bShouldRemove)
{
return FReply::Handled();
}

FScopedTransaction Transaction(LOCTEXT(“RemoveModifiersTransaction”, “Removing Animation Modifier(s)”));

TArray<UAnimationModifiersAssetUserData*> AssetUserData;

// Retrieve asset user data for selected sequences
for (UAnimSequence* AnimationSequence : AnimSequences)
{
// DROBINSON FIX PART 1
AssetUserData.Add(AnimationSequence->GetAssetUserData());
}

// For each added modifier create add a new instance to each of the user data entries, using the one(s) set up in the window as template(s)
for (int32 i = 0; i < Modifiers.Num(); ++i)
{
UAnimationModifier* Modifier = Modifiers[i];
checkf(Modifier, TEXT(“Invalid selected modifier”));

// Revert modifiers from anim assets
if (bShouldRevert)
{
// DROBINSON FIX PART 2
for (int a = 0, aMax = AnimSequences.Num(); a < aMax; ++a)
{
// DROBINSON FIX PART 3
if (AssetUserData[a] == nullptr)
continue;

// DROBINSON FIX PART 4
UAnimationModifier* const* ModifierInstanceInSequence = AssetUserData[a]->GetAnimationModifierInstances().FindByPredicate([Modifier](const UAnimationModifier* TestModifier)
{
return Modifier->GetClass() == TestModifier->GetClass();
});

if (ModifierInstanceInSequence)
{
// DROBINSON FIX PART 5
UAnimSequence* AnimSequence = AnimSequences[a];
(*ModifierInstanceInSequence)->RevertFromAnimationSequence(AnimSequence);

// Revert can not fail, thus we can always mark reverted
if ((*ModifierInstanceInSequence)->HasLegacyPreviousAppliedModifierOnSkeleton())
{
checkf(AnimSequence->GetSkeleton() != nullptr, TEXT(“Invalid skeleton for anim sequence”));
(*ModifierInstanceInSequence)->RemoveLegacyPreviousAppliedModifierOnSkeleton(AnimSequence->GetSkeleton());
}
}
}
}

// Remove modifiers from user assets
for (UAnimationModifiersAssetUserData* UserData : AssetUserData)
{
// DROBINSON FIX PART 5
if (UserData == nullptr)
continue;

UAnimationModifier* const* ModifierInstanceInUserData = UserData->GetAnimationModifierInstances().FindByPredicate([Modifier](const UAnimationModifier* TestModifier)
{
return Modifier->GetClass() == TestModifier->GetClass();
});

if (ModifierInstanceInUserData)
{
UserData->Modify();
UserData->RemoveAnimationModifierInstance(*ModifierInstanceInUserData);
}
}
}

if (WidgetWindow.IsValid())
{
WidgetWindow.Pin()->RequestDestroyWindow();
}

return FReply::Handled();
}`

Steps to Reproduce

  1. Select multiple AnimSequence assets in the Content Browser and select “Animation Modifier(s) > Add Modifiers” from the context menu
  2. Add a MotionExtractorModifier with the “Remove Curve On Revert” option selected and apply it to the selected AnimSequence assets.
  3. Apply the Animation Modifiers by selecting “Animation Modifier(s) > Apply Modifiers” from the context menu of the selected AnimSequence assets.
  4. Verify that the selected animations now have a “root_translation_Y” track added to them.
  5. Multi-select the animation assets from the Content Browser and select “Animation Modifier(s) > Remove Modifiers” from the context menu
  6. Select the MotionExtractorModifier Animation Modifier from the list and then apply to the selected assets, answering Yes to the question “Should the Animation Modifiers be reverted before removing them?” question and the “Are you sure?” question.
  7. Open the animation assets and note that the “root_translation_Y” curve was only removed from one of them.

Hi David,

Thanks for the detailed information and code diff, I’ve logged UE-298399 to ensure this gets integrated into 5.7!

Cheers,

Jurre