diff --git a/gif/SKILL.md b/gif/SKILL.md index f95e07a..c7991d4 100644 --- a/gif/SKILL.md +++ b/gif/SKILL.md @@ -76,9 +76,18 @@ bottom of this skill) as `{"name": "gifsmith.py", "file_id": ""}`, then write ONE `code_exec` call: ```python -# gifsmith re-exports the drawing vocabulary, so this one import is enough: +# gifsmith re-exports the drawing vocabulary, so this one import is enough. +# The COMPLETE importable list (there is nothing else — don't guess, and don't +# spend a call on dir(gifsmith)): +# Animation +# Arc Circle Ellipse FancyBboxPatch PathPatch Path Polygon Rectangle Wedge +# Image ImageDraw ImageFont (PIL, for mode="pil") +# np plt (numpy / pyplot, for anything else) +# GIF_MAX_SECONDS GIF_MAX_BYTES from gifsmith import Animation, Circle, Ellipse, Rectangle, Polygon, Wedge, np anim = Animation(width=640, height=480, fps=15) # canvas locked here +# Animation() takes ONLY width, height, fps, dpi, bg (background= also works). +# Everything about the output — format, size, audio — is set on render(). # ---- cast: defined ONCE, called from every scene ---- def draw_steve(ax, x, y, rage=0.0): diff --git a/gif/scripts/gifsmith.py b/gif/scripts/gifsmith.py index 3b127cf..63bf74f 100644 --- a/gif/scripts/gifsmith.py +++ b/gif/scripts/gifsmith.py @@ -37,6 +37,7 @@ fix just that scene. It also writes /workspace/still.png: a labelled contact sheet, one tile per scene, for the critique pass. """ +import difflib import glob import math import os @@ -52,10 +53,34 @@ from PIL import Image, ImageDraw, ImageFont # noqa: E402 # Re-exported so one import line covers the whole drawing vocabulary — a missing # `from matplotlib.patches import ...` is otherwise a NameError that costs a pass. +# +# PathPatch and Path are here because agents assumed they already were: two +# separate workers wrote `from gifsmith import PathPatch` and got an ImportError, +# because the module exported SOME matplotlib artists and not others with no way +# to tell which from outside. The rule now is "the drawing vocabulary, whole". from matplotlib.patches import ( # noqa: E402,F401 - Arc, Circle, Ellipse, FancyBboxPatch, Polygon, Rectangle, Wedge) + Arc, Circle, Ellipse, FancyBboxPatch, PathPatch, Polygon, Rectangle, Wedge) +from matplotlib.path import Path # noqa: E402,F401 import numpy as np # noqa: E402,F401 +# The public surface, stated rather than discovered. Without this, `dir(gifsmith)` +# returned 24 names of which nine were incidental imports (glob, os, shutil, +# subprocess, sys, traceback...) — several of them things a scene author should +# never touch — and a worker spent a whole code_exec call running exactly that +# to find out what it could import. Keep in sync when adding a re-export. +__all__ = [ + "Animation", + # matplotlib artists (mode="mpl") + "Arc", "Circle", "Ellipse", "FancyBboxPatch", "PathPatch", "Path", + "Polygon", "Rectangle", "Wedge", + # PIL (mode="pil") + "Image", "ImageDraw", "ImageFont", + # the two libraries themselves, for anything not re-exported above + "np", "plt", + # caps a scene author may want to read + "GIF_MAX_SECONDS", "GIF_MAX_BYTES", +] + # A GIF longer than this balloons past what's worth shipping as a GIF. GIF_MAX_SECONDS = 20 # Keep a GIF comfortably under Discord's ~10 MiB inline limit. @@ -124,7 +149,35 @@ class _Scene: class Animation: """Collects scenes, renders them to a locked-size frame sequence, encodes.""" - def __init__(self, width=640, height=480, fps=15, dpi=100, bg="white"): + def __init__(self, width=640, height=480, fps=15, dpi=100, bg="white", **kwargs): + # `background` is accepted as an alias for `bg` because it is the obvious + # synonym and agents reached for it; `force` is rejected loudly because + # `render(force="mp4")` DOES take it, which makes it read like an + # Animation-level setting. Both were real failures, one code_exec round + # trip each, and both are the kind a better message fixes for free. + if "background" in kwargs: + bg = kwargs.pop("background") + if kwargs: + valid = ["width", "height", "fps", "dpi", "bg (or background)"] + bad = sorted(kwargs) + hints = [] + for k in bad: + near = difflib.get_close_matches( + k, ["width", "height", "fps", "dpi", "bg", "background"], n=1, cutoff=0.6) + if near: + hints.append(f"{k!r} — did you mean {near[0]!r}?") + elif k == "force": + hints.append("'force' belongs to render(force='mp4'/'gif'), " + "not to Animation()") + else: + hints.append(repr(k)) + raise TypeError( + "Animation() got unexpected keyword argument(s): " + + "; ".join(hints) + + ". Valid: " + ", ".join(valid) + + ". Everything about the OUTPUT (format, size, audio) is set on " + "render(), not here.") + # H.264 needs even dimensions; lock them here so the encoder never fails. self.width = int(width) // 2 * 2 self.height = int(height) // 2 * 2 @@ -217,10 +270,46 @@ class Animation: # ------------------------------------------------------------------- render + def _dry_run_scenes(self): + """Render every scene ONCE at t=0 before the real pass. + + Scene functions fail at render time, not definition time, so a NameError + in scene 4 used to surface only after scenes 0-3 had been fully rendered + — and, when the pipe was open, after frames had already gone into the + encoder. Two production workers lost a whole render each to + `RuntimeError: every scene failed`, learning about one broken name per + attempt. + + One frame per scene costs almost nothing and reports EVERY broken scene + in a single pass. A scene that fails here is marked and skipped by the + real loop rather than re-run to fail identically; if they ALL fail there + is nothing to encode, so say so now instead of after the setup. + """ + for i, sc in enumerate(self._scenes): + try: + (self._render_pil if sc.mode == "pil" else self._render_mpl)(sc, 0.0) + except Exception: # noqa: BLE001 + sc.error = traceback.format_exc(limit=6) + print(f"[gifsmith] SCENE {i} ({sc.title!r}) FAILED its t=0 dry " + f"run — skipped, other scenes continue:\n{sc.error}", + file=sys.stderr, flush=True) + broken = [i for i, sc in enumerate(self._scenes) if sc.error] + if broken and len(broken) == len(self._scenes): + raise RuntimeError( + "every scene failed before rendering started — see the " + f"{len(broken)} traceback(s) above. They usually share ONE " + "cause: a helper called with a keyword it does not take, or a " + "name that differs by a character. Fix it and re-run the whole " + "program in a fresh code_exec call, remembering gifsmith.py in " + "files_in. Nothing was encoded and no GPU time was spent.") + return broken + def render(self, long_edge=None, force=None, audio=None): if not self._scenes: raise RuntimeError("no scenes registered — decorate at least one " "function with @anim.scene(...)") + # Before ANY setup: no scratch dir, no ffmpeg pipe, no frames. + self._dry_run_scenes() self._scratch, free_mb = _pick_scratch() long_edge = long_edge or max(self.width, self.height) @@ -256,6 +345,8 @@ class Animation: per_scene = max(1, CONTACT_SHEET_MAX_TILES // len(self._scenes)) for i, sc in enumerate(self._scenes): + if sc.error: + continue # already reported by the dry run; don't fail it twice start = self._count n = max(1, int(round(sc.seconds * self.fps))) try: