Skip to content

fix(#1045): de-duplicate GRAVITY and the 128.0 fall terminal; remove afloat-stall disclosure - #1130

Merged
djhenry merged 3 commits into
mainfrom
worktree-fix-1045
Sep 17, 2026
Merged

djhenry merged 3 commits into
mainfrom
worktree-fix-1045

Conversation

@djhenry

@djhenry djhenry commented Sep 16, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes #1045. physics.rs's header claimed to be the single source of truth for physics constants, but that was falsified by its own file:

  • fall_damage() locally shadowed GRAVITY with its own const GRAVITY: f32 = 120.0; instead of reading the module's real one.
  • The controller's fall-speed clamp (movement's private MAX_FALL = 128.0) and fall_damage's local TERMINAL = 128.0 shared both a name-shape and a numeric value despite being different quantities — a velocity clamp on the controller vs. a damage-curve clamp on a derived impact velocity — a same-name-different-quantity trap for anyone sweeping by name later.

This applies the issue's own preferred fix (option 1):

  • Promoted the controller's terminal fall speed into physics.rs as FALL_TERMINAL_VELOCITY, with a doc comment explicitly warning it must not be confused with fall_damage's clamp.
  • movement now imports FALL_TERMINAL_VELOCITY/GRAVITY from eqoxide_core::physics instead of defining its own MAX_FALL.
  • Deleted fall_damage's local GRAVITY shadow entirely — it now uses the real module constant.
  • Renamed fall_damage's local TERMINAL to IMPACT_VELOCITY_CAP, a name that can't be mistaken for the controller's terminal.
  • Left eqoxide-nav::collision's unrelated MAX_FALL = 120.0 (a fall-search distance cap) untouched — the issue explicitly warns against conflating it with the other two, and it's a genuinely different constant used for a different purpose.

As a side effect, MAX_FALL as a name now exists exactly once in the whole repository (collision.rs's unrelated one), which resolves the issue's flagged "trap for anyone sweeping by name" concern.

Also: removed the afloat-stall disclosure (#776/#801) entirely

Bundled into this branch on review: the afloat_stall signal — "body afloat in water, driven to swim somewhere, making no net progress for 3.0s" — was purely observational (a throttled log line plus a player.afloat_stall field on GET /v1/observe/debug), with no engine/physics behavior depending on it. Unattended-AI-dev feature bloat, and splitting an affordance like this out per-medium (water only, nothing analogous on land) doesn't make sense as a permanent feature. Removed entirely:

  • AfloatStall/AfloatStallClock/AfloatFrame and crates/eqoxide-core/src/afloat.rs outright.
  • ControllerView's disclosures()/publish_disclosures()/invalidate_disclosures() API, collapsed to a single pub hold field with invalidate_hold().
  • CharacterController::afloat_stall()/disclosures(), leaving just hold().
  • All downstream call sites in GameState, the HTTP observe handler, the IPC/net publish paths, and app.rs's two render-frame publish sites.
  • The dedicated test sections in eqoxide-core, eqoxide-ipc, eqoxide-http, eqoxide-net, and movement.rs, and the documented afloat_stall field from docs/http-api.md.

Test plan

🤖 Generated with Claude Code

https://claude.ai/code/session_01CWrs7cDd1NAJmaLscBjgcj

physics.rs's header claimed single-source-of-truth for physics constants,
but fall_damage() locally shadowed GRAVITY with its own value, and the
controller's fall-speed clamp (movement's private MAX_FALL = 128.0) shared
both name-shape and numeric value with fall_damage's TERMINAL = 128.0
despite being different quantities (a velocity clamp vs. a damage-curve
clamp) — a same-name-different-quantity trap for anyone sweeping by name.

Promote the controller's terminal into physics.rs as FALL_TERMINAL_VELOCITY
(the movement module now imports it instead of defining its own MAX_FALL),
delete fall_damage's local GRAVITY shadow, and rename its local damage-curve
clamp from TERMINAL to IMPACT_VELOCITY_CAP so it can't be mistaken for the
controller's terminal. eqoxide-nav::collision's unrelated MAX_FALL (a
fall-search distance cap) is untouched, per the issue's own caution against
conflating it with the other two.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CWrs7cDd1NAJmaLscBjgcj
The signal — "body afloat in water, driven to swim somewhere, making no
net progress for 3.0s" — was purely observational: a throttled log line
plus a player.afloat_stall field on GET /v1/observe/debug. No engine or
physics behavior ever depended on it, and splitting an "unattended AI
dev" affordance out per-medium (water only, nothing analogous on land)
doesn't make sense as a permanent feature.

Deletes AfloatStall/AfloatStallClock/AfloatFrame and crates/eqoxide-core/
src/afloat.rs outright, and follows every downstream site the compiler
surfaced: ControllerView's disclosures()/publish_disclosures()/
invalidate_disclosures() API collapses to a single `pub hold` field with
invalidate_hold(); CharacterController drops afloat_stall()/disclosures()
down to hold() alone; GameState, the HTTP observe handler, the IPC/net
publish paths, and app.rs's two render-frame publish call sites all
follow. Removes the dedicated test sections in eqoxide-core,
eqoxide-ipc, eqoxide-http, eqoxide-net, and movement.rs, and the
documented afloat_stall field from docs/http-api.md.

cargo check/clippy --workspace --all-targets --all-features and the full
test suite are clean (0 failed).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CWrs7cDd1NAJmaLscBjgcj
@djhenry djhenry changed the title fix(#1045): de-duplicate GRAVITY and the 128.0 fall terminal fix(#1045): de-duplicate GRAVITY and the 128.0 fall terminal; remove afloat-stall disclosure Sep 16, 2026
Independent code review of b0bcd34 found four dangling references the compiler
couldn't catch, in files the afloat removal's own edits didn't touch:

- eqoxide-http/src/lib.rs: two spots still described ControllerView::hold as one
  half of a "pair of disclosures" and named the now-deleted
  ControllerView::publish_disclosures method.
- eqoxide-core/src/game_state.rs's begin_zone_in doc: "the two controller
  disclosures cleared below" / "those two lines" — only one field, one line,
  remains.
- eqoxide-net/src/gameplay.rs: a comment still said stream_position mirrors the
  view's "disclosures" (plural) into GameState; it mirrors hold alone now.
- docs/designs/2026-08-05-contact-probe-blind-band.md cited the now-deleted
  a_swimmer_hauling_out_at_a_legitimate_bank_never_raises_the_afloat_stall test
  by name, including in a "must stay green" regression list for unrelated,
  still-pending design work. Annotated both citations as retired rather than
  rewriting the historical measurement record.

cargo check --workspace --all-targets --all-features clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CWrs7cDd1NAJmaLscBjgcj
@djhenry
djhenry merged commit 0906aa1 into main Sep 17, 2026
1 check passed
@djhenry
djhenry deleted the worktree-fix-1045 branch September 17, 2026 00:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant