feat(smash): fifty-one abilities stop sharing three pictures - #1035
Merged
Conversation
`Cue` was a three-variant enum when it was written and five by tonight, and it was the only thing an ability could ask to be seen as. Bone Explosion, Water Splash and Fish Flurry all drew `Cue::Explosion`, so bones, water and fish were one grey puff. The enum was never really the problem: the protocol layer under it could spell five particles, so five was all there was to ask for. Now that `Particle` is the whole registry, the seam carries a `Particles` instead -- a particle, a point and a shape -- and an ability composes what it wants. `Cue` is gone, along with `play_cue` and the adapter's table of which particle each variant meant, because that mapping was only ever a hosting decision standing in for a missing vocabulary. What replaces it is not the same enum renamed. `module::visuals` holds one function per recurring moment -- a blast, a teleport, a death, a burn, a poison -- so those keep looking the same wherever they happen, and a kit that wants something else writes it inline against `Particles` without widening anything. The two `[PLACEHOLDER]` cues are now the particles vanilla actually uses. A burn was pinned to `crit` and a poison to half-power `dragon_breath`; they are `minecraft:flame` and `minecraft:entity_effect` in poison green. `tools/smash-match.py` gains the claim that every declared ability was seen: each one fired by a real client has to be answered by a `ClientboundLevelParticlesPacket` with a non-zero count, alongside the sound claim it already made.
andrewgazelka
force-pushed
the
feat/smash-particle-seam
branch
from
July 28, 2026 11:48
13e9a2f to
f111fcf
Compare
Benchmark Results for generalComparing to 48acbc9 |
andrewgazelka
added a commit
that referenced
this pull request
Jul 28, 2026
Twenty of the fifty-one abilities fire a projectile, and until now a client saw none of them. `projectile::fire` created a game-half entity with `Flight`, `Payload` and `FiredBy`, integrated it, detected the hit and dealt the damage — all correct, all tested against the mock, and all invisible, because nothing ever told a client the thing existed. A Skeleton's Barrage sent five arrows' worth of damage and not one arrow. ## The flight stays where it is It is authoritative for the hit, it is deterministic, and `tests/abilities.rs` and `smash-e2e` both rest on it; moving it would fork test from production. What was missing was only the picture, so only the picture is added. Each projectile now carries a **`Visual(EntityKind)`** — a generated enum named directly, the choice #1035 made for particles. Exact where a vanilla entity is the thing (an arrow is `Arrow`, a wither skull a `WitherSkull`, a thrown cub a `Wolf`) and the closest always-rendered projectile where it is not (a thrown coal is a `SmallFireball`, an ink pellet a `Snowball`), each `[APPROXIMATED]` marked at the call site. **`crate::draw`**, on the host, turns that into an entity a client draws: it decorates the existing projectile with the components hyperion's egress reads — kind, position, per-tick velocity, facing — and enqueues the spawn. The client dead-reckons the arc from the velocity the `add_entity` packet carries, which is how vanilla draws a thrown projectile between updates. It is the **same entity**, not a shadow, so when `fly` destructs it on the hit the drawn thing dies too and hyperion's despawn observer tells every client to drop it — no second lifetime to keep in step. ## Two things it deliberately does not do Each has a reason that is a property of the engine, not a preference. **No `Owner`.** That is what `update_projectile_positions` requires to integrate and collide an entity; omitting it is what stops hyperion re-flying and re-hitting a projectile `Flight` already owns. **Not `spawn::spawn`**, which makes a *new* entity. Decorating the existing one is the whole reason despawn is free. ## The honest limitation The drawn arc and the hit are computed by two integrators that do not share constants — hyperion's client dead-reckoning uses the vanilla gravity for the kind, `Flight` uses what the ability set. They agree exactly for a zero-gravity projectile (a hook, a line of cows) and drift for a heavy one over its flight. The projectile vanishes at the hit either way, because the hit is what destructs it. Sharing one integrator is the larger change `docs/smash-design.md` flags; this is the visible-now half. ## Proof, both halves of the seam **`every_projectile_that_flies_can_be_seen`** fires every ability and asserts that whatever projectiles appeared carry a `Visual` — the input to drawing, for all twenty, checked in the game half against the mock. It sweeps the roster rather than a hand-list of projectile abilities, so a kit added tomorrow that forgets the visual fails here. **`tools/smash-match.py`** decodes `add_entity` and proves "a projectile was drawn" on the wire: over the sweep, firing the abilities produces `add_entity` packets, which before `draw` existed never left the server. During the sweep the only new entities are projectiles, because every player spawned before it began. ### Guard broken and watched to fail, then restored `.set(visual)` removed from `fire`: ``` Skeleton / Barrage put 2 projectile(s) in the world with no Visual Spider / Needler put 6 projectile(s) in the world with no Visual Sky Squid / Ink Shotgun put 7 projectile(s) in the world with no Visual ... and every other projectile ability ``` ## What this does not settle The wire half cannot be run to green from here: my `smash-e2e` attempts die on the load box's connection sweep, as they have all session. The in-process `Visual` proof is the local evidence; the `add_entity` census is structural and runs on the gate. One call site in `tests/relationship_traits.rs` (from #1041/#1042) also calls `fire`; its signature fix is included — three lines, no reformatting of that file. ## Checks `cargo test -p smash` (21 binaries) and `nix run --accept-flake-config .#lint` both green on `d11ca7a`, which includes the flecs trait pass.
Benchmark Results for generalComparing to 2970b28 |
andrewgazelka
added a commit
that referenced
this pull request
Jul 28, 2026
A Java enum's constant order is a wire table. `FriendlyByteBuf.writeEnum` sends `ordinal()`; `ByteBufCodecs.idMapper` sends a field each constant declares. Either way the declaration order in the jar is as much a definition with a source of truth as a registry is, and hand-transcribing one is the same defect — with the extra hazard that an ordinal carries no name on the wire, so a transcription one constant out is a wrong value rather than a failed lookup. Eleven of them were typed out in this crate. `EquipmentSlot` twice, once per module, with different accessor names on the two copies. ## Why exactly these eleven, which is the interesting part A hand-written ordinal enum here is almost exactly the set whose enclosing packet the extractor refused. Where a packet's layout is recovered in full, `build.rs::lower_enum` already turns the enums it mentions into closed `#[repr]` Rust enums from `protocol.json` — twenty of them exist in `OUT_DIR` right now, and nobody typed any of them. `protocol.json` carries 42 extracted enum definitions with full constant lists; the other 22 never reach Rust because they sit inside a refused packet or data component. `AttributeModifier$Operation` is the clean proof: it *is* extracted, with its three constants, and it is *also* hand-written in `item/payload.rs`, purely because `dataComponent/minecraft:attribute_modifiers` is a refusal. So these eleven are the ones no recovered layout reaches — carried by `set_player_team`, `set_objective`, `set_equipment`, `commands`, `level_chunk_with_light`, or by the boss bar packet that until #1038 was extracted as carrying no bytes at all. `JAVA_ENUMS` in `nix/extract-protocol.py` reads them out of the decompiled class directly rather than waiting for those layouts to be recovered. | generated | Java class | numbered by | n | |---|---|---|---| | `BossBarColor` | `world.BossEvent$BossBarColor` | ordinal | 7 | | `BossBarOverlay` | `world.BossEvent$BossBarOverlay` | ordinal | 5 | | `ChatTypeParameter` | `network.chat.ChatTypeDecoration$Parameter` | ordinal | 3 | | `Direction` | `core.Direction` | ordinal | 6 | | `DisplaySlot` | `world.scores.DisplaySlot` | `id` | 19 | | `EquipmentSlot` | `world.entity.EquipmentSlot` | ordinal | 8 | | `HeightmapKind` | `world.level.levelgen.Heightmap$Types` | `id` | 6 | | `ObjectiveRenderType` | `world.scores.criteria.ObjectiveCriteria$RenderType` | ordinal | 2 | | `TeamCollisionRule` | `world.scores.Team$CollisionRule` | `id` | 4 | | `TeamColor` | `world.scores.TeamColor` | `id` | 16 | | `TeamVisibility` | `world.scores.Team$Visibility` | `id` | 4 | 80 constants. Same conventions as #1021: `#[repr(uN)]` with the discriminant being the number, `id()` an `as` cast, `name()` off a static table, `from_name` a binary search over a table the generator sorted, closed with no `Unknown`, and a `const` assertion per file proving the discriminant folds. ## The two numberings are declared, not guessed `EquipmentSlot` sends `ordinal()` from `ClientboundSetEquipmentPacket` (`output.writeByte(slotType.ordinal())`) and `s.id` from its own `STREAM_CODEC` (`idMapper(BY_ID, s -> s.id)`), and the two disagree: `OFFHAND` is 1 by ordinal and 5 by id. Which numbering a class uses is the second element of its `JAVA_ENUMS` entry, resolved against the source and named in the generated module's own docs. Nothing infers it, because a value read with the wrong table is a wrong value rather than a failed lookup, and the two tables are the same shape. The extractor refuses rather than approximates: a class that has moved, an id field that has gone, a constant the id table does not mention, or a set that is not dense `0..n-1` all fail the extraction with the class named. ## Checked against the server, not against the parse These constants are read out of decompiled Java by a regex. No test written from that same reading could catch a misreading of it, so the numbers are taken from the other side: `nix/java/VanillaEncoder.java` now emits `java_enum.<Type>.<CONSTANT>` for all 80, and for the six classes that publish a `StreamCodec` it gets the number by **running Mojang's encoder and reading the `VarInt` back out** (asserting the encoding is exactly one `VarInt` and no more). For the five with no codec it reads `DisplaySlot.id()` or `ordinal()` off the class. `EquipmentSlot` deliberately uses the ordinal and not its `STREAM_CODEC`, with the reason at the call site. `tests/java_enum.rs` then walks `ALL` for every type and compares both directions: every variant against the server's number for the name it claims, and the count of `java_enum.<Type>.` fixtures against the number of variants. The second half is what catches a *dropped* constant, which the first half cannot see. ## Guards broken, and what they said Each mutation was checked to have applied (`assert t.count(old) == 1`, then grep for the replacement in the file) and to parse, before being run. **G — a discriminant one out** (`TeamColor::DarkBlue = 1` → `= 2`). Caught, but **not by the test I was aiming at**: ``` error[E0081]: discriminant value `2` assigned more than once --> crates/hyperion-minecraft-proto/src/generated/java_enum/team_color.rs:22:1 ``` Reporting it as it happened: this mutation never reached the test, so it proves the compiler and not the fixture check. Hence H. **H — two entries swapped in the generated name table**, which compiles cleanly and is what a sorting bug in the generator would actually look like. Both tests fire: ``` panicked at tests/java_enum.rs:34: assertion `left == right` failed: TeamColor::Black disagrees with the server about its number left: 0 right: 1 panicked at tests/java_enum.rs:88: assertion `left == right` failed left: None right: Some(Black) ``` **I — the extractor drops a constant**, patched into `scan_java_enums` so the whole pipeline runs on a realistic misparse (`BossBarColor` loses `WHITE`). Regenerated, rebuilt, and the count half fires: ``` panicked at tests/java_enum.rs:42: assertion `left == right` failed: BossBarColor has 6 variants but the server declares 7: ["java_enum.BossBarColor.PINK", ..., "java_enum.BossBarColor.WHITE"] left: 6 right: 7 ``` All three restored; `nix run .#sync-minecraft-proto` reproduces the committed tables and `minecraft-proto-generated` passes. ## Call sites that changed, and one naming decision `DisplaySlot::to_id()` → `id()` (three tests and one line in `events/smash/src/adapter.rs`). `HeightmapKind::from_id` returned `Result`, the generated one returns `Option`, so `world/chunk.rs` does the `ok_or` itself with the reason for not clamping kept at the site. The two `EquipmentSlot` copies collapse into one, so `from_raw`/`from_ordinal` and `to_raw`/`ordinal` become `from_id`/`id`, plus one private `slot_byte` in `packets/play/entity.rs` because that packet packs the number into a byte with a continuation bit and the narrowing should happen once. `EquipmentSlot::MainHand` becomes `Mainhand`, and `OffHand` becomes `Offhand`. Mojang's constants are `MAINHAND` and `OFFHAND`, one word each; `MainHand` needs a dictionary the generator does not have, and hand-maintaining an exception list is the thing this PR removes. Two call sites in `crates/hyperion/src/simulation/inventory.rs`. ## Decisions on the hand-written enums this PR does *not* generate Listed because a hand-written type nobody has decided about is the thing to avoid, not the existence of hand-written types. - **`ChatVisiblity`, `ParticleStatus`** — generatable, blocked. Neither class is in the decompiled tree: `nix/minecraft-data.nix` selects classes referenced *one hop* from `net/minecraft/network`, and both are reached only via `net.minecraft.server.level.ClientInformation`, which is itself one hop out. Filed as ENG-10940. The extractor sees the gap today — both degrade to a bare `varint` noted `enum ordinal`, with the class name dropped. - **`StringArgumentKind`** — `com.mojang.brigadier.arguments.StringArgumentType$StringType`. Brigadier is a separate artifact and the decompile covers `net/minecraft` only. Staying hand-written; the reason is now at the type. - **`NamedColor`, `Decoration`** — `ChatFormatting`, but travelling as JSON/NBT *names* rather than numbers, and `Decoration` is a reordered subset. Their truth lives in DFU `MapCodec`s, which this extractor cannot read at all. Staying hand-written; see the note on the text module in the enumeration. - **`EntityDataSerializer`** — not a Java enum. `EntityDataSerializers` is a static registry of fields, numbered by `registerSerializer` call order, so it needs a different reader. Not attempted. - **`ComponentType`** — ENG-10872, three enums over one registry, blocked on a hand-transcribed `shape()` whose variant order is load-bearing. - **`HumanoidArm`** — the pure duplicate. `build.rs` already emits it into `OUT_DIR/types.rs`; the copy in `packets/configuration.rs` exists because that module was written in parallel with the generator and landed first. Deliberately left for its own change, because `packets/mod.rs` documents **eleven packets defined twice for exactly the same reason** and `HumanoidArm` is one thread of that knot rather than a separate cleanup. Filed as ENG-10941 with the eleven named. ## Checks `cargo test --workspace`, `cargo clippy --workspace --all-targets --all-features -- -D warnings` (CI's exact command) and `cargo fmt --all --check` all green. `nix build .#checks.aarch64-darwin.{minecraft-proto-coverage,minecraft-proto-json,minecraft-proto-generated,minecraft-encoder-fixtures,tool-paths}` all green. `minecraft-literals` is **red on `main`** and not from this branch: `events/smash/tests/visuals.rs` gained `minecraft:entity.player.hurt_on_fire` in #1035, one over the baseline of 322. Untouched here, reported rather than fixed. Co-authored-by: agent <agent@ix.dev>
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #1035 +/- ##
==========================================
+ Coverage 52.61% 54.07% +1.45%
==========================================
Files 337 347 +10
Lines 30819 32497 +1678
Branches 1194 1234 +40
==========================================
+ Hits 16216 17572 +1356
- Misses 14339 14644 +305
- Partials 264 281 +17
... and 10 files with indirect coverage changes 🚀 New features to boost your workflow:
|
andrewgazelka
added a commit
that referenced
this pull request
Jul 28, 2026
…1051) ## Effect `nix run .#minecraft-literals` is green on main again (324/324). It went red because two PRs force-merged past a queued CI each added a raw `minecraft:` sound string without updating the baseline: - **#1047** (bounds) — `minecraft:item.shield.block` in `arena.rs`, the push-back thud. - **#1035** (seam) — `minecraft:entity.player.hurt_on_fire` in `visuals.rs`. ## Why the baseline, not ALLOWED or a migration Both are `Shows.sound` / `play_sound` strings — the identical stringly-typed sound debt already tracked in the baseline for all fifteen kits' hurt sounds. The baseline is "debt with a name on it"; ALLOWED is for literals that are correct to keep. So they belong in the baseline. Resynced with the checker's own `--write`, which added exactly those two and nothing else. ## The real fix, deferred with a name Migrating `Shows.sound: &'static str` and the `play_sound` call sites to the generated `SoundEvent` enum retires this whole baseline section — both of these have variants (`SoundEvent::ItemShieldBlock`, `SoundEvent::EntityPlayerHurtOnFire`). That is the generate-from-source follow-up, not this gate unblock. ## Note for the class This is the second gate (after fmt) that force-merging against slow CI left red on main by landing past a queued check. ENG-10938 already recommends making the Formatting gate non-skippable under `--admin`; the same argument applies to `minecraft-literals` and `lint`. fmt itself is now clean again (`cargo fmt --all --check` rc=0), paid down by #1043's style commit, so no sweep is needed — only the permanent gate. ## Checks `python3 nix/check-minecraft-literals.py --root . --baseline nix/minecraft-literal-baseline.json` → 324/324, rc=0. `cargo fmt --all --check` → rc=0. Baseline-only change, no code touched.
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
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.
The seam half of the effects work, following #1030.
Cueis deleted; an ability now names any of the 125 particles and composes its own shape.The abilities agent should rebase on this. It touches 22 files, 15 of them kits, and every kit change is one line plus an import.
Why
Cuehad to go rather than growIt was three variants when written and five by tonight. It was also the only thing an ability could ask to be seen as, so Bone Explosion, Water Splash and Fish Flurry all drew
Cue::Explosionand bones, water and fish were one grey puff. The enum was never really the problem — the protocol layer under it could spell five particles, so five was all there was to ask for. #1030 removed that constraint.Server::particles(Particles)replacesServer::cue.Particlesishyperion::effects::Effect<'static>, re-exported throughserver.rsfor the same reasonTextis: a second copy of a builder is a second thing to keep in step.play_cueand the adapter's table of which particle each variant meant are gone, because that mapping was only ever a hosting decision standing in for a missing vocabulary.module::visualsis not the same enum renamedOne function per recurring moment —
blast,teleport,death,burn,venom— so an ability going off looks the same wherever it happens. The difference fromCueis that these are plain functions returning aParticles, not a closed set: a kit that wants something else writes it inline and does not have to widen anything.teleportis a 40-point sphere rather than a puff, because a teleport has a shape and the eye reads a shell as an arrival and a scatter as damage.The two placeholders are closed
Cue::Burnwas pinned tocritandCue::Venomto half-powerdragon_breath, both marked[PLACEHOLDER], because the proto carried neither real particle. They are nowminecraft:flameandminecraft:entity_effectin vanilla's own poison green (0x4E9331). That is items 3 on the abilities agent's list.Shows.cuebecomesShows.effect: fn(Vec3) -> Particles— a barefnfor the same reasonprojectile::Payload::on_hitis one: it is built in aconstand has nowhere to put a capture. It also means an affliction picks its own shape and density, not just its colour.Wire assertion
tools/smash-match.pygains a sixth sweep claim, "every declared ability was seen": each of the 51 abilities, fired by a real client, must be answered by aClientboundLevelParticlesPacketwith a non-zero count. It sits beside the sound claim it already made and is collected the same way, from every client rather than one, because the caster is not always in their own broadcast channel.That is the gate #1030 could not have: an effect needs something to emit it, and until this PR nothing did.
Guards broken, and what they said
Five new ones. The harness now refuses a mutant that fails to compile, after that produced two false "passes" in #1030.
a burn ticked and nothing was drawna poison is a tinted potion effect, not a shape, andassertion left != right failed / left: Flamecritplaceholderassertion left == right failed / left: Crit right: Flameteleport drew a particle 64.32 blocks from Vec3(12.5, 64.0, -3.5), at Vec3(0.0, 1.0, 0.0)blast sends a packet that draws nothingGuard 19 did not exist until breaking it showed that. The first attempt at it reported
GUARD DID NOT FIRE: deletingCuemoved the picture from a field the compiler checks to a function pointer it also checks, but nothing anywhere asserted the picture still arrived. An affliction whose particle call was dropped would tick, hurt, make its noise, be invisible, and pass the entire suite.tests/visuals.rsandMockServer::particles()exist because of that.Checks
nix run .#test— 741 passed, 1 skippednix run .#lint,.#fmt— cleanchecks.minecraft-literals— greenRebased on
54de2ba. The seven kit conflicts with #1032 were resolved by taking main's body wholesale and re-applying the mechanical rewrite to it, so none of that PR's tuning is dropped.Note for the registry charter
Particlescrossing the seam meansevents/smash/src/server.rsnow nameshyperion::effectsas well ashyperion_minecraft_proto. The seam's doc claims the game is host-agnostic; that was already only half true, sinceTextcomes from the proto crate. If strict host-agnosticism is wanted back, the move is to putEffect/Shapeinhyperion-minecraft-proto— they are pure data over a particle and a point — which needsglamin that crate or its ownVec3. Flagging rather than doing.🤖 Generated with Claude Code