Merge pull request #115 from DramaticShape/dev

1.6.1

.modkitignore

@@ -4,6 +4,8 @@
 # The SDK suite and the probes it grew out of. A shipped test that requires
 # an engine module reads as a private require against the archive
 # (CONTRIBUTING-mods.md "What the PR must contain", 2).
+tests/arena_config.lua
+tests/arena_editor.lua
 tests/arena_pick.lua
 tests/battle_shots.lua
 tests/dramatic_shape_test.lua

CHANGELOG.md

@@ -1,5 +1,24 @@
 # Changelog
 
+## Unreleased
+
+### Fixed
+
+- **Exeggutor, Tangela and Magmar stand as models in STADIUM battles, and
+  Pidgeot and Dodrio stop animating garbled.** Five species animate with
+  hermite keyframes rather than packed per-frame streams, and the extractor
+  read the animation flags byte from the wrong half of its u16 -- the half
+  that is always zero -- so it decoded their keyframe tables as streams.
+  For Exeggutor, Tangela and Magmar the result exploded so hard the packer
+  declined them to flat battle pics; Pidgeot and Dodrio stayed models but
+  played the garbage. All five now decode the way the game's own sampler
+  (src/17300.c) does, and no species is held off the field any more.
+
+  Model packs built by an older version of the mod are detected by a
+  revision stamp in the install marker and rebuilt from the ROM on the next
+  launch (or shadowed by a checkout's freshly packed set) rather than
+  trusted.
+
 ## 1.6.0
 
 ### Added

lib/StadiumBuild.lua

@@ -216,13 +216,14 @@ end
 
 StadiumBuild.stance = stance
 
--- Whether this species' standby loop is corrupt in the source data.
+-- Whether this species' standby loop is corrupt as extracted.
 --
--- A handful come out of the extraction with animations that throw bones
--- hundreds of units off the body -- Exeggutor, Tangela and Magmar, whose
--- channel streams the game's own index arithmetic evidently reads differently
--- from the way this does. Played, they look like a Pokemon coming apart; the
--- mod would rather stand them still.
+-- No species trips this today. Exeggutor, Tangela and Magmar used to, when
+-- the flags byte was misread and their hermite-keyframe animations were
+-- decoded as packed streams, throwing bones hundreds of units off the body.
+-- It stays as the guard against the next extraction bug: played, a broken
+-- idle looks like a Pokemon coming apart, and the mod would rather show
+-- the sprite fallback (see StadiumMon).
 --
 -- The test is deliberately narrow, because "differs from the bind pose" is NOT
 -- brokenness. It is asked only of the STANDBY loop -- the one animation that

lib/StadiumFragment.lua

@@ -22,9 +22,10 @@
 -- everything else is render state this rebuilds from the texture table
 -- instead.
 --
--- THE ANIMATIONS are packed per-frame streams of 12- or 16-bit fields. The
--- game also has a hermite-keyframe mode (flags & 8); it is ported for
--- completeness but no animation of any of the 151 battle Pokemon uses it.
+-- THE ANIMATIONS are packed per-frame streams of 12- or 16-bit fields for
+-- 146 of the 151 species. The other five -- Pidgeot, Dodrio, Exeggutor,
+-- Tangela and Magmar -- use the game's hermite-keyframe mode (flags & 8),
+-- some for every animation, some for a subset.
 --
 -- ------- arithmetic, and why there are no bit operations here
 --
@@ -86,10 +87,31 @@ local function roundHalfEven(x)
   return f + 1
 end
 
+-- Python's round(x, nd). Multiplying by 10^nd first is WRONG: the multiply
+-- itself rounds, and a value just under a decimal boundary can land exactly
+-- on it -- Magmar's attack scale track holds 1.008874999..., whose *1e5 is
+-- exactly 100887.5, and the fold then differed from the oracle by one bit.
+-- Python rounds the EXACT binary expansion, and so does LuaJIT's own float
+-- formatter, so the digits come from string.format instead.
 local function roundTo(x, nd)
   if nd == 0 then return roundHalfEven(x) end
-  local m = 10 ^ nd
-  return roundHalfEven(x * m) / m
+  if x ~= x or x == math.huge or x == -math.huge then return x end
+  -- A true decimal tie -- the exact expansion terminating one digit past nd
+  -- with a 5 -- only a dyadic value can produce, and its zeros then run out
+  -- forever; any other double shows a nonzero digit within ~18 places. The
+  -- formatter rounds such a tie away from zero where Python rounds it to
+  -- even, so it is the one case the digits cannot settle.
+  local s = string.format("%." .. (nd + 24) .. "f", x)
+  local dot = s:find(".", 1, true)
+  local tail = s:sub(dot + nd + 1)
+  if not tail:match("^50*$") then
+    return tonumber(string.format("%." .. nd .. "f", x))
+  end
+  local head = s:sub(1, dot + nd)         -- sign, integer part, nd digits
+  local last = head:byte(-1) - 48
+  if last % 2 == 0 then return tonumber(head) end
+  -- an odd digit bumps to even with no carry
+  return tonumber(head:sub(1, -2) .. string.char(head:byte(-1) + 1))
 end
 
 StadiumFragment.roundHalfEven = roundHalfEven
@@ -572,7 +594,9 @@ Anim.__index = Anim
 local function newAnim(frag, off)
   return setmetatable({
     f = frag, off = off,
-    flags      = frag:u8(off),
+    -- the u16 at +0: the flag bits live in its LOW byte, so a u8 read at +0
+    -- gets the always-zero high byte and silently hides hermite mode
+    flags      = frag:u16(off),
     startFrame = frag:u16(off + 4),
     loopStart  = frag:u16(off + 6),
     nChannels  = frag:u16(off + 8),
@@ -592,20 +616,20 @@ function Anim:chan(i)
            oRot = f:u16(o + 6), oTrans = f:u16(o + 8) }
 end
 
--- Packed per-frame streams (flags & 8 == 0), which is what every animation of
--- every battle Pokemon actually uses.
+-- Packed per-frame streams (flags & 8 == 0).
 --
--- A count of 0 means the component has no stream at all. The game's own index
--- arithmetic (offset + count - 1) then runs off the front of the array -- for
--- Tangela's idle the scale "array" is two entries long and the computed index
--- is 99 -- so an empty channel is read as "keep the bind-pose value", which is
--- what the nil return means to the caller.
+-- A count of 0 means the component has no stream. In the ROM that only ever
+-- happens in HERMITE animations, where a count under 2 means "the offset
+-- field IS the constant value" -- no packed animation of any of the 151
+-- species carries an empty channel, so the bind-pose fallback (the nil
+-- return) is dead code kept as a safety net.
 function Anim:transPacked(c, frame)
   if c.nTrans == 0 then return nil end
   local wide = floor(self.flags / 4) % 2 == 1
   local bits = wide and 16 or 12
   if c.nTrans == 1 then
-    if wide then return c.oTrans + 0.0 end
+    -- func_80016848 reads the u16 offset field back as a SIGNED constant
+    if wide then return signed(c.oTrans, 16) + 0.0 end
     return floor(signed(c.oTrans * 16 % 65536, 16) / 16) + 0.0
   end
   local i = c.oTrans + (frame < c.nTrans - 1 and frame or c.nTrans - 1)
@@ -626,9 +650,9 @@ function Anim:scalePacked(c, frame)
   return self.f:s16(self.scaleData + i * 2) / 1000.0
 end
 
--- Hermite keyframes (flags & 8). Ported for completeness: no animation of any
--- of the 151 battle Pokemon sets that flag, which was measured rather than
--- assumed.
+-- Hermite keyframes (flags & 8): Pidgeot, Dodrio, Exeggutor, Tangela and
+-- Magmar. src/17300.c func_80016934 (narrow, 6-byte keys) and func_80016B30
+-- (wide, 8-byte keys with a separate out-tangent).
 function Anim:hermite(base, n, frame, wide)
   local f = self.f
   local stride = wide and 8 or 6
@@ -673,7 +697,10 @@ function Anim:rotKey(c, frame)
                        floor(c.interp / 2) % 2 == 1) / 10.0
   end
   deg = deg % 360.0
-  return floor(deg / 360.0 * 65536.0)
+  -- func_80016DE0 returns s16: the f32 -> s16 cast WRAPS an angle above 180
+  -- degrees to its negative twin, and the pack writer clamps i16, so an
+  -- unwrapped value would pin at 32767 instead
+  return signed(floor(deg / 360.0 * 65536.0) % 65536, 16)
 end
 
 function Anim:scaleKey(c, frame)
@@ -721,7 +748,8 @@ Aux.__index = Aux
 local function newAux(frag, off)
   return setmetatable({
     f = frag,
-    flags      = frag:u8(off),
+    flags      = frag:u16(off),   -- low byte, same layout as Anim
+
     startFrame = frag:u16(off + 4),
     loopStart  = frag:u16(off + 6),
     nChannels  = frag:u16(off + 8),

lib/StadiumInstall.lua

@@ -56,6 +56,13 @@ StadiumInstall.MARKER = StadiumInstall.DIR .. "/pack.info"
 -- than misread. Must track StadiumPack's magic.
 StadiumInstall.FORMAT = "DSM3"
 
+-- Bumped when the packs' CONTENT changes without the byte layout moving, so
+-- a cache built by an older extractor is rebuilt rather than trusted. Rev 2
+-- is the hermite-animation decode fix: the five keyframe species (Pidgeot,
+-- Dodrio, Exeggutor, Tangela, Magmar) come out garbled or bind-posed from
+-- any rev-1 build.
+StadiumInstall.REV = 2
+
 StadiumInstall.COUNT = 151
 
 -- Named ROM files, then any ROM at all sitting in the folder.
@@ -138,9 +145,10 @@ local function readMarker()
   if not (f and isFile(StadiumInstall.MARKER)) then return nil end
   local ok, text = pcall(f.read, StadiumInstall.MARKER)
   if not (ok and type(text) == "string") then return nil end
-  local format, count, md5 = text:match("^(%S+)%s+(%d+)%s*(%S*)")
+  local format, count, md5, rev = text:match("^(%S+)%s+(%d+)%s*(%S*)%s*(%S*)")
   if not format then return nil end
-  return { format = format, count = tonumber(count), md5 = md5 }
+  return { format = format, count = tonumber(count), md5 = md5,
+           rev = tonumber(rev) }
 end
 
 -- Whether a complete, current set of packs is on disk.
@@ -150,7 +158,8 @@ function StadiumInstall.ready()
   if readyCache ~= nil then return readyCache end
   local m = readMarker()
   readyCache = (m ~= nil and m.format == StadiumInstall.FORMAT
-                and m.count == StadiumInstall.COUNT) and true or false
+                and m.count == StadiumInstall.COUNT
+                and m.rev == StadiumInstall.REV) and true or false
   return readyCache
 end
 
@@ -315,8 +324,9 @@ function StadiumInstall.step()
     local wrote = #job.failed == 0 and job.total > 0
     if wrote and f then
       pcall(f.write, StadiumInstall.MARKER,
-            ("%s %d %s\n"):format(StadiumInstall.FORMAT, job.total,
-                                  tostring(job.md5 or "")))
+            ("%s %d %s %d\n"):format(StadiumInstall.FORMAT, job.total,
+                                     tostring(job.md5 or ""),
+                                     StadiumInstall.REV))
       readyCache = nil
       StadiumPack.forget()
     end

lib/StadiumMon.lua

@@ -205,32 +205,23 @@ end
 -- card, which is a per-POKEMON decline rather than a per-battle one: a fight
 -- can perfectly well have a model on one side and a pic on the other.
 --
--- ------- the three the extraction cannot read
+-- ------- staticPose: the corrupt-idle escape hatch
 --
--- Exeggutor, Tangela and Magmar come out of the ROM with standby loops that
--- throw bones hundreds of units off the body -- the game's own index
--- arithmetic evidently reads those channel streams differently from the way
--- this does. StadiumBuild.idleIsBroken measures it and the pack carries the
--- verdict as `staticPose`.
+-- StadiumBuild.idleIsBroken measures whether a species' standby loop throws
+-- bones off the body, and the pack carries the verdict as `staticPose`. A
+-- species so marked DECLINES here -- the Game Boy's own battle sprite
+-- stands on the tile instead, drawn by the same 2D-3D path every species
+-- uses when its model is unavailable -- because a bind pose held for a
+-- whole fight reads as broken, not as "this one does not animate".
 --
--- That flag used to mean "stand this one still in its bind pose", on the
--- reasoning that a Pokemon standing there looking like itself beats one
--- coming apart. It does -- but only just. A bind pose is a RIGGING pose, not
--- a portrait: arms out, neck straight, nothing where the artwork put it. Set
--- among a hundred and forty-eight species that breathe, the three that hold
--- a T-pose for the whole fight do not read as "these ones do not animate",
--- they read as broken -- which they are.
---
--- So they decline instead, and the Game Boy's own battle sprite stands on
--- the tile in front of the same arena, lit the same way, drawn by the same
--- 2D-3D path every species uses when its model is unavailable. The mode
--- already had that fallback for a species with no pack at all; this is three
--- more species taking it.
---
--- Keyed on the DATA rather than on a list of dex numbers on purpose: the
--- test that produced the flag is in the packer, so a re-extraction that
--- fixes those streams -- or breaks a fourth species -- moves this with it
--- and nothing here has to be edited.
+-- No species is marked today. Exeggutor, Tangela and Magmar used to be:
+-- their animations are hermite keyframes (flags & 8), the extractor misread
+-- the flags byte and decoded them as packed streams, and the exploding
+-- result tripped the detector (Pidgeot and Dodrio were garbled by the same
+-- bug, just not hard enough to trip it). The detector stays, keyed on the
+-- DATA rather than a list of dex numbers, so a future extraction bug that
+-- corrupts a species' idle falls back to the sprite instead of coming
+-- apart on the field -- and nothing here has to be edited when it does.
 function StadiumMon:setSpecies(dex)
   if dex == self.species then return self.rig ~= nil end
   if self.rig then self.rig:release() end

lib/StadiumPack.lua

@@ -55,14 +55,22 @@ local floor = math.floor
 --
 -- DIR is inside the mod, and exists for a developer checkout that has run
 -- tools/stadium_pack.py -- which is also how the oracle the Lua extractor is
--- tested against gets built. It is second because a locally built cache
--- should win over whatever a checkout happens to have lying around.
+-- tested against gets built. It is second because a locally built CURRENT
+-- cache should win over whatever a checkout happens to have lying around --
+-- current as judged by StadiumInstall's marker, so a cache an old extractor
+-- built does not shadow a fresh set (see readPack).
 StadiumPack.CACHE_DIR = "dramatic_shape/stadium"
 StadiumPack.DIR = "assets/stadium"
 
 local function readPack(species)
+  -- The cache only counts when StadiumInstall's marker says it is a
+  -- complete, CURRENT build -- an old cache (a rev the extractor has since
+  -- fixed, a format that moved) must not shadow a fresh shipped set, and a
+  -- half-written folder must not be read at all. Required lazily: Install
+  -- requires this module at load, so the reverse edge cannot be taken then.
   local rel = ("%s/%03d.dsm"):format(StadiumPack.CACHE_DIR, species)
-  if love and love.filesystem and love.filesystem.getInfo then
+  if love and love.filesystem and love.filesystem.getInfo
+     and V.require("StadiumInstall").ready() then
     local okInfo, info = pcall(love.filesystem.getInfo, rel, "file")
     if okInfo and info then
       local ok, bytes = pcall(love.filesystem.read, rel)
@@ -83,9 +91,11 @@ end
 -- contract and the packer's CONTEXTS must stay identical to it.
 -- Position 2 was called "hit" until the move table was read against it: it
 -- is the animation most of a species' MOVES play, which makes it the default
--- attack and not a damage reaction (see StadiumMon's STATES). The name is a
--- label on a position -- the format is the ORDER -- so renaming it changes no
--- bytes, but it has to match tools/stadium_pack.py's CONTEXTS.
+-- attack and not a damage reaction (see StadiumMon's STATES). The slot TABLE
+-- is indexed by position, but the name also reaches the packed files: the
+-- packers bake it into the animation NAME strings, so it has to match
+-- tools/stadium_pack.py's CONTEXTS *and* pipeline/battle.py's CONTEXT_SLOTS,
+-- or the oracle diff reports every species.
 StadiumPack.CONTEXT = {
   "idle", "attack_default", "faint", "entrance", "reaction_169", "reaction_170",
   "reaction_171", "reaction_172", "reaction_173", "reaction_174",

lib/Water.lua

@@ -940,7 +940,16 @@ vec2 waveUV(vec2 tc, vec2 col) {
 // can afford it -- the colour is a colour, and tc/sc arrived through
 // LOVE's mediump plumbing whatever this signature says -- and the maths
 // below runs on the stage default the moment the values touch a local.
-vec4 effect(mediump vec4 color, Image tex, mediump vec2 tc, mediump vec2 sc) {
+//
+// Which precision that has to BE is not ours to know: LOVE 12 forward-
+// declares effect() under a different one, and pins that matched 11's
+// prototype are the mismatch there -- the same refusal, from the other
+// side, with the water falling back to flat. So the qualifier is a define
+// the Lua side fills in, and Water.shader compiles the pinned form first
+// and the bare one only if that is refused. Whichever prototype a runtime
+// brought, one of the two agrees with it.
+vec4 effect(EFFECT_PREC vec4 color, Image tex, EFFECT_PREC vec2 tc,
+            EFFECT_PREC vec2 sc) {
   // THE DEPTH TEST, done here because the buffer that would have done it is
   // detached for the length of this pass so it can be READ (see the header).
   // Same comparison, same buffer, same result: a building in front of a pond
@@ -1133,7 +1142,7 @@ end
 
 Water._trainSource = trainSource       -- named for the suite
 
-local function source(grid)
+local function source(grid, bare)
   local src = SHADER_SRC:gsub("//@CRATERS", (craterSource():gsub("%%", "%%%%")))
   src = src:gsub("//@TRAINS", (trainSource():gsub("%%", "%%%%")))
   local head = ("#define RAY_STEPS %d\n#define RAY_REFINE %d\n"
@@ -1141,6 +1150,12 @@ local function source(grid)
     :format(Water.RAY_STEPS, Water.RAY_REFINE, Water.WAVE_STEPS,
             Water.WAVE_STRIDE)
   if grid then head = head .. "#define VOXEL_GRID 1\n" end
+  -- effect()'s parameter precision -- see the signature for why it cannot
+  -- simply be spelled there. Empty is a define all the same: the params
+  -- then carry the stage default, which is what a prototype declared
+  -- without qualifiers wants.
+  head = head .. (bare and "#define EFFECT_PREC\n"
+                        or "#define EFFECT_PREC mediump\n")
   return head .. src
 end
 
@@ -1159,6 +1174,13 @@ function Water.shader(grid)
       shaders[grid] = false
     else
       local ok, sh = pcall(love.graphics.newShader, source(grid))
+      if not ok then
+        -- the pinned prototype was the wrong one for this runtime; the bare
+        -- one is the only other shape there is, and a driver that refuses
+        -- both was never going to draw this water anyway
+        local bareOk, bareSh = pcall(love.graphics.newShader, source(grid, true))
+        if bareOk then ok, sh = bareOk, bareSh end
+      end
       if not ok and V and V.mod and V.mod.log then
         -- once, where it can be read: the fallback is flat water, which is
         -- easy to look at and impossible to diagnose without this line

manifest.json

@@ -1,7 +1,7 @@
 {
   "id": "DRAMATIC_SHAPE",
   "name": "Dramatic Shape Voxel Mod",
-  "version": "1.6.0",
+  "version": "1.6.1",
   "api": 2,
   "entry": "main.lua",
   "profile": "content",
@@ -10,7 +10,7 @@
   "priority": 100,
   "dependencies": [],
   "optional_dependencies": [],
-  "conflicts": [],
+  "conflicts": ["ds_fp_ceiling"],
   "permissions": [
     "engine_internals"
   ],

model_extract/pipeline/battle.py

@@ -28,9 +28,12 @@ CONTEXT_SLOTS = {
     165: ('idle', 'code',
           'The standby loop. func_8432B0A4 restores this slot whenever the Pokemon '
           'returns to neutral, and it resolves to animation 0 for all 151 species.'),
-    166: ('hit', 'data',
+    166: ('attack_default', 'data',
           'Resolves to animation 2 for 149/151 species, the same animation slots '
-          '178-181 use in the reaction paths.'),
+          '178-181 use in the reaction paths. Called "hit" until the move table '
+          'was read against it: it is the animation most of a species\' MOVES '
+          'play, the default attack rather than a damage reaction (the name has '
+          'to match lib/StadiumPack.lua\'s CONTEXT and StadiumBuild\'s CONTEXTS).'),
     167: ('faint', 'data',
           'The referenced animation always ends far from the standing pose - the '
           'model collapses to 0.03-0.84x its idle height, or leaves the frame '

model_extract/pipeline/build.py

@@ -107,7 +107,7 @@ def bind_extent(data):
 
 # Which context name wins when several claim the same animation. The battle
 # table points many slots at one clip, and these are the ones worth naming.
-NAME_PREF = ['idle', 'hit', 'faint', 'entrance', 'struggle', 'flinch']
+NAME_PREF = ['idle', 'attack_default', 'faint', 'entrance', 'struggle', 'flinch']
 
 
 def label_animations(data, rows, moves):
@@ -294,7 +294,7 @@ def main(argv):
     moves_out = []
     if move_rows:
         default_anim = {p['species']: next(
-            (a['index'] for a in p['animations'] if 'hit' in a.get('contexts', [])), -1)
+            (a['index'] for a in p['animations'] if 'attack_default' in a.get('contexts', [])), -1)
             for p in manifest['pokemon']}
         for mid in range(1, battle.N_MOVES + 1):
             users, tally, ndiff = [], collections.Counter(), 0

model_extract/pipeline/fragment.py

@@ -315,7 +315,11 @@ class Animation:
     def __init__(self, frag, off):
         f = self.f = frag
         self.off = off
-        self.flags     = f.u8(off)
+        # The flags live in the LOW byte of the u16 at +0 -- reading the byte
+        # AT +0 gets the always-zero high byte, which silently turns every
+        # hermite animation (flags & 8: Pidgeot, Dodrio, Exeggutor, Tangela,
+        # Magmar) into a packed-stream read of keyframe tables.
+        self.flags     = f.u16(off)
         self.startFrame= f.u16(off + 4)
         self.loopStart = f.u16(off + 6)
         self.nChannels = f.u16(off + 8)
@@ -333,16 +337,20 @@ class Animation:
                     oRot=f.u16(o + 6), oTrans=f.u16(o + 8))
 
     # -- packed stream sampling (flags & 8 == 0) --------------------------
-    # A count of 0 means the component has no stream at all. The game's index
-    # arithmetic (offset + count - 1) then runs off the front of the array -- for
-    # Tangela's idle the scale "array" is two entries long and the computed index
-    # is 99 -- so an empty channel is treated as "keep the bind-pose value".
+    # A count of 0 means the component has no stream. In the ROM that only
+    # ever happens in HERMITE animations, where a count under 2 means "the
+    # offset field IS the constant value" -- no packed animation of any of the
+    # 151 species carries an empty channel, so the bind-pose fallback here is
+    # dead code kept as a safety net.
     def _trans_packed(self, c, frame):
         if c['nTrans'] == 0:
             return None
         bits = 16 if (self.flags & 4) else 12
         if c['nTrans'] == 1:
-            return float(c['oTrans'] if (self.flags & 4) else signed((c['oTrans'] * 16) & 0xFFFF, 16) >> 4)
+            # (s16) casts both ways: func_80016848 reads the u16 offset field
+            # back as a signed constant.
+            return float(signed(c['oTrans'], 16) if (self.flags & 4)
+                         else signed((c['oTrans'] * 16) & 0xFFFF, 16) >> 4)
         i = c['oTrans'] + min(frame, c['nTrans'] - 1)
         return float(bitfield(self.f, self.transData, i, bits))
 
@@ -404,7 +412,11 @@ class Animation:
         else:
             deg = self._hermite(self.rotData + c['oRot'] * 2, c['nRot'], frame, c['interp'] & 2) / 10.0
         deg %= 360.0
-        return int(deg / 360.0 * 65536.0)
+        # func_80016DE0 returns s16: the f32 -> s16 cast WRAPS an angle above
+        # 180 degrees to its negative twin. Same binary angle either way, but
+        # the packer stores i16 with clamping, so an unwrapped 350-degree
+        # value would pin at 32767 (= 180 degrees) instead.
+        return signed(int(deg / 360.0 * 65536.0) & 0xFFFF, 16)
 
     def _scale_key(self, c, frame):
         if c['nScale'] < 2:
@@ -439,7 +451,7 @@ class AuxAnimation:
 
     def __init__(self, frag, off):
         f = self.f = frag
-        self.flags     = f.u8(off)
+        self.flags     = f.u16(off)         # low byte, same layout as Animation
         self.startFrame= f.u16(off + 4)
         self.loopStart = f.u16(off + 6)
         self.nChannels = f.u16(off + 8)

tests/dramatic_shape_test.lua

@@ -5581,11 +5581,15 @@ for _, dex in ipairs({ 25, 6, 95, 143 }) do
   end
 end
 
--- the three species whose standby loop is corrupt in the source extraction
--- are marked to hold their bind pose instead of coming apart
-T.eq(Pack.load(126).staticPose, true,
-  "Magmar is held at its bind pose -- its source animations are broken")
-T.eq(pikachu.staticPose, false, "and a species with good data is not")
+-- No species is held at its bind pose any more. Magmar (with Pidgeot,
+-- Dodrio, Exeggutor and Tangela) uses the game's hermite-keyframe animation
+-- mode, which the extractor used to misread as packed streams -- the flags
+-- byte was read at +0, the always-zero high half of the u16 -- and the
+-- packer then declared their exploding standby loops corrupt. The broken-
+-- idle detector stays as the guard, so this asserts it no longer fires.
+T.eq(Pack.load(126).staticPose, false,
+  "Magmar animates -- its hermite animations decode correctly now")
+T.eq(pikachu.staticPose, false, "and a packed-stream species does too")
 end)()
 
 Pipelines.reset()

tests/stadium_shots.lua

@@ -44,10 +44,14 @@ return function(game)
     { "ONIX", "DIGLETT" },        -- the tallest against nearly the shortest
     { "ZUBAT", "TENTACRUEL" },    -- hovers above its origin / hangs below it
     { "GASTLY", "CATERPIE" },     -- a floater and the smallest in the set
-    -- the two species whose animation data is corrupt at source: BOTH of
-    -- these must come out as flat battle pics standing on their tiles, not
-    -- as models (see StadiumMon.setSpecies). Tangela is the third.
+    -- the five hermite-keyframe species (flags & 8), which the extractor
+    -- used to misread as packed streams: Exeggutor, Tangela and Magmar came
+    -- out so garbled the packer declined them to flat pics, and Pidgeot and
+    -- Dodrio animated garbled. All five must now stand as MODELS in a sane
+    -- standby loop (see StadiumFragment's animation notes).
     { "EXEGGUTOR", "MAGMAR" },
+    { "PIDGEOT", "TANGELA" },
+    { "DODRIO", "PIDGEOTTO" },    -- with its packed-stream evolution as control
     { "GYARADOS", "SNORLAX" },    -- the two biggest bodies in the set
   }
 

tools/stadium_pack.py

@@ -229,11 +229,9 @@ def stance(data):
     Measured on the BIND POSE, which is the one pose in the set that can be
     trusted for this.  Two things recommend it.  It reproduces the verified
     glTF export bit for bit on all 151 species, so it is measuring the same
-    skeleton the reference implementation agreed with; and it is immune to
-    the animation quirks a handful of species carry (Exeggutor's idle throws
-    limbs hundreds of units off the body from frame 1 on, and Magmar's does
-    something similar) -- quirks that would otherwise decide how big every
-    OTHER frame of those species is drawn.
+    skeleton the reference implementation agreed with; and it does not move
+    with the animations, so no single clip's excursion decides how big every
+    other frame of that species is drawn.
 
     The floor is the interesting number, and it reads cleanly: 119 of the
     151 sit within 5% of zero, which says the model origin IS where the game
@@ -249,13 +247,15 @@ def stance(data):
 
 
 def idle_is_broken(data, idle):
-    """Whether this species' standby loop is corrupt in the source data.
+    """Whether this species' standby loop is corrupt as extracted.
 
-    A handful of species come out of the extraction with animations that
-    throw bones hundreds of units off the body -- Exeggutor, Tangela and
-    Magmar, whose channel streams the game's own index arithmetic evidently
-    reads differently from the way the exporter does.  Played, they look
-    like a Pokemon coming apart; the mod would rather stand them still.
+    No species trips this today.  Exeggutor, Tangela and Magmar used to:
+    their animations are hermite keyframes (flags & 8), the extractor read
+    the flags byte at +0 -- the always-zero high half of the u16 -- and
+    decoded keyframe tables as packed streams, which threw bones hundreds
+    of units off the body.  The detector stays as the guard against the
+    next extraction bug: played, a broken idle looks like a Pokemon coming
+    apart, and the mod would rather show the sprite fallback.
 
     The test is deliberately narrow, because "differs from the bind pose" is
     NOT brokenness.  It is asked only of the STANDBY loop, which is the one