-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Call .finish()
for every animation in AnimationGroup
on finish
#3951
Conversation
This could be a fix to the issue found in ManimCommunity#3950
If you want to skip the Wall Texts go to suggested solutions. You can also check for the Issue at this next link: #3950 "Manim 0.18.1: Updaters don't work with objects which have been added using LaggedStart()" So based on the method However If we see at the tested_code, and follow the definitions to
So here's 3 possible solutions
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello, and thanks for your PR!
I'm the author of the PR #3542. I accidentally introduced this bug after attempting to fix the following issue:
There was a bug where, if you used a "reverse" rate_func which intended to make the submobjects reach an alpha == 0 state, when AnimationGroup.finish() gets called, it would call all of its subanimations' finish() method which would make all the submobjects be reset to an alpha == 1 state instead. To fix this, an AnimationGroup.interpolate(1) was added to the AnimationGroup.finish() method.
I realized that it would be more difficult than that: in order to accomplish that, an option would be to add an initial_alpha: float = 0.0
parameter to Animation.begin()
and a final_alpha: float = 1.0
parameter to Animation.finish()
, among other things. In that way, I would allow an animation to finish with, for example, alpha=0.0
, which is useful for reversed animations. That was something I discussed with Benjamin Hackl a long time ago, without a definitive solution, and it should be done in another PR.
The issue is: it seems I forgot to revert that change in AnimationGroup.finish()
. So, thanks for making this PR to fix it!
I would say that the correct code for AnimationGroup.finish()
should be the following:
def finish(self) -> None:
for anim in self.animations:
anim.finish()
self.anims_begun[:] = True
self.anims_finished[:] = True
if self.suspend_mobject_updating:
self.group.resume_updating()
Notice that:
- The
self.interpolate(1)
is gone, because the effect I intended to cause with it won't work for now. - The
for anim in self.animations: anim.finish()
is moved outside theif
, because it should always be executed, like it was originally.
Perhaps it's also a good idea for us to add a test for this in order to prevent future regressions. |
TL;DR: if you could please modify def finish(self) -> None:
for anim in self.animations:
anim.finish()
self.anims_begun[:] = True
self.anims_finished[:] = True
if self.suspend_mobject_updating:
self.group.resume_updating() that would be great! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
.finish()
for every animation in AnimationGroup
on finish
* First draft of Changelog for 0.19.0 * fix typos * added 3967 * Add #4037 * Update count 97 -> 98 * Add #4039 * Add #3930 and #4044 * Add this PR to changelog * Add small description to changelog * Add #3924, #3951, and #4038 * Bump Manim version to 0.19.0 * Update CITATION.cff to be more consistent * remove github-security[bot] * Feedback + missing PRs * add newly merged PRs * added more details to highlights + breaking changes * bump date * sort PRs in breaking changes section and include 3964 * sort highlights section * fix: add required configuration key in .readthedocs.yml * Update docs/source/changelog/0.19.0-changelog.rst --------- Co-authored-by: Benjamin Hackl <[email protected]>
Fix Animation's getting toggled off but not back on
Overview: What does this pull request change?
Animations sent through
AnimationGroup
andLaggedStart
will have their updaters toggled off without toggling them back on after the GroupAnimation finishes.Motivation and Explanation: Why and how do your changes improve the library?
Users playing the animations of their Mobjects in AnimationGroup will find themselves troubled due to updaters silently getting turned off but not back on after the AnimationGroup finishes
Links to added or changed documentation pages
N/A
Further Information and Comments
This could be a fix to the issue found in #3950
Reviewer Checklist