feat(SOLSYS-42): Blender Earth/Moon billboards in Sol→Centauri cinematic - #45
Open
ThomasAFink wants to merge 8 commits into
Open
feat(SOLSYS-42): Blender Earth/Moon billboards in Sol→Centauri cinematic#45ThomasAFink wants to merge 8 commits into
ThomasAFink wants to merge 8 commits into
Conversation
Replace Earth and Moon scatter dots with camera-facing flyby sprites during FuncAnimation when --blender-bodies is set, writing a separate blender-stem GIF so the classic dotted cinematic stays unchanged.
Stop cycling cropped flyby GIF frames through plot_surface. Sample Earth/Moon Blender texture packs into lit disks and draw them during FuncAnimation after the camera projection is set.
Organize blender cinematic outputs, add blender --pipeline, and tune Earth spin / Moon pace / fixed globe size. Checkpoint before switching to Blender turntable frame reuse.
Add fixed-camera transparent day/night spin renders (--spin) and draw those RGBA frames at Earth/Moon during FuncAnimation instead of Python texture globes.
Comments now match Blender spin-loop compositing; fake-blender debug outputs removed from sol_centauri/blender.
Composite Earth/Moon spin loops into the Sol→Centauri tour, keep Luna locked to orbital phase with smooth frame blending, and add rendered spin assets plus blender cinematic GIFs.
|
Warning Review limit reached
Next review available in: 36 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis change adds Blender Earth and Moon spin-loop generation, sprite loading, and cinematic billboard compositing. It adds CLI commands for spin rendering and the complete Blender-to-Sol–Centauri pipeline, with documentation, tests, and local output ignore rules. ChangesBlender body rendering and cinematic integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant BlenderCLI
participant renderPlanetSpin
participant BlenderBodySpriteAtlas
participant SolCentauriCinematicAnimator
BlenderCLI->>renderPlanetSpin: render Earth and Moon spin loops
renderPlanetSpin-->>BlenderBodySpriteAtlas: provide frame directories
BlenderBodySpriteAtlas-->>SolCentauriCinematicAnimator: provide interpolated RGBA sprites
SolCentauriCinematicAnimator->>SolCentauriCinematicAnimator: project and composite body billboards
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| blend = float(wrapped - np.floor(wrapped)) | ||
| if blend <= 1e-6: | ||
| return frames[index0] | ||
| if blend >= 1.0 - 1e-6: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (9)
animate/scenes/blender/flyby_scene.py (2)
59-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a type annotation for
cameraPath, and document the return type difference.Two small clarity items:
cameraPathin_bodyJobSkeletonhas no annotation. UseSequence[FlybyCameraSample].renderPlanetFlybyreturns GIF paths, butrenderPlanetSpinreturns frame directories. The names and signatures look identical, so state the returned value in therenderPlanetSpindocstring.Also applies to: 266-275
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@animate/scenes/blender/flyby_scene.py` at line 59, Add the Sequence[FlybyCameraSample] annotation to cameraPath in _bodyJobSkeleton, ensuring the required typing and sample symbols are used. Update renderPlanetSpin’s docstring to explicitly state that it returns frame-directory paths, distinguishing this from renderPlanetFlyby’s GIF-path result.
51-66: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider passing the built
BodySceneinto_bodyJobSkeleton.
buildSpinJobandbuildFlybyJobeach callbuildBodyScene, and_bodyJobSkeletoncalls it again for the same body.buildBodySceneconstructsPlanetCatalogandMoonCatalogon every call, andspinFramesDirectoryadds one more call.renderPlanetSpintherefore rebuilds the catalogs several times per theme.Accept the already-built
bodySceneas a parameter in_bodyJobSkeleton, or drop the duplicate call in the two public builders.Also applies to: 133-134
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@animate/scenes/blender/flyby_scene.py` around lines 51 - 66, Avoid rebuilding the same BodyScene in the job-building flow: update _bodyJobSkeleton to accept and reuse an already-built BodyScene, then pass that instance from buildSpinJob and buildFlybyJob instead of calling buildBodyScene again. Ensure spinFramesDirectory and renderPlanetSpin continue using the shared scene where applicable, while preserving existing frame-count and appearance behavior.tests/test_blender_body_sprites.py (2)
120-141: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the dot fallback.
The stated objective is a fallback to scatter dots when assets are missing. The tests confirm the atlas is
Nonewhen the flag is off, and confirm overlay painting when assets exist. No test confirms thatupdatepaints no overlay image and still draws markers whenuseBlenderBodies=Trueand the atlas has no frames.Add a case with an empty spin directory that asserts
bodyOverlay.get_images()is empty.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_blender_body_sprites.py` around lines 120 - 141, Add a test covering the missing-assets fallback: construct SolCentauriCinematicAnimator with useBlenderBodies=True and an empty spin directory, invoke update, and assert bodyOverlay.get_images() is empty while the fallback dot markers are still drawn. Reuse the existing test setup and update/marker assertions rather than changing production behavior.
150-158: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe animator constructor loads real repository assets before the temp atlas replaces it.
SolCentauriCinematicAnimator(..., useBlenderBodies=True)buildsBlenderBodySpriteAtlas(themeName)with the defaultoutput/animate/blenderdirectory. The test then overwritesanimator.blenderSprites. So the test result depends on which spin frames exist in the working tree, and it decodes those PNGs for no purpose.Add an output-directory parameter to the animator (or inject the atlas) so the test never reads the repository output tree.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_blender_body_sprites.py` around lines 150 - 158, Update SolCentauriCinematicAnimator to accept an output-directory parameter or injected BlenderBodySpriteAtlas, and use it when constructing blenderSprites. Pass the temporary directory from the test at construction time so animator.update(0) never loads assets from the default repository output tree; remove the post-construction atlas replacement.render.py (2)
357-364: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConfirm that the
blendersubparser exposes the stars CSV, or reuse the shared default.
_handleBlenderCommandhardcodes'data/nearby_stars_30.csv'. Every other command reads that path from an argument with the same default. The--pipelinemode therefore cannot use a different catalog.Add
--starsto theblendersubparser, or move the default into one module-level constant that all commands use.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@render.py` around lines 357 - 364, The _handleBlenderCommand pipeline path currently hardcodes the stars catalog and ignores command-line configuration. Add a --stars argument to the blender subparser and pass args.stars as starsCsvPath, or centralize the existing default in a shared module-level constant and reuse it across all commands.
273-273: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the frame-count default sentinel.
48 if frames == 120 else framesappears twice, and72 if args.frames == 120 else args.framesfollows the same pattern. The literal120is the parser default, so the three sites all depend on that default staying unchanged. Add one helper, for example_modeFrameCount(frames, default), and use the parser default as a named constant.Also applies to: 368-368, 380-380
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@render.py` at line 273, Extract the parser’s 120 frame default into a named constant and add a shared helper such as _modeFrameCount(frames, default) for converting that sentinel to the mode-specific count. Replace the duplicated ternaries at the spin-frame and args.frames call sites around lines 273, 368, and 380 with the helper, passing the appropriate mode default while preserving current behavior.animate/blender_body_sprites.py (2)
35-39: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider storing spin frames as
uint8and converting per use.Each frame is kept as
float32RGBA. For the default spin output (512×512, 48 frames) that is about 192 MB per body, so Earth plus Moon holds roughly 384 MB in memory for each animator instance.renderSolCentauriCinematicAnimationsbuilds one animator per theme, so the peak stays high.Store the loaded PNGs as
uint8and normalize only inside_blendSpinFrames. The consumer inanimate/scenes/sol_centauri_cinematic.py(_flushBlenderBodyOverlays) converts back touint8anyway.♻️ Proposed change
frames: list[np.ndarray] = [] for path in paths: with Image.open(path) as image: - frames.append(np.asarray(image.convert('RGBA'), dtype=np.float32) / 255.0) + frames.append(np.asarray(image.convert('RGBA'), dtype=np.uint8)) return framesThen in
_blendSpinFrames, cast the selected frames with.astype(np.float32) / 255.0before mixing, and adjust_resizeRgbaaccordingly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@animate/blender_body_sprites.py` around lines 35 - 39, Store loaded frames in uint8 form in the frame-loading logic instead of normalizing them to float32. Update _blendSpinFrames to convert the selected frames to float32 and divide by 255.0 before blending, and adjust _resizeRgba to operate consistently with this representation while preserving the existing output behavior.
42-47: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider caching resized frames.
_resizeRgbaruns a LANCZOS resize on every call. The cinematic callsearthFrameandmoonFrameonce per animation frame, so the same source frames are resized repeatedly across the 120-frame loop and again for the second theme. A small dictionary keyed by(index0, index1, blendBucket, size), or a pre-resized frame list per requested resolution, would remove most of that work.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@animate/blender_body_sprites.py` around lines 42 - 47, The resize path repeatedly performs LANCZOS work for identical frames; add caching around _resizeRgba or its callers so resized results are reused, keyed by source frame identity/blend bucket and target size. Ensure the cache is shared across the 120-frame loop and both themes while preserving the existing fast path for already-correct dimensions.animate/scenes/sol_centauri_cinematic.py (1)
1191-1223: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
_flushBlenderBodyLabelsrecomputes the offset with hardcoded Moon values.Three issues in this method:
halfWidthAuis deleted, andself._viewHalfWidthAuis read instead. Remove the parameter, or use it.openCloseup=TrueandbodyScale=0.35are hardcoded. Only the Moon queues a label today, so any future Earth label would be offset by the Moon radius.radiusAu or 0.0silently places the label on the body center when_blenderBillboardRadiusAureturnsNone.Carry the billboard radius in the
_pendingBlenderLabelstuple at queue time. The overlay flush already computes that radius for the same body in the same frame.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@animate/scenes/sol_centauri_cinematic.py` around lines 1191 - 1223, Update _flushBlenderBodyLabels and the _pendingBlenderLabels queue to carry each body’s computed billboard radius at enqueue time, rather than deleting halfWidthAu or recomputing it with Moon-specific openCloseup and bodyScale values. Use the queued radius directly when calculating fracRadius, and handle the existing None result explicitly instead of converting it to zero and placing labels at the body center.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@animate/scenes/blender/flyby_scene.py`:
- Around line 290-308: Update renderPlanetSpin to render each theme into a
temporary frames directory rather than deleting the persistent directory before
_runBlenderFlybyJob. After the render completes and framePaths is non-empty,
replace the existing persistent directory with the temporary directory; ensure
failures or empty renders leave the previously successful spin frames intact.
In `@output/animate/blender/planets/earth/earth_spin_dark_job.json`:
- Around line 460-463: The appearance job serialization emits machine-specific
absolute texture paths. Update BodyAppearance.toJobDict or its
texture-resolution step to write repository-relative paths, and update
render_flyby.py to resolve those paths from the repository root; regenerate the
affected jobs with portable color/clouds entries. Apply this to
output/animate/blender/planets/earth/earth_spin_dark_job.json:460-463,
output/animate/blender/planets/earth/earth_spin_light_job.json:460-463,
output/animate/blender/moons/moon/moon_spin_dark_job.json:457-465, and
output/animate/blender/moons/moon/moon_spin_light_job.json:457-465. Also
determine whether these *_job.json files should be covered by the existing
ignore rules or remain tracked.
In `@render.py`:
- Around line 265-292: Update _runBlenderCinematicPipeline to handle
single-theme execution explicitly: either reject pipeline runs unless theme is
"all", or emit a clear warning that the unselected cinematic theme will use
scatter dots. Keep the existing all-theme behavior unchanged and ensure the
printed pipeline status accurately reflects the selected behavior.
---
Nitpick comments:
In `@animate/blender_body_sprites.py`:
- Around line 35-39: Store loaded frames in uint8 form in the frame-loading
logic instead of normalizing them to float32. Update _blendSpinFrames to convert
the selected frames to float32 and divide by 255.0 before blending, and adjust
_resizeRgba to operate consistently with this representation while preserving
the existing output behavior.
- Around line 42-47: The resize path repeatedly performs LANCZOS work for
identical frames; add caching around _resizeRgba or its callers so resized
results are reused, keyed by source frame identity/blend bucket and target size.
Ensure the cache is shared across the 120-frame loop and both themes while
preserving the existing fast path for already-correct dimensions.
In `@animate/scenes/blender/flyby_scene.py`:
- Line 59: Add the Sequence[FlybyCameraSample] annotation to cameraPath in
_bodyJobSkeleton, ensuring the required typing and sample symbols are used.
Update renderPlanetSpin’s docstring to explicitly state that it returns
frame-directory paths, distinguishing this from renderPlanetFlyby’s GIF-path
result.
- Around line 51-66: Avoid rebuilding the same BodyScene in the job-building
flow: update _bodyJobSkeleton to accept and reuse an already-built BodyScene,
then pass that instance from buildSpinJob and buildFlybyJob instead of calling
buildBodyScene again. Ensure spinFramesDirectory and renderPlanetSpin continue
using the shared scene where applicable, while preserving existing frame-count
and appearance behavior.
In `@animate/scenes/sol_centauri_cinematic.py`:
- Around line 1191-1223: Update _flushBlenderBodyLabels and the
_pendingBlenderLabels queue to carry each body’s computed billboard radius at
enqueue time, rather than deleting halfWidthAu or recomputing it with
Moon-specific openCloseup and bodyScale values. Use the queued radius directly
when calculating fracRadius, and handle the existing None result explicitly
instead of converting it to zero and placing labels at the body center.
In `@render.py`:
- Around line 357-364: The _handleBlenderCommand pipeline path currently
hardcodes the stars catalog and ignores command-line configuration. Add a
--stars argument to the blender subparser and pass args.stars as starsCsvPath,
or centralize the existing default in a shared module-level constant and reuse
it across all commands.
- Line 273: Extract the parser’s 120 frame default into a named constant and add
a shared helper such as _modeFrameCount(frames, default) for converting that
sentinel to the mode-specific count. Replace the duplicated ternaries at the
spin-frame and args.frames call sites around lines 273, 368, and 380 with the
helper, passing the appropriate mode default while preserving current behavior.
In `@tests/test_blender_body_sprites.py`:
- Around line 120-141: Add a test covering the missing-assets fallback:
construct SolCentauriCinematicAnimator with useBlenderBodies=True and an empty
spin directory, invoke update, and assert bodyOverlay.get_images() is empty
while the fallback dot markers are still drawn. Reuse the existing test setup
and update/marker assertions rather than changing production behavior.
- Around line 150-158: Update SolCentauriCinematicAnimator to accept an
output-directory parameter or injected BlenderBodySpriteAtlas, and use it when
constructing blenderSprites. Pass the temporary directory from the test at
construction time so animator.update(0) never loads assets from the default
repository output tree; remove the post-construction atlas replacement.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 47c592b4-0188-4280-97be-76b2275a6864
⛔ Files ignored due to path filters (198)
output/animate/blender/moons/moon/moon_spin_dark.gifis excluded by!**/*.gifoutput/animate/blender/moons/moon/moon_spin_dark/frame_0000.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0001.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0002.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0003.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0004.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0005.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0006.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0007.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0008.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0009.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0010.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0011.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0012.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0013.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0014.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0015.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0016.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0017.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0018.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0019.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0020.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0021.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0022.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0023.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0024.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0025.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0026.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0027.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0028.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0029.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0030.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0031.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0032.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0033.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0034.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0035.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0036.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0037.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0038.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0039.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0040.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0041.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0042.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0043.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0044.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0045.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0046.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_dark/frame_0047.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light.gifis excluded by!**/*.gifoutput/animate/blender/moons/moon/moon_spin_light/frame_0000.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0001.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0002.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0003.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0004.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0005.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0006.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0007.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0008.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0009.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0010.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0011.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0012.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0013.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0014.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0015.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0016.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0017.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0018.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0019.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0020.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0021.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0022.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0023.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0024.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0025.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0026.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0027.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0028.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0029.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0030.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0031.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0032.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0033.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0034.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0035.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0036.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0037.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0038.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0039.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0040.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0041.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0042.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0043.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0044.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0045.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0046.pngis excluded by!**/*.pngoutput/animate/blender/moons/moon/moon_spin_light/frame_0047.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark.gifis excluded by!**/*.gifoutput/animate/blender/planets/earth/earth_spin_dark/frame_0000.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0001.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0002.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0003.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0004.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0005.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0006.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0007.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0008.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0009.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0010.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0011.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0012.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0013.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0014.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0015.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0016.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0017.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0018.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0019.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0020.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0021.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0022.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0023.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0024.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0025.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0026.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0027.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0028.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0029.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0030.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0031.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0032.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0033.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0034.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0035.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0036.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0037.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0038.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0039.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0040.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0041.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0042.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0043.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0044.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0045.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0046.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_dark/frame_0047.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light.gifis excluded by!**/*.gifoutput/animate/blender/planets/earth/earth_spin_light/frame_0000.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0001.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0002.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0003.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0004.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0005.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0006.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0007.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0008.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0009.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0010.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0011.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0012.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0013.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0014.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0015.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0016.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0017.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0018.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0019.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0020.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0021.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0022.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0023.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0024.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0025.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0026.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0027.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0028.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0029.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0030.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0031.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0032.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0033.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0034.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0035.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0036.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0037.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0038.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0039.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0040.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0041.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0042.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0043.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0044.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0045.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0046.pngis excluded by!**/*.pngoutput/animate/blender/planets/earth/earth_spin_light/frame_0047.pngis excluded by!**/*.pngoutput/animate/sol_centauri/blender/sol_centauri_cinematic_blender_dark.gifis excluded by!**/*.gifoutput/animate/sol_centauri/blender/sol_centauri_cinematic_blender_light.gifis excluded by!**/*.gif
📒 Files selected for processing (14)
.gitignoreREADME.mdanimate/blender_body_sprites.pyanimate/scenes/blender/README.mdanimate/scenes/blender/flyby_camera.pyanimate/scenes/blender/flyby_scene.pyanimate/scenes/blender/render_flyby.pyanimate/scenes/sol_centauri_cinematic.pyoutput/animate/blender/moons/moon/moon_spin_dark_job.jsonoutput/animate/blender/moons/moon/moon_spin_light_job.jsonoutput/animate/blender/planets/earth/earth_spin_dark_job.jsonoutput/animate/blender/planets/earth/earth_spin_light_job.jsonrender.pytests/test_blender_body_sprites.py
Comment on lines
+290
to
+308
| for themeName in themes: | ||
| framesDirectory = spinFramesDirectory(bodyName, themeName, outputDirectory=outputRoot) | ||
| if framesDirectory.exists(): | ||
| shutil.rmtree(framesDirectory) | ||
| framesDirectory.mkdir(parents=True, exist_ok=True) | ||
| job = buildSpinJob( | ||
| bodyName, | ||
| theme=themeName, | ||
| frameCount=frameCount, | ||
| resolution=resolution, | ||
| fps=fps, | ||
| framesDirectory=framesDirectory, | ||
| ) | ||
| jobPath = bodyDirectory / f'{stem}_spin_{themeName}_job.json' | ||
| writeFlybyJob(job, jobPath) | ||
| _runBlenderFlybyJob(jobPath) | ||
| framePaths = sorted(framesDirectory.glob('frame_*.png')) | ||
| if not framePaths: | ||
| raise RuntimeError(f'No spin frames rendered for theme={themeName}') |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Delete the existing spin frames only after the render succeeds.
renderPlanetSpin removes the persistent frames directory before it runs Blender. If _runBlenderFlybyJob raises, or Blender writes no frames, the previously good spin loop is already gone. The cinematic then falls back to scatter dots with no way to recover except a successful re-render.
Render into a temporary directory, then replace the persistent directory after the frame check passes.
🛡️ Proposed fix
for themeName in themes:
framesDirectory = spinFramesDirectory(bodyName, themeName, outputDirectory=outputRoot)
- if framesDirectory.exists():
- shutil.rmtree(framesDirectory)
- framesDirectory.mkdir(parents=True, exist_ok=True)
+ stagingDirectory = framesDirectory.with_name(f'{framesDirectory.name}_staging')
+ if stagingDirectory.exists():
+ shutil.rmtree(stagingDirectory)
+ stagingDirectory.mkdir(parents=True, exist_ok=True)
job = buildSpinJob(
bodyName,
theme=themeName,
frameCount=frameCount,
resolution=resolution,
fps=fps,
- framesDirectory=framesDirectory,
+ framesDirectory=stagingDirectory,
)
jobPath = bodyDirectory / f'{stem}_spin_{themeName}_job.json'
writeFlybyJob(job, jobPath)
_runBlenderFlybyJob(jobPath)
- framePaths = sorted(framesDirectory.glob('frame_*.png'))
+ framePaths = sorted(stagingDirectory.glob('frame_*.png'))
if not framePaths:
raise RuntimeError(f'No spin frames rendered for theme={themeName}')
+ if framesDirectory.exists():
+ shutil.rmtree(framesDirectory)
+ stagingDirectory.rename(framesDirectory)
+ framePaths = sorted(framesDirectory.glob('frame_*.png'))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for themeName in themes: | |
| framesDirectory = spinFramesDirectory(bodyName, themeName, outputDirectory=outputRoot) | |
| if framesDirectory.exists(): | |
| shutil.rmtree(framesDirectory) | |
| framesDirectory.mkdir(parents=True, exist_ok=True) | |
| job = buildSpinJob( | |
| bodyName, | |
| theme=themeName, | |
| frameCount=frameCount, | |
| resolution=resolution, | |
| fps=fps, | |
| framesDirectory=framesDirectory, | |
| ) | |
| jobPath = bodyDirectory / f'{stem}_spin_{themeName}_job.json' | |
| writeFlybyJob(job, jobPath) | |
| _runBlenderFlybyJob(jobPath) | |
| framePaths = sorted(framesDirectory.glob('frame_*.png')) | |
| if not framePaths: | |
| raise RuntimeError(f'No spin frames rendered for theme={themeName}') | |
| for themeName in themes: | |
| framesDirectory = spinFramesDirectory(bodyName, themeName, outputDirectory=outputRoot) | |
| stagingDirectory = framesDirectory.with_name(f'{framesDirectory.name}_staging') | |
| if stagingDirectory.exists(): | |
| shutil.rmtree(stagingDirectory) | |
| stagingDirectory.mkdir(parents=True, exist_ok=True) | |
| job = buildSpinJob( | |
| bodyName, | |
| theme=themeName, | |
| frameCount=frameCount, | |
| resolution=resolution, | |
| fps=fps, | |
| framesDirectory=stagingDirectory, | |
| ) | |
| jobPath = bodyDirectory / f'{stem}_spin_{themeName}_job.json' | |
| writeFlybyJob(job, jobPath) | |
| _runBlenderFlybyJob(jobPath) | |
| framePaths = sorted(stagingDirectory.glob('frame_*.png')) | |
| if not framePaths: | |
| raise RuntimeError(f'No spin frames rendered for theme={themeName}') | |
| if framesDirectory.exists(): | |
| shutil.rmtree(framesDirectory) | |
| stagingDirectory.rename(framesDirectory) | |
| framePaths = sorted(framesDirectory.glob('frame_*.png')) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@animate/scenes/blender/flyby_scene.py` around lines 290 - 308, Update
renderPlanetSpin to render each theme into a temporary frames directory rather
than deleting the persistent directory before _runBlenderFlybyJob. After the
render completes and framePaths is non-empty, replace the existing persistent
directory with the temporary directory; ensure failures or empty renders leave
the previously successful spin frames intact.
Comment on lines
+460
to
+463
| "textures": { | ||
| "color": "/Users/tom/Desktop/SOLSYS/data/textures/bodies/earth/color.png", | ||
| "clouds": "/Users/tom/Desktop/SOLSYS/data/textures/bodies/earth/clouds.png" | ||
| }, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Committed spin job files embed absolute local texture paths. All four generated job files record texture paths under /Users/tom/Desktop/SOLSYS/.... The shared root cause is the appearance serialization, which writes machine-absolute paths, combined with committing these generated files. The paths expose a local username and do not resolve on any other machine or in CI, so a re-run of render.py blender --spin fails to find the textures.
output/animate/blender/planets/earth/earth_spin_dark_job.json#L460-L463: replace thecolorandcloudsabsolute paths with paths relative to the repository root, or remove the file from version control.output/animate/blender/planets/earth/earth_spin_light_job.json#L460-L463: apply the same change to thecolorandcloudsentries.output/animate/blender/moons/moon/moon_spin_dark_job.json#L457-L465: apply the same change to thecolorentry.output/animate/blender/moons/moon/moon_spin_light_job.json#L457-L465: apply the same change to thecolorentry.
Fix the writer in BodyAppearance.toJobDict (or its texture resolution step) to emit repository-relative paths, and resolve them against the repository root inside animate/scenes/blender/render_flyby.py. The stack also adds .gitignore rules for local Blender artifacts; confirm whether these *_job.json files should be ignored too.
📍 Affects 4 files
output/animate/blender/planets/earth/earth_spin_dark_job.json#L460-L463(this comment)output/animate/blender/planets/earth/earth_spin_light_job.json#L460-L463output/animate/blender/moons/moon/moon_spin_dark_job.json#L457-L465output/animate/blender/moons/moon/moon_spin_light_job.json#L457-L465
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@output/animate/blender/planets/earth/earth_spin_dark_job.json` around lines
460 - 463, The appearance job serialization emits machine-specific absolute
texture paths. Update BodyAppearance.toJobDict or its texture-resolution step to
write repository-relative paths, and update render_flyby.py to resolve those
paths from the repository root; regenerate the affected jobs with portable
color/clouds entries. Apply this to
output/animate/blender/planets/earth/earth_spin_dark_job.json:460-463,
output/animate/blender/planets/earth/earth_spin_light_job.json:460-463,
output/animate/blender/moons/moon/moon_spin_dark_job.json:457-465, and
output/animate/blender/moons/moon/moon_spin_light_job.json:457-465. Also
determine whether these *_job.json files should be covered by the existing
ignore rules or remain tracked.
Comment on lines
+265
to
+292
| def _runBlenderCinematicPipeline( | ||
| *, | ||
| theme: str, | ||
| frames: int, | ||
| outputDirectory: str, | ||
| starsCsvPath: str, | ||
| ) -> None: | ||
| """Earth+Moon spin loops, then Sol→Centauri cinematic compositing those frames.""" | ||
| spinFrames = 48 if frames == 120 else frames | ||
| for bodyName in ('Earth', 'Moon'): | ||
| print(f'[pipeline] Blender spin loop → {bodyName}') | ||
| paths = renderPlanetSpin( | ||
| bodyName, | ||
| theme=theme, | ||
| frameCount=spinFrames, | ||
| outputDirectory=outputDirectory, | ||
| ) | ||
| for path in paths: | ||
| print(f'Spin ready → {path}') | ||
|
|
||
| print('[pipeline] Sol→Centauri cinematic with Blender spin billboards') | ||
| renderSolCentauriCinematicAnimations( | ||
| figureSizeInches=ANIMATE_FIGURE_SIZE_INCHES, | ||
| dpi=ANIMATE_DPI_CINEMATIC, | ||
| starsCsvPath=starsCsvPath, | ||
| useBlenderBodies=True, | ||
| ) | ||
| print('[pipeline] done') |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
--pipeline with a single theme still renders both cinematic themes.
_runBlenderCinematicPipeline forwards args.theme to renderPlanetSpin, so --theme dark renders only the dark spin loops. renderSolCentauriCinematicAnimations then always renders both the light and the dark cinematic. The light run finds no light spin frames and silently falls back to scatter dots, which contradicts the printed pipeline message.
Either require --theme all for --pipeline, or print a clear warning that the other theme will use dots.
🛡️ Proposed guard
"""Earth+Moon spin loops, then Sol→Centauri cinematic compositing those frames."""
+ if theme != 'all':
+ print(
+ f'[pipeline] warning: --theme {theme} renders spin frames for one theme only. '
+ 'The other cinematic theme falls back to scatter dots.'
+ )
spinFrames = 48 if frames == 120 else frames📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def _runBlenderCinematicPipeline( | |
| *, | |
| theme: str, | |
| frames: int, | |
| outputDirectory: str, | |
| starsCsvPath: str, | |
| ) -> None: | |
| """Earth+Moon spin loops, then Sol→Centauri cinematic compositing those frames.""" | |
| spinFrames = 48 if frames == 120 else frames | |
| for bodyName in ('Earth', 'Moon'): | |
| print(f'[pipeline] Blender spin loop → {bodyName}') | |
| paths = renderPlanetSpin( | |
| bodyName, | |
| theme=theme, | |
| frameCount=spinFrames, | |
| outputDirectory=outputDirectory, | |
| ) | |
| for path in paths: | |
| print(f'Spin ready → {path}') | |
| print('[pipeline] Sol→Centauri cinematic with Blender spin billboards') | |
| renderSolCentauriCinematicAnimations( | |
| figureSizeInches=ANIMATE_FIGURE_SIZE_INCHES, | |
| dpi=ANIMATE_DPI_CINEMATIC, | |
| starsCsvPath=starsCsvPath, | |
| useBlenderBodies=True, | |
| ) | |
| print('[pipeline] done') | |
| def _runBlenderCinematicPipeline( | |
| *, | |
| theme: str, | |
| frames: int, | |
| outputDirectory: str, | |
| starsCsvPath: str, | |
| ) -> None: | |
| """Earth+Moon spin loops, then Sol→Centauri cinematic compositing those frames.""" | |
| if theme != 'all': | |
| print( | |
| f'[pipeline] warning: --theme {theme} renders spin frames for one theme only. ' | |
| 'The other cinematic theme falls back to scatter dots.' | |
| ) | |
| spinFrames = 48 if frames == 120 else frames | |
| for bodyName in ('Earth', 'Moon'): | |
| print(f'[pipeline] Blender spin loop → {bodyName}') | |
| paths = renderPlanetSpin( | |
| bodyName, | |
| theme=theme, | |
| frameCount=spinFrames, | |
| outputDirectory=outputDirectory, | |
| ) | |
| for path in paths: | |
| print(f'Spin ready → {path}') | |
| print('[pipeline] Sol→Centauri cinematic with Blender spin billboards') | |
| renderSolCentauriCinematicAnimations( | |
| figureSizeInches=ANIMATE_FIGURE_SIZE_INCHES, | |
| dpi=ANIMATE_DPI_CINEMATIC, | |
| starsCsvPath=starsCsvPath, | |
| useBlenderBodies=True, | |
| ) | |
| print('[pipeline] done') |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@render.py` around lines 265 - 292, Update _runBlenderCinematicPipeline to
handle single-theme execution explicitly: either reject pipeline runs unless
theme is "all", or emit a clear warning that the unselected cinematic theme will
use scatter dots. Keep the existing all-theme behavior unchanged and ensure the
printed pipeline status accurately reflects the selected behavior.
Defer the opening Earth label onto the body overlay (with correct body scale) and reset overlay limits after imshow so the globe no longer covers the word.
Start the blender cinematic zoomed on Earth alone for ~2 spin turns, then ease out to reveal Luna; classic dotted open is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--blender-bodies(in-animation billboards, not GIF splicing)--spin/--pipeline), unit tests, README commands, and rendered light/dark blender cinematic GIFsCloses #42
Test plan
pytest tests/test_blender_body_sprites.pypython render.py blender --pipeline(or reuse existing spin frames) thenpython render.py animate --system sol_centauri --blender-bodies--blender-bodiesoutput/animate/sol_centauri/blender/sol_centauri_cinematic_blender_{light,dark}.gifSummary by CodeRabbit
New Features
Documentation
Tests