Light refactor in animations

4 files changed, 128 insertions, 59 deletions

diff --git a/animation.scm b/animation.scm
index f166bff..5753bc1 100644
--- a/animation.scm
+++ b/animation.scm
@@ -26,16 +26,27 @@
 (define (frame-by-idx frames frame-idx)
   (list-ref frames (modulo frame-idx (length frames))))
 
+;; A "timed" frame is =(tile-id tick-budget)= — a two-element proper list.
+;; A bare tile id is a number (or other atom).
+(define (timed-frame? frame-def)
+  (and (pair? frame-def)
+       (pair? (cdr frame-def))
+       (null? (cddr frame-def))))
+
 ;; The tile ID is 1-indexed.
 (define (frame->tile-id frames frame-idx)
   (let ((frame-def (frame-by-idx frames frame-idx)))
-    (if (list? frame-def)
+    (if (timed-frame? frame-def)
 	(car frame-def)
 	frame-def)))
 
-(define (frame->duration animation frame-idx)
+;; Tick budget for one frame, in this order:
+;; 1. Per-frame duration from =(tile duration)= in #:frames
+;; 2. Else the animation alist's #:duration
+;; 3. Else +default-anim-duration+ (global default)
+(define (animation-frame-duration animation frame-idx)
   (let ((frame-def (frame-by-idx (animation-frames animation) frame-idx)))
-    (if (list? frame-def)
+    (if (timed-frame? frame-def)
 	(cadr frame-def)
 	(animation-ref animation #:duration +default-anim-duration+))))
 
@@ -63,22 +74,26 @@
 ;; Advance the animation tick/frame counter for one game tick.
 ;; Pass the animation table for this entity's type.
 ;; Entities without #:anim-name are returned unchanged.
+;; Tick threshold is always (animation-frame-duration anim frame) — per-frame pair,
+;; then animation #:duration, then +default-anim-duration+. #:anim-duration
+;; on the entity is set every step to that resolved value.
 
 (define (advance-animation entity anim dt)
-  (let ((tick     (+ dt (entity-ref entity #:anim-tick 0)))
-        (duration (entity-ref entity #:duration +default-anim-duration+))
-        (frames   (animation-frames anim))
-        (frame    (entity-ref entity #:anim-frame 0)))
+  (let* ((frame    (entity-ref entity #:anim-frame 0))
+         (frames   (animation-frames anim))
+         (duration (animation-frame-duration anim frame))
+         (tick     (+ dt (entity-ref entity #:anim-tick 0))))
     (if (>= tick duration)
         (let ((new-frame-id (modulo (+ frame 1) (length frames))))
           (entity-set-many entity
 			   `((#:anim-tick . 0)
 			     (#:anim-frame . ,new-frame-id)
 			     (#:tile-id . ,(frame->tile-id frames new-frame-id))
-			     (#:duration . ,(frame->duration anim new-frame-id)))))
+			     (#:anim-duration . ,(animation-frame-duration anim new-frame-id)))))
         (entity-set-many entity
 			 `((#:anim-tick . ,tick)
-			   (#:tile-id . ,(frame->tile-id frames frame)))))))
+			   (#:tile-id . ,(frame->tile-id frames frame))
+			   (#:anim-duration . ,duration))))))
 
 (define (animate-entity entity animations dt)
   (let* ((anim-name (entity-ref entity #:anim-name #f))
diff --git a/docs/animation.org b/docs/animation.org
index 4a3939e..6704034 100644
--- a/docs/animation.org
+++ b/docs/animation.org
@@ -72,37 +72,54 @@ need to call anything per frame just to make the animation play.
 
 ** Animation data shape
 
-An animation is an alist with three keys:
+An animation is an alist with these keys:
 
 | Key          | Required | Meaning                                           |
 |--------------+----------+---------------------------------------------------|
 | =#:name=     | yes      | Symbol used to select this animation.             |
 | =#:frames=   | yes      | List of frames (see below).                       |
-| =#:duration= | no       | Default ticks-per-frame. Used when a frame        |
-|              |          | does not carry its own duration. Defaults to 10.  |
+| =#:duration= | no       | Default /per-frame/ budget for *bare* frame      |
+|              |          | entries (see [[*Duration resolution][Duration resolution]]). |
 
-=#:frames= can take two forms, and you can mix them inside one
-animation only if you're careful about what each element means.
+** =#:frames= layouts (prefab plist style)
 
-*Simple form* — a list of bare tile ids. Each frame holds for the
-animation's =#:duration= ticks (or 10 if no duration is given):
+You can combine bare tile ids and per-frame pairs in the same
+=#:frames= list, but each element must be unambiguous:
 
-#+begin_src scheme
-(#:name attack #:frames (28 29) #:duration 10)
-#+end_src
+- *Bare ids* — every element is a tile id on its own (number or
+  symbol, depending on your data). All such frames share one budget:
+  the animation's =#:duration=, or the global default if the
+  animation omits =#:duration=.
+
+  #+begin_src scheme
+  (#:name attack #:frames (1 2 3) #:duration 100)
+  #+end_src
+
+- *Per-frame budgets* — a frame is a two-element /proper/ list
+  ='(tile-id budget)=. That frame always uses =budget=; the
+  animation's top-level =#:duration= does not apply to it. Lists with
+  more than two elements are /not/ treated as timed frames (they fall
+  through to the bare-id rules and will not behave as intended).
+
+  #+begin_src scheme
+  (#:name walk #:frames ((1 100) (2 300) (3 25)))
+  #+end_src
+
+- *Bare ids, animation-wide default omitted* — same as the first case,
+  but the budget for every bare frame is the global default
+  =+default-anim-duration+= (100 in =animation.scm=, same units as
+  engine =dt= — typically milliseconds).
+
+  #+begin_src scheme
+  (#:name idle #:frames (1 2 3))
+  #+end_src
 
-*Timed form* — a list of =(tile-id duration-ticks)= pairs. Each frame
-carries its own duration; the animation's top-level =#:duration= is
-ignored for that frame:
+*Uniform example* (bare frames, shared =#:duration=):
 
 #+begin_src scheme
-(#:name walk #:frames ((28 10) (29 1000)))
+(#:name attack #:frames (28 29) #:duration 10)
 #+end_src
 
-In the timed form above, tile 28 shows for 10 ticks and tile 29 shows
-for 1000 ticks — a deliberately uneven cycle useful for things like
-blink-idle animations.
-
 The =#:animations= entity key holds a *list* of these animation
 records, one per named animation:
 
@@ -112,10 +129,30 @@ records, one per named animation:
               (#:name jump #:frames (30)))
 #+end_src
 
-For a real working example of both forms side by side, see the
-=timed-frames= and =std-frames= prefabs in
+For a real working example of timed vs uniform frames side by side,
+see the =timed-frames= and =std-frames= prefabs in
 =demo/assets/animation-prefabs.scm=.
 
+** Duration resolution
+
+For the frame at index =i= in =#:frames=, =animation-frame-duration= resolves the
+tick budget in this order (and =advance-animation= always uses this —
+there is no separate “entity override” for the threshold):
+
+1. *Per-frame* — if the entry is a two-element list ='(tile-id budget)=,
+   use =budget=.
+2. *Animation default* — otherwise use the animation alist's
+   =#:duration= when present.
+3. *Global default* — otherwise =+default-anim-duration+= (100 in
+   =animation.scm=).
+
+The same resolution runs on every =apply-animation= tick: the
+accumulated =#:anim-tick= is compared to =(animation-frame-duration anim
+(entity-ref entity #:anim-frame))=, and =#:anim-duration= on the
+entity is set to that resolved value /every/ step so it always
+reflects the budget for the *current* frame index (whether the frame
+advanced this tick or not).
+
 ** The apply-animation pipeline step
 
 =apply-animation= is the *last* stage of =default-engine-update=,
@@ -150,12 +187,19 @@ When the step does run, =animate-entity= looks up the current
 animation by =#:anim-name= in the entity's =#:animations= list, and
 =advance-animation= does the actual work:
 
-- Increment =#:anim-tick= by 1.
-- If tick exceeds the current frame's duration, advance
-  =#:anim-frame= (modulo the number of frames), reset =#:anim-tick=
-  to 0, and write the new frame's =#:tile-id= onto the entity.
-- Otherwise, just write the current frame's =#:tile-id= and the
-  incremented tick.
+- Resolve the current frame's tick budget with =animation-frame-duration= (see
+  [[*Duration resolution][Duration resolution]]); mirror that value
+  onto =#:anim-duration= on the entity.
+- Accumulate =#:anim-tick= by the frame delta =dt= (same units as the
+  budgets above).
+- If the tick reaches or exceeds that budget, advance =#:anim-frame=
+  (modulo the number of frames), reset =#:anim-tick= to 0, update
+  =#:anim-duration= to the /new/ frame's resolved budget, and write the
+  new frame's =#:tile-id= onto the entity.
+- Otherwise, write the current frame's =#:tile-id=, the incremented
+  tick, and keep =#:anim-duration= equal to the /current/ frame's
+  resolved budget (unchanged value unless you changed =#:frames= or
+  animation =#:duration= out of band).
 
 The renderer reads =#:tile-id= directly — so as long as the pipeline
 ran, what ends up on screen is always the current frame's tile.
@@ -317,23 +361,20 @@ Two details worth noticing:
 
 ** Per-frame durations for non-uniform timing
 
-Use the =(tile-id duration)= frame form when you want a cycle where
-one frame lingers and another flashes past. The canonical example
-(from =demo/assets/animation-prefabs.scm=) is a blink-heavy idle:
+Use the two-element ='(tile-id budget)= frame form when you want a
+cycle where one frame lingers and another flashes past. The canonical
+example (from =demo/assets/animation-prefabs.scm=) is a blink-heavy
+idle:
 
 #+begin_src scheme
 (#:name idle #:frames ((28 10) (29 1000)))
 #+end_src
 
 Tile 28 shows for 10 ticks (a quick flash), then tile 29 holds for
-1000 ticks (a long eye-open pose), then the cycle repeats. You can
-mix this with the simple form across different animations in the
-same entity — it's the frame shape that matters, not the animation.
-
-Per-frame durations override any top-level =#:duration= on the
-animation; in fact the top-level =#:duration= is only consulted when
-=advance-animation= falls back to the default of 10 (see
-=frame->duration= in =animation.scm=).
+1000 ticks (a long eye-open pose), then the cycle repeats. Per-frame
+budgets are step /1/ in [[*Duration resolution][Duration resolution]];
+they override the animation's top-level =#:duration=, which in turn
+overrides the global default for bare frames only.
 
 ** Disabling animation on an entity without touching =#:animations=
 
@@ -345,8 +386,8 @@ to list =animation= in its =#:skip-pipelines=:
 (entity-set entity #:skip-pipelines '(animation))
 #+end_src
 
-This leaves =#:animations=, =#:anim-name=, =#:anim-frame=, and
-=#:anim-tick= untouched. When you remove =animation= from
+This leaves =#:animations=, =#:anim-name=, =#:anim-frame=,
+=#:anim-tick=, and =#:anim-duration= untouched. When you remove =animation= from
 =#:skip-pipelines= later, the cycle resumes exactly where it left
 off. Contrast with =(entity-set entity #:animations #f)=, which
 strips the animation data entirely and would require you to rebuild
diff --git a/docs/physics.org b/docs/physics.org
index 233ffc8..9c57cc1 100644
--- a/docs/physics.org
+++ b/docs/physics.org
@@ -255,7 +255,10 @@ entity list.
 *** apply-animation
 
 - *Reads*: =#:animations=, =#:anim-name=, =#:anim-frame=, =#:anim-tick=
-- *Writes*: =#:anim-tick=, =#:anim-frame=, =#:tile-id=, =#:duration=
+  (the tick budget each step is =animation-frame-duration= on the current
+  animation + frame index — see =docs/animation.org=).
+- *Writes*: =#:anim-tick=, =#:anim-frame=, =#:tile-id=, =#:anim-duration=
+  (=#:anim-duration= mirrors the resolved budget for the current frame).
 - *Guard*: only runs when =#:animations= is present.
 - Advances the per-entity animation state machine and updates
   =#:tile-id= so the renderer draws the correct sprite. Fully
diff --git a/tests/animation-test.scm b/tests/animation-test.scm
index 00dd217..7ee634c 100644
--- a/tests/animation-test.scm
+++ b/tests/animation-test.scm
@@ -22,13 +22,22 @@
     (test "wraps around" 0 (frame->tile-id '((0 10) (1 10)) 2))
     (test "frame 1 of (27 28)" 28 (frame->tile-id '((27 10) (28 10)) 1))))
 
-(test-group "frame->duration"
-  (test "first frame, frames (0)" 100 (frame->duration '((#:frames . ((0 100))))
+(test-group "animation-frame-duration"
+  (test "first frame, frames (0)" 100 (animation-frame-duration '((#:frames . ((0 100))))
 						       0))
-  (test "wraps around" 100 (frame->duration '((#:frames . ((0 100) (1 200))))
+  (test "wraps around" 100 (animation-frame-duration '((#:frames . ((0 100) (1 200))))
 					    0))
-  (test "frame 1 of (27 28)" 200 (frame->duration '((#:frames . ((27 100) (28 200))))
+  (test "frame 1 of (27 28)" 200 (animation-frame-duration '((#:frames . ((27 100) (28 200))))
 						  1))
+  (test "bare frame uses animation #:duration"
+    4
+    (animation-frame-duration (anim #:name 'x #:frames '(0 1) #:duration 4) 0))
+  (test "bare frame falls back to global default without animation #:duration"
+    100
+    (animation-frame-duration (anim #:name 'x #:frames '(0 1)) 0))
+  (test "timed frame ignores animation #:duration"
+    10
+    (animation-frame-duration (anim #:name 'x #:frames '((0 10) (1 20)) #:duration 4) 0))
   )
 
 (test-group "set-animation"
@@ -57,7 +66,7 @@
            (stepped (animate-entity e anims 200)))
       (test "ticks resets on frame switch" 0 (entity-ref stepped #:anim-tick))
       (test "sets tile-id on 10th tick" 1 (entity-ref stepped #:tile-id))
-      (test "sets duration to frame duration" 20 (entity-ref stepped #:duration))))
+      (test "sets anim-duration to frame duration" 20 (entity-ref stepped #:anim-duration))))
   (test-group "Empty"
     (let* ((e (entity #:type 'player)))
       (test "unchanged entity without anim-name" e (animate-entity e '() 1)))))
@@ -66,11 +75,12 @@
   (test-group "animated entity"
     (let* ((anims (list (anim #:name 'walk #:frames '(2 3) #:duration 4)))
            (e (entity #:type 'player #:anim-name 'walk #:anim-frame 0 #:anim-tick 0 #:animations anims))
-	   (stepped-entity (apply-animation #f e 10)))
-      (test "Updated animated entity" 10 (entity-ref stepped-entity #:anim-tick)))
-    (let* ((e (entity #:type 'static))
-	   (stepped-entity (apply-animation #f e 10)))
-      (test "unchanged static entity" #f (entity-ref stepped-entity #:anim-tick)))))
+	   (stepped-entity (apply-animation #f e 3)))
+      (test "accumulates anim-tick against frame duration" 3 (entity-ref stepped-entity #:anim-tick))
+      (test "mirrors current frame duration on entity" 4 (entity-ref stepped-entity #:anim-duration))))
+  (let* ((e (entity #:type 'static))
+         (stepped-entity (apply-animation #f e 10)))
+    (test "unchanged static entity" #f (entity-ref stepped-entity #:anim-tick))))
 
 (test-end "animation")
 (test-exit)