server status works - #2
Merged
Merged
Conversation
andrewgazelka
pushed a commit
that referenced
this pull request
Sep 21, 2024
andrewgazelka
added a commit
that referenced
this pull request
Jul 27, 2026
Super Smash Mobs was a library with 47 passing tests and no way to play
it.
This gives it a host half, and along the way fixes the two reasons no
hyperion
game server was joinable at all.
## Getting it running
```sh
nix run .#certs # once
nix run .#smash # game server with a proxy inside it, on localhost:25565
```
`nix run .#smash-dev` is the deployed shape — game server, separate
proxy —
under `dev`'s rebuild-and-restart watcher.
## The host half
Four new files under `events/smash/src/`, and **no file under
`src/module/`
changed**, which was the design doc's own test of whether the seam was
in the
right place.
| file | what it does |
|---|---|
| `adapter.rs` | implements the nine-method `Server` trait against
hyperion; the only file importing both |
| `mirror.rs` | hyperion position, facing, ground state and velocity
onto the game's mirrors |
| `input.rs` | packet events into ability activation, melee damage and
kit hotbars |
| `command.rs` | `/kit` and `/kits` |
| `main.rs` | clap args and the entry point |
Writes cross the seam as a queue drained once per tick in `PostUpdate`
rather
than as immediate world edits. `Server` is called from inside flecs
observers —
ability activation, the damage pipeline, the lobby's phase transitions —
where
taking a second mutable borrow of a component a running query holds is a
runtime abort rather than a compile error. The cost is one tick of
latency on
knockback.
`Cue` and `HotbarItem` are the game's closed vocabulary; the mapping
onto real
sounds, particles and items is in the adapter.
## Two bugs that made every game server unjoinable
**No client ever received the Login packet.** `player_joins` is the
system that
calls `player_join_world`, and it reads its work list off
`Comms::skins_rx`.
Nothing in the tree ever sent on `skins_tx` — both the offline path and
the
Mojang-fetch path set the `PlayerSkin` component directly. Every client
authenticated, the proxy counted it as connected, and it sat on *Joining
world…* forever. `crates/hyperion/src/ingress/mod.rs` now routes both
paths
through the channel.
**No `certs` app existed.** The game server and the proxy authenticate
over
mTLS and neither starts without a certificate, but `nix run .#dev` and
`nix run .#proxy` passed no cert flags, so both died immediately. Added
the
app; `dev` and `proxy` now pass the flags.
Also: the embedded proxy inside the game-server process hardcoded its
cert
paths to bare filenames in the working directory, so it could only ever
start
by coincidence. `ProxyAddress` carries them now; defaults are unchanged.
## Proof it works
`tools/smash-client.py` is a scripted 1.20.1 client that distinguishes
"authenticated" from "in the world" — the distinction the proxy's
connection
count cannot make. Two clients, kits, an ability and a hit:
```
[Alpha] <- LoginSuccessS2C (authenticated, NOT yet in the world)
[Alpha] <- GameJoinS2C entity_id=1411 ** REACHED PLAY STATE **
[Alpha] <- PlayerPositionLookS2C (12.0, 34.0, 0.0) teleport_id=1
[Alpha] -> CommandExecutionC2S /kit Skeleton
[Alpha] <- GameMessageS2C {"text":"Kit set to Skeleton."}
[Alpha] <- HealthUpdateS2C health=20.00
[Alpha] <- HealthUpdateS2C health=16.36
[Alpha] <- EntityVelocityUpdateS2C entity=1411 velocity=(3729, 1530, 3729)
[Bravo] <- GameJoinS2C entity_id=1412 ** REACHED PLAY STATE **
[Bravo] -> CommandExecutionC2S /kit irongolem
[Bravo] <- GameMessageS2C {"text":"Kit set to Iron Golem."}
[Bravo] -> PlayerInteractItemC2S right click #1
[Bravo] -> PlayerInteractItemC2S right click #2
[Bravo] -> PlayerInteractEntityC2S attack entity=1411
[Bravo] <- OverlayMessageS2C {"text":"That ability is recharging."}
```
Every number is the game's own. Iron Golem's melee is 7, Skeleton's 12
armour
points reduce it 48%, and 20 − 7×0.52 = **16.36** exactly. The knockback
resolves to 0.659 horizontal and 0.391 vertical blocks per tick, which
is
`0.2 + 0.48K` and `min(0.2K, 0.4 + 0.04K) + 0.2` for K = 0.957 — the
grounded
term confirming the mirror is feeding `OnGround` correctly. The second
right-click coming back *recharging* is what proves the click reached
the
ability gate and set a cooldown.
## What does not work yet
- **The match never starts.** `LobbyConfig::min_players` is 4 and `alive
<= 1`
ends a match, so everything above happens in the hub phase. Nothing is
wrong
with the lobby; four scripted clients would exercise it and I ran two.
- **No automated end-to-end test.** The proof above is a manual run of
`tools/smash-client.py`. The regression this most wants — a client
reaching
the Login packet — deserves a test, and I did not write one.
- **`nix run .#smash-dev` was not run end to end**, only its component
commands with identical flags: port 25565 on this machine is held by
another
agent's stack.
- The arena sits on top of `GenMapModule`'s downloaded world rather than
floating platforms, so the kill plane at y=0 is under solid ground.
- Still missing from the game itself, unchanged by this PR: the Smash
Crystal,
hunger drain, the double-jump impulse, and seventeen of the twenty-one
kits.
## Gates
```
$ nix run .#lint → rc=0
$ nix run .#test → 154 tests run: 154 passed, 1 skipped
$ nix run .#fmt -- --check → rc=0
```
`nix flake check` fails on `packages.*` with "Cargo.lock contains
multiple git
dependencies with the same name-version"; that reproduces on
`origin/main`
with this branch stashed, so it is not from here.
andrewgazelka
added a commit
that referenced
this pull request
Jul 28, 2026
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.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.
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
…r (ENG-10914) (#1040) ## The bug (ENG-10914): a single movement packet crashes the whole server `move_player_pos` / `move_player_pos_rot` take three client-supplied `f64`s and apply them with only a block-collision check, **no coordinate bound**. A joined client sending one `MovePlayerPos { x: 2_000_000.0, y: 65.0, z: 4.0 }` takes the process down for every connected player. `x = 2_000_000` is far *inside* the Minecraft world border (29,999,984) but its chunk key `2_000_000 >> 4 = 125_000` overflows the `i16` this server keys chunks by. That overflow reaches the chunk math two ways from the same packet: - **Debug build** (the dev server): `Blocks::get_blocks`, reached synchronously from the move handler's collision check, does `as_i16vec2()` which silently wraps, and the wrapped range trips `debug_assert!(start.x >= start_xz.x)` at `blocks/mod.rs:297`. - **Release build** (production): the asserts vanish; the position is stored and the next tick's `Position::to_chunk` does `i16::try_from(125_000).unwrap()` and panics. ## The fix Two defects, fixed at two layers: 1. **Load-bearing — bound the coordinate at the move handler.** `change_position_or_correct_client` now rejects a proposed position that is non-finite or outside the representable chunk range *before* it reaches any chunk math, and snaps the client back with a teleport correction (matching vanilla, which refuses movement past the border rather than clamping onto it). The enforced bound is the **`i16` chunk-range safety limit** (`MAX_SAFE_BLOCK` = `(i16::MAX - 1) * 16` = 524,256), *not* the world border — the crashing packet is inside the border, so the border alone would still admit it. The non-finite check comes first, because `NaN as i32` is 0 (a silent teleport to origin) and `inf as i32` saturates and then wraps. 2. **Defence in depth — make the conversions total.** `Position::to_chunk` now clamps instead of `try_from().unwrap()`, and `Blocks::get_blocks` clamps its query into the representable block window before deriving chunk coordinates, so the `as_i16vec2()` wrap can no longer turn a far query into a bogus chunk range. A coordinate-to-chunk conversion that can crash is wrong regardless of who calls it. ## Gate the class, not the instance `unwrap`/`expect`/`panic!` on client-derived values is a class of remote-crash bug. `handlers.rs` — the serverbound play-packet module, where every byte comes from a client — now carries `#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic)]`. The next such call in that module is a build failure, not a P0. **Proven it fires:** injecting `Some(1).unwrap()` into `move_player_pos` failed clippy with `used unwrap() on an Option value`, pointing at the deny. Enumeration of the class downstream of client input (see the report for detail): fixed the client-reachable sites in `pose.rs::to_chunk` and `blocks/mod.rs::get_blocks` (the `as_i16vec2` wraps). The remaining `unwrap`s in `blocks/mod.rs` (chunk-relative indices `0..=15`, block-offset `& 15`) are infallible by loop/bit invariants and left in place. One `expect` in the proto NBT decoder (`nbt/mod.rs:609,623`) is flagged for a follow-up fuzz/property gate on the decoder rather than fixed here (out of tested scope). ## Wire assertion (the regression test) New `oob-move-e2e` gate (`nix run .#oob-move-e2e`, `checks.oob-move-e2e`): a scripted client reaches play and sends the ENG-10914 packet plus the NaN and `+inf` vectors, then asserts the **server keeps ticking** (it answers a teleport correction after every hostile move, and the driver confirms the game server is still listening) — not merely a return value. **Broke the guard and watched it fail.** With the fix reverted, the first hostile packet crashed the server: ``` thread 'main' panicked at crates/hyperion/src/simulation/blocks/mod.rs:297:17: assertion failed: start.x >= start_xz.x ``` ``` [oob] -> hostile MovePlayerPos #1/3: x=2_000_000 (finite, past the i16 chunk range; ENG-10914) [oob] connection lost: server closed the connection [oob] RESULT: the server dropped the connection after a movement packet. A single malformed MovePlayerPos crashed it for every connected player (ENG-10914). the game server stopped listening during the session (RUNEXIT=1) ``` With the fix restored: ``` [oob] -> hostile MovePlayerPos #1/3: x=2_000_000 (finite, past the i16 chunk range; ENG-10914) [oob] -> hostile MovePlayerPos #2/3: x=NaN (non-finite) [oob] -> hostile MovePlayerPos #3/3: x=+inf (non-finite) [oob] RESULT: the server refused 3 out-of-bounds / non-finite MovePlayerPos packets and kept ticking (ENG-10914 fixed) (RUNEXIT=0) ``` Unit/integration coverage in `crates/hyperion/tests/oob_position.rs` and a `to_chunk` unit test pin the defence-in-depth layer. ## Note The new gate's port offset was first set to 10000, which aliases onto the base game-server port (`gameServerPort - proxyPort == 10000`); filed as ENG-10933 and worked around with offset 9500. See the comment in `flake.nix`. Closes ENG-10914. 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
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.
Uh oh!
There was an error while loading. Please reload this page.