KybernetikGames / animancer

Documentation for the Animancer Unity Plugin.
63 stars 8 forks source link

AnimancerEvents are swallowed when time is synchronized #326

Closed nickjohnson-dev closed 6 days ago

nickjohnson-dev commented 7 months ago

Environment

Description

(Not sure if I would call it a bug, but it is a pitfall I finally found a workaround for. I wanted to call it to your attention @KybernetikGames in case there is a solution that could be built into the lib, and also wanted to get this out there for others who may run into it.)

When using a TimeSynchronizationGroup along with a ClipTransition that includes AnimancerEvents, any events that are caught between the gulf between the "stored time" and the "synced time" are lost during a sync. From a brief reading of the code it would happen with the new TimeSynchronizer as well, since it uses the same calculation for the time to sync to.

The workaround I finally found after a lot of tricky debugging around the events not firing, was to loop over the transition's events when time was synced successfully, and invoke any events that were caught in that gap.

var storedTime = this.CurrentTransition.State?.NormalizedTime ?? 0;
var didStoreTimes = this.StoreTimes();
this.Animancer.Play(this.CurrentTransition);

if (didStoreTimes && this.SyncTimes()) {
  var syncedTime = this.CurrentTransition.State?.NormalizedTime ?? 0;
  if (this.CurrentTransition.State?.Events != null) {
    this.CurrentTransition.State.Events.ForEach(e => {
      if (e.normalizedTime > storedTime && e.normalizedTime <= syncedTime) {
        e.callback?.Invoke();
      }
    });
  }
}

I am not sure if it would be appropriate for the goals of the library, but maybe a flag could be available on the TimeSynchronizationGroup/TimeSynchronizer to use something like state.MoveTime instead of setting the time directly.

If it does not align, I at least hope that this workaround is helpful for reference purposes if anyone else is running into a similar issue.

Hopefully I have covered the reproduction steps well enough above, but if not, I would be happy to provide more details.

Thanks for the great library, it has really enabled a completely transformed workflow for me on the game I'm building.

KybernetikGames commented 7 months ago

Have you tried changing the synchronizer to use MoveTime?

I can't remember why I didn't make it that way in the first place, but it would be a lot nicer than needing to manually go through all the events like that.

nickjohnson-dev commented 7 months ago

I am AFK for the evening and didn't get a chance to try modifying the synchronizer to use MoveTime myself. I will give it a shot tomorrow morning though and report back here.

nickjohnson-dev commented 7 months ago

Just got a chance to try it out and it seems to achieve the same effect as my looping through events externally. 🥳

Definitely a less leaky workaround.

I'll defer to you to post an official example of the code change if you would like, since I believe it is part of the Pro package's code.

Thanks again! 🙏

KybernetikGames commented 7 months ago

Did you need to do anything other than swap state.Time = ... to state.MoveTime(...)? The synchronizer scripts are in the Utilities folder which is included in Animancer Lite so it's fine to post your changes, but I figured it would be super straightforward.

Also, do you like the old TimeSynchronizationGroup or the new TimeSynchronizer system better? I'm planning on removing the old system because it seems rather clunky compared to the new one.

nickjohnson-dev commented 7 months ago

I didn't need to do anything complex, no. I just made this change:

// state.Time = NormalizedTime.Value * state.Length + deltaTime * state.EffectiveSpeed;
state.MoveTime(NormalizedTime.Value * state.Length + deltaTime * state.EffectiveSpeed, false);

I hadn't tried actually implementing the new Synchronizer until this afternoon after you asked the question. It took me just a bit to figure out how I needed to use it compared to the TimeSynchronizationGroup, but after understanding it, I agree it is a much more elegant solution.

I already have a very tightly tracked state machine for my character that was being used to select which AnimationSet/Transition should be active, and I was able to simply use that machine's state enum as the TimeSynchronizer Group and remove a lot of extra code.

KybernetikGames commented 1 month ago

Animancer v8.0 is now available for Alpha Testing and includes this fix..

KybernetikGames commented 6 days ago

Animancer v8.0 is now fully released.