Skip to content

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