fantasycalendar / FoundryVTT-Sequencer

This module implements a basic pipeline that can be used for managing the flow of a set of functions, effects, sounds, and macros.
Other
47 stars 25 forks source link

addAnimatedProperties cannot be used more than once #261

Closed MrVauxs closed 3 months ago

MrVauxs commented 3 months ago

When calling addAnimatedProperties twice, the second usage causes a Maximum call stack error.

The problem lies here: https://github.com/fantasycalendar/FoundryVTT-Sequencer/blob/79b8f35f87b4695b72d9bb4ba2b0796556592ce3/src/canvas-effects/canvas-effect.js#L1219-L1229

What the code is doing is merging the two animations together for the flag update. Problem is that Sequencer processes the animations as they arrive and stores the finished version as the flag, instead of the plain data object. During this processing, it changes the newData.animations[number].target from a mere string, such as "sprite", to a recursive SpriteSheet class, causing the error.

The solution is to revert that change before being sent (see ex. 1), or not have the processing change the underlying data (ex. 2).

Example 1 of fixing the issue:

newData.animations = (newData.animations ?? []).concat(
        foundry.utils.deepClone(animationsToAdd)
      ).map(obj => ({ ...obj, target: "sprite" }));

This would be much better done if you could retrieve the string form of the class, but I couldn't find any such property. So this example is just a proof of concept and not production ready code (ex. does animating the sprite object work the same? I doubt it, but have not tested).

Example 2 Changing the following line and subsequent code to account for the change. https://github.com/fantasycalendar/FoundryVTT-Sequencer/blob/79b8f35f87b4695b72d9bb4ba2b0796556592ce3/src/canvas-effects/canvas-effect.js#L2804

MrVauxs commented 3 months ago

Sidenote, it would probably be best to discard no longer needed animations? 🤔 Since they are one off, once they are done they don't really need to be in the flags anymore, let alone shared with other clients.

Haxxer commented 3 months ago

Fixed in 3.2.11