fix(smash): put every kit's first ability on the key a hand rests on

[andrewgazelka]

The bug

Twelve of the fifteen Super Smash Mobs kits left hotbar slot 0 empty and
numbered their abilities from 1. A vanilla client selects slot 0 on its own when
it reaches the world and no packet in the join sequence changes that, so twelve
kits handed a player a bar whose first ability could not be fired until they
scrolled. An operator saw it in a real client: an empty first square, then a
sword, an axe and a block.

Nothing failed. Every ability was present, correctly bound and reachable, and no
gate had an opinion about which key a hand starts on.

The layout, and why it is a shift

Enumerated from source, all fifteen kits already declared their abilities in
ascending slot order with the ultimate last, and the three that were right
(Enderman, Skeleton, Slime) were exactly the three whose first ability was
numbered 0:

enderman       0 1 2 8    ok        skeleton   0 1 2 8    ok
slime          0 1   8    ok        blaze        1 2 8     no slot 0
guardian         1 2 3 8            iron_golem   1 2 3 8
...twelve in all

Slot 8 was already consistent across the whole roster, so the far-right
convention for the Smash Crystal was understood and simply not applied at the
left end. Every kit's starting abilities were contiguous, just starting from the
wrong number. So the whole set was one key to the right of where it belonged and
the fix is a shift.

The alternative reading, that slot 0 was reserved for a melee weapon not yet
implemented, does not hold: KitStats documents that a kit hits for the same
amount with any item including a fist, nothing in kit.rs, server.rs or
adapter.rs has any notion of a weapon item, and Op::SetHotbar clears the
inventory and refills it purely from abilities, so nothing could ever have
occupied slot 0.

Why the values were not the whole problem

There is nothing a per-ability slot: u8 can be checked against on its own.
slot: 1 is wrong only in relation to the rest of its kit, and the doc comment
on AbilitySpec showed a slot: 1 (Enderman's Blink, genuinely its second
ability) that new kit files then copied as their first. That is the shape that
let twelve kits drift.

So the field is gone. KitBuilder::ability hands out slots in declaration order
from 0 and KitBuilder::ultimate always takes ULTIMATE_SLOT. The order a kit
file declares its abilities in is the layout. Fifty-one hand-written numbers
deleted, and the empty first key is now unreachable rather than merely absent
today. Declaring more starting abilities than fit to the left of the ultimate is
an assert at module init rather than an ability nobody can press.

The gates

events/smash/tests/hotbar.rs sweeps kit::registry and ability::manifest,
following the pattern tests/sound.rs set, so a kit added tomorrow is covered
the moment its module runs. Four invariants, and each earns its place:

• slot 0 is filled on every kit. The operator's bug, stated as itself, so a
  failure log reads "Blaze leaves slot 0 empty" rather than a puzzle about sets.
• starting abilities run from 0 with no holes. Contiguity and not merely
  "starts at 0", because a gap is a dead key in the middle of the bar and two or
  three abilities have no business being spread across nine keys.
• every ultimate sits in the last slot. It is the one binding a player
  carries across every mob, which is worth nothing if a kit can move it.
• no slot is off the end of the bar. Worth having independently: this is the
  one that catches a typo, and set_hotbar returns an error the adapter
  discards, so an ability in slot 12 is silently absent rather than loud.

Plus two that read one layer out: playing every kit on a real player and reading
kit::hotbar, which is the function push_stale_hotbars calls, because a
perfect declaration can still be dropped on instantiation; and granting a Smash
Crystal, which reaches the bar through a different path from kit::apply.

nix run .#smash-hotbar-e2e proves it over the protocol. One client stands in
the hub (the lobby needs four to start a countdown), changes kit fifteen times,
and holds the ClientboundContainerSetSlot packets it receives against the
server's own /abilities output: slot 0 filled, keys exactly the ones the
registry names, items matching, no holes, and slot 8 empty until a crystal is
picked up. Seventy-six checks. Framing comes from tools/client-26.2.py by way
of smash-match.py, the same route every other scripted client takes. Ports
default off smash-identity-e2e's, so it does not collide with the pair already
sharing +4000 (ENG-10834).

Watching them fail

Reintroducing the bug as next_slot: 1:

test starting_abilities_fill_the_left_of_the_bar_with_no_holes ... FAILED
a kit's starting abilities must occupy slot 0 upwards with no gaps:
["Blaze lays out {1, 2}, wanted {0, 1}", ... all fifteen]

