We are seeing all the AnimNotifies of the AnimMontage getting fired when loop occurs. This didn’t happen back in UE4 but it’s now happening in UE5.5. We’ve found some differences in loop data.
In FAnimMontageInstance::SetSequencerMontagePosition(), the following function is called twice on loop before the notifies are evaluated. It happens both in UE4 and UE5 but there seems to be a change that makes the values differ between engine versions.
UE4 - Treated as NotMoved in FMontageSubStepper::Advance()
- ForcedNextFromPosition=41.39f - ForcedNextToPosition=41.40f
- ForcedNextFromPosition=0.0f - ForcedNextToPosition=0.0f
UE5 - Treated as Moved in FMontageSubStepper::Advance()
- ForcedNextFromPosition=41.39f - ForcedNextToPosition=41.40f
- ForcedNextFromPosition=0.0f - ForcedNextToPosition=0.12f
NotMoved value makes the Notifies not to be gathered, whereas the Moved value makes them gather from 41.39f to 0.12f, taking ALL of AnimNotifies in the Montage.
UE5’s ForcedNextPosition seems to be the correct behaviour, where the remaining DeltaTime after the loop happened is still used for animation consistency. The problem here is that the Notify looks that it should be gathered twice, once per SetNextPositionWithEvents call, rather than taking the whole range. This issue didn’t affect UE4 as the remaining DeltaTime wasn’t used, so the second SetNextPositionWithEvents was treated as a 0.0f -> 0.0f move (NotMoved), avoiding the Notifies to be evaluated.
MontageInstanceToUpdate->SetNextPositionWithEvents(InFromPosition, InToPosition);
Hi, thanks for reporting this. As far as I’m aware, this isn’t a bug that we’ve run into previously. There are multiple issues going on with this, which is why it’s taken a while to dig into. I can see that this is a behaviour change since 4.27 as you mentioned. And it does seem to be because Sequencer will now perform two updates to the dynamic montage when the section loops which exposes pre-existing issues with the montage code.
The specific reason that all the notifies are being sampled is because we aren’t accounting for ForcedNextFromPosition in the FAnimMontageInstance::Advance code when we sample the notifies. And this bug also appears to affect sampling of root motion. In that method, the previous update time is stored in PreviousSubStepPosition. That is always set to the update time from the previous call to FAnimMontageInstance::Advance. And although the Sequencer code is calling SetNextPositionWithEvents twice, it isn’t advancing the montage twice. So we end up in a situation where PreviousSubStepPosition is the previous update from the last frame the montage was advanced (prior to the loop) and the new update time (Position) is ForcedNextToPosition.
Unfortunately, the Montage code isn’t designed to deal with previous and current position times that have looped like this. There’s an assumption (which is true with regular montage playback code) that when the montage loops, the playback will be clamped on the final frame of the section. And then playback will start from the first frame of the section the next time that Advance is called. What Sequencer is doing (and really any calls to SetNextPositionWithEvents) can break this assumption. That causes code like FAnimMontageInstance::HandleEvents to think the montage is being played backwards (since the current position is smaller than the previous position). And that’s why you end up with all the notifies being sampled.
My current thinking is that the proper way to fix this is to have PreviousSubStepPosition in FAnimMontageInstance::Advance be set to ForcedNextFromPosition. As you’ve seen, with the two calls that Sequencer makes to SetNextPositionWithEvents when looping, that will set PreviousSubStepPosition to 0.0 when FAnimMontageInstance::Advance is called later in the frame. And that will mean that only the notifies from the start of the animation to ForcedNextToPosition are sampled.
Obviously, that still leaves a problem in that notifies at the end of the animation (the section of the delta time prior to the loop) can be missed. After some testing of the other Sequencer code path that plays animations directly on the mesh (rather than using dynamic montages) that also seems to suffer from this same problem where notifies at the end of the animation can be missed. So I think we need to do a larger piece of work at the Sequencer level to resolve that.
I’ve attached a patch with the changes that I described to change how PreviousSubStepPosition is set. Could you try integrating them to see if that at least prevents all of the notifies from being triggered?
Great, thanks for letting me know that resolved the issue for you.
Yeah, I’ve created this JIRA for the specific issue with PreviousSubStepPosition not being updated when FAnimMontageInstance::SetNextPositionWithEvents is called. I’ll also have to discuss with the Sequencer team what they want to do about the remaining issue (notifies that sit at the very end of the animation that could be missed in the frame that the loop happens). Any fix there will likely be more involved so it could take a while.
Hi, thanks for looking into this. We’ve integrated this patch into our project and notify bug looks to be fixed. Greatly appreciated. We’re going with this fix for now as it solves our current issues.
Is there any bug tracking link or official post we can follow for an official fix for this bug in a future engine version? Good to track for future merges.
Thanks again.