UnitPortrait's animation plays synchronously with unitResponse audios.

[bearU369]

UnitPortraits would play the talk animation as long as the unitResponse's audio being played, instead of playing the whole talk animation.

[Retera]

Seems like a good idea for a change; I'm surprised i had forgotten to do this already.

But I think it might be sort of spaghetti code if we put SequenceUtils.randomPortraitSequence(...) inside of MdxComplexInstance::updateAnimations. Currently, I usually had animation selection and management outside of MdxComplexInstance.

In the long term, there is a design conflict at play here because the MdxComplexInstance is originally a copy of Ghostwolf's viewer from the mdx-m3-viewer repo, and has viewer functions like setting the sequence loop mode. But as I struggled with emulating Warcraft 3, typically I use the MODEL_LOOP and then have animation management in a class that manages MdxComplexInstance from the outside, such as the class RenderUnit.UnitAnimationRenderListenerImpl.

I think that in the longer term, to more cleanly emulate Warcraft 3 the best thing might be if the behaviors of RenderUnit.UnitAniimationRenderListenerImpl where all or partially added to MdxComplexInstance. But currently if we did that, it would break all kinds of stuff.

Pending that sort of future refactor, for the time being I would be more tempted to think that the target animation length state information managing how long we play Portrait Talk should be inside of MeleeUI.Portrait class given the way that things are currently done. Looking back at it, it looked like that Portrait class is already checking for sequenceEnded on every frame update. I might suggest we change that Portrait-class-specific check to instead first check if the time elapsed since we started talking has reached the target animation length, use that to determine whether to include TALK secondary tag, and then fire call the call to play the animation if either our desired tags have changed or the animation ended.

If we manage these portrait animation decisions in the MeleeUI.Portrait class, then we also push the if(this.animationTargetDuration != 0) { check to somewhere that doesn't run for every model in the 3D world scene. So if we have 100 footmen onscreen, checking this for every one increases general case workload but if it's managed only for the portrait then we might minorly save performance, too.

[bearU369]

Pending that sort of future refactor, for the time being I would be more tempted to think that the target animation length state information managing how long we play Portrait Talk should be inside of MeleeUI.Portrait class given the way that things are currently done. Looking back at it, it looked like that Portrait class is already checking for sequenceEnded on every frame update. I might suggest we change that Portrait-class-specific check to instead first check if the time elapsed since we started talking has reached the target animation length, use that to determine whether to include TALK secondary tag, and then fire call the call to play the animation if either our desired tags have changed or the animation ended.

If we manage these portrait animation decisions in the MeleeUI.Portrait class, then we also push the if(this.animationTargetDuration != 0) { check to somewhere that doesn't run for every model in the 3D world scene. So if we have 100 footmen onscreen, checking this for every one increases general case workload but if it's managed only for the portrait then we might minorly save performance, too.

I didn't know that Portrait has its own update method within. Thanks, I'll be looking at it. I initially thought a timed animation would be something be used in the scripting aspect of the base game but after some refresher in the Trigger Editor, it's something that isn't supported AFAIK. So I guess there aren't needed much.

Basically how long portrait talk should play is by getting the milliseconds of the sound duration and have that animation on-loop until a certain time has elapsed over that milliseconds, to which we declare that the sequence has ended. I believe it's something similar you did for delays between portrait talks. Similar to what I already did in the previous commit except not worrying about the spaghetti code with random imports.

Moving the changes from MdxComplexInstance to Portrait should be easy and better for minimal overhead. 👍