Skip to content

fix(settings): reject hero settings members for slots the hero lacks - #438

Merged
Teakowa merged 1 commit into
mainfrom
fix/429-hero-settings-check
Oct 13, 2026
Merged

Teakowa merged 1 commit into
mainfrom
fix/429-hero-settings-check

Conversation

@e54-bot

@e54-bot e54-bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #429.

The audit's remaining live defect for this issue was a validation gap: a
hero-scoped settings member like Doctrine { enableAbility3: Off } passed
check and then failed emission with a missing setting mapping error —
or worse, risked spelling a malformed %1$s row for a hero that has no
corresponding ability slot at all.

check_emission's walk_heroes now rejects such a member when it is not
raw, has no reviewed per-hero display label, and the gameplay catalog
proves the hero lacks the ability slot. Unknown heroes keep the previous
behavior, and slots the hero does own (e.g. Doctrine ability1Cooldown%,
whose label is composed from gameplay names) still emit and round-trip.

Investigated audit findings already covered on main (PR #433):

  • wreckingBall canonical mapping — wrecking_ball_hero_settings_emit
    and wrecking_ball_zh_label_binds_secondary_fire cover the en and zh
    label bindings; no stale hammond mapping remains.
  • Mode-inherited settings — mode_inherited_setting_resolves_typed and
    game_mode_start_all_slots_filled_resolves cover typed resolution;
    self-referential suggestions were not reproducible.
  • ability1Cooldown% under rowless heroes — resolved through the
    gameplay-name template; the new check keys on missing ability slots,
    not on absent generated rows.

Test plan

  • cargo test -p workshop-rs --lib audit_regressions — 26 pass,
    including the new hero_member_without_slot_is_rejected_by_check
  • cargo test --workspace --all-targets — all suites pass
  • cargo fmt --all --check, cargo clippy --workspace --all-targets -- -D warnings, workshop-catalog-gen -- check, git diff --check

check_emission now rejects a hero-scoped settings member when no reviewed
label names it and gameplay data proves the hero has no corresponding
ability slot, instead of letting emission fail with a missing-mapping
error or spell a malformed placeholder. Unknown heroes and generated
labels keep existing behavior.

Fixes #429
@Teakowa
Teakowa merged commit 7974a03 into main Oct 13, 2026
5 checks passed
@Teakowa
Teakowa deleted the fix/429-hero-settings-check branch October 13, 2026 11:53
Teakowa added a commit that referenced this pull request Oct 15, 2026
…466)

Adds a standalone check_emit_agreement target that runs parsed sources and producer-built settings trees through parse -> check -> emit and asserts the accept/reject verdict is identical at every reachable stage.

Covered areas per #459: hero settings members (the #438 slot-guard shape and its accepted counterparts) and catalogued versus uncatalogued keyed blocks (the #413/#424 carried-block shapes), including the producer-only RawGroup pass-through marker and unwritable leaf shapes.

Sensitivity was verified in both directions before commit: dropping the hero-slot check guard makes check accept members emit still rejects, and reverting the catalogued carried-block arm makes check reject blocks emit still accepts; in both cases the new tests fail as designed.

No source changes were needed: the covered surface is currently consistent.

Fixes #459

Co-authored-by: Teakowa <git@teakowa.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Settings: check accepts input that emit mishandles for wrecking ball, mode-inherited keys, and rowless hero ability keys

2 participants