feat(proto): every vanilla registry is a closed Rust enum - #1021
Merged
Conversation
## What a registry id costs now
Nothing. Not "almost nothing" -- the compiler emits no instructions for it.
`tools/registry-enum-asm.sh` compiles four probes at `--release` and prints
what they became:
```
# rustc 1.97.1, aarch64-apple-darwin
_probe_opaque_sound_id: # fn(SoundEvent) -> i32 { sound.id().0 }
ret
_probe_opaque_block_id: # fn(Block) -> i32 { block.id().0 }
ret
_probe_named_sound_id: # fn() -> i32 { SoundEvent::EntityArrowHit.id().0 }
mov w0, #85
ret
_probe_match_sound: # a three-arm match over 1968 variants
mov w8, #2
cmp w0, #1168
csel w8, w8, wzr, eq
cmp w0, #85
csinc w0, w8, wzr, ne
ret
```
The first two are the ones worth reading. `sound.id()` on a value the
optimiser cannot see through is a bare `ret`: the discriminant *is* the network
id, so the number is already in the return register and there is no conversion
to perform. `probe_named_sound_id` is the constant. `probe_match_sound` is five
branchless instructions with no jump table and no arm for a case that cannot
occur.
That is evidence I produced and can be reproduced by anyone with the script.
The *enforced* version of the same claim is a `const` assertion at the bottom
of all 94 generated registry files:
```rust
const _: () = assert!(Fluid::ALL[4].id().0 == 4);
const _: () = assert!(core::mem::size_of::<Fluid>() == 1);
```
A `const` context cannot call anything the compiler is unable to fold. If
`id()` ever stops being a cast off the discriminant, that line stops building.
Both forms are here because they prove different halves: the `const` covers the
compile-time-known variant on every build, the assembly covers the runtime
value once.
## The shape
`crates/hyperion-minecraft-proto/src/generated/registry/` is 95 files: one per
registry, plus a `mod.rs`. 94 of them carry an enum with one variant per entry
and 6854 variants between them.
```rust
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)]
#[repr(u16)]
pub enum SoundEvent {
/// `minecraft:entity.arrow.hit` (id 85).
EntityArrowHit = 85,
...
}
```
Four rules, and each one is a decision rather than a default.
**Closed.** No `Unknown(u32)`, no `#[non_exhaustive]`. A value inside the
server has already been proven to be in the set, and an open enum would make
every consumer write an arm for a case that cannot occur.
**`TryFrom` at the decode boundary only.** `from_id` is the one door for a
number that might be garbage, and it is the only fallible way in. Everything
else holds a value that is already proven, so there is nothing to check.
**Encoding is infallible.** `id()` is `const fn`, returns `RegistryId`, and
cannot fail.
**Lookups are a table or a cast, never a `HashMap` and never a match cascade.**
`id()` is the cast. `name()` is one load from a static indexed by the
discriminant. `from_name()` is a binary search over a permutation the generator
sorted, and is for names that are genuinely only known at run time -- one read
out of a world file, or typed into a command.
The repr is the narrowest that holds the registry: `Fluid` is one byte,
`SoundEvent` is two. `Option<SoundEvent>` is also two, because a closed enum
with room left in its discriminant gives `Option` its niche for free, so the
decode boundary costs no extra byte either.
## Nothing is curated, with one exception that is written down where it applies
Every registry the extractor reports gets an enum whether or not anything uses
it today. A hand-picked subset is a gap that surfaces the day somebody needs
the registry nobody picked, and the compile cost that would justify curating is
not real: the five biggest registries together build in 0.71s.
The exception is `minecraft:particle_type`, and it is not curation -- it is one
registry whose generator needs a different input. `nix/extract-protocol.py`
gives up on `ParticleTypes#STREAM_CODEC` ("dispatched codec: the layout depends
on a runtime type"), so `protocol.json` has the 125 names and nothing about
their bodies, and 13 of the 125 have one: `block` carries a block state, `dust`
a colour and a scale, `vibration` a destination and a flight time. An enum
generated from this file would be id-only and structurally unable to say that,
while being the more discoverable of the two spellings.
`nix/generate-particles.py` reads the decompiled codecs instead and owns
`crate::particle`.
The reason is written at three places so it cannot expire into an agreement
nobody remembers: `NO_ENUM` in `nix/generate-rust.py`, the module doc of the
generated `registry/particle_type.rs` (which keeps its name table, because that
is what the other generator pins its ids against), and `NO_ENUM` in
`tests/registry_enum.rs`, which fails if the set of carve-outs ever changes
without somebody editing all three.
## Every guard here was watched failing
Not "added a check". Broken, run, output read, restored.
**The `const` assertion, given an `id()` that is not the discriminant.**
Changed `RegistryId(self as i32)` to `+ 1` in one file:
```
error[E0080]: evaluation panicked: assertion failed: Fluid::ALL[4].id().0 == 4
--> crates/hyperion-minecraft-proto/src/generated/registry/fluid.rs:126:15
|
126 | const _: () = assert!(Fluid::ALL[4].id().0 == 4);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ evaluation of `_` failed here
```
**The compile-fail doctest, given a variant that exists.** Replaced the
invented `SoundEvent::EntityArrowHitDeluxe` with the real `EntityArrowHit`:
```
test .../registry/mod.rs - generated::registry (line 47) - compile fail ... FAILED
Test compiled successfully, but it's marked `compile_fail`.
```
**The sorted-table assumption, given a table that is not sorted.** The one the
brief warned about: a wrong order fails as a lookup miss and reads as "that
entry does not exist". Swapped two entries of `Fluid`'s by-name permutation:
```
thread 'every_name_resolves_through_the_binary_search' panicked at
crates/hyperion-minecraft-proto/tests/registry_enum.rs:71:13:
assertion `left == right` failed: Fluid: from_name(minecraft:flowing_lava) missed;
the by-name table is out of order
left: None
right: Some(3)
```
The order is not assumed anywhere. The generator sorts it, and the test puts
all 6854 names back through `from_name` rather than spot-checking, so a wrong
permutation is caught by name.
**The carve-out list, emptied.**
```
thread 'every_registry_but_the_named_carve_out_has_an_enum' panicked:
assertion `left == right` failed: a registry lost or gained an enum; if that was
deliberate, say so in NO_ENUM here and at the exclusion site in nix/generate-rust.py
left: ["minecraft:particle_type"]
right: []
```
**The generator's own collision refusal.** Made `variant()` drop digits, which
is what a careless normalisation would do:
```
minecraft:command_argument_type: `minecraft:vec3` and `minecraft:vec2` both become
the variant `Vec`; teach variant() a disambiguation rather than dropping one
```
This is the check the brief asked about specifically. Rust identifiers are
case-sensitive, so a collision is an exact string collision and comparing the
converted strings is the whole test; it runs in the generator rather than in a
test because a colliding identifier does not compile and the useful failure is
the one naming which two entries did it. On the real data it finds none: 6979
entry names across 95 registries all convert to distinct identifiers.
## What this does not do
**No call site is migrated.** Deliberate, and sequenced with four other agents
working in this repo tonight. `entity_type::EntityType` is still the
hand-written struct -- a `&'static str` plus an `i32`, 16 bytes -- and
`generated::registry::EntityType` is the one-byte enum beside it. That is one
concept with two implementations and it is the next PR, not this one: it
renames `EntityType::PIG` to `EntityType::Pig` across 121 call sites, which is
the change most likely to conflict with somebody else's open branch.
**The 26 runtime validation sites are all still there.** Counted, not
estimated, and the list is the baseline the next two PRs move:
```
$ rg -n --type rust -e '\.id_of\(' -e 'ItemKind::from_str' -e 'BlockKind::from_str' \
-e 'entity_type\(name' -e 'block_state::state_id\(' -e 'default_state_id\(' \
crates events | grep -v '/tests/' | grep -v '/generated/'
26
```
Some of those are legitimate -- the anvil world loader really is resolving a
name it read out of a file -- and the ones that are not include
`events/smash/src/adapter.rs:800`, `ItemKind::from_str(bare).unwrap_or(ItemKind::Stick)`,
which silently hands back a stick for a name nobody typed correctly. Saying
"now zero-cost" while that line exists would be marketing. What this PR claims
is narrower and is proved above: a registry value, once you have one, costs
nothing to name, to match on, or to put on the wire.
**`minecraft:data_component_type` now has three enums over it**, and this PR
added the third. `generated::data_component::DataComponent` and
`item::component_type::ComponentType` were already a duplicate pair on main;
generating the registry enum uniformly makes it three rather than carving out a
second exception. The alternative was a second carve-out for a reason that is
about this crate's history rather than about the data, which is exactly the
kind of exception that accumulates. Filed as ENG-10872 to collapse them.
**Two things I did not check.** Whether the 95 enums slow a cold workspace
build measurably -- I measured the five biggest in isolation (0.71s) and the
crate's own build, not the workspace's wall clock. And whether rustdoc on 6854
variants is tolerable; `cargo doc` was never run.
## Gates
| gate | result |
| --- | --- |
| `nix build .#checks.aarch64-darwin.minecraft-proto-generated` | rc=0 |
| `cargo clippy --workspace --all-targets` | rc=0 |
| `cargo fmt --all --check` | rc=0 |
| `cargo test -p hyperion-minecraft-proto --test registry_enum` | rc=0, 11 tests |
| `cargo test -p hyperion-minecraft-proto --doc` | rc=0, incl. 1 compile-fail |
| `flake8 nix/generate-rust.py` | rc=0 |
The nix check is the load-bearing one: it reruns the whole extraction from the
jar and diffs the result against the committed tree, so the 97 files here are
what the pipeline produces rather than what I happened to write.
---
AI-authored (Claude) under human direction.
Benchmark Results for generalComparing to e4bbcc2 |
andrewgazelka
added a commit
that referenced
this pull request
Jul 28, 2026
…rce (#1028) fix(tools): scripted clients read protocol.json, not each other's source ## What broke, and how it got through #1021 turned `crates/hyperion-minecraft-proto/src/generated/registry.rs` into a directory. Three Python clients under `tools/` were reading that file as text and pulling registry names out of it with a regex. They kept "building", passed every Rust gate, and failed twenty minutes into CI inside four end-to-end checks: ``` FileNotFoundError: [Errno 2] No such file or directory: '/nix/store/bavm52as33izbs3kl5zihvszxgskz7lw-source/crates/hyperion-minecraft-proto/src/generated/registry.rs' enforced checks that failed: completions-e2e smash-e2e smash-hotbar-e2e smash-hud-e2e ``` A path in a Python string is invisible to cargo and to every Rust grep. I searched for `generated::registry` before landing #1021 and found the Rust callers; I did not search for `generated/registry.rs`, and nothing made me. `smash-selector-e2e` is the same bug one merge later: `tools/smash-selector.py` read `src/entity_type.rs`, which #1024 deleted, with a regex matching `Self::new("...", n)` — a spelling that stopped existing when entity types became a plain enum. CI had not reached it yet. Running the whole gate locally found it: ``` enforced checks that failed: completions-e2e smash-e2e smash-hotbar-e2e smash-hud-e2e smash-selector-e2e ``` ## The fix is that they stop reading Rust Pointing three regexes at three new paths would have restored green and left the same trap armed. All three now call one reader: ```python def registry_entries(registry: str) -> list[str]: """A vanilla registry's entry names, in network-id order.""" ``` in `tools/client-26.2.py`, reading `crates/hyperion-minecraft-proto/protocol.json` — the same file `build.rs` reads and the same file every generated table comes out of. `protocol.json` joins the e2e client fileset in `nix/e2e.nix`. Two bugs went with the scrapers. `completions-check.py` carried a comment explaining that it had to index from `entries: &[` rather than from the declaration, because the registry's own name sits in the same table and looks exactly like an entry — getting it wrong shifted every id by one and turned `brigadier:string` into `brigadier:long`. `smash-match.py` compensated for the same thing with a bare `[1:]`. Neither is needed: `entries` in the JSON is the entries, and the index is the id. ## The gate, because the class outlives the instance `nix flake check .#tool-paths` fails when any tracked `.py`, `.sh` or `.nix` file names a repository path that does not exist. Only strings anchored at a real top-level directory count, and anything containing a format placeholder or a glob is skipped, so prose is not mistaken for a claim. It earned itself immediately. Its first run, before I had looked at `smash-selector.py`, found the bug #1024 had just introduced: ``` 4 repository paths named in scripts, 1 of them missing MISSING tools/smash-selector.py:89 crates/hyperion-minecraft-proto/src/entity_type.rs ``` Broken deliberately as well, by putting the dead path back into `completions-check.py`: ``` 4 repository paths named in scripts, 1 of them missing MISSING tools/completions-check.py:44 crates/hyperion-minecraft-proto/src/generated/registry.rs A script names a path this tree does not have. Nothing else catches this: a path in a string is invisible to cargo and to every Rust grep, so it fails inside whichever gate runs that script. Either fix the path, or -- better -- stop reaching into another component's source and read its committed data instead. ``` Restored, and it reports `3 repository paths named in scripts, 0 of them missing`. This is the cheap half of the fix and it is worth saying which half. It cannot tell that a regex has stopped matching a file that still exists — only that the file is gone. What stops the other half is the three clients no longer parsing Rust at all. ## Gates | gate | result | | --- | --- | | `nix build .#checks.aarch64-darwin.completions-e2e` | rc=0, `RESULT: every completion claim held` | | `nix build .#checks.aarch64-darwin.tool-paths` | rc=0, 3/3 | | `nix eval .#checks.aarch64-darwin.flake-gate.drvPath` | rc=0 | | `cargo fmt --all --check` | rc=0 | | `flake8` on all five changed scripts | rc=0 for everything this change touched | `completions-e2e` is the one that was failing and it is the one I ran to completion. The other four e2e checks share the same reader through `smash-match.py`'s `load_item_names` and `smash-selector.py`'s `entity_names`, both of which are exercised by the unit-level check above, but I did not wait on all five locally -- each compiles the whole server. CI runs them. Two lint findings in `tools/smash-selector.py` (E301, E302) are pre-existing; I checked by stashing and re-running. Not fixed here. --- AI-authored (Claude) under human direction.
andrewgazelka
added a commit
that referenced
this pull request
Jul 28, 2026
## The gate first, the migration second A `"minecraft:..."` string in Rust is an unproven claim. Nothing checks that the name exists, that this version still spells it that way, or that the registry it indexes is the one the caller meant, so a typo or a Mojang rename becomes a `None` on whichever tick that value was next needed. Every static registry is a closed enum since #1021, so the name can be a variant and the compiler answers all three questions at the line that asked. Fixing occurrences by hand does not stop the class. `nix flake check .#minecraft-literals` does, and it is the point of this change; the migration below is what it enabled, not what it is for. ## Counted from the source, not from a regex over lines The brief warned about this and it was warranted. A regex cannot tell `//! minecraft:pig` in a doc comment from `"minecraft:pig"` in code, and this repo's doc comments name registry entries constantly -- `entity_kind.rs` alone has dozens. `nix/check-minecraft-literals.py` walks each file the way Rust lexes it: line comments, nested block comments, char literals and lifetimes are stepped over, and raw strings (`r#"..."#`) keep their contents. What that finds across every file in `git ls-files '*.rs'`: | | files | literals | | --- | --- | --- | | generated, recognised by the `@generated` header | 102 | 10009 | | hand-written | 56 | 765 | 765, against the 659 an earlier pass measured. I did not reconstruct how that number was reached, so I cannot say which 106 it missed, only that a Rust-aware scan finds more. Generated files are exempted by their own header rather than by a path list, so a new generated file is covered the day it appears and a file that stops being generated stops being exempt. ## Two kinds of exemption, and they are different on purpose `ALLOWED` is a path with a reason: a file where these literals are correct and are expected to stay. Two entries, 430 names between them. - `crates/hyperion/src/net/protocol/registries.rs` -- the element names of the 29 **dynamic** registries. `protocol.json` does not carry those, so they have no enum. The file is a name table of the same kind as generated output and its own header already says it should be deleted once the proto crate generates dynamic registry contents. - `crates/hyperion-minecraft-proto/tests/registry_enum.rs` -- the test proving the enums cover the registries has to name entries as strings to check that the string form resolves, and names deliberate non-entries to check that they do not. The baseline is a path with a *list*: names that have not been migrated yet. It is debt with a name on it, it ratchets in both directions, and the intended end state is that it is empty and the baseline file is deleted. 386 at the start of this change, 323 after the migration below, and 327 after rebasing onto a main that had grown four more while this branch was open. I chose a ratchet over a hard failure for the remainder because most of them are in `events/smash/**` and `crates/hyperion/src/simulation/**`, where three other agents have open branches tonight. Migrating those files now would conflict with work in flight for no gain the gate does not already give: the gate refuses the *next* one either way. ## What was migrated, and why this one `minecraft:command_argument_type`, 63 names across two files, chosen because it was the largest single cluster and because it fixes something the brief names directly: **encoding could fail**. ```rust -fn argument_type_id(name: &str) -> Result<i32> { - let id = registry::COMMAND_ARGUMENT_TYPE - .id_of(name) - .ok_or_else(|| Error::InvalidIdentifier(name.to_owned()))?; - i32::try_from(id).map_err(|_| Error::InvalidIdentifier(name.to_owned())) -} ``` That was a linear scan over 57 entries, returning a `Result`, run for every argument of every command in the tree, so that this server could discover at run time whether `brigadier:double` still existed. `ArgumentType::to_id` is now `const fn` and infallible, and `ArgumentType::Empty` and `ArgumentType::Registry` hold a `CommandArgumentType` rather than a bare `RegistryId` -- so the id cannot be one the registry does not have, rather than being checked against it. `decode` changed in the same direction and is the interesting half: it used to resolve the incoming id to a *name* and then match on string literals, which is 57 string comparisons in the worst case, and silently fell through to `Self::Empty` for a name it did not recognise. It now resolves through `CommandArgumentType::from_id` once -- the single decode boundary -- and matches on variants. `crates/hyperion/src/simulation/command.rs` lost its own duplicate `argument_type_id`, and `empty_argument` went from `-> anyhow::Result<...>` to a `const fn`, which deleted a `?` from 43 call sites. Runtime validation sites on non-decode paths, by the command in the previous PR's body: 26 before, 22 after. The four removed are the two `argument_type_id` helpers and the two `id_of` calls inside them. The remaining 22 are enumerated in the baseline and in that list; several are legitimate -- the anvil world loader really is resolving a name it read out of a file -- and the worst is `events/smash/src/adapter.rs:800`, `ItemKind::from_str(bare).unwrap_or(ItemKind::Stick)`, which hands back a stick for a name nobody typed correctly. That file belongs to another agent tonight. ## The gate, broken in every direction and watched **A new raw name appears.** Added a second biome lookup to `translate.rs`: ``` 387 raw registry names in Rust string literals, 386 allowed by the baseline NEW crates/hyperion/src/simulation/blocks/translate.rs minecraft:sunflower_plains 1 raw registry name(s) that were not there before. Every static registry is a closed enum in hyperion_minecraft_proto::generated::registry, so name the variant instead: a typo or a Mojang rename is then a compile error here rather than a None at run time. If the name belongs to a dynamic registry and genuinely has no enum, add the file to ALLOWED in nix/check-minecraft-literals.py with the reason. ``` **Names disappear and the baseline stops bounding anything.** This is not a hypothetical; it is what the command-argument migration produced before the baseline was tightened: ``` 323 raw registry names in Rust string literals, 386 allowed by the baseline GONE crates/hyperion-minecraft-proto/src/packets/play/player.rs brigadier:double GONE crates/hyperion-minecraft-proto/src/packets/play/player.rs brigadier:float ... 85 lines ... 63 raw registry name(s) are gone, so the baseline is looser than the code and has stopped bounding anything. Tighten it with: nix run .#sync-minecraft-literals ``` **The claim about comments, tested rather than asserted.** Added a doc comment naming `minecraft:block_pos` and `"minecraft:vec3"`, and a block comment containing `"minecraft:item_stack"`, to `command.rs`. The count stayed at 323 and no `NEW` line appeared, which is the whole reason this is a lexer and not a `rg` invocation. A line-based check would have reported three regressions there. ## The gate caught something before it even merged While this branch was open, #1023 landed on `main`. Rebasing onto it, with no prompting and no edit to the checker: ``` 327 raw registry names in Rust string literals, 323 allowed by the baseline NEW events/smash/src/module/kits/blaze.rs minecraft:entity.player.hurt_on_fire NEW events/smash/src/module/kits/spider.rs minecraft:entity.player.hurt_sweet_berry_bush NEW events/smash/tests/abilities.rs minecraft:entity.arrow.shoot NEW events/smash/tests/contract.rs minecraft:entity.player.hurt_on_fire ``` Four sound names written as strings in a PR authored after the enums existed, which is the class this is meant to stop and a fair demonstration that it does not stop itself. They are in the baseline rather than migrated: those files have work in flight against them tonight and the gate refuses the *next* one either way. Had this check been on `main` an hour earlier, #1023 would have been asked to write `SoundEvent::EntityPlayerHurtOnFire` and the four would not exist. ## Gates | gate | result | | --- | --- | | `nix build .#checks.aarch64-darwin.minecraft-literals` | rc=0, 327/327 | | `nix eval .#checks.aarch64-darwin.flake-gate.drvPath` | rc=0 | | `cargo clippy --workspace --all-targets` | rc=0 | | `cargo fmt --all --check` | rc=0 | | `cargo test --workspace` | rc=0 | | `flake8 nix/check-minecraft-literals.py` | rc=0 | Because #1007 made CI enforce the flake checks by name, `minecraft-literals` runs in CI the day this merges with no edit to `nix/ci/flake-gate.nix`. ## What I did not do **327 names are still raw.** Named above, bounded by the baseline, and left deliberately rather than overlooked. **The dynamic registries still have no enum.** 430 of the 765 hand-written literals are the one file that lists them, and generating them properly means teaching `nix/generate-registry-data.py` to emit enums the way `generate-rust.py` now does. That would delete the largest `ALLOWED` entry and is the obvious next step; it is not this PR. Filed as ENG-10887. **The worst remaining fallback is filed rather than fixed.** ENG-10884 covers `ItemKind::from_str(bare).unwrap_or(ItemKind::Stick)`, which is in another agent's file this week. **`nix flake check` was not run whole.** The three checks this change touches were built individually and `flake-gate` was evaluated, but the full check set compiles a server and I did not wait on it. ## One red gate here is not mine `cargo clippy --all-targets --all-features -- -D warnings` fails on `events/smash/tests/abilities.rs:595`, an unused `use flecs_ecs::prelude::*;`. It arrived with #1023, whose own Clippy check failed and which was merged anyway, so it is red on `main` independently of this branch. Removing that one line locally makes the whole workspace pass, which is how I know nothing here contributes to it. Left alone: that file has another agent's work in flight against it tonight. Worth recording how I nearly missed it. `cargo clippy --workspace --all-targets` reported clean three times on this tree, because the test target was cached and because it does not pass `-D warnings`. CI's invocation is `cargo clippy --all-targets --all-features -- -D warnings` and the difference turns a warning into a failure. Running anything less than CI's exact command is not a check. --- AI-authored (Claude) under human direction.
andrewgazelka
added a commit
that referenced
this pull request
Jul 28, 2026
## The gate first, the migration second A `"minecraft:..."` string in Rust is an unproven claim. Nothing checks that the name exists, that this version still spells it that way, or that the registry it indexes is the one the caller meant, so a typo or a Mojang rename becomes a `None` on whichever tick that value was next needed. Every static registry is a closed enum since #1021, so the name can be a variant and the compiler answers all three questions at the line that asked. Fixing occurrences by hand does not stop the class. `nix flake check .#minecraft-literals` does, and it is the point of this change; the migration below is what it enabled, not what it is for. ## Counted from the source, not from a regex over lines The brief warned about this and it was warranted. A regex cannot tell `//! minecraft:pig` in a doc comment from `"minecraft:pig"` in code, and this repo's doc comments name registry entries constantly -- `entity_kind.rs` alone has dozens. `nix/check-minecraft-literals.py` walks each file the way Rust lexes it: line comments, nested block comments, char literals and lifetimes are stepped over, and raw strings (`r#"..."#`) keep their contents. What that finds across every file in `git ls-files '*.rs'`: | | files | literals | | --- | --- | --- | | generated, recognised by the `@generated` header | 102 | 10009 | | hand-written | 56 | 765 | 765, against the 659 an earlier pass measured. I did not reconstruct how that number was reached, so I cannot say which 106 it missed, only that a Rust-aware scan finds more. Generated files are exempted by their own header rather than by a path list, so a new generated file is covered the day it appears and a file that stops being generated stops being exempt. ## Two kinds of exemption, and they are different on purpose `ALLOWED` is a path with a reason: a file where these literals are correct and are expected to stay. Two entries, 430 names between them. - `crates/hyperion/src/net/protocol/registries.rs` -- the element names of the 29 **dynamic** registries. `protocol.json` does not carry those, so they have no enum. The file is a name table of the same kind as generated output and its own header already says it should be deleted once the proto crate generates dynamic registry contents. - `crates/hyperion-minecraft-proto/tests/registry_enum.rs` -- the test proving the enums cover the registries has to name entries as strings to check that the string form resolves, and names deliberate non-entries to check that they do not. The baseline is a path with a *list*: names that have not been migrated yet. It is debt with a name on it, it ratchets in both directions, and the intended end state is that it is empty and the baseline file is deleted. 386 at the start of this change, 323 after the migration below, and 327 after rebasing onto a main that had grown four more while this branch was open. I chose a ratchet over a hard failure for the remainder because most of them are in `events/smash/**` and `crates/hyperion/src/simulation/**`, where three other agents have open branches tonight. Migrating those files now would conflict with work in flight for no gain the gate does not already give: the gate refuses the *next* one either way. ## What was migrated, and why this one `minecraft:command_argument_type`, 63 names across two files, chosen because it was the largest single cluster and because it fixes something the brief names directly: **encoding could fail**. ```rust -fn argument_type_id(name: &str) -> Result<i32> { - let id = registry::COMMAND_ARGUMENT_TYPE - .id_of(name) - .ok_or_else(|| Error::InvalidIdentifier(name.to_owned()))?; - i32::try_from(id).map_err(|_| Error::InvalidIdentifier(name.to_owned())) -} ``` That was a linear scan over 57 entries, returning a `Result`, run for every argument of every command in the tree, so that this server could discover at run time whether `brigadier:double` still existed. `ArgumentType::to_id` is now `const fn` and infallible, and `ArgumentType::Empty` and `ArgumentType::Registry` hold a `CommandArgumentType` rather than a bare `RegistryId` -- so the id cannot be one the registry does not have, rather than being checked against it. `decode` changed in the same direction and is the interesting half: it used to resolve the incoming id to a *name* and then match on string literals, which is 57 string comparisons in the worst case, and silently fell through to `Self::Empty` for a name it did not recognise. It now resolves through `CommandArgumentType::from_id` once -- the single decode boundary -- and matches on variants. `crates/hyperion/src/simulation/command.rs` lost its own duplicate `argument_type_id`, and `empty_argument` went from `-> anyhow::Result<...>` to a `const fn`, which deleted a `?` from 43 call sites. Runtime validation sites on non-decode paths, by the command in the previous PR's body: 26 before, 22 after. The four removed are the two `argument_type_id` helpers and the two `id_of` calls inside them. The remaining 22 are enumerated in the baseline and in that list; several are legitimate -- the anvil world loader really is resolving a name it read out of a file -- and the worst is `events/smash/src/adapter.rs:800`, `ItemKind::from_str(bare).unwrap_or(ItemKind::Stick)`, which hands back a stick for a name nobody typed correctly. That file belongs to another agent tonight. ## The gate, broken in every direction and watched **A new raw name appears.** Added a second biome lookup to `translate.rs`: ``` 387 raw registry names in Rust string literals, 386 allowed by the baseline NEW crates/hyperion/src/simulation/blocks/translate.rs minecraft:sunflower_plains 1 raw registry name(s) that were not there before. Every static registry is a closed enum in hyperion_minecraft_proto::generated::registry, so name the variant instead: a typo or a Mojang rename is then a compile error here rather than a None at run time. If the name belongs to a dynamic registry and genuinely has no enum, add the file to ALLOWED in nix/check-minecraft-literals.py with the reason. ``` **Names disappear and the baseline stops bounding anything.** This is not a hypothetical; it is what the command-argument migration produced before the baseline was tightened: ``` 323 raw registry names in Rust string literals, 386 allowed by the baseline GONE crates/hyperion-minecraft-proto/src/packets/play/player.rs brigadier:double GONE crates/hyperion-minecraft-proto/src/packets/play/player.rs brigadier:float ... 85 lines ... 63 raw registry name(s) are gone, so the baseline is looser than the code and has stopped bounding anything. Tighten it with: nix run .#sync-minecraft-literals ``` **The claim about comments, tested rather than asserted.** Added a doc comment naming `minecraft:block_pos` and `"minecraft:vec3"`, and a block comment containing `"minecraft:item_stack"`, to `command.rs`. The count stayed at 323 and no `NEW` line appeared, which is the whole reason this is a lexer and not a `rg` invocation. A line-based check would have reported three regressions there. ## The gate caught something before it even merged While this branch was open, #1023 landed on `main`. Rebasing onto it, with no prompting and no edit to the checker: ``` 327 raw registry names in Rust string literals, 323 allowed by the baseline NEW events/smash/src/module/kits/blaze.rs minecraft:entity.player.hurt_on_fire NEW events/smash/src/module/kits/spider.rs minecraft:entity.player.hurt_sweet_berry_bush NEW events/smash/tests/abilities.rs minecraft:entity.arrow.shoot NEW events/smash/tests/contract.rs minecraft:entity.player.hurt_on_fire ``` Four sound names written as strings in a PR authored after the enums existed, which is the class this is meant to stop and a fair demonstration that it does not stop itself. They are in the baseline rather than migrated: those files have work in flight against them tonight and the gate refuses the *next* one either way. Had this check been on `main` an hour earlier, #1023 would have been asked to write `SoundEvent::EntityPlayerHurtOnFire` and the four would not exist. ## Gates | gate | result | | --- | --- | | `nix build .#checks.aarch64-darwin.minecraft-literals` | rc=0, 327/327 | | `nix eval .#checks.aarch64-darwin.flake-gate.drvPath` | rc=0 | | `cargo clippy --workspace --all-targets` | rc=0 | | `cargo fmt --all --check` | rc=0 | | `cargo test --workspace` | rc=0 | | `flake8 nix/check-minecraft-literals.py` | rc=0 | Because #1007 made CI enforce the flake checks by name, `minecraft-literals` runs in CI the day this merges with no edit to `nix/ci/flake-gate.nix`. ## What I did not do **327 names are still raw.** Named above, bounded by the baseline, and left deliberately rather than overlooked. **The dynamic registries still have no enum.** 430 of the 765 hand-written literals are the one file that lists them, and generating them properly means teaching `nix/generate-registry-data.py` to emit enums the way `generate-rust.py` now does. That would delete the largest `ALLOWED` entry and is the obvious next step; it is not this PR. Filed as ENG-10887. **The worst remaining fallback is filed rather than fixed.** ENG-10884 covers `ItemKind::from_str(bare).unwrap_or(ItemKind::Stick)`, which is in another agent's file this week. **`nix flake check` was not run whole.** The three checks this change touches were built individually and `flake-gate` was evaluated, but the full check set compiles a server and I did not wait on it. ## Also in here: a stale reference in my own generator `nix/generate-rust.py` explained the `particle_type` carve-out by saying the particle enum is owned separately "the same way `nix/generate-entity-types.py` owns entity types". #1024 -- mine, two commits earlier -- deleted that script. So the comment justified a real decision by appealing to a precedent that no longer existed, which leaves the next reader either hunting a missing file or concluding the carve-out was arbitrary. It also named `nix/generate-particles.py` and `src/particle.rs` as though they were present. They are not on `main` yet. The reason for the carve-out is a fact about the data and is true today; the ownership is an arrangement that lands separately. The comment now says the first without asserting the second, and the reciprocal note in the generated `registry/particle_type.rs` changed with it, so the two cannot drift. Worth naming the general shape, because a gate in this very PR nearly caught it and did not. `tool-paths` only matches paths at the start of a string literal; this one was inside backticks in the middle of a sentence. A comment can name a file that does not exist and nothing anywhere objects. I have not widened the pattern -- matching every path-shaped substring in every comment would flag prose constantly -- so this stays a thing a reader catches, and one did. ## A note on what a green local clippy is worth `cargo clippy --workspace --all-targets` reported this tree clean three times while CI was failing, for two reasons that will recur. It does not pass `-D warnings`, so an unused import is a warning locally and an error in CI. And cargo caches per target, so a test target nobody touched is not re-checked and a warning that arrived with somebody else's merge stays invisible. `touch`ing the file made it appear at once. CI's invocation is `cargo clippy --all-targets --all-features -- -D warnings` (flake.nix:185). Anything less is not the same check, and this PR is verified with that one. Filed as ENG-10894 along with the red it exposed, which #1029 has since fixed. --- AI-authored (Claude) under human direction.
andrewgazelka
added a commit
that referenced
this pull request
Jul 28, 2026
…1025) feat(proto): a raw `minecraft:` name in Rust is now a build failure ## The gate first, the migration second A `"minecraft:..."` string in Rust is an unproven claim. Nothing checks that the name exists, that this version still spells it that way, or that the registry it indexes is the one the caller meant, so a typo or a Mojang rename becomes a `None` on whichever tick that value was next needed. Every static registry is a closed enum since #1021, so the name can be a variant and the compiler answers all three questions at the line that asked. Fixing occurrences by hand does not stop the class. `nix flake check .#minecraft-literals` does, and it is the point of this change; the migration below is what it enabled, not what it is for. ## Counted from the source, not from a regex over lines The brief warned about this and it was warranted. A regex cannot tell `//! minecraft:pig` in a doc comment from `"minecraft:pig"` in code, and this repo's doc comments name registry entries constantly -- `entity_kind.rs` alone has dozens. `nix/check-minecraft-literals.py` walks each file the way Rust lexes it: line comments, nested block comments, char literals and lifetimes are stepped over, and raw strings (`r#"..."#`) keep their contents. What that finds across every file in `git ls-files '*.rs'`: | | files | literals | | --- | --- | --- | | generated, recognised by the `@generated` header | 102 | 10009 | | hand-written | 56 | 765 | 765, against the 659 an earlier pass measured. I did not reconstruct how that number was reached, so I cannot say which 106 it missed, only that a Rust-aware scan finds more. Generated files are exempted by their own header rather than by a path list, so a new generated file is covered the day it appears and a file that stops being generated stops being exempt. ## Two kinds of exemption, and they are different on purpose `ALLOWED` is a path with a reason: a file where these literals are correct and are expected to stay. Two entries, 430 names between them. - `crates/hyperion/src/net/protocol/registries.rs` -- the element names of the 29 **dynamic** registries. `protocol.json` does not carry those, so they have no enum. The file is a name table of the same kind as generated output and its own header already says it should be deleted once the proto crate generates dynamic registry contents. - `crates/hyperion-minecraft-proto/tests/registry_enum.rs` -- the test proving the enums cover the registries has to name entries as strings to check that the string form resolves, and names deliberate non-entries to check that they do not. The baseline is a path with a *list*: names that have not been migrated yet. It is debt with a name on it, it ratchets in both directions, and the intended end state is that it is empty and the baseline file is deleted. 386 at the start of this change, 323 after the migration below, and 327 after rebasing onto a main that had grown four more while this branch was open. I chose a ratchet over a hard failure for the remainder because most of them are in `events/smash/**` and `crates/hyperion/src/simulation/**`, where three other agents have open branches tonight. Migrating those files now would conflict with work in flight for no gain the gate does not already give: the gate refuses the *next* one either way. ## What was migrated, and why this one `minecraft:command_argument_type`, 63 names across two files, chosen because it was the largest single cluster and because it fixes something the brief names directly: **encoding could fail**. ```rust -fn argument_type_id(name: &str) -> Result<i32> { - let id = registry::COMMAND_ARGUMENT_TYPE - .id_of(name) - .ok_or_else(|| Error::InvalidIdentifier(name.to_owned()))?; - i32::try_from(id).map_err(|_| Error::InvalidIdentifier(name.to_owned())) -} ``` That was a linear scan over 57 entries, returning a `Result`, run for every argument of every command in the tree, so that this server could discover at run time whether `brigadier:double` still existed. `ArgumentType::to_id` is now `const fn` and infallible, and `ArgumentType::Empty` and `ArgumentType::Registry` hold a `CommandArgumentType` rather than a bare `RegistryId` -- so the id cannot be one the registry does not have, rather than being checked against it. `decode` changed in the same direction and is the interesting half: it used to resolve the incoming id to a *name* and then match on string literals, which is 57 string comparisons in the worst case, and silently fell through to `Self::Empty` for a name it did not recognise. It now resolves through `CommandArgumentType::from_id` once -- the single decode boundary -- and matches on variants. `crates/hyperion/src/simulation/command.rs` lost its own duplicate `argument_type_id`, and `empty_argument` went from `-> anyhow::Result<...>` to a `const fn`, which deleted a `?` from 43 call sites. Runtime validation sites on non-decode paths, by the command in the previous PR's body: 26 before, 22 after. The four removed are the two `argument_type_id` helpers and the two `id_of` calls inside them. The remaining 22 are enumerated in the baseline and in that list; several are legitimate -- the anvil world loader really is resolving a name it read out of a file -- and the worst is `events/smash/src/adapter.rs:800`, `ItemKind::from_str(bare).unwrap_or(ItemKind::Stick)`, which hands back a stick for a name nobody typed correctly. That file belongs to another agent tonight. ## The gate, broken in every direction and watched **A new raw name appears.** Added a second biome lookup to `translate.rs`: ``` 387 raw registry names in Rust string literals, 386 allowed by the baseline NEW crates/hyperion/src/simulation/blocks/translate.rs minecraft:sunflower_plains 1 raw registry name(s) that were not there before. Every static registry is a closed enum in hyperion_minecraft_proto::generated::registry, so name the variant instead: a typo or a Mojang rename is then a compile error here rather than a None at run time. If the name belongs to a dynamic registry and genuinely has no enum, add the file to ALLOWED in nix/check-minecraft-literals.py with the reason. ``` **Names disappear and the baseline stops bounding anything.** This is not a hypothetical; it is what the command-argument migration produced before the baseline was tightened: ``` 323 raw registry names in Rust string literals, 386 allowed by the baseline GONE crates/hyperion-minecraft-proto/src/packets/play/player.rs brigadier:double GONE crates/hyperion-minecraft-proto/src/packets/play/player.rs brigadier:float ... 85 lines ... 63 raw registry name(s) are gone, so the baseline is looser than the code and has stopped bounding anything. Tighten it with: nix run .#sync-minecraft-literals ``` **The claim about comments, tested rather than asserted.** Added a doc comment naming `minecraft:block_pos` and `"minecraft:vec3"`, and a block comment containing `"minecraft:item_stack"`, to `command.rs`. The count stayed at 323 and no `NEW` line appeared, which is the whole reason this is a lexer and not a `rg` invocation. A line-based check would have reported three regressions there. ## The gate caught something before it even merged While this branch was open, #1023 landed on `main`. Rebasing onto it, with no prompting and no edit to the checker: ``` 327 raw registry names in Rust string literals, 323 allowed by the baseline NEW events/smash/src/module/kits/blaze.rs minecraft:entity.player.hurt_on_fire NEW events/smash/src/module/kits/spider.rs minecraft:entity.player.hurt_sweet_berry_bush NEW events/smash/tests/abilities.rs minecraft:entity.arrow.shoot NEW events/smash/tests/contract.rs minecraft:entity.player.hurt_on_fire ``` Four sound names written as strings in a PR authored after the enums existed, which is the class this is meant to stop and a fair demonstration that it does not stop itself. They are in the baseline rather than migrated: those files have work in flight against them tonight and the gate refuses the *next* one either way. Had this check been on `main` an hour earlier, #1023 would have been asked to write `SoundEvent::EntityPlayerHurtOnFire` and the four would not exist. ## Gates | gate | result | | --- | --- | | `nix build .#checks.aarch64-darwin.minecraft-literals` | rc=0, 327/327 | | `nix eval .#checks.aarch64-darwin.flake-gate.drvPath` | rc=0 | | `cargo clippy --workspace --all-targets` | rc=0 | | `cargo fmt --all --check` | rc=0 | | `cargo test --workspace` | rc=0 | | `flake8 nix/check-minecraft-literals.py` | rc=0 | Because #1007 made CI enforce the flake checks by name, `minecraft-literals` runs in CI the day this merges with no edit to `nix/ci/flake-gate.nix`. ## What I did not do **327 names are still raw.** Named above, bounded by the baseline, and left deliberately rather than overlooked. **The dynamic registries still have no enum.** 430 of the 765 hand-written literals are the one file that lists them, and generating them properly means teaching `nix/generate-registry-data.py` to emit enums the way `generate-rust.py` now does. That would delete the largest `ALLOWED` entry and is the obvious next step; it is not this PR. Filed as ENG-10887. **The worst remaining fallback is filed rather than fixed.** ENG-10884 covers `ItemKind::from_str(bare).unwrap_or(ItemKind::Stick)`, which is in another agent's file this week. **`nix flake check` was not run whole.** The three checks this change touches were built individually and `flake-gate` was evaluated, but the full check set compiles a server and I did not wait on it. ## Also in here: a stale reference in my own generator `nix/generate-rust.py` explained the `particle_type` carve-out by saying the particle enum is owned separately "the same way `nix/generate-entity-types.py` owns entity types". #1024 -- mine, two commits earlier -- deleted that script. So the comment justified a real decision by appealing to a precedent that no longer existed, which leaves the next reader either hunting a missing file or concluding the carve-out was arbitrary. It also named `nix/generate-particles.py` and `src/particle.rs` as though they were present. They are not on `main` yet. The reason for the carve-out is a fact about the data and is true today; the ownership is an arrangement that lands separately. The comment now says the first without asserting the second, and the reciprocal note in the generated `registry/particle_type.rs` changed with it, so the two cannot drift. Worth naming the general shape, because a gate in this very PR nearly caught it and did not. `tool-paths` only matches paths at the start of a string literal; this one was inside backticks in the middle of a sentence. A comment can name a file that does not exist and nothing anywhere objects. I have not widened the pattern -- matching every path-shaped substring in every comment would flag prose constantly -- so this stays a thing a reader catches, and one did. ## A note on what a green local clippy is worth `cargo clippy --workspace --all-targets` reported this tree clean three times while CI was failing, for two reasons that will recur. It does not pass `-D warnings`, so an unused import is a warning locally and an error in CI. And cargo caches per target, so a test target nobody touched is not re-checked and a warning that arrived with somebody else's merge stays invisible. `touch`ing the file made it appear at once. CI's invocation is `cargo clippy --all-targets --all-features -- -D warnings` (flake.nix:185). Anything less is not the same check, and this PR is verified with that one. Filed as ENG-10894 along with the red it exposed, which #1029 has since fixed. --- AI-authored (Claude) under human direction.
andrewgazelka
added a commit
that referenced
this pull request
Jul 28, 2026
…place them (#1030) Effects primitives for Super Smash Mobs: the engine layer an ability composes out of. No `events/smash/` changes — the seam (`Server::particles`, deleting `Cue`) follows as a second, small PR so the abilities agent rebases once and deliberately. ## The particle table is generated, and checked against Mojang's encoder `minecraft:particle_type` is the one registry the general registry codegen (#1021) deliberately skips, and the exclusion note there names this script. It cannot emit this one: `ParticleTypes.STREAM_CODEC` is a dispatch, so `nix/extract-protocol.py` marks it `unresolved` and `protocol.json` carries only the 125 names — not which entries write a body, nor what shape. `nix/generate-particles.py` reads the dispatch table out of the decompiled Java, where every `register` call names its option class beside the particle, and reads each option class's own `StreamCodec` to learn the body. It emits two enums following #1021's conventions exactly — `#[repr(u16)]`, discriminant is the network id, `const fn id`, `name()` off a static table, `from_id`/`from_name` at the decode boundary only, closed, no `Unknown`: - `ParticleKind` — the id on its own, 125 unit variants. - `Particle<'a>` — the type together with its options. Thirteen payload shapes. There is no `Particle::from_id`, and the module says why: a type id alone does not say how many bytes follow, so turning one back into a particle means reading the body. **The reading of the Java is only as good as the parse, so it is checked against the encoder it was parsed from.** `nix/java/VanillaEncoder.java` now drives Mojang's own `ParticleTypes.STREAM_CODEC` over all thirteen shapes, and `tests/play_particles.rs` compares byte for byte, both bare and inside `level_particles`: ``` particle.block -> 0101 particle.item -> 36c407030000 particle.dust -> 15ffff000040000000 particle.dust_color_transition -> 16ffff0000ff0000ff3fc00000 particle.entity_effect -> 1c80336699 particle.vibration -> 37000000004000003ffe28 particle.trail -> 383ff8000000000000c002000000000000400e000000000000ff00ff001e ``` Anything the script does not already understand is a hard failure rather than a skipped entry — a particle silently omitted would encode as another particle's id the next time the registry shifted. `nix run .#sync-minecraft-particles` regenerates; `checks.minecraft-particles` fails on drift, alongside the other six. ## `hyperion::effects` ```rust Effect::burst(particle, at) / line(from, to) / ring(c, r) / disc(c, r) / sphere(c, r) .count(n).offset(spread).speed(v).points(n).normal(n).long_distance(b) .emit(world) // one bundle, one broadcast .expanding(to, seconds).emit(world) // a ParticleEmitter entity a system ages apply_impulse(entity, delta) / set_velocity(..) / knock_back(entity, from, magnitude, lift) knockback_impulse(origin, target, magnitude, lift) -> Vec3 quantized(v) -> Vec3 // what the client will actually see players_within(world, center, radius, except) -> Vec<Hit> // nearest first nearest_player(..) -> Option<Hit>; Hit::falloff(radius) spawn(world, kind, at) -> EntityView launch(world, kind, at, velocity, shooter) -> EntityView Lifetime::new(seconds) // expires and despawns itself ``` Only particles is a flecs `Module`, because only particles have state outliving the call that started them. The emitter ages on flecs' delta time, not on any game clock, so an effect started in the hub does not freeze — same reason #1023's `Effect` did. `smash::module::knockback::strength` stays where it is and stays the only health-scaled knockback in the game; this layer takes an already-computed magnitude. ## Two bugs found while building it **Nothing outside `PacketState::Play` ever sent `RemoveEntities`.** `remove_player_from_visibility` is gated on it, so a projectile, a dropped item or a turret the server destructed stayed drawn on every client for the rest of the match. `SpawnModule` now has the observer for everything else. **`HyperionCore` never imported `SpatialModule`,** although `update_projectile_positions` (via `EgressModule`) calls `spatial::get_first_collision`, which reads the `SpatialIndex` singleton. A world that spawned an engine projectile panicked with `Component hyperion::spatial::SpatialIndex is not registered` on the next tick. bedwars imported it by hand; smash did not, so the panic was one arrow away. Found by the new spawn test, which panicked exactly that way before the import went in. ## What I did not do - **No e2e gate for the new shapes.** A gate needs something to *emit* an effect, and nothing does until the seam PR lands. The wire assertions here are byte-level against Mojang's own encoder, which is more precise about content than an e2e is, but it is not a client seeing a packet. The e2e peer belongs with the seam PR and I will add it there. - **The area query is a scan, not the BVH.** `SpatialIndex` is rebuilt empty every frame because nothing in production adds `Spatial`; ENG-10879. The fence is in `area.rs`'s module comment with the count it holds at (sixteen players, fine; a thousand, not). - **No `SoundEvent` enum.** Positional sound at an arbitrary point already works and I was not going to build a second path for it. The 1968 string sound ids remain. - `nix/generate-rust.py:288` refers to `nix/generate-entity-types.py`, which #1024 deleted. Not my file, not touched. ## Every new guard broken, and what it said Eighteen. Each mutation applied, the test run, the output recorded, the mutation reverted; full suite green again afterwards. | # | Mutation | Failure | |---|---|---| | 1 | dust colour as a `VarInt`, not a fixed int | `dust (minecraft:dust) does not encode the way the jar does` / left `[21, 128, 128, 252, 255, 15, ...]` right `[21, 255, 255, 0, 0, ...]` | | 2 | `Dust = 200`, an id drifted from the registry | `assertion left == right failed: dust` / left `200` right `21` | | 3 | `BlockStateId::from_raw` accepts anything | `assertion failed: BlockStateId::from_raw(-1).is_err()` | | 4 | ring basis seeded from a fixed axis | `out of the plane by 3 with normal Vec3(1.0, 0.0, 0.0)` | | 5 | sphere bunched toward one pole | `64 of 64 above the equator` | | 6 | disc drawn as a ring | `0 of 64 in the inner half` | | 7 | area query forgets to exclude the caster | left `[Entity(906), Entity(907)]` right `[Entity(907)]` | | 8 | area query stops sorting by distance | left `[Entity(910), Entity(909), Entity(908)]` right `[Entity(909), Entity(910), Entity(908)]` | | 9 | knockback steered by vertical separation | `Vec3(0.3577709, 1.1155418, 0.0)` | | 10 | `quantized` reports what was asked, not what was sent | `Vec3(0.8, 0.4, 0.0) did not survive the wire` / left `Vec3(0.7999756, 0.3999878, 0.0)` right `Vec3(0.8, 0.4, 0.0)` | | 11 | `long_distance` dropped before the packet | `long_distance sets it` | | 12 | `offset` dropped before the packet | left `(0.0, 0.25, 0.125)` right `(0.5, 0.25, 0.125)` | | 13 | committed particle table hand-edited | `committed particle table is stale; run: nix run .#sync-minecraft-particles` plus the diff | | 14 | generator meets a codec it has never seen | `GeyserBaseParticleOptions: 'ByteBufCodecs.FLOAT' is a codec this script has never seen. Read its Java and add it to PRIMITIVES, COMPOSITES or ID_MAPPERS; guessing at a body shifts every byte after the particle.` | | 15 | Mojang renames the `register` helper | `125 registry entries have no register() call in ParticleTypes.java, starting with ['minecraft:angry_villager', ...]; the regex no longer matches every declaration` | | 16 | `launch` forgets the owner | `an arrow with no owner collides with the bow that fired it` / left `None` right `Some(Entity(919))` | | 17 | a lifetime never expires | `outlived its lifetime` | | 18 | an entity destructed the tick it spawns | `expired before its time` | **Two of these were wrong the first time, and only running them showed it.** #2 and #10 as first written failed to *compile* rather than failing the assertion, which proves nothing about the guard; both were rewritten as mutations that build. And #16 first "passed" — the mutation had matched the same text inside a doc comment above the code, so the mutation harness (`replace(..., 1)`) had silently edited a comment and left the real line alone. Re-anchored on a unique two-line span, it fails as above. A mutation that does not compile and a mutation that does not apply look identical to a green test run. ## Checks - `nix run .#test` — 714 passed, 1 skipped - `nix run .#lint` — clean for everything in this diff - `nix run .#fmt` — clean - `checks.minecraft-particles`, `checks.minecraft-encoder-fixtures`, `checks.minecraft-proto-json` — green **`origin/main` at `a25c3a9` is already red on lint**, from #1023: `events/smash/tests/abilities.rs:595` has an unused `use flecs_ecs::prelude::*;`. Verified by stashing this branch's changes and linting that target at `a25c3a9`. Not touched here — the `bow` worktree already has the one-line deletion staged. ## Also filed - ENG-10879 — the spatial BVH is rebuilt empty every tick; nothing in production adds `Spatial`. - ENG-10893 — `git stash` is shared across worktrees, so one agent's `pop` takes another agent's stash. Nearly lost a colleague's work-in-progress to it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>
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>
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.
feat(proto): every vanilla registry is a closed Rust enum
What a registry id costs now
Nothing. Not "almost nothing" -- the compiler emits no instructions for it.
tools/registry-enum-asm.shcompiles four probes at--releaseand printswhat they became:
The first two are the ones worth reading.
sound.id()on a value theoptimiser cannot see through is a bare
ret: the discriminant is the networkid, so the number is already in the return register and there is no conversion
to perform.
probe_named_sound_idis the constant.probe_match_soundis fivebranchless instructions with no jump table and no arm for a case that cannot
occur.
That is evidence I produced and can be reproduced by anyone with the script.
The enforced version of the same claim is a
constassertion at the bottomof all 94 generated registry files:
A
constcontext cannot call anything the compiler is unable to fold. Ifid()ever stops being a cast off the discriminant, that line stops building.Both forms are here because they prove different halves: the
constcovers thecompile-time-known variant on every build, the assembly covers the runtime
value once.
The shape
crates/hyperion-minecraft-proto/src/generated/registry/is 95 files: one perregistry, plus a
mod.rs. 94 of them carry an enum with one variant per entryand 6854 variants between them.
Four rules, and each one is a decision rather than a default.
Closed. No
Unknown(u32), no#[non_exhaustive]. A value inside theserver has already been proven to be in the set, and an open enum would make
every consumer write an arm for a case that cannot occur.
TryFromat the decode boundary only.from_idis the one door for anumber that might be garbage, and it is the only fallible way in. Everything
else holds a value that is already proven, so there is nothing to check.
Encoding is infallible.
id()isconst fn, returnsRegistryId, andcannot fail.
Lookups are a table or a cast, never a
HashMapand never a match cascade.id()is the cast.name()is one load from a static indexed by thediscriminant.
from_name()is a binary search over a permutation the generatorsorted, and is for names that are genuinely only known at run time -- one read
out of a world file, or typed into a command.
The repr is the narrowest that holds the registry:
Fluidis one byte,SoundEventis two.Option<SoundEvent>is also two, because a closed enumwith room left in its discriminant gives
Optionits niche for free, so thedecode boundary costs no extra byte either.
Nothing is curated, with one exception that is written down where it applies
Every registry the extractor reports gets an enum whether or not anything uses
it today. A hand-picked subset is a gap that surfaces the day somebody needs
the registry nobody picked, and the compile cost that would justify curating is
not real: the five biggest registries together build in 0.71s.
The exception is
minecraft:particle_type, and it is not curation -- it is oneregistry whose generator needs a different input.
nix/extract-protocol.pygives up on
ParticleTypes#STREAM_CODEC("dispatched codec: the layout dependson a runtime type"), so
protocol.jsonhas the 125 names and nothing abouttheir bodies, and 13 of the 125 have one:
blockcarries a block state,dusta colour and a scale,
vibrationa destination and a flight time. An enumgenerated from this file would be id-only and structurally unable to say that,
while being the more discoverable of the two spellings.
nix/generate-particles.pyreads the decompiled codecs instead and ownscrate::particle.The reason is written at three places so it cannot expire into an agreement
nobody remembers:
NO_ENUMinnix/generate-rust.py, the module doc of thegenerated
registry/particle_type.rs(which keeps its name table, because thatis what the other generator pins its ids against), and
NO_ENUMintests/registry_enum.rs, which fails if the set of carve-outs ever changeswithout somebody editing all three.
Every guard here was watched failing
Not "added a check". Broken, run, output read, restored.
The
constassertion, given anid()that is not the discriminant.Changed
RegistryId(self as i32)to+ 1in one file:The compile-fail doctest, given a variant that exists. Replaced the
invented
SoundEvent::EntityArrowHitDeluxewith the realEntityArrowHit:The sorted-table assumption, given a table that is not sorted. The one the
brief warned about: a wrong order fails as a lookup miss and reads as "that
entry does not exist". Swapped two entries of
Fluid's by-name permutation:The order is not assumed anywhere. The generator sorts it, and the test puts
all 6854 names back through
from_namerather than spot-checking, so a wrongpermutation is caught by name.
The carve-out list, emptied.
The generator's own collision refusal. Made
variant()drop digits, whichis what a careless normalisation would do:
This is the check the brief asked about specifically. Rust identifiers are
case-sensitive, so a collision is an exact string collision and comparing the
converted strings is the whole test; it runs in the generator rather than in a
test because a colliding identifier does not compile and the useful failure is
the one naming which two entries did it. On the real data it finds none: 6979
entry names across 95 registries all convert to distinct identifiers.
What this does not do
No call site is migrated. Deliberate, and sequenced with four other agents
working in this repo tonight.
entity_type::EntityTypeis still thehand-written struct -- a
&'static strplus ani32, 16 bytes -- andgenerated::registry::EntityTypeis the one-byte enum beside it. That is oneconcept with two implementations and it is the next PR, not this one: it
renames
EntityType::PIGtoEntityType::Pigacross 121 call sites, which isthe change most likely to conflict with somebody else's open branch.
The 26 runtime validation sites are all still there. Counted, not
estimated, and the list is the baseline the next two PRs move:
Some of those are legitimate -- the anvil world loader really is resolving a
name it read out of a file -- and the ones that are not include
events/smash/src/adapter.rs:800,ItemKind::from_str(bare).unwrap_or(ItemKind::Stick),which silently hands back a stick for a name nobody typed correctly. Saying
"now zero-cost" while that line exists would be marketing. What this PR claims
is narrower and is proved above: a registry value, once you have one, costs
nothing to name, to match on, or to put on the wire.
minecraft:data_component_typenow has three enums over it, and this PRadded the third.
generated::data_component::DataComponentanditem::component_type::ComponentTypewere already a duplicate pair on main;generating the registry enum uniformly makes it three rather than carving out a
second exception. The alternative was a second carve-out for a reason that is
about this crate's history rather than about the data, which is exactly the
kind of exception that accumulates. Filed as ENG-10872 to collapse them.
Two things I did not check. Whether the 95 enums slow a cold workspace
build measurably -- I measured the five biggest in isolation (0.71s) and the
crate's own build, not the workspace's wall clock. And whether rustdoc on 6854
variants is tolerable;
cargo docwas never run.Gates
nix build .#checks.aarch64-darwin.minecraft-proto-generatedcargo clippy --workspace --all-targetscargo fmt --all --checkcargo test -p hyperion-minecraft-proto --test registry_enumcargo test -p hyperion-minecraft-proto --docflake8 nix/generate-rust.pyThe nix check is the load-bearing one: it reruns the whole extraction from the
jar and diffs the result against the committed tree, so the 97 files here are
what the pipeline produces rather than what I happened to write.
AI-authored (Claude) under human direction.