fix(combat): #298 — admit player targets to melee/missile attack and the camera
Selecting a PKLite player and attacking did nothing: with auto-target on it retargeted to the nearest monster, with auto-target off it logged "combat: attack ignored; no creature target found". Spells on the same target worked, which was the clue. Root cause: CombatTargetPolicy.IsHostileMonster:31-33 rejects any candidate carrying BfPlayer BEFORE reaching ObjectIsAttackable, so the both-PKLite pool match at SelectedObjectHealthPolicy.cs:70-71 was unreachable for players. Melee and missile targeting never supported player targets at all — the gate is named IsHostileMonster and does exactly what it says. Nobody could hit it until69ba9486made PK Lite reachable. Retail uses ONE predicate for monsters and players, with no player exclusion: ClientCombatSystem::ExecuteAttack @0x0056BB70 gates unconditionally on ObjectIsAttackable @0x0056A600 (creature type, Free-PK short-circuit on either side, then IsPlayer -> bothPK || bothPKLite, else BF_ATTACKABLE with pets excluded). acdream already ported that predicate verbatim; it was simply unreachable. The fix SPLITS the two concerns rather than relaxing the shared predicate: explicit-target admission routes through ObjectIsAttackable, while auto-target ACQUISITION keeps the monster-only gate. That is required by register row IA-19 — explicit product direction that Auto Target must never select NPCs, players or pets. IA-19 is not overridden here; its own justification promises "manual player-selection commands remain available", and that promise was never implemented, so this makes the row true. Review confirmed no path lets auto-acquisition select a player: every automatic Select is fed by a FindClosest* filtered through IsHostileMonster. Review also found a second site with the same bug, which the first pass froze in place on my instruction: retail gates combat-camera tracking on the SAME predicate as the attack. ClientCombatSystem::UpdateTargetTracking @0x0056A950 reads GetAttackTarget() then gates CameraSet::TrackTarget on ObjectIsAttackable. Ours used the monster-only gate, so with ViewCombatTarget on by default the attack would land while the camera refused to track the opponent — user-visible in exactly the duel this fix enables. GetCombatCameraTargetPoint now uses the wide predicate. IA-19 does not reach the camera: it performs no acquisition, only presentation on an already-chosen target. The first pass had added a source comment asserting IA-19 covered it; that comment and the matching text in docs/ISSUES.md are corrected, since a wrong citation is how a real divergence becomes invisible. Depends on9b1e6fc6(#297): the both-PKLite arm needs the LOCAL player's own bit to be live. Review confirmed both admission sites read ClientObjectTable on every call, so this is not inert in production. Newly reachable and now pinned: ObjectIsAttackable's pet-exclusion arm, which CombatTargetPolicy rejected before it could ever run. Follow-ups filed: #304 (SelectionInteractionController.GetSelectedOrClosestCombatTarget has no production caller — one of the two widened call sites is dead code), #305 (HeadlessGameplayOperations has the identical pre-existing bug, so the graphical/headless hosts now diverge). Gates: complete Release solution 10,904 passed / 4 skipped / 0 failed (baseline 10,900). Adversarial + retail-conformance review PASS after one FAIL round; the predicate was re-verified branch-for-branch against 0x0056A600 since it goes live here for the first time. Camera fix discrimination-verified by revert. Connected acceptance NOT run — needs a live two-client PKLite session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
9b1e6fc637
commit
bc0077a55f
11 changed files with 410 additions and 25 deletions
|
|
@ -500,19 +500,31 @@ it. Do #297 FIRST — #298 depends on it.
|
|||
`BF_ATTACKABLE` with pets excluded. We already have that predicate ported
|
||||
verbatim and correctly at `SelectedObjectHealthPolicy.cs:41-78` — it is
|
||||
simply unreachable.
|
||||
**DO NOT fix by relaxing the shared predicate.** `IsHostileMonster` also
|
||||
backs auto-target ACQUISITION (`CombatAttackTargetSource.cs:80`,
|
||||
`WorldSelectionQuery.cs:255`) and the combat camera
|
||||
(`WorldSelectionQuery.cs:264-269`), and relaxing it would violate register row
|
||||
**IA-19**, explicit product direction that auto-target must never select
|
||||
NPCs, players or pets. Retail's own auto-target DOES admit players
|
||||
(@0x0056C040 pc:377318-377327), so retail and IA-19 genuinely disagree here.
|
||||
Fix shape: SPLIT explicit-target admission (-> `ObjectIsAttackable`,
|
||||
retail-exact) from auto-acquisition (-> keep `IsHostileMonster`, IA-19
|
||||
intact). IA-19's own text already promises "manual player-selection commands
|
||||
remain available"; that promise is currently unimplemented, which is the real
|
||||
gap. Not affected: the health bar (`SelectedObjectHealthPolicy.cs:32` already
|
||||
admits `BfPlayer`) and the vivid target indicator.
|
||||
**DO NOT fix by relaxing `IsHostileMonster`'s automatic-acquisition
|
||||
scope.** `IsHostileMonster` also backs auto-target ACQUISITION
|
||||
(`CombatAttackTargetSource.cs:80`, `WorldSelectionQuery.cs:280`), and
|
||||
relaxing it would violate register row **IA-19**, explicit product
|
||||
direction that auto-target must never select NPCs, players or pets.
|
||||
Retail's own auto-target DOES admit players (@0x0056C040
|
||||
pc:377318-377327), so retail and IA-19 genuinely disagree here — for
|
||||
acquisition only.
|
||||
**The combat camera is NOT an IA-19 concern, despite an earlier draft of
|
||||
this note claiming otherwise.** Retail `ClientCombatSystem::
|
||||
UpdateTargetTracking` @0x0056A950 (pc:375691-375696) gates
|
||||
`CameraSet::TrackTarget` on the SAME `ObjectIsAttackable` predicate as
|
||||
`ExecuteAttack` @0x0056BB98, not the narrow monster-only policy. The
|
||||
camera performs no acquisition of its own — it only tracks whatever the
|
||||
player already selected — so `WorldSelectionQuery.GetCombatCameraTargetPoint`
|
||||
must route through the wide predicate exactly like explicit-target
|
||||
admission. (Landed: `GetCombatCameraTargetPoint` now calls
|
||||
`IsAttackableTarget`.)
|
||||
Fix shape: SPLIT explicit-target admission AND the combat camera
|
||||
(-> `ObjectIsAttackable`/`IsAttackableTarget`, retail-exact) from
|
||||
auto-acquisition (-> keep `IsHostileMonster`, IA-19 intact). IA-19's own
|
||||
text already promises "manual player-selection commands remain
|
||||
available"; that promise was unimplemented before this fix. Not affected:
|
||||
the health bar (`SelectedObjectHealthPolicy.cs:32` already admits
|
||||
`BfPlayer`) and the vivid target indicator.
|
||||
Correct model to copy: spells already work on PKLite players because
|
||||
`RetailSpellTargetPolicy.cs:40-46` treats `BF_PLAYER` as an ACCEPT and never
|
||||
calls `ObjectIsAttackable` — the client checks target-TYPE compatibility and
|
||||
|
|
@ -580,6 +592,41 @@ it. Do #297 FIRST — #298 depends on it.
|
|||
`LiveEntityCollisionBuilder` and therefore no live-entity target shadows), so
|
||||
this is shape rather than a defect. Filed from the #297 delta review.
|
||||
|
||||
## Follow-ups from the #298 fix and its review — 2026-08-03
|
||||
|
||||
- **#304 — OPEN — `SelectionInteractionController.GetSelectedOrClosestCombatTarget`
|
||||
has no production caller. LOW/shape.** Grep confirms only tests reach it
|
||||
(`GetSelectedOrClosestCombatTarget:114-121`); no `GameplayInputActionRouter`
|
||||
or other App wiring calls it. The #298 fix widened it correctly (explicit
|
||||
selection now checks `IWorldSelectionQuery.IsAttackableTarget` instead of
|
||||
`IsHostileMonster`), matching `CombatAttackTargetSource`'s live path
|
||||
defensively, but it currently exists only to keep the (also unused-in-
|
||||
production) `IsAttackableTarget` member exercised by four `IWorldSelectionQuery`
|
||||
fakes. Fix shape: delete the dead method (and, if nothing else calls
|
||||
`IsAttackableTarget` through this interface after that, the interface member
|
||||
and its fake stubs too) — or find the caller that was supposed to exist and
|
||||
wire it. Filed from the #298 review.
|
||||
|
||||
- **#305 — OPEN — `HeadlessGameplayOperations.GetSelectedOrClosestTarget` has
|
||||
the same player-exclusion bug #298 fixed for graphical hosts.
|
||||
MEDIUM.** `src/AcDream.Headless/Hosting/HeadlessGameplayOperations.cs:235-244`
|
||||
checks explicit selection via `RuntimeHostileTargetQuery.IsHostile`
|
||||
(`src/AcDream.Runtime/Gameplay/RuntimeHostileTargetQuery.cs:73-101`), which is
|
||||
the monster-only `CombatTargetPolicy.IsHostileMonster` gate — structurally
|
||||
identical to the bug #298 fixed in `CombatAttackTargetSource`/
|
||||
`SelectionInteractionController`. A headless bot that explicitly selects a
|
||||
compatible-PK player and attacks will fall through to `SelectClosestTarget()`
|
||||
(since `AutoTarget` is hardcoded `true` at `:128`) instead of attacking the
|
||||
selected player. Pre-existing (not introduced by #298 — confirmed by the
|
||||
same `9966b531`/`3361a8d7`/`2644d1d5`/`0f2d98c5` diff boundary #297 used), but
|
||||
the graphical/headless behavioral *divergence* is new as of the #298 commit,
|
||||
and Slice K makes headless a first-class host, so the gap is now live for bot
|
||||
PvP. Fix shape: same split as #298 — add an `ObjectIsAttackable`-backed
|
||||
explicit-admission query to `RuntimeHostileTargetQuery` (or a sibling) and
|
||||
route `GetSelectedOrClosestTarget`'s explicit branch through it, leaving
|
||||
`FindClosest`/auto-acquisition on the narrow policy. Filed from the #298
|
||||
review.
|
||||
|
||||
## C3c placement cutover — 2026-08-02
|
||||
|
||||
- **#276 — OPEN — SpawnPlacementSettler discards the settle's resolved
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue