From b08073983b8ac8b871c34c2a422c150e097e3be1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9rome=20Eertmans?= <jeertmans@icloud.com>
Date: Sat, 18 May 2024 10:17:25 +0200
Subject: [PATCH] fix(lib): prevent filename collision (#429)

* fix(lib): prevent filename collision

Apparently, ManimCE can produce two different animations with the same name (i.e., the same hash). As documented, ManimGL would any produce files with the same name so this fix was needed.

Closes #428

* chore(lib): update comment

chore(lib): update comment

* chore(tests): add test

* chore(tests): remove redundant underscore

* chore(docs): add changelog entry
---
 CHANGELOG.md            |  6 ++++++
 manim_slides/config.py  | 10 +++++++---
 manim_slides/convert.py | 24 ++++++++++++++++++++++--
 tests/test_convert.py   | 22 ++++++++++++++++++++++
 4 files changed, 57 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b402f4b..59754cd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 (unreleased)=
 ## [Unreleased](https://github.com/jeertmans/manim-slides/compare/5.1.7...HEAD)
 
+(unreleased-fixed)=
+### Fixed
+
+- Fix combining assets from multiple scenes to avoid filename collision.
+  [#429](https://github.com/jeertmans/manim-slides/pull/429)
+
 (v5.1.7)=
 ## [v5.1.7](https://github.com/jeertmans/manim-slides/compare/v5.1.6...v5.1.7)
 
diff --git a/manim_slides/config.py b/manim_slides/config.py
index 3fd4f91..93dd69d 100644
--- a/manim_slides/config.py
+++ b/manim_slides/config.py
@@ -324,15 +324,19 @@ class PresentationConfig(BaseModel):  # type: ignore[misc]
             f.write(self.model_dump_json(indent=2))
 
     def copy_to(
-        self, folder: Path, use_cached: bool = True, include_reversed: bool = True
+        self,
+        folder: Path,
+        use_cached: bool = True,
+        include_reversed: bool = True,
+        prefix: str = "",
     ) -> "PresentationConfig":
         """Copy the files to a given directory."""
         for slide_config in self.slides:
             file = slide_config.file
             rev_file = slide_config.rev_file
 
-            dest = folder / file.name
-            rev_dest = folder / rev_file.name
+            dest = folder / f"{prefix}{file.name}"
+            rev_dest = folder / f"{prefix}{rev_file.name}"
 
             slide_config.file = dest
             slide_config.rev_file = rev_dest
diff --git a/manim_slides/convert.py b/manim_slides/convert.py
index aba86c0..774f799 100644
--- a/manim_slides/convert.py
+++ b/manim_slides/convert.py
@@ -404,8 +404,28 @@ class RevealJS(Converter):
 
             full_assets_dir.mkdir(parents=True, exist_ok=True)
 
-            for presentation_config in self.presentation_configs:
-                presentation_config.copy_to(full_assets_dir, include_reversed=False)
+            num_presentation_configs = len(self.presentation_configs)
+
+            if num_presentation_configs > 1:
+                # Prevent possible name collision, see:
+                # https://github.com/jeertmans/manim-slides/issues/428
+                # With ManimCE, this can happen when caching is disabled as filenames are
+                #   'uncached_000x.mp4'
+                # With ManimGL, this can easily occur since filenames are just basic integers...
+                num_digits = len(str(num_presentation_configs - 1))
+
+                def prefix(i: int) -> str:
+                    return f"s{i:0{num_digits}d}_"
+
+            else:
+
+                def prefix(i: int) -> str:
+                    return ""
+
+            for i, presentation_config in enumerate(self.presentation_configs):
+                presentation_config.copy_to(
+                    full_assets_dir, include_reversed=False, prefix=prefix(i)
+                )
 
         dest.parent.mkdir(parents=True, exist_ok=True)
 
diff --git a/tests/test_convert.py b/tests/test_convert.py
index 791dfea..2cf4d0e 100644
--- a/tests/test_convert.py
+++ b/tests/test_convert.py
@@ -153,6 +153,28 @@ class TestConverter:
         file_contents = Path(out_file).read_text()
         assert "manim" in file_contents.casefold()
 
+    @pytest.mark.parametrize("num_presentation_configs", (1, 2))
+    def test_revealjs_multiple_scenes_converter(
+        self,
+        tmp_path: Path,
+        presentation_config: PresentationConfig,
+        num_presentation_configs: int,
+    ) -> None:
+        out_file = tmp_path / "slides.html"
+        RevealJS(
+            presentation_configs=[
+                presentation_config for _ in range(num_presentation_configs)
+            ]
+        ).convert_to(out_file)
+        assert out_file.exists()
+        assets_dir = Path(tmp_path / "slides_assets")
+        assert assets_dir.is_dir()
+
+        got = sum(1 for _ in assets_dir.iterdir())
+        expected = num_presentation_configs * len(presentation_config.slides)
+
+        assert got == expected
+
     @pytest.mark.parametrize("frame_index", ("first", "last"))
     def test_pdf_converter(
         self, frame_index: str, tmp_path: Path, presentation_config: PresentationConfig