test every_kit_puts_something_on_the_key_a_hand_rests_on ... FAILED
these kits leave hotbar slot 0 empty, so a player who spawns and right-clicks
fires nothing: ["Blaze", "Chicken", "Cow", ... all fifteen]

test playing_any_kit_hands_out_a_bar_that_starts_at_slot_zero ... FAILED

The same change against the e2e gate, which is the one that reads the wire:

FAIL Blaze fills slot 0, the key a client has selected when it spawns
     (bar: 1:minecraft:iron_sword, 2:minecraft:iron_axe)
FAIL Blaze runs from slot 0 upwards with no holes: [1, 2]
... 30 failures, rc=1

Making next_slot step by 2 fires the contiguity check only, and raising
ULTIMATE_SLOT past HOTBAR_SLOTS fires the range check only, so each
invariant was watched failing on its own cause.

What depended on the old numbering

Three things, and two of them would have failed quietly:

• tools/smash-match.py --ability-slot defaulted to 3 for Iron Golem's Seismic
  Slam, now slot 2. A stale value produced no message at all: an empty slot
  has no registry entry to disagree with, so the check pressed a key holding
  nothing, saw no motion, and reported a broken knockback model. It is now a
  recorded failure the report reads, since the old complaint was appended to a
  list already folded into a proof by the time the match phase runs.
• tests/modularity.rs asked slot 3 for a ground requirement and got Ok(()),
  which is what activate returns for a slot holding nothing. Its cooldown test
  passed on slot 1 while measuring a different ability than it named.
• tests/verification.rs drives four test-local kits by slot. Skeleton, one of
  the three already correct, is deliberately left on slot 1 there: slot 0 is
  Barrage, which charges rather than firing, so a bare use_slot on it never
  reaches the cooldown that test is about.

Verdict on the API shape

The values were wrong, but so was the shape. A per-ability slot number with no
notion of the kit's layout is unfalsifiable in isolation, and the doc example
made the wrong value the default thing to copy. Deriving the layout from
declaration order is what makes the class impossible rather than merely gated,
and it costs a kit author nothing: no kit in the roster wanted a
non-contiguous bar.

Slot stays a plain component and not a relation. A slot is a scalar property of
an ability, not a reference to another entity: there is no slot entity to point
at, and no sharing for a relation to earn its keep on, unlike (PlaysOnCast, sound) where the sound really is interned and shared across abilities. Making
it a relation would mint nine singletons to encode a u8 and turn
granted_in_slot, which runs on every right-click, from a component read into a
target lookup. The layout is likewise not stored on the kit: walking
(Grants, ability) already derives it, and a second copy is a thing that can
disagree.

Verified locally

Rebased onto b5ce627, which closed ENG-10817 and made every attribute of
checks enforced in CI subtractively. So checks.smash-hotbar-e2e and
checks.smash-hotbar-e2e-app are enforced the day they land, with no edit to
nix/ci/flake-gate.nix, which is what nix eval confirms:

$ nix eval --json .#checks.aarch64-darwin --apply \
    'c: builtins.filter (n: builtins.match ".*hotbar.*" n != null) (builtins.attrNames c)'
["smash-hotbar-e2e","smash-hotbar-e2e-app"]

The +6000 port offset is for nix run only; the sandboxed check picks a free
port, so it cannot collide with the pair already sharing +4000 (ENG-10834).

Run locally on aarch64-darwin, after the rebase:

• nix run .#test: 636 passed, 1 skipped (the count moved with refactor: delete three crates and one file that nothing compiles against #1010 and refactor: fixtures and a two-character crate stop being top-level members #1011
  deleting crates on main; it was 656 before the rebase)
• nix run .#lint: clean
• nix run .#fmt: clean
• nix run .#smash-hotbar-e2e: RESULT ok, 0 checks failed, 76 passes
• nix run .#smash-e2e: RESULT a whole match ran at protocol 776, rc=0. This
  is the gate whose --ability-slot moves, and it proved knockback from an
  ability on the new slot: P4 took (-2.227, 0.646, -2.227) blocks/tick, which
  solves to the same strength horizontally and vertically and so matches the
  model rather than a double impulse.

Not done, and worth knowing:

• The mutation gate (nix run .#mutants) was not run.
• The other four e2e gates were not re-run, since nothing here touches their
  paths. smash-e2e was, because this moves its --ability-slot.
• Neither e2e gate was run on linux. CI will be the first to do that, and per
  ci: enforce the flake checks, by name, instead of exempting all of them #1007 the sandboxed smash gates do now work there.
• cargo test -p smash segfaulted once under parallel test threads, in
  game_flow, and passed on the next run and every run with
  --test-threads=1. Pre-existing flake in flecs registration, not caused by
  this change, and unfiled: it is outside this change's scope.

Authored by Claude Code (Opus) at the operator's direction.

[github-actions]

Benchmark Results for general

ray_intersection/aabb_size_0.1                     [  15.0 ns ...  15.0 ns ]      -0.36%
ray_intersection/aabb_size_1                       [  14.6 ns ...  14.5 ns ]      -0.21%
ray_intersection/aabb_size_10                      [  14.6 ns ...  14.6 ns ]      -0.17%
ray_intersection/ray_distance_1                    [   1.1 ns ...   1.1 ns ]      +0.26%
ray_intersection/ray_distance_5                    [   1.1 ns ...   1.1 ns ]      +0.06%
ray_intersection/ray_distance_20                   [   1.1 ns ...   1.1 ns ]      -0.57%
overlap/no_overlap                                 [  12.3 ns ...  12.4 ns ]      +0.45%
overlap/partial_overlap                            [  12.4 ns ...  12.4 ns ]      +0.19%
overlap/full_containment                           [  11.5 ns ...  11.5 ns ]      +0.20%
point_containment/inside                           [   4.7 ns ...   4.7 ns ]      -0.23%
point_containment/outside                          [   4.7 ns ...   4.7 ns ]      +0.07%
point_containment/boundary                         [   4.7 ns ...   4.7 ns ]      +0.05%

Comparing to b5ce627

[github-actions]

Benchmark Results for general

ray_intersection/aabb_size_0.1                     [  17.4 ns ...  17.4 ns ]      -0.29%
ray_intersection/aabb_size_1                       [  17.2 ns ...  17.2 ns ]      -0.07%
ray_intersection/aabb_size_10                      [  17.2 ns ...  17.2 ns ]      +0.03%
ray_intersection/ray_distance_1                    [   1.5 ns ...   1.5 ns ]      +0.20%
ray_intersection/ray_distance_5                    [   1.5 ns ...   1.5 ns ]      -0.03%
ray_intersection/ray_distance_20                   [   1.5 ns ...   1.5 ns ]      +0.25%
overlap/no_overlap                                 [  16.0 ns ...  16.0 ns ]      +0.16%
overlap/partial_overlap                            [  15.9 ns ...  15.9 ns ]      +0.34%
overlap/full_containment                           [  14.9 ns ...  14.9 ns ]      +0.16%
point_containment/inside                           [   6.0 ns ...   6.0 ns ]      -0.03%
point_containment/outside                          [   6.1 ns ...   6.1 ns ]      +0.00%
point_containment/boundary                         [   6.0 ns ...   6.0 ns ]      -0.01%

Comparing to 205da58

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 47.09%. Comparing base (b5ce627) to head (c928075).
⚠️ Report is 2 commits behind head on main.

@@            Coverage Diff             @@
##             main    #1012      +/-   ##
==========================================
- Coverage   47.17%   47.09%   -0.09%     
==========================================
  Files         238      238              
  Lines       27097    27054      -43     
  Branches     1087     1087              
==========================================
- Hits        12784    12741      -43     
  Misses      14077    14077              
  Partials      236      236              

Files with missing lines	Coverage Δ
events/smash/src/adapter.rs	0.00% <ø> (ø)
events/smash/src/module/kit.rs	92.03% <100.00%> (+0.26%)	⬆️
events/smash/src/module/kits/blaze.rs	100.00% <ø> (ø)
events/smash/src/module/kits/chicken.rs	97.34% <ø> (-0.07%)	⬇️
events/smash/src/module/kits/cow.rs	100.00% <ø> (ø)
events/smash/src/module/kits/creeper.rs	100.00% <ø> (ø)
events/smash/src/module/kits/enderman.rs	100.00% <ø> (ø)
events/smash/src/module/kits/guardian.rs	90.06% <ø> (-0.26%)	⬇️
events/smash/src/module/kits/iron_golem.rs	92.80% <ø> (-0.23%)	⬇️
events/smash/src/module/kits/skeleton.rs	96.72% <ø> (-0.11%)	⬇️
... and 7 more

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.