tweenjs
Fix dynamic to with array values
fixes https://github.com/tweenjs/tween.js/issues/555 (in particular, see the plan at the bottom, and note that the fix for v17 has already been published in v17.6.0).
target: v19
@lguminski This does not include your example that modifies the interpolation array length. I think this is a good first step.
I'm planning to make some re-factors that would make it easier to implement that later, if we want it, as an additional feature.
This is a breaking change because now modifying an object outside of an animation is no longer dynamic by default. Previously, it was possible to dynamically modify the object passed to to() as in the "dynamic to" example where fox chases a rabbit, but it was not possible to dynamically modify interpolation arrays. Now, by default nothing is dynamic (for objects and interpolation arrays) and objects will not be modified, unless dynamic is set to true.
@mikebolt @lguminski @dalisoft What are your thoughts about how it currently works? Regardless if we make the default value of dynamic be true or false, either way is still a breaking change. Question is, which default value do you like more?
Currently default is false.
Note, in previous PRs, I converted tests.js to tests.ts (we can verify our types while we write tests, yay!), and I also previously added a test for dynamic to modifying an object (search for dynamic in tests.ts).
additional idea
I am also thinking that a further improvement would be that instead of injecting starting values into interpolation arrays, we can simply read the starting values from _valuesStart, and hence we would never need to modify the objects passed into to() but still achieve the dynamic or non-dynamic behavior depending on the boolean value. With this approach, the only code that would ever modify an object passed into to() would be the end user's own code, never our Tween code.
I'm going to try the additional idea I mentioned above first. If it isn't too tricky, then it will:
- eliminate the need for a
dynamicoption (the dynamic-to feature will always be available) - prevent Tween from ever writing to the user's object that was passed into
tween.to(), except in the case that values are relative string values (e.g.+20).
We could additionally add an option like hasRelative that would be required in order to opt into the relative value feature (which would modify the user's object).
There's no way to allow both the relative-values and dynamic-to features while never writing to the end user's to object. At least, I don't yet see a simple way to do it: If user wants to use relative values, we can copy the object then apply the relative values, but that means we are using a copy so dynamic-to doesn't work as the user's values are not applied to the copy. Conversely, if the user wants dynamic values, that will work if we don't copy the user's object, but then relative values will be applied to the user's object thus modifying it.
Well, we could move the relative-value checking into update instead of handling them only initially in start, but that would mean the relative-value handling cost will move from an initialization cost to an update cost (the new cost will happen on every update of the animation instead of once at the beginning).
I suppose we should have some performance tests to see what the actual perf difference is.
I arrived late to the party. Sorry about that. I just commented under the original issue, and now I am seeing here that independently you arrived to similar conclusion.
I'm going to try the additional idea I mentioned above first. If it isn't too tricky, then it will:
- eliminate the need for a dynamic option (the dynamic-to feature will always be available)
- prevent Tween from ever writing to the user's object that was passed into tween.to(), except in the case that values are relative string values (e.g. +20).
First, I am happy to see it, because I did not like the dynamic option. Secondly I also agree that the user's object that was passed into tween.to() should not be altered by tween, as it violates ownership over memory. It is fine if tween manages its own buffers/caches, but user's original data must not be altered, as we don't know what one plans to do with the data later. Perhaps the data is used later after the tween finishes, and then the fact that tween changed it breaks code.
@trusktr summing up, I believe that these are right choices.
