Moguri / blend2bam

A CLI tool to convert Blender blend files to Panda3D BAM files
MIT License
68 stars 18 forks source link

blend2bam Loader fails if temp directory is on another disk #86

Closed izzyboris closed 1 year ago

izzyboris commented 1 year ago

My system temporary directory is on another disk (Windows C:) from my model files (G:, a network drive). This triggers an interesting sequence that prevents blend2bam from running as a Panda Loader. Issue was introduced in the master branch, not present in the 0.21 tag.

  1. loader.py:BlendLoader.load_file uses NamedTemporaryFile to create a filename for the temporary output path to use
  2. shutil.move in cli.py:convert() cannot use os.rename between different disks, so falls back to opening the file in 'wb' mode for a data copy
  3. This file is already held open by NamedTemporaryFile and can't be opened for write, generating a PermissionDenied exception.

Because cli.py doesn't use NamedTemporaryFile to create a temp path, only the Loader plugin, this doesn't fail when running blend2bam manually.

Fix might be to either:

  1. bamfile.close() immediately inside the NamedTemporaryFile context manager
  2. Use a simple tempfile.mktemp() to generate the name instead

The close() in Option 1 deletes the tempfile that was just created, doing extra work to no benefit, so Option 2 is probably cleaner.

Moguri commented 1 year ago

I switched to using a temp directory, which is how I simplified this mess with NamedTemporaryFile on Windows for the cli. I believe I have a fix pushed to master, but I do not have a test environment to repro the issue. Could you please test and confirm?

izzyboris commented 1 year ago

That almost works but panda complains about the returned path. I think it's missing a p3d.Filename? This works for me:

diff --git a/blend2bam/loader.py b/blend2bam/loader.py
index 76a64d4..7ffbeb5 100644
--- a/blend2bam/loader.py
+++ b/blend2bam/loader.py
@@ -36,4 +36,4 @@ class BlendLoader:

             options = p3d.LoaderOptions(options)
             options.flags |= p3d.LoaderOptions.LF_no_cache
-            return loader.load_sync(bamfilepath, options=options)
+            return loader.load_sync(p3d.Filename.from_os_specific(bamfilepath), options=options)
Moguri commented 1 year ago

Yup, I accidentally changed bamfilepath from a panda3d.core.Filename to a str (os.path-based file path). I pushed the fix you suggested.

izzyboris commented 1 year ago

That works better now, thank you!