CreateAnimation causes new animation to be unsavable due to notifiers having source animation as Outer

Development Character & Animation
, ,
1

See repro steps

2

Steps to Reproduce
CreateAnimation causes new animation to be unsavable due to notifiers having source animation as Outer.

  1. Import animation
  2. Add a notifier that has a UObject (e.g. Resume Clothing Sim)
  3. Run the provided code (works the first time, since I use StaticDuplicateObject
  4. Run the provided code again
  5. Notice you cannot save the newly created animation, since it complains about a private outer.
  6. Inspect in the debugger and notice the outer object for the notifier is the original animation instead of the copy

If there is a better way of creating a new animation based on an existing one, let me know.

Destroying the old one before StaticDuplicate is not an option, since it causes issues with references to it.

In 5.4 I could just StaticDuplicate all the time, but 5.5. added a check due to some refcount of a subobject.

void ATestActor::TestCreateAnimation(UAnimSequence* OrigSequence)
{
  //FString SourceAnimationPath = OrigSequence->GetPathName();
  FString CopyName = OrigSequence->GetName() + TEXT("_Copy");
  FString CopyPath;
  FName HashedName;

  UPackage* OrigPackage = OrigSequence->GetPackage();
  //UPackage* OrigPackage = LoadPackage(nullptr, *SourceAnimationPath, ELoadFlags::LOAD_Async);
  CopyPath = OrigPackage->GetPathName();
  CopyPath.RemoveFromEnd(OrigSequence->GetName());
  CopyPath.Append(CopyName);

  HashedName = FName(*CopyName);
  UPackage* DestinationPackage = CreatePackage(*CopyPath);

  UAnimSequence* SequenceCopy = Cast<UAnimSequence>(StaticLoadObject(UAnimSequence::StaticClass(), DestinationPackage, *CopyName));

  if (SequenceCopy)
  {
    SequenceCopy->CreateAnimation(OrigSequence);
  }
  else
  {
    SequenceCopy = DuplicateObject<UAnimSequence>(OrigSequence, DestinationPackage, HashedName);
  }
}
3

Hi, thanks for flagging this. It looks like the way that UAnimSequence::CreateAnimation is attempting to copy the notifies isn’t valid. This has likely been broken for quite some time. We actually have a helper function to allow for copying of notifies between sequences which should give the correct behaviour. Can you try changing CreateAnimation to the following and let me know whether that works for you:

bool UAnimSequence::CreateAnimation(UAnimSequence* Sequence)
{
	if(Sequence)
	{
		ResetAnimation();
 
		CopyDataModel(Sequence->GetDataModelInterface());
		Controller->SetModel(DataModelInterface);
		UE::Anim::CopyNotifies(Sequence, this, false);
		AnimNotifyTracks = Sequence->AnimNotifyTracks;
 
		Controller->NotifyPopulated();
 
		return true;
	}
	
	return false;
}
4

Just a quick update on this. I was looking at checking in the change into 5.7 and I realized there was a mistake in the code I provided. The updated function should actually look like this:

bool UAnimSequence::CreateAnimation(UAnimSequence* Sequence)
{
	if(Sequence)
	{
		ResetAnimation();
 
		CopyDataModel(Sequence->GetDataModelInterface());
		Controller->SetModel(DataModelInterface);
		Controller->NotifyPopulated();
 
		UE::Anim::CopyNotifies(Sequence, this, false);
 
		return true;
	}
	
	return false;
}

That should get things working for you.

5

Seems to work, though we prefer not to do source changes.

The code below seems to do the same for me, but let’s me avoid source change.

		SequenceCopy->CreateAnimation(OrigSequence);
		SequenceCopy->DeleteNotifyTrackData();
		UE::Anim::CopyNotifies(OrigSequence, SequenceCopy, false);

Thank you for the help =)

6

Yeah, that makes sense if you don’t want to make source modifications. I’ve committed the fix for 5.7, so you should be able to remove your custom code if/when you upgrade. Thanks again for flagging this.