Detangle `ModelAnimation` time-handling from `ModelAnimationCollection`
- Dominant language
- JavaScript
- Stars
- 15.7k
- Forks
- 3.9k
- Avg merge
- 4d 6h
- Merged PRs (30d)
- 34
Description
In #10314, `ModelExperimentalAnimation` and `ModelExperimentalAnimationCollection` were implemented based on `ModelAnimation` and `ModelAnimationCollection` respectively. I noticed that the `update` code in `ModelAnimationCollection` relied a lot on `ModelAnimation`'s internal variables. In fact, `ModelAnimation` does no updating itself: it only acts as a set of variables that `ModelAnimationCollection` draws from in its update function. This doesn't feel like the best design, though if any disagree with me feel free to say.
It might be cleaner to create a function `ModelExperimentalAnimation.update(frameState)`, then let it handle the time within its own context. All that `ModelExperimentalAnimationCollection` would have to do in its `update` function is something like:
```
for(let i = 0; i < animations.length; i++) {
animations[i].update(frameState);
}
```
The `ModelExperimentalAnimation` may need to communicate with the collection in some way if the animation has `removeOnStop = true`, and the animation has stopped. But I imagine this is doable.
Contributor guide
Research direction
Start by reading the update logic in ModelAnimationCollection and comparing it with ModelExperimentalAnimationCollection and ModelExperimentalAnimation. Trace the internal variables shared by the collection and animation, then define completion as animation time being handled by the individual animation while collection updates delegate to each animation and preserve removeOnStop behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- computer-graphics
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100