Repository: playcanvas/engine (js)

Issue:
`AnimComponent#reset` bad processing on parameters
AnimComponent#reset

https://github.com/playcanvas/engine/blob/256dd83c21750e8c84cd1eb4cafd9000c34f4573/src/framework/components/anim/component.js#L455

The reset method just copy references of parameter objects from origin stateGraph.parameters. If I reset an AnimComponent instance, then all changes, I made on the instance, will applied to the origin stateGraph.parameters. That makes the aseet dirty, and I cannot "reset" the AnimComponent instance anymore. 

The reset method should works as how AnimComponent#loadStateGraph does.  

https://github.com/playcanvas/engine/blob/256dd83c21750e8c84cd1eb4cafd9000c34f4573/src/framework/components/anim/component.js#L364



Pull request:
[Anim Component] Fix for resetting parameters
Resetting the anim component was causing its parameters object to reference parameters directly from the state graph resource. This could lead to other anim components that are instantiated with this resource having incorrect initial data. The parameters object is now deep copied when resetting the graph.

The reference that each anim controller had to the parameters object was also broken when resetting, leading to subsequent parameter changes not being reflected in previously instantiated anim controllers. This has been fixed by passing each controller a function to retrieve parameters directly from the anim component's parameters object.

Fixes #4709

I confirm I have read the [contributing guidelines](https://github.com/playcanvas/engine/blob/master/.github/CONTRIBUTING.md) and signed the [Contributor License Agreement](https://docs.google.com/a/playcanvas.com/forms/d/1Ih69zQfJG-QDLIEpHr6CsaAs6fPORNOVnMv5nuo0cjk/viewform).


Patch:
diff --git a/src/framework/anim/controller/anim-controller.js b/src/framework/anim/controller/anim-controller.js
index f570ecbf7..e3c579906 100644
--- a/src/framework/anim/controller/anim-controller.js
+++ b/src/framework/anim/controller/anim-controller.js
@@ -29,20 +29,21 @@ class AnimController {
      * @param {object[]} states - The list of states used to form the controller state graph.
      * @param {object[]} transitions - The list of transitions used to form the controller state
      * graph.
-     * @param {object[]} parameters - The anim components parameters.
      * @param {boolean} activate - Determines whether the anim controller should automatically play
      * once all {@link AnimNodes} are assigned animations.
      * @param {import('../../../core/event-handler.js').EventHandler} eventHandler - The event
      * handler which should be notified with anim events.
-     * @param {Set} consumedTriggers - Used to set triggers back to their default state after they
+     * @param {Function} findParameter - Retrieves a parameter which is used to control the transition between states.
+     * @param {Function} consumeTrigger - Used to set triggers back to their default state after they
      * have been consumed by a transition.
      */
-    constructor(animEvaluator, states, transitions, parameters, activate, eventHandler, consumedTriggers) {
+    constructor(animEvaluator, states, transitions, activate, eventHandler, findParameter, consumeTrigger) {
         this._animEvaluator = animEvaluator;
         this._states = {};
         this._stateNames = [];
         this._eventHandler = eventHandler;
-        this._consumedTriggers = consumedTriggers;
+        this._findParameter = findParameter;
+        this._consumeTrigger = consumeTrigger;
         for (let i = 0; i < states.length; i++) {
             this._states[states[i].name] = new AnimState(
                 this,
@@ -60,7 +61,6 @@ class AnimController {
         });
         this._findTransitionsFromStateCache = {};
         this._findTransitionsBetweenStatesCache = {};
-        this._parameters = parameters;
         this._previousStateName = null;
         this._activeStateName = ANIM_STATE_START;
         this._playing = false;
@@ -227,7 +227,7 @@ class AnimController {
         const conditions = transition.conditions;
         for (let i = 0; i < conditions.length; i++) {
             const condition = conditions[i];
-            const parameter = this.findParameter(condition.parameterName);
+            const parameter = this._findParameter(condition.parameterName);
             switch (condition.predicate) {
                 case ANIM_GREATER_THAN:
                     if (!(parameter.value > condition.value)) return false;
@@ -339,9 +339,9 @@ class AnimController {
         // turn off any triggers which were required to activate this transition
         for (let i = 0; i < transition.conditions.length; i++) {
             const condition = transition.conditions[i];
-            const parameter = this.findParameter(condition.parameterName);
+            const parameter = this._findParameter(condition.parameterName);
             if (parameter.type === ANIM_PARAMETER_TRIGGER) {
-                this._consumedTriggers.add(condition.parameterName);
+                this._consumeTrigger(condition.parameterName);
             }
         }
 
@@ -584,7 +584,7 @@ class AnimController {
     }
 
     findParameter(name) {
-        return this._parameters[name];
+        return this._findParameter(name);
     }
 }
 
diff --git a/src/framework/anim/controller/anim-state.js b/src/framework/anim/controller/anim-state.js
index 5ce9403e2..348468986 100644
--- a/src/framework/anim/controller/anim-state.js
+++ b/src/framework/anim/controller/anim-state.js
@@ -44,7 +44,6 @@ class AnimState {
         this._speed = speed;
         this._loop = loop;
         this._hasAnimations = false;
-        const findParameter = this._controller.findParameter.bind(this._controller);
         if (blendTree) {
             this._blendTree = this._createTree(
                 blendTree.type,
@@ -56,7 +55,7 @@ class AnimState {
                 blendTree.children,
                 blendTree.syncAnimations,
                 this._createTree,
-                findParameter
+                this._controller.findParameter
             );
         } else {
             this._blendTree = new AnimNode(this, null, name, 1.0, speed);
diff --git a/src/framework/components/anim/component.js b/src/framework/components/anim/component.js
index 85ca529e4..868f95629 100644
--- a/src/framework/components/anim/component.js
+++ b/src/framework/components/anim/component.js
@@ -290,10 +290,10 @@ class AnimComponent extends Component {
             animEvaluator,
             states,
             transitions,
-            this._parameters,
             this._activate,
             this,
-            this._consumedTriggers
+            this.findParameter.bind(this),
+            this.consumeTrigger.bind(this)
         );
         this._layers.push(new AnimComponentLayer(name, controller, this, weight, blendType));
         this._layerIndices[name] = layerIndex;
@@ -324,6 +324,18 @@ class AnimComponent extends Component {
         return this._addLayer({ name, states, transitions, weight, mask, blendType });
     }
 
+    _assignParameters(stateGraph) {
+        this._parameters = {};
+        const paramKeys = Object.keys(stateGraph.parameters);
+        for (let i = 0; i < paramKeys.length; i++) {
+            const paramKey = paramKeys[i];
+            this._parameters[paramKey] = {
+                type: stateGraph.parameters[paramKey].type,
+                value: stateGraph.parameters[paramKey].value
+            };
+        }
+    }
+
     /**
      * Initializes component animation controllers using the provided state graph.
      *
@@ -359,15 +371,7 @@ class AnimComponent extends Component {
      */
     loadStateGraph(stateGraph) {
         this._stateGraph = stateGraph;
-        this._parameters = {};
-        const paramKeys = Object.keys(stateGraph.parameters);
-        for (let i = 0; i < paramKeys.length; i++) {
-            const paramKey = paramKeys[i];
-            this._parameters[paramKey] = {
-                type: stateGraph.parameters[paramKey].type,
-                value: stateGraph.parameters[paramKey].value
-            };
-        }
+        this._assignParameters(stateGraph);
         this._layers = [];
 
         let containsBlendTree = false;
@@ -458,7 +462,7 @@ class AnimComponent extends Component {
      * playing before it will continue playing.
      */
     reset() {
-        this._parameters = Object.assign({}, this._stateGraph.parameters);
+        this._assignParameters(this._stateGraph);
         for (let i = 0; i < this._layers.length; i++) {
             const layerPlaying = this._layers[i].playing;
             this._layers[i].reset();
@@ -627,6 +631,14 @@ class AnimComponent extends Component {
         Debug.log(`Cannot set parameter value. No parameter found in anim controller named "${name}" of type "${type}"`);
     }
 
+    findParameter(name) {
+        return this._parameters[name];
+    }
+
+    consumeTrigger(name) {
+        this._consumedTriggers.add(name);
+    }
+
     /**
      * Returns a float parameter value by name.
      *


Test patch:
diff --git a/test/framework/anim/controller/anim-controller.test.mjs b/test/framework/anim/controller/anim-controller.test.mjs
index 98e323aed..316e5a97d 100644
--- a/test/framework/anim/controller/anim-controller.test.mjs
+++ b/test/framework/anim/controller/anim-controller.test.mjs
@@ -72,20 +72,22 @@ describe('AnimController', function () {
         const animBinder = new AnimComponentBinder({ entity: graph }, graph, 'layer', {}, 0);
         animBinder.resolve = () => {};
         const animEvaluator = new AnimEvaluator(animBinder);
+        const parameters = {
+            'param': {
+                'name': 'param',
+                'type': 'FLOAT',
+                'value': 0.5
+            }
+        };
+        const consumedTriggers = new Set();
         controller = new AnimController(
             animEvaluator,
             states,
             transitions,
-            {
-                'param': {
-                    'name': 'param',
-                    'type': 'FLOAT',
-                    'value': 0.5
-                }
-            }, // parameters
             true, // activate
             null, // event handler
-            new Set() // consumed triggers
+            name => parameters[name],
+            name => consumedTriggers.add(name)
         );
         // add tracks
         const curves = [new AnimCurve(['path/to/entity'], 0, 0, INTERPOLATION_LINEAR)];