Skip to content

Commit

Permalink
fix(lib): prevent filename collision (#429)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
jeertmans authored May 18, 2024
1 parent 993acf0 commit b080739
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 5 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
10 changes: 7 additions & 3 deletions manim_slides/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -324,15 +324,19 @@ def to_file(self, path: Path) -> None:
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
Expand Down
24 changes: 22 additions & 2 deletions manim_slides/convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -404,8 +404,28 @@ def convert_to(self, dest: Path) -> None:

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)

Expand Down
22 changes: 22 additions & 0 deletions tests/test_convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,28 @@ def test_revealjs_converter(
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
Expand Down

0 comments on commit b080739

Please sign in to comment.