Repository: ManimCommunity/manim
File: manim/scene/scene.py (Python)

Code before:
            return self

    def replace(self, old_mobject: Mobject, new_mobject: Mobject) -> None:
        """Replace one mobject in the scene with another, preserving draw order.

        If ``old_mobject`` is a submobject of some other Mobject (e.g. a
        :class:`.Group`), the new_mobject will replace it inside the group,
        without otherwise changing the parent mobject.

        Parameters
        ----------
        old_mobject
            The mobject to be replaced. Must be present in the scene.
        new_mobject
            A mobject which must not already be in the scene.

        """
        if old_mobject is None or new_mobject is None:
            raise ValueError("Specified mobjects cannot be None")

        def replace_in_list(
            mobj_list: list[Mobject], old_m: Mobject, new_m: Mobject
        ) -> bool:
            # Avoid duplicate references to the same object in self.mobjects
            if new_m in mobj_list:
                mobj_list.remove(new_m)

            # We use breadth-first search because some Mobjects get very deep and
            # we expect top-level elements to be the most common targets for replace.
            for i in range(0, len(mobj_list)):
                # Is this the old mobject?
                if mobj_list[i] == old_m:
                    # If so, write the new object to the same spot and stop looking.
                    mobj_list[i] = new_m
                    return True
            # Now check all the children of all these mobs.
            for mob in mobj_list:  # noqa: SIM110
                if replace_in_list(mob.submobjects, old_m, new_m):
                    # If we found it in a submobject, stop looking.
                    return True
            # If we did not find the mobject in the mobject list or any submobjects,
            # (or the list was empty), indicate we did not make the replacement.
            return False

        # Make use of short-circuiting conditionals to check mobjects and then
        # foreground_mobjects
        replaced = replace_in_list(
            self.mobjects, old_mobject, new_mobject
        ) or replace_in_list(self.foreground_mobjects, old_mobject, new_mobject)

        if not replaced:

Review comment (bug):
There is a very subtle bug which is introduced due to an edge case: what if `new_mobject is old_mobject`? This might happen if someone wants to replace Mobject `a` with Mobject `b` and doesn't notice that `a` and `b` are references to the same thing.
```py
sq = Square()
self.add(sq)
self.replace(sq, sq)  # ValueError: Could not find Square in scene
```

To fix this, instead of removing the Mobject, it could simply return `True` in this case: the old Mobject was found and "replaced", except it does not require being replaced, because it's the same Mobject. Logging a warning in this case might also be useful.

_Edit: this goes to show that we really need a custom data structure for our Mobject lists in general, instead of doing the same patches everywhere..._

```suggestion
            if new_m in mobj_list:
                if old_m is new_m:
                    # In this case, one could say that the old Mobject was already found.
                    # No replacement is needed, since old_m is new_m, so no action is required.
                    # This might be unexpected, so raise a warning.
                    logging.warning(
                        f"Attempted to replace {type(old_m).__name__} "
                        "with itself in Scene.mobjects."
                    ) 
                    return True
                mobj_list.remove(new_m)
```

Code after:
                self.add(mob)
                curr_mobjects += mob.get_family()  # type: ignore[arg-type]

    def remove(self, *mobjects: Mobject) -> Self:
        """
        Removes mobjects in the passed list of mobjects
        from the scene and the foreground, by removing them
        from "mobjects" and "foreground_mobjects"

        Parameters
        ----------
        *mobjects
            The mobjects to remove.
        """
        if config.renderer == RendererType.OPENGL:
            mobjects_to_remove = []
            meshes_to_remove: set[Object3D] = set()
            mobject_or_mesh: Mobject
            for mobject_or_mesh in mobjects:
                if isinstance(mobject_or_mesh, Object3D):
                    meshes_to_remove.add(mobject_or_mesh)
                else:
                    mobjects_to_remove.append(mobject_or_mesh)
            self.mobjects = restructure_list_to_exclude_certain_family_members(
                self.mobjects,
                mobjects_to_remove,
            )

            def lambda_function(mesh: Object3D) -> bool:
                return mesh not in set(meshes_to_remove)

            self.meshes = list(
                filter(lambda_function, self.meshes),
            )
            return self
        else:
            assert config.renderer == RendererType.CAIRO
            for list_name in "mobjects", "foreground_mobjects":
                self.restructure_mobjects(mobjects, list_name, False)
            return self

    def replace(self, old_mobject: Mobject, new_mobject: Mobject) -> None:
        """Replace one mobject in the scene with another, preserving draw order.

        If ``old_mobject`` is a submobject of some other Mobject (e.g. a
        :class:`.Group`), the new_mobject will replace it inside the group,
        without otherwise changing the parent mobject.

        Parameters
        ----------
        old_mobject