Fix name collision between VectorScene.moving_mobjects and Scene.moving_mobjects#4726
Open
m-awais-khan wants to merge 1 commit intoManimCommunity:mainfrom
Open
Fix name collision between VectorScene.moving_mobjects and Scene.moving_mobjects#4726m-awais-khan wants to merge 1 commit intoManimCommunity:mainfrom
VectorScene.moving_mobjects and Scene.moving_mobjects#4726m-awais-khan wants to merge 1 commit intoManimCommunity:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a runtime crash in LinearTransformationScene caused by VectorScene reusing the attribute name moving_mobjects, which conflicts with Scene.moving_mobjects (used internally during play(...)), by renaming the vector-scene-specific list to avoid the collision.
Changes:
- Renamed
VectorScene’s internal tracking list fromself.moving_mobjectstoself.vector_moving_mobjects. - Updated internal call sites that add/use these tracked mobjects during transformations.
- Updated a related docstring reference (but some documentation still references the old name).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
791
to
+806
| Adds the mobject to the special list | ||
| self.moving_mobject, and adds a property | ||
| to the mobject called mobject.target, which | ||
| keeps track of what the mobject will move to | ||
| or become etc. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| mobject | ||
| The mobjects to add to the list | ||
|
|
||
| target_mobject | ||
| What the moving_mobject goes to, etc. | ||
| """ | ||
| mobject.target = target_mobject | ||
| self.add_special_mobjects(self.moving_mobjects, mobject) | ||
| self.add_special_mobjects(self.vector_moving_mobjects, mobject) |
Comment on lines
1089
to
1092
| """ | ||
| This method returns an animation that moves a mobject | ||
| in "self.moving_mobjects" to its corresponding .target value. | ||
| in "self.vector_moving_mobjects" to its corresponding .target value. | ||
| func is a function that determines where the .target goes. |
Comment on lines
697
to
708
| def setup(self) -> None: | ||
| # The has_already_setup attr is to not break all the old Scenes | ||
| if hasattr(self, "has_already_setup"): | ||
| return | ||
| self.has_already_setup = True | ||
| self.background_mobjects: list[Mobject] = [] | ||
| self.foreground_mobjects: list[Mobject] = [] | ||
| self.transformable_mobjects: list[Mobject] = [] | ||
| self.moving_vectors: list[Mobject] = [] | ||
| self.transformable_labels: list[MathTex] = [] | ||
| self.moving_mobjects: list[Mobject] = [] | ||
| self.vector_moving_mobjects: list[Mobject] = [] | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes a critical name collision bug in
VectorScenewhereLinearTransformationScenecrashes duringapply_matrixif a UI element (like a label) was previously animated.Root Cause:
VectorSceneinitializes a listself.moving_mobjectsto keep track of vectors and objects that should undergo linear transformation.However, the base class
Sceneoverwritesself.moving_mobjectsevery timeself.play(...)is called, using it to track the mobjects currently animating.If a user calls
self.play(FadeIn(label))right beforeself.apply_matrix(), the matrix transformation accidentally tries to transform the UI elements from the previous animation instead of the grid and transformable mobjects! This results inValueError: zip() argument 3 is longer than arguments 1-2whenstrict=Trueis enforced in theTransform.Fix:
Renamed all internal references of
self.moving_mobjectsinsideVectorSceneandLinearTransformationScenetoself.vector_moving_mobjectsto avoid collision withScene.moving_mobjects.