From c50db9bfc0057da53b545d1902790b8f193dfadd Mon Sep 17 00:00:00 2001 From: Sam Sung <51686767+samwhosung@users.noreply.github.com> Date: Fri, 16 Oct 2026 20:03:00 +0700 Subject: [PATCH] A player-clippy gate lints the shipped player build (#1471) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No gate built the shipped player graph outside tests. The player-tests gate compiles with `cfg(test)`, and clippy ran only on the dev feature set, so the player build's own warnings landed unseen: four unused re-exports that only dev probes used. A new `player-clippy` gate runs `cargo clippy -p benilla -p benilla-app --no-default-features -- -D warnings` in `scripts/gates.sh`, in CI, and in `scripts/check.sh` when it reaches those crates. `docs/METHOD.md` names it. The four re-exports are deleted: `ground_derived`, `GroundClamped`, `BodyPartsDesc` and `ATTACH_SLOT_NAMES`. Their dev users import them from the defining modules, with no `allow` left. Test: an unused import added to `net.rs` fails the new gate. Fixes #1465 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 2 ++ .../benilla-app/src/capture/probes/dress_census.rs | 5 +++-- .../benilla-app/src/capture/probes/ground_census.rs | 6 ++---- .../benilla-app/src/capture/probes/visual_census.rs | 3 ++- crates/benilla-app/src/debug_panel/inspect.rs | 4 ++-- crates/benilla-app/src/entities.rs | 9 ++++----- crates/benilla-app/src/net.rs | 10 +++------- crates/benilla-app/src/net/motion.rs | 6 +++--- crates/benilla-app/src/weapon_trail/tests.rs | 3 ++- docs/METHOD.md | 2 +- scripts/check.sh | 4 ++++ scripts/gates.sh | 12 +++++++++--- 12 files changed, 37 insertions(+), 29 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 69c1c9ccc..462330389 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,5 +60,7 @@ jobs: run: scripts/sync-reference.py benilla-re --check - name: pass-span-lint run: scripts/pass-span-lint.py + - name: player-clippy + run: cargo clippy -p benilla -p benilla-app --no-default-features -- -D warnings - name: player-tests run: cargo test -p benilla-formats -p benilla-app --no-default-features --lib diff --git a/crates/benilla-app/src/capture/probes/dress_census.rs b/crates/benilla-app/src/capture/probes/dress_census.rs index 5336770ad..f8047a520 100644 --- a/crates/benilla-app/src/capture/probes/dress_census.rs +++ b/crates/benilla-app/src/capture/probes/dress_census.rs @@ -14,7 +14,8 @@ use benilla_protocol::EntityKind; use bevy::prelude::*; use super::ProbeClock; -use crate::entities::{Equipment, HeldAttached, ATTACH_SLOT_NAMES}; +use crate::entities::equipment::ATTACH_SLOT_NAMES; +use crate::entities::{Equipment, HeldAttached}; use crate::names::NameCache; use crate::net::{Guid, NetEntity, ObjectStore, SelfPlayer}; @@ -61,7 +62,7 @@ fn fire_dress_census( names: Res, body: Query<(Entity, &Transform), With>, // The self body's parts, one `DRESS_PART` line each. - body_parts: crate::entities::BodyPartsDesc, + body_parts: crate::entities::attach::BodyPartsDesc, entities: Query, // `parts=`/`mats=`: mesh parts under the unit and the distinct materials they bind. children: Query<&Children>, diff --git a/crates/benilla-app/src/capture/probes/ground_census.rs b/crates/benilla-app/src/capture/probes/ground_census.rs index 93f3834d6..98c1b0b0e 100644 --- a/crates/benilla-app/src/capture/probes/ground_census.rs +++ b/crates/benilla-app/src/capture/probes/ground_census.rs @@ -16,11 +16,9 @@ use bevy::prelude::*; use super::ProbeClock; use crate::names::NameCache; +use crate::net::motion::spline::{ground_derived, GroundClamped}; use crate::net::NetCommands; -use crate::net::{ - ground_derived, CreatureSwimming, Embodied, GroundClamped, Guid, NetEntity, RemoteMotion, - SelfPlayer, Spline, -}; +use crate::net::{CreatureSwimming, Embodied, Guid, NetEntity, RemoteMotion, SelfPlayer, Spline}; /// A drop past this (yd) counts as sunk: clear of the clamp's small corrections, under a storey. const SUNK_YD: f32 = 0.5; diff --git a/crates/benilla-app/src/capture/probes/visual_census.rs b/crates/benilla-app/src/capture/probes/visual_census.rs index be1de7f6c..ea1c66347 100644 --- a/crates/benilla-app/src/capture/probes/visual_census.rs +++ b/crates/benilla-app/src/capture/probes/visual_census.rs @@ -18,7 +18,8 @@ use benilla_protocol::EntityKind; use bevy::prelude::*; use super::ProbeClock; -use crate::entities::{FallbackCube, HeldAttached, VisualAttached, ATTACH_SLOT_NAMES}; +use crate::entities::equipment::ATTACH_SLOT_NAMES; +use crate::entities::{FallbackCube, HeldAttached, VisualAttached}; use crate::names::NameCache; use crate::net::{Guid, NetEntity, SelfPlayer}; use crate::target::hover::pick_model_roots; diff --git a/crates/benilla-app/src/debug_panel/inspect.rs b/crates/benilla-app/src/debug_panel/inspect.rs index b954cb132..d76d05ba5 100644 --- a/crates/benilla-app/src/debug_panel/inspect.rs +++ b/crates/benilla-app/src/debug_panel/inspect.rs @@ -69,7 +69,7 @@ type MotionReadout = ( Option<&'static crate::net::RemoteMotion>, Option<&'static crate::net::UnitMoveModes>, // The ground clamp's memo, for the card's `ground` line. - Option<&'static crate::net::GroundClamped>, + Option<&'static crate::net::motion::spline::GroundClamped>, // Where the unit stands now, which the `ground` line reads the terrain against. &'static Transform, ); @@ -533,7 +533,7 @@ pub(super) fn inspect_ui( None => moving, } }); - // The ground clamp's memo ([`crate::net::GroundClamped`]): `z` is drawn, `seat` the server's + // The ground clamp's memo ([`crate::net::motion::spline::GroundClamped`]): `z` is drawn, `seat` the server's // last pose, `drop` = `seat - z` the clamp's correction, `terrain` the MCNK height. `walk` is // the swept step on a server path and `idle` the settle from the seat; an idle miss stays at // the seat, a walk miss descends. diff --git a/crates/benilla-app/src/entities.rs b/crates/benilla-app/src/entities.rs index 40e1786e9..82cabeac4 100644 --- a/crates/benilla-app/src/entities.rs +++ b/crates/benilla-app/src/entities.rs @@ -25,7 +25,7 @@ use display::{ }; /// Attaching a visual to each net entity: skeleton, animation, character geosets and skin, fade. -mod attach; +pub(crate) mod attach; use attach::{attach_entity_visuals, build_dressup_preview, build_glue_pet, build_glue_preview}; /// Composited body skins by look, built off the main thread. @@ -37,7 +37,7 @@ mod carried_light; use carried_light::spawn_carried_lights; /// Equipment from the descriptor and `ItemDisplayInfo`: armour, held items, helm and shoulders. -mod equipment; +pub(crate) mod equipment; use equipment::{attach_held_items, resolve_corpse_equipment, resolve_equipment}; /// Item and enchant glows, hung on the item's own attachment points. @@ -86,11 +86,10 @@ mod chain_beam; pub(crate) use chain_beam::ChainHops; use chain_beam::{simulate_chain_beams, spawn_chain_beams}; // `equip_slot` is the one InventoryType → slot table; the dressing room places items by it. -pub(crate) use attach::{equip_slot, BodyPartsDesc}; +pub(crate) use attach::equip_slot; pub(crate) use equipment::ItemDisplays; +pub(crate) use equipment::{attach_id, DressKey, HeldAttached}; pub(crate) use equipment::{BoneAttach, Equipment}; -// For the `WOW_DRESS_CENSUS` instrument: what a body wears against what it resolved. -pub(crate) use equipment::{attach_id, DressKey, HeldAttached, ATTACH_SLOT_NAMES}; // Only the tests that build a `DressKey` name this type. #[cfg(test)] pub(crate) use equipment::ItemModelKind; diff --git a/crates/benilla-app/src/net.rs b/crates/benilla-app/src/net.rs index e8f0f4207..ed7d99aff 100644 --- a/crates/benilla-app/src/net.rs +++ b/crates/benilla-app/src/net.rs @@ -22,7 +22,7 @@ use benilla_world::schedule::WorldStage; mod apply; pub(crate) mod handlers; pub(crate) mod io; -mod motion; +pub(crate) mod motion; mod objects; pub(crate) use objects::tear_down; mod session; @@ -41,13 +41,9 @@ use motion::{ // `pub(crate)`: `creature_anim` reads the shuffle latch it produces, and a test runs both. pub(crate) use motion::drive_display_facing; pub(crate) use motion::{ - ground_derived, grounded_y, jump_seed, CreatureSwimming, FacingStep, RemoteMotion, Spline, - SplineStopped, UnitMoveModes, + grounded_y, jump_seed, CreatureSwimming, FacingStep, RemoteMotion, Spline, SplineStopped, + UnitMoveModes, }; -// Only the ground-census probe reads this, so a build without the instruments leaves it unused. -// Not a `cfg`: seam knowledge lives in three fixed places, and this file is not one of them. -#[allow(unused_imports)] -pub(crate) use motion::GroundClamped; /// The net subsystem: spawns the background IO threads and drives the per-frame event drain. pub(crate) struct NetPlugin { diff --git a/crates/benilla-app/src/net/motion.rs b/crates/benilla-app/src/net/motion.rs index 6416f3bdf..8b4b78acb 100644 --- a/crates/benilla-app/src/net/motion.rs +++ b/crates/benilla-app/src/net/motion.rs @@ -11,7 +11,7 @@ mod facing; mod modes; mod relay; mod remote; -mod spline; +pub(crate) mod spline; #[cfg(test)] mod tests; @@ -26,12 +26,12 @@ pub(crate) use remote::jump_seed; pub(crate) use remote::RemoteMotion; pub(in crate::net) use remote::{apply_move, arrival_snap, trace_relay, RelayOutcome}; pub(super) use remote::{drain_pending_moves, extrapolate_remote_units}; +pub(crate) use spline::grounded_y; pub(super) use spline::{ create_spline, ground_clamp_creatures, mark_swimming_creatures, monster_move_spline, sample_splines, trace_create_spline, trace_move_snap, }; -pub(crate) use spline::{ground_derived, grounded_y}; -pub(crate) use spline::{CreatureSwimming, GroundClamped, Spline, SplineStopped}; +pub(crate) use spline::{CreatureSwimming, Spline, SplineStopped}; /// The Bevy rotation of a wire yaw (radians about WoW +Z): a Bevy +Y yaw with no 180° offset, /// unlike doodad and WMO placement. diff --git a/crates/benilla-app/src/weapon_trail/tests.rs b/crates/benilla-app/src/weapon_trail/tests.rs index b9cea1521..e5064d976 100644 --- a/crates/benilla-app/src/weapon_trail/tests.rs +++ b/crates/benilla-app/src/weapon_trail/tests.rs @@ -234,7 +234,8 @@ fn only_whirlwind_outruns_the_ring() { // The latch: armed by the kit, consumed by the unit's next animation, one slot per unit. use crate::creature_anim::AnimDriver; -use crate::entities::{HeldAttached, ATTACH_SLOT_NAMES}; +use crate::entities::equipment::ATTACH_SLOT_NAMES; +use crate::entities::HeldAttached; /// A unit holding a trail-capable weapon in each hand, plus the app that fires them. fn latch_app() -> (App, Entity, [Entity; 2]) { diff --git a/docs/METHOD.md b/docs/METHOD.md index 63ae8ff9c..65e5f57d8 100644 --- a/docs/METHOD.md +++ b/docs/METHOD.md @@ -78,7 +78,7 @@ crate of its own or a fork, and benilla opens a piece of itself to such a crate - **The land gates**: `cargo fmt --all -- --check`, `cargo clippy --workspace --all-targets -- -D warnings`, `cargo test --workspace`, the doc-link lint (a full `cargo doc` of the workspace) and the render-pass lint, the vendored-reference and message-catalog checks, the player - build without the `dev` feature with its own tests, and the engine boot checks, as + build without the `dev` feature, linted and with its own tests, and the engine boot checks, as `scripts/gates.sh`. It runs on the tree that lands, before it lands; it is memoized on the tree. The tests that read the install or the addon corpus skip where the data is absent, and the gate prints the count; where the data is, a skip is a failure (`BENILLA_REQUIRE_DATA=1`). diff --git a/scripts/check.sh b/scripts/check.sh index 5292037f6..49151d7d0 100755 --- a/scripts/check.sh +++ b/scripts/check.sh @@ -112,6 +112,10 @@ run() { run fmt cargo fmt --all -- --check run clippy cargo clippy $pflags --all-targets -- -D warnings +# The player build (no `dev` feature) is linted on its own: the run above sees only the dev set. +case " $pkgs " in *" benilla-app "* | *" benilla "*) + run player-clippy cargo clippy -p benilla -p benilla-app --no-default-features -- -D warnings ;; +esac # --no-fail-fast: one red test binary does not hide the failures in the ones after it. run test env BENILLA_SKIP_LOG="$skips" cargo test --no-fail-fast $pflags # Data-gated tests skip without the install or the addon corpus and libtest swallows their line, diff --git a/scripts/gates.sh b/scripts/gates.sh index 12b6f3dc5..58645001d 100755 --- a/scripts/gates.sh +++ b/scripts/gates.sh @@ -2,9 +2,9 @@ # The land gate chain: every gate in order, stopping at the first failure. # # fmt, clippy, reference, test, test-no-install, vm-parity, doc-links, message-catalog, -# pass-span-lint, player-tests, enforcer, enforcer-no-install. A failure prints the gate's log tail -# and exits nonzero; a green run stamps the tree, which then re-greens without running. -# GATES_FORCE=1 runs the chain anyway, +# pass-span-lint, player-clippy, player-tests, enforcer, enforcer-no-install. A failure prints the +# gate's log tail and exits nonzero; a green run stamps the tree, which then re-greens without +# running. GATES_FORCE=1 runs the chain anyway, # WOW_DATA picks the install (set and empty: none) and TMPDIR holds the scratch files; the script # sets BENILLA_REQUIRE_DATA, BENILLA_ADDON_CORPUS, BENILLA_SKIP_LOG and WOW_WORLDVIEW_CHECK for # its gates. BENILLA_RE names the benilla-re checkout the reference gate compares `reference/` with. @@ -170,6 +170,12 @@ run message-catalog scripts/gen-message-catalog.py --check # wgpu validation error on Vulkan and a silent no-op on Metal and DX12, which no macOS run sees. run pass-span-lint scripts/pass-span-lint.py +# player-clippy: the player build's own lints, which no other gate sees. `clippy` runs the dev +# feature set and player-tests compiles under `cfg(test)`, where test code uses imports and items +# the shipped library does not; this lints the shipped library and the launcher, with no +# `--all-targets`, so a warning confined to the player graph fails here. +run player-clippy cargo clippy -p benilla -p benilla-app --no-default-features -- -D warnings + # player-tests: the player build, `benilla-app` without `dev`, which compiles out the debug panel, # perf HUD, inspector, capture harness and probes, so it fails when any other code names one of # them. Its test build compiles every line of the player graph but the `benilla` launcher, a shim