Skip to content

Add support for negative z-index in AnimationGroup #4389

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Merzlikin-Matvey
Copy link

Overview: What does this pull request change?

Fixes an AnimationGroup behaviour with negative z_index Mobjects. More about this problem you can see in these issues:
#3334 and #3914

Motivation and Explanation: Why and how do your changes improve the library?

Mobjects with negative z_index have problems with playing in animations. This PR fixes it

Links to added or changed documentation pages

There are no changes to the documentation

Further Information and Comments

I had already opened a pull request, but I decided to close it due to many issues with the previous solution. I have taken all the shortcomings, now there are no performance or graphical test issues.

Reviewer Checklist

  • The PR title is descriptive enough for the changelog, and the PR is labeled correctly
  • If applicable: newly added non-private functions and classes have a docstring including a short summary and a PARAMETERS section
  • If applicable: newly added functions and classes are tested

Copy link
Member

@behackl behackl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Please take a look at my suggestion.

@@ -904,7 +931,7 @@ def get_moving_mobjects(self, *animations: Animation) -> list[Mobject]:
# as soon as there's one that needs updating of
# some kind per frame, return the list from that
# point forward.
animation_mobjects = [anim.mobject for anim in animations]
animation_mobjects = self.recursively_unpack_animation_groups(*animations)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it works, I'd prefer a simpler solution not requiring a new method; something along the lines of the suggestion from my previous review:

Suggested change
animation_mobjects = self.recursively_unpack_animation_groups(*animations)
animation_mobjects = []
for anim in animations:
animation_mobject.extend(anim.mob.get_family())

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

2 participants