Skip to content

Fix duplicate references in Scene.mobjects after ReplacementTransform with existing target mobject #4242

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

Conversation

irvanalhaq9
Copy link
Contributor

@irvanalhaq9 irvanalhaq9 commented May 9, 2025

Overview: What does this pull request change?

Fix #4238

Fix duplicate mobject reference issue when using ReplacementTransform if the target mobject is already in the scene.

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

Previously, if the target mobject was already in the scene, ReplacementTransform would replace the source mobject in the scene.mobjects list without removing the existing reference to the target mobject. This caused scene.mobjects to contain two references to the same object (i.e., the target mobject), leading to unexpected behavior in #4238.

Before this fix:

from manim import Circle, ReplacementTransform, Scene, Square
scene = Scene()
c = Circle()
sq = Square()
scene.add(c, sq)
print(scene.mobjects)    # [Circle, Square]
scene.play(ReplacementTransform(c, sq))
print(scene.mobjects)    # [Square, Square]  <- Duplicate reference to Square
mob = scene.mobjects
print(mob[0] is mob[1])  # True

After this fix:

from manim import Circle, ReplacementTransform, Scene, Square
scene = Scene()
c = Circle()
sq = Square()
scene.add(c, sq)
print(scene.mobjects)    # [Circle, Square]
scene.play(ReplacementTransform(c, sq))
print(scene.mobjects)    # [Square]  <- Only one reference remains

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

Sorry, something went wrong.

Copy link
Contributor

@chopan050 chopan050 left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! I left one important comment:

@github-project-automation github-project-automation bot moved this from 🆕 New to 👀 In review in Dev Board Jul 25, 2025
irvanalhaq9 and others added 3 commits July 26, 2025 06:01
Co-authored-by: Francisco Manríquez Novoa <49853152+chopan050@users.noreply.github.com>
@irvanalhaq9
Copy link
Contributor Author

@chopan050 Thanks for the review! I've committed the suggested change.

Copy link
Contributor

@chopan050 chopan050 left a comment

Choose a reason for hiding this comment

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

Thanks!

@github-project-automation github-project-automation bot moved this from 👀 In review to 👍 To be merged in Dev Board Jul 26, 2025
@chopan050 chopan050 changed the title Fix duplicate references in scene.mobjects after ReplacementTransform with existing target mobject Fix duplicate references in Scene.mobjects after ReplacementTransform with existing target mobject Jul 26, 2025
@chopan050 chopan050 enabled auto-merge (squash) July 26, 2025 01:19
@chopan050 chopan050 merged commit dea245a into ManimCommunity:main Jul 26, 2025
21 of 22 checks passed
@github-project-automation github-project-automation bot moved this from 👍 To be merged to ✅ Done in Dev Board Jul 26, 2025
@irvanalhaq9 irvanalhaq9 deleted the fix-duplicate-reference-in-replacement-transform branch July 26, 2025 01:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

ReplacementTransform leaves behind a “ghost” mobject when the target is already on‑screen
2 participants