UAnimMontage::GetAnimNotifiesFromDeltaPositions doesn't get notifications from referenced anim sequences

Hello,

When using the following function to set the position of an anim single node instance manually

if (UAnimSingleNodeInstance* Instance = Mesh->GetSingleNodeInstance()) { Instance->SetPosition(Position, bFireNotifies); }I noticed that the audio notify located on the anim sequence base wasn’t being activated and that only the notifications present on the UAnimMontage asset were being used. I tested this with various notifies in the anim sequence asset and confirmed none triggered whilst the montage ones did.

on further investigation, i saw that the call in

UAnimSingleNodeInstance::SetPositionWithPreviousTime

for getting the notification contexts goes to the anim sequence base one only as it is not overwrittein for anim montages

UAnimSequenceBase * SequenceBase = Cast<UAnimSequenceBase> (CurrentAsset); if (SequenceBase) { SequenceBase->GetAnimNotifiesFromDeltaPositions(InPreviousTime, Proxy.GetCurrentTime(), NotifyContext); }However, when you look at the animcomposite class which inherits from the same AnimCompositeBase class as UAnimMontage does. You can see that it accounts for this when getting the anim notifies for the composite

void UAnimComposite::GetAnimNotifiesFromDeltaPositions(const float& PreviousPosition, const float & CurrentPosition, FAnimNotifyContext& NotifyContext) const { Super::GetAnimNotifiesFromDeltaPositions(PreviousPosition, CurrentPosition, NotifyContext); AnimationTrack.GetAnimNotifiesFromTrackPositions(PreviousPosition, CurrentPosition, NotifyContext); }

Is this expected behaviour in the UAnimMontage asset?

locally i have changed the code to override the function in the UAnimMontage with the following

`void UAnimMontage::GetAnimNotifiesFromDeltaPositions(const float& PreviousPosition, const float& CurrentPosition, FAnimNotifyContext& NotifyContext) const
{
// Get notifies from the montage tracks
Super::GetAnimNotifiesFromDeltaPositions(PreviousPosition, CurrentPosition, NotifyContext);

// Now we have the notifications from the anim montage tracks we also need to supply the notifies from the anim sequences being referenced
for (const FSlotAnimationTrack& SlotTrack : SlotAnimTracks)
{
// Check all anim track segments as if the positions are out of the range of the segment, then it will return early – saves checking twice
// Checking all segments instead of just the active one means we can catch any notifies we might skip over if the two positions start and end on two different animations
for (const FAnimSegment& Segment : SlotTrack.AnimTrack.AnimSegments)
{
SlotTrack.AnimTrack.GetAnimNotifiesFromTrackPositions(PreviousPosition, CurrentPosition, NotifyContext);
}
}
}`

and this seems to work.

i was just wondering if this could / should be integrated at the engine level rather than just a local change?

Steps to Reproduce
Use

GetAnimNotifiesFromDeltaPositionsand see no notifications are given back from the anim sequence of a montage in the FAnimNotifyContext& NotifyContext

or

if (UAnimSingleNodeInstance* Instance = Mesh->GetSingleNodeInstance()) { Instance->SetPosition(Position, bFireNotifies); }and take notice that animation notifies don’t fire if they are placed on the anim sequence asset and not the anim montage asset.

Hi, thanks for reporting this. It does look like this is a bug with the UAnimSingleNodeInstance implementation. I was curious about why playback of a montage via the regular animation blueprint codepath doesn’t suffer the same issue, and it’s because we have the following code in FAnimMontageInstance::HandleEvents:

`// Queue all notifies fired from the AnimMontage’s Notify Track.
{
// We already break up AnimMontage update to handle looping, so we guarantee that PreviousPos and CurrentPos are contiguous.
Montage->GetAnimNotifiesFromDeltaPositions(PreviousTrackPos, CurrentTrackPos, NotifyContext);

// For Montage only, remove notifies marked as ‘branching points’. They are not queued and are handled separately.
Montage->FilterOutNotifyBranchingPoints(NotifyContext.ActiveNotifies);

// Queue active non-‘branching point’ notifies.
AnimInstance->NotifyQueue.AddAnimNotifies(NotifyContext.ActiveNotifies, NotifyWeight);
}

// Queue all notifies fired by all the animations within the AnimMontage. We’ll do this for all slot tracks.
{
TMap<FName, TArray> NotifyMap;

for (auto SlotTrack = Montage->SlotAnimTracks.CreateIterator(); SlotTrack; ++SlotTrack)
{
TArray& CurrentSlotNotifies = NotifyMap.FindOrAdd(SlotTrack->SlotName);

// Queue active notifies from current slot.
{
NotifyContext.ActiveNotifies.Reset();
SlotTrack->AnimTrack.GetAnimNotifiesFromTrackPositions(PreviousTrackPos, CurrentTrackPos, NotifyContext);
Swap(CurrentSlotNotifies, NotifyContext.ActiveNotifies);
}
}

// Queue active unfiltered notifies from slot tracks.
AnimInstance->NotifyQueue.AddAnimNotifies(NotifyMap, NotifyWeight);
}`So there, we’re manually sampling the notifies on both the montage track and the dependent animation tracks.

As a result, I think that we should do something similar in UAnimSingleNodeInstance::SetPositionWithPreviousTime. So I would go with a slightly different approach to your fix, by adding the following code to that method:

`if (SequenceBase)
{
FAnimTickRecord TickRecord;
FAnimNotifyContext NotifyContext(TickRecord);
NotifyQueue.Reset(GetSkelMeshComponent());

SequenceBase->GetAnimNotifiesFromDeltaPositions(InPreviousTime, Proxy.GetCurrentTime(), NotifyContext);
// FIX START
if (UAnimMontage* Montage = Cast(SequenceBase))
{
for (auto SlotTrack = Montage->SlotAnimTracks.CreateIterator(); SlotTrack; ++SlotTrack)
{
SlotTrack->AnimTrack.GetAnimNotifiesFromTrackPositions(InPreviousTime, Proxy.GetCurrentTime(), NotifyContext);
}
}

if (NotifyContext.ActiveNotifies.Num() > 0)
{
// single node instance only has 1 asset at a time
NotifyQueue.AddAnimNotifies(NotifyContext.ActiveNotifies, 1.0f);
}
// FIX END

TriggerAnimNotifies(0.f);

}`The reason for going with that approach, rather than modifying UAnimMontage::GetAnimNotifiesFromDeltaPositions, is that because we’re already manually sampling the montage tracks in FAnimMontageInstance::HandleEvents, I think you’ll end up with duplicate notifies with your approach when playing back a montage via an animation blueprint.

We could also refactor the montage notify sampling code so that it was all done via the various GetAnimNotifiesFromDeltaPositions methods, and change FAnimMontageInstance::HandleEvents to not manually sample the montage and the animations, but that’s a bigger change, so it comes with more risk.

Another third option would be to change UAnimSingleNodeInstance::SetPositionWithPreviousTime to call into FAnimMontageInstance::Advance when dealing with a Montage asset, rather than just calling UAnimSequenceBase::SetPosition. That would then go down the same code path as with an anim blueprint that ends up calling into FAnimMontageInstance::HandleEvents. But again, that’s a riskier change.

So there are a few different options here that I’ll discuss with the dev team, and we’ll get one of them implemented.

Sounds great thank you!

I’ll pop the fix you suggested in locally until the correct change comes down line, thank you! :slight_smile:

No problem. I’ll leave this thread open for now just in case you run into any problems with the change. It’ll auto-close in a few weeks. I’ve also created a JIRA that you can track the status of the issue from to see when a fix goes in.