refactor(proto): one EntityType, and it is the generated enum - #1024
Merged
Conversation
## Two implementations of one concept, down to one
`generated::registry::EntityType` arrived in the previous PR as one of 94
registry enums. `entity_type::EntityType` was already here, generated by its
own script into its own shape. Both described `minecraft:entity_type`, both
were generated from the same `protocol.json`, and `tests/entity_type.rs`
existed solely to check they agreed -- a test whose necessity was the defect.
This deletes the older one. What goes with it:
| deleted | lines |
| --- | --- |
| `crates/hyperion-minecraft-proto/src/entity_type.rs` | 544 |
| `nix/generate-entity-types.py` | 171 |
| `crates/hyperion-minecraft-proto/tests/entity_type.rs` | 51 |
| `entityTypeCodegen`, `generatedEntityTypes`, `syncEntityTypesScript`, `entityTypesUpToDate` in `nix/minecraft-data.nix` | 47 |
| `minecraft-entity-types` check, `sync-minecraft-entity-types` app, `minecraft-entity-types-rust` package in `flake.nix` | 3 |
One generator fewer, one nix check fewer, one sync command fewer, and the
staleness of what remains is covered by `minecraft-proto-generated`, which
already diffs the whole `src/generated` tree.
## What a caller writes now
`EntityType::PIG` is `EntityType::Pig`. 121 call sites, all but two of them in
`crates/hyperion/src/simulation/entity_kind.rs`, which is the table pairing a
flecs `EntityKind` tag with the vanilla type it spawns as.
The rename was not done with a SCREAMING-to-Pascal regex. It reads the variant
list out of the generated file and maps each old constant to the variant the
generator actually emitted, so a name whose conversion is not the obvious one
fails loudly rather than silently producing an identifier that happens to
compile as something else. Every one of the 121 mapped; the script reported no
unmapped names.
The rest of the surface:
| was | is |
| --- | --- |
| `entity_type(name)` | `EntityType::from_name(name)` |
| `ENTITY_TYPES` | `EntityType::ALL` |
| `ENTITY_TYPE_COUNT` | `EntityType::COUNT` |
| `hyperion_minecraft_proto::entity_type::EntityType` | `...::generated::registry::EntityType` |
`id()` and `name()` are spelled the same and mean the same, so no call site of
either changed.
## What this costs, in bytes
The retired struct was a `&'static str` and an `i32`, which with padding is 16
bytes, and every copy of an `EntityKind`'s entity type moved a pointer. The
enum is one byte: 158 entries fit in a `u8` discriminant, and that discriminant
is the network id. `Option<EntityType>` -- which is what
`EntityKind::entity_type` returns for the four kinds this version has no type
for -- is also one byte, because the closed enum leaves 98 unused values for
the niche.
`from_name` went from a linear scan over 158 entries to a binary search, though
that is the least interesting part: the only caller is `EntityKind::named`,
which runs when a name arrives from outside.
## The pins the deleted test was carrying
`tests/entity_type.rs` did three things. Two of them -- the table matching the
registry, and every name resolving to its own entry -- are now structurally
impossible to get wrong, because there is one table rather than two, and
`tests/registry_enum.rs` already puts all 6854 names in the crate through
`from_name`.
The third was worth keeping and has moved rather than gone:
```rust
#[test]
fn the_entity_type_ids_are_the_26_2_ones() {
assert_eq!(registry::EntityType::Pig.id(), RegistryId(100));
assert_eq!(registry::EntityType::Player.id(), RegistryId(156));
}
```
Every other test in that file would pass against a table that was wrong in the
same way twice. `minecraft:entity_type` was renumbered between 1.20.1 and 26.2,
and the symptom of holding the old numbering is not an error, it is the entity
*next to* the one that was asked for appearing in the world. So is
`a_type_this_version_dropped_does_not_resolve`, which pins that
`minecraft:boat` is gone (split per wood in 1.21.2) and that `pig` without the
namespace is not a name.
## Gates
| gate | result |
| --- | --- |
| `cargo build --workspace` | rc=0 |
| `cargo clippy --workspace --all-targets` | rc=0 |
| `cargo fmt --all --check` | rc=0 |
| `cargo test --workspace` | rc=0 |
| `cargo test -p hyperion-minecraft-proto --test registry_enum` | rc=0, 13 tests |
| `nix eval .#checks.aarch64-darwin.flake-gate.drvPath` | rc=0 |
The flake-gate eval is the one that matters for the deletions: #1007 made CI
enforce the flake checks by name, so removing three attributes from `flake.nix`
would break the gate's own evaluation if any of them were still referenced.
They are not.
## What I did not do
**No literal migration.** `EntityKind::named` still takes a `&str`, and the
callers that hand it one still hand it one. That is stage 3 and the gate that
goes with it.
**`EntityKind` itself is untouched.** It is 125 flecs tags whose relationship
to `minecraft:entity_type` is a 125-arm match, and four of those arms have no
vanilla type at all (`Gui`, and three others named in the last arm). Collapsing
`EntityKind` into `EntityType` would be a real design change to the simulation
layer, not a rename, and it belongs to whoever owns that layer.
---
AI-authored (Claude) under human direction.
andrewgazelka
force-pushed
the
feat/entity-type-enum
branch
from
July 28, 2026 10:57
35afcc9 to
3ba49f4
Compare
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #1024 +/- ##
=======================================
Coverage ? 51.13%
=======================================
Files ? 335
Lines ? 30052
Branches ? 1137
=======================================
Hits ? 15368
Misses ? 14438
Partials ? 246
🚀 New features to boost your workflow:
|
Benchmark Results for generalComparing to a5db463 |
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. ## 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>
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
1 participant
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.
refactor(proto): one EntityType, and it is the generated enum
Two implementations of one concept, down to one
generated::registry::EntityTypearrived in the previous PR as one of 94registry enums.
entity_type::EntityTypewas already here, generated by itsown script into its own shape. Both described
minecraft:entity_type, bothwere generated from the same
protocol.json, andtests/entity_type.rsexisted solely to check they agreed -- a test whose necessity was the defect.
This deletes the older one. What goes with it:
crates/hyperion-minecraft-proto/src/entity_type.rsnix/generate-entity-types.pycrates/hyperion-minecraft-proto/tests/entity_type.rsentityTypeCodegen,generatedEntityTypes,syncEntityTypesScript,entityTypesUpToDateinnix/minecraft-data.nixminecraft-entity-typescheck,sync-minecraft-entity-typesapp,minecraft-entity-types-rustpackage inflake.nixOne generator fewer, one nix check fewer, one sync command fewer, and the
staleness of what remains is covered by
minecraft-proto-generated, whichalready diffs the whole
src/generatedtree.What a caller writes now
EntityType::PIGisEntityType::Pig. 121 call sites, all but two of them incrates/hyperion/src/simulation/entity_kind.rs, which is the table pairing aflecs
EntityKindtag with the vanilla type it spawns as.The rename was not done with a SCREAMING-to-Pascal regex. It reads the variant
list out of the generated file and maps each old constant to the variant the
generator actually emitted, so a name whose conversion is not the obvious one
fails loudly rather than silently producing an identifier that happens to
compile as something else. Every one of the 121 mapped; the script reported no
unmapped names.
The rest of the surface:
entity_type(name)EntityType::from_name(name)ENTITY_TYPESEntityType::ALLENTITY_TYPE_COUNTEntityType::COUNThyperion_minecraft_proto::entity_type::EntityType...::generated::registry::EntityTypeid()andname()are spelled the same and mean the same, so no call site ofeither changed.
What this costs, in bytes
The retired struct was a
&'static strand ani32, which with padding is 16bytes, and every copy of an
EntityKind's entity type moved a pointer. Theenum is one byte: 158 entries fit in a
u8discriminant, and that discriminantis the network id.
Option<EntityType>-- which is whatEntityKind::entity_typereturns for the four kinds this version has no typefor -- is also one byte, because the closed enum leaves 98 unused values for
the niche.
from_namewent from a linear scan over 158 entries to a binary search, thoughthat is the least interesting part: the only caller is
EntityKind::named,which runs when a name arrives from outside.
The pins the deleted test was carrying
tests/entity_type.rsdid three things. Two of them -- the table matching theregistry, and every name resolving to its own entry -- are now structurally
impossible to get wrong, because there is one table rather than two, and
tests/registry_enum.rsalready puts all 6854 names in the crate throughfrom_name.The third was worth keeping and has moved rather than gone:
Every other test in that file would pass against a table that was wrong in the
same way twice.
minecraft:entity_typewas renumbered between 1.20.1 and 26.2,and the symptom of holding the old numbering is not an error, it is the entity
next to the one that was asked for appearing in the world. So is
a_type_this_version_dropped_does_not_resolve, which pins thatminecraft:boatis gone (split per wood in 1.21.2) and thatpigwithout thenamespace is not a name.
Gates
cargo build --workspacecargo clippy --workspace --all-targetscargo fmt --all --checkcargo test --workspacecargo test -p hyperion-minecraft-proto --test registry_enumnix eval .#checks.aarch64-darwin.flake-gate.drvPathThe flake-gate eval is the one that matters for the deletions: #1007 made CI
enforce the flake checks by name, so removing three attributes from
flake.nixwould break the gate's own evaluation if any of them were still referenced.
They are not.
What I did not do
No literal migration.
EntityKind::namedstill takes a&str, and thecallers that hand it one still hand it one. That is stage 3 and the gate that
goes with it.
EntityKinditself is untouched. It is 125 flecs tags whose relationshipto
minecraft:entity_typeis a 125-arm match, and four of those arms have novanilla type at all (
Gui, and three others named in the last arm). CollapsingEntityKindintoEntityTypewould be a real design change to the simulationlayer, not a rename, and it belongs to whoever owns that layer.
AI-authored (Claude) under human direction.