diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 4fd57d2c..a2cc8d9e 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -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 diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index 84ec328f..39b1adc4 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -56,7 +56,7 @@ accepted-divergence entries (#96, #49, #50). | IA-15 | D.2b gameplay UI is our own `UiHost`/`UiRoot` retained tree, not a byte-port of Keystone. `RetailUiRuntime` owns the production import/mount graph; `RetailWindowManager`/typed handles centralize registry, raise, focus/capture cleanup, lifecycle events, reverse-order grouped controller teardown, removable Silk input subscriptions, schema-v2 per-character/per-resolution automatic layouts with per-window authored-geometry revisions, and portable named `saveui/loadui` profiles; `RetailWindowFrame` is the single production/Studio mount contract for imported-chrome and shared-wrapper windows. Production LayoutDesc imports include vitals `0x2100006C`, chat `0x21000006`, toolbar `0x21000016`, character `0x2100002E`, inventory `0x21000023` plus mounted `0x21000024/22/21`, dialog catalog `0x2100003C`, radar `0x21000074`, and external container `0x21000008` (shared bevel plus a user-directed compact 700-pixel initial content width instead of its authored 800-pixel root). The dialog context/queue/callback lifecycle is now a named-client port; only its retained rendering remains under this Keystone adaptation. | `src/AcDream.App/UI/RetailUiRuntime.cs`; `src/AcDream.App/UI/RetailWindowManager.cs`; `src/AcDream.App/UI/RetainedPanelControllerGroup.cs`; `src/AcDream.App/UI/UiHost.cs`; `src/AcDream.App/UI/RetailWindowLayoutPersistence.cs`; `src/AcDream.App/UI/Layout/RetailWindowFrame.cs`; `src/AcDream.App/UI/Layout/RetailDialogFactory.cs`; `src/AcDream.App/UI/Layout/RetailConfirmationDialogView.cs`; `src/AcDream.App/UI/Layout/ExternalContainerController.cs`; `src/AcDream.App/UI/Layout/LayoutImporter.cs`; binding supply in `GameWindow.cs` | Keystone has no matching PDB/decomp, so we preserve its observable ElementDesc/state/input behavior from DAT, named client call sites, and live evidence while using modern retained ownership. Real RenderSurfaces and imported element geometry remain the visual oracle; the external strip's initial width follows the connected visual direction and remains horizontally resizable to its authored extent or the viewport edge. | Persistence and low-level widget rendering are behaviorally reconstructed from retail semantics rather than a Keystone byte-port; lifecycle edge cases remain constrained by conformance tests. The external strip opens 100 pixels narrower than the raw LayoutDesc before user/persistence resizing. | Production LayoutDesc objects; `DialogFactory @ 0x004773C0..0x00478470`; `gmExternalContainerUI @ 0x004CBAD0..0x004CBFE0`; `docs/research/2026-07-13-retail-dialog-factory-pseudocode.md`; Keystone behavior notes in `docs/research/retail-ui/` | | IA-17 | Toolbar chrome is toolkit-supplied through the central `RetailWindowFrame` mount (`UiCollapsibleFrame` 8-piece bevel) because LayoutDesc `0x21000016` carries no baked frame. It also supports a toolkit-defined collapse-to-one-row (bottom-edge resize snapping between a row-1-only and a two-row height, row-2 visibility tied to the stop) — retail's real collapse is keystone.dll (no decomp) and the DAT stacks both rows always. | `src/AcDream.App/UI/Layout/RetailWindowFrame.cs`; `src/AcDream.App/UI/UiCollapsibleFrame.cs`; toolbar policy in `GameWindow.cs`; spec: `docs/superpowers/specs/2026-06-20-d2b-toolbar-collapse-design.md` | The central mount now owns wrapper geometry/registration uniformly; border-over-content prevents the row-2 right cap from poking through | The collapse stops remain a toolkit reconstruction rather than a byte-port of Keystone behavior | gmToolbarUI WM chrome (keystone.dll, no PDB); no bevel ids in LayoutDesc 0x21000016 (toolbar dump) | | IA-18 | Effect overlay tile (enum 0x10000005) is a `ReplaceColor` SURFACE SOURCE — pure-white pixels in the composited drag icon are replaced PER-PIXEL with the same (x,y) pixel of the effect tile (the SURFACE overload `SurfaceWindow::ReplaceColor` 0x004415b0), preserving the tile's texture/gradient; the tile itself is NOT blitted as an additional layer. This IS faithful retail behavior. **Anti-regression: do NOT re-implement this as a blit layer NOR as a flat-color replace (it is a per-pixel surface copy).** | `src/AcDream.App/UI/IconComposer.cs` (`ReplaceWhiteFromSurface`) | Faithful port of `IconData::RenderIcons` @407614 → the SURFACE overload `ReplaceColor` 0x004415b0 (`dst[x,y]=src[x,y]` where `dst==white`); confirmed via clean Ghidra decompile + named decomp + visual (the Energy Crystal's blue is a gradient, 2026-06-17). | A blit-layer or flat-color re-implementation would show the wrong effect look (no gradient) — the visual-verification regression that retired the mean-color approximation | `IconData::RenderIcons` acclient_2013_pseudo_c.txt:407524; `ReplaceColor` SURFACE overload 0x004415b0:71656; `docs/research/2026-06-17-stateful-icon-RESOLVED.md` | -| IA-19 | Automatic combat acquisition is narrowed to attackable non-player monsters. Retail `AutoTarget` falls back to `SelectNext(SELECTION_TYPE_COMPASS_ITEM)`, whose combat filter can also admit attackable enemy players in compatible PK states. | `src/AcDream.Core/Combat/CombatTargetPolicy.cs`; consumers `src/AcDream.App/Interaction/WorldSelectionQuery.cs` (`IsHostileMonster`/`FindClosestHostileMonster`) and `SelectionInteractionController.cs` (`SelectClosestCombatTarget`) | Explicit product direction: Auto Target must never select NPCs, players, pets, or other objects; manual player-selection commands remain available | In PK play, Auto Target will not acquire an otherwise valid hostile player as retail would; the player must be selected manually | `ClientCombatSystem::AutoTarget @ 0x0056BC80`; `CPlayerSystem::SelectNext @ 0x0055F9A0`; `ClientCombatSystem::ObjectIsAttackable @ 0x0056A600` | +| IA-19 | Automatic combat acquisition is narrowed to attackable non-player monsters. Retail `AutoTarget` falls back to `SelectNext(SELECTION_TYPE_COMPASS_ITEM)`, whose combat filter can also admit attackable enemy players in compatible PK states. | `src/AcDream.Core/Combat/CombatTargetPolicy.cs`; consumers `src/AcDream.App/Interaction/WorldSelectionQuery.cs` (`IsHostileMonster`/`FindClosestHostileMonster`) and `SelectionInteractionController.cs` (`SelectClosestCombatTarget`). This row is auto-acquisition-only: as of #298, explicit-target admission and the combat camera route through the separate, retail-exact `WorldSelectionQuery.IsAttackableTarget` (`ObjectIsAttackable`-backed) instead, so a compatible-PK player is a valid manual attack/camera target — do not assume one predicate still serves both concerns. | Explicit product direction: Auto Target must never select NPCs, players, pets, or other objects; manual player-selection commands remain available | In PK play, Auto Target will not acquire an otherwise valid hostile player as retail would; the player must be selected manually | `ClientCombatSystem::AutoTarget @ 0x0056BC80`; `CPlayerSystem::SelectNext @ 0x0055F9A0`; `ClientCombatSystem::ObjectIsAttackable @ 0x0056A600` | | IA-20 | The basic combat bar keeps dark-red media `0x0600715E` visible as the centered middle baseline. Retail skill-gates field `0x100005EF` to trained Recklessness; the separate bright child remains faithful live `SetPowerbarLevel` feedback from the absolute left edge. | `src/AcDream.App/UI/UiScrollbar.cs`; child-policy extraction in `src/AcDream.App/UI/Layout/DatWidgetFactory.cs` | Explicit connected visual direction: the dark middle track remains present behind live attack charge; the exact skill-gated treatment remains tracked by AP-112 | Untrained characters retain the dark-red baseline where retail may leave only the gray track; trained/untrained Recklessness presentation is not distinguishable | `gmCombatUI::RecvNotice_SetPowerbarLevel @ 0x004CC0E0`; `gmCombatUI::ListenToElementMessage @ 0x004CC430`; LayoutDesc `0x21000073` | | IA-21 | When ACE sends player BoolProperty `68` (`SpellComponentsRequired`) false, acdream presents the retail scarab/prismatic-taper formula even without a directly carried school focus. With component enforcement enabled, retail's exact focus/infusion versus account-customized selection remains intact. | `src/AcDream.App/Spells/SpellComponentRequirementService.cs` | A component-disabled server has no actionable legacy recipe; explicit product direction is that this client/server mode uses the modern scarab/taper component presentation | A custom server could expect retail's legacy recipe to remain visible even though casting consumes no components | `ClientMagicSystem::AreSpellComponentsRequired @ 0x00567B90`; `ClientMagicSystem::GetAppropriateSpellFormula @ 0x00567D50`; `CSpellBase::InqScarabOnlyFormula @ 0x00597050` | diff --git a/src/AcDream.App/Combat/CombatAttackTargetSource.cs b/src/AcDream.App/Combat/CombatAttackTargetSource.cs index b2596d39..eaea1d9b 100644 --- a/src/AcDream.App/Combat/CombatAttackTargetSource.cs +++ b/src/AcDream.App/Combat/CombatAttackTargetSource.cs @@ -38,7 +38,7 @@ internal sealed class CombatAttackTargetSource : ICombatAttackTargetSource public uint? GetSelectedOrClosestCombatTarget(bool autoTarget) { if (_selection.SelectedObjectId is { } selected - && IsHostileMonster(selected)) + && IsAttackableExplicitTarget(selected)) { return selected; } @@ -89,8 +89,52 @@ internal sealed class CombatAttackTargetSource : ICombatAttackTargetSource return best; } + /// + /// Automatic-acquisition eligibility (register row IA-19): narrowed to + /// non-player, non-pet, attackable monsters. Backs + /// only — never explicit + /// selection. + /// private bool IsHostileMonster(uint serverGuid) { + if (!TryGetLiveCombatCandidate(serverGuid, out ClientObject? candidate)) + return false; + + uint playerGuid = _player.ServerGuid; + return (candidate.Type & ItemType.Creature) != 0 + && CombatTargetPolicy.IsHostileMonster( + playerGuid, + _objects.Get(playerGuid), + candidate); + } + + /// + /// Explicit-target admission for a user-issued attack (#298). Retail + /// ClientCombatSystem::ExecuteAttack @ 0x0056BB70 gates + /// unconditionally on ObjectIsAttackable @ 0x0056A600, with no + /// player exclusion — a compatible-PK player is a valid attack target. + /// This is deliberately a different, wider predicate than + /// , which stays narrowed to monsters for + /// automatic acquisition per register row IA-19. + /// + private bool IsAttackableExplicitTarget(uint serverGuid) + { + if (!TryGetLiveCombatCandidate(serverGuid, out ClientObject? candidate)) + return false; + + uint playerGuid = _player.ServerGuid; + return SelectedObjectHealthPolicy.ObjectIsAttackable( + playerGuid, + _objects.Get(playerGuid), + serverGuid, + candidate); + } + + private bool TryGetLiveCombatCandidate( + uint serverGuid, + [System.Diagnostics.CodeAnalysis.NotNullWhen(true)] out ClientObject? candidate) + { + candidate = null; uint playerGuid = _player.ServerGuid; if (serverGuid == playerGuid || !_liveEntities.TryGetInteractionEligibleRecord( @@ -107,12 +151,7 @@ internal sealed class CombatAttackTargetSource : ICombatAttackTargetSource return false; } - ClientObject? candidate = _objects.Get(serverGuid); - return candidate is not null - && (candidate.Type & ItemType.Creature) != 0 - && CombatTargetPolicy.IsHostileMonster( - playerGuid, - _objects.Get(playerGuid), - candidate); + candidate = _objects.Get(serverGuid); + return candidate is not null; } } diff --git a/src/AcDream.App/Interaction/SelectionInteractionController.cs b/src/AcDream.App/Interaction/SelectionInteractionController.cs index 9c788f91..eb7ee866 100644 --- a/src/AcDream.App/Interaction/SelectionInteractionController.cs +++ b/src/AcDream.App/Interaction/SelectionInteractionController.cs @@ -104,10 +104,17 @@ internal sealed class SelectionInteractionController _items.PlaceIn3D(payload, target); } + /// + /// Explicit selection uses the wider ObjectIsAttackable-backed + /// admission (#298) so a compatible-PK player is a valid attack target; + /// falling back to auto-acquisition still uses the narrower + /// monster-only + /// policy (register row IA-19). + /// public uint? GetSelectedOrClosestCombatTarget(bool autoTarget) { if (_selection.SelectedObjectId is { } selected - && _query.IsHostileMonster(selected)) + && _query.IsAttackableTarget(selected)) { return selected; } diff --git a/src/AcDream.App/Interaction/WorldSelectionQuery.cs b/src/AcDream.App/Interaction/WorldSelectionQuery.cs index 08375d88..ada79dc7 100644 --- a/src/AcDream.App/Interaction/WorldSelectionQuery.cs +++ b/src/AcDream.App/Interaction/WorldSelectionQuery.cs @@ -44,6 +44,7 @@ internal interface IWorldSelectionQuery string Describe(uint serverGuid); bool IsCreature(uint serverGuid); bool IsHostileMonster(uint serverGuid); + bool IsAttackableTarget(uint serverGuid); ClosestCombatTarget? FindClosestHostileMonster(); bool IsUseable(uint serverGuid); bool IsPickupable(uint serverGuid); @@ -229,6 +230,14 @@ internal sealed class WorldSelectionQuery return (GetItemType(serverGuid) & ItemType.Creature) != 0; } + /// + /// Automatic-acquisition eligibility (register row IA-19): narrowed to + /// non-player, non-pet, attackable monsters. Backs + /// only — never explicit + /// selection, and never the combat camera (retail gates camera tracking + /// on ObjectIsAttackable, not this narrower policy — see + /// ). + /// public bool IsHostileMonster(uint serverGuid) => IsCreature(serverGuid) && CombatTargetPolicy.IsHostileMonster( @@ -236,6 +245,27 @@ internal sealed class WorldSelectionQuery _objects.Get(_playerGuid()), _objects.Get(serverGuid)); + /// + /// Explicit-target admission for a user-issued attack (#298), and the + /// combat camera's tracking gate. Retail + /// ClientCombatSystem::ExecuteAttack @ 0x0056BB70 and + /// UpdateTargetTracking @ 0x0056A950 both gate unconditionally on + /// ObjectIsAttackable @ 0x0056A600, with no player exclusion — a + /// compatible-PK player is a valid attack target and a valid camera + /// target. This is deliberately a different, wider predicate than + /// , which stays narrowed to monsters for + /// AUTOMATIC ACQUISITION ONLY per register row IA-19 — IA-19 does not + /// reach explicit selection or the camera (a presentation gate on an + /// already-chosen target, not an acquisition path). + /// + public bool IsAttackableTarget(uint serverGuid) + => IsCreature(serverGuid) + && SelectedObjectHealthPolicy.ObjectIsAttackable( + _playerGuid(), + _objects.Get(_playerGuid()), + serverGuid, + _objects.Get(serverGuid)); + public bool ShouldShowHealth(uint serverGuid) => SelectedObjectHealthPolicy.ShouldQueryHealth( _playerGuid(), @@ -261,8 +291,18 @@ internal sealed class WorldSelectionQuery return best; } + /// + /// #298 follow-up: retail ClientCombatSystem::UpdateTargetTracking + /// @ 0x0056A950 (pc:375691-375696) gates CameraSet::TrackTarget + /// on ObjectIsAttackable @ 0x0056A600 — the SAME wide predicate as + /// ExecuteAttack, not the narrower automatic-acquisition policy. + /// The camera performs no acquisition of its own; it only tracks a + /// target the player already selected, so routing it through + /// does not touch register row IA-19 + /// (which scopes itself to automatic acquisition). + /// public Vector3? GetCombatCameraTargetPoint(uint serverGuid) - => IsHostileMonster(serverGuid) + => IsAttackableTarget(serverGuid) && TryGetInteractionTarget(serverGuid, out WorldInteractionTarget target) ? target.Entity.Position + Vector3.Transform(new Vector3(0f, 0f, 0.5f), target.Entity.Rotation) diff --git a/tests/AcDream.App.Tests/Combat/CombatAttackTargetSourceTests.cs b/tests/AcDream.App.Tests/Combat/CombatAttackTargetSourceTests.cs index d211e9a3..6405a374 100644 --- a/tests/AcDream.App.Tests/Combat/CombatAttackTargetSourceTests.cs +++ b/tests/AcDream.App.Tests/Combat/CombatAttackTargetSourceTests.cs @@ -32,6 +32,142 @@ public sealed class CombatAttackTargetSourceTests harness.Targets.GetSelectedOrClosestCombatTarget(autoTarget: false)); } + /// + /// #298: explicit selection of a compatible-PK player is a valid attack + /// target. Retail ObjectIsAttackable @ 0x0056A600 admits a player + /// target when both sides carry the same PKLite status, and this is + /// reachable ONLY through explicit selection — never automatic + /// acquisition (register row IA-19). + /// + [Fact] + public void ExplicitSelectedPkLitePlayerIsAcceptedWithoutAutoTarget() + { + var harness = new Harness(); + harness.SetLocalPlayerPvpFlags(SelectedObjectHealthPolicy.BfPkLiteStatus); + const uint target = 0x7000_0002u; + harness.Add( + target, + new Vector3(2f, 0f, 0f), + attackable: false, + isPlayer: true, + extraFlags: SelectedObjectHealthPolicy.BfPkLiteStatus); + harness.Selection.Select(target, SelectionChangeSource.Keyboard); + + Assert.Equal( + target, + harness.Targets.GetSelectedOrClosestCombatTarget(autoTarget: false)); + } + + /// + /// #298: explicit selection of a player whose PK pool does not match + /// (neither side is PKLite/PK/Free) is still refused — the fix widens + /// admission to compatible-PK players, it does not admit every player. + /// + [Fact] + public void ExplicitSelectedNonPkPlayerIsRefused() + { + var harness = new Harness(); + const uint target = 0x7000_0003u; + harness.Add( + target, + new Vector3(2f, 0f, 0f), + attackable: false, + isPlayer: true); + harness.Selection.Select(target, SelectionChangeSource.Keyboard); + + Assert.Null( + harness.Targets.GetSelectedOrClosestCombatTarget(autoTarget: false)); + } + + /// + /// #298 follow-up: explicit selection no longer short-circuits on + /// PetOwnerId != 0 the way CombatTargetPolicy.IsHostileMonster + /// does (`:33`) — it now reaches ObjectIsAttackable's OWN pet arm + /// (retail `else if (esi->pwd._pet_owner == 0)` @ 0x0056a683) for the + /// first time. That arm still refuses an attackable, owned pet, so the + /// end behavior is unchanged; this pins reachability through the new + /// path, not just the Core predicate (see + /// SelectedObjectHealthPolicyTests.ObjectIsAttackable_AttackablePetIsRejected). + /// + [Fact] + public void ExplicitSelectedAttackablePetIsRefused() + { + var harness = new Harness(); + const uint pet = 0x7000_0007u; + harness.Add( + pet, + new Vector3(2f, 0f, 0f), + attackable: true, + petOwnerId: Player); + harness.Selection.Select(pet, SelectionChangeSource.Keyboard); + + Assert.Null( + harness.Targets.GetSelectedOrClosestCombatTarget(autoTarget: false)); + } + + /// + /// Mutation guard for : + /// with nothing explicitly selected, auto-acquisition must still filter + /// the nearer compatible-PK player out and pick the farther monster. This + /// does NOT exercise the explicit-selection branch (nothing is selected), + /// so it does not by itself prove the #298 split kept + /// narrow — it proves + /// nobody swapped FindClosestHostileMonster's gate for the wider + /// predicate. See + /// for the case that actually forces the narrow policy to reject the + /// only candidate. + /// + [Fact] + public void AutoTargetNeverAcquiresAPlayerOverAMonster() + { + var harness = new Harness(); + harness.SetLocalPlayerPvpFlags(SelectedObjectHealthPolicy.BfPkLiteStatus); + const uint monster = 0x7000_0004u; + const uint pkLitePlayer = 0x7000_0005u; + harness.Add(monster, new Vector3(8f, 0f, 0f), attackable: true); + harness.Add( + pkLitePlayer, + new Vector3(1f, 0f, 0f), + attackable: false, + isPlayer: true, + extraFlags: SelectedObjectHealthPolicy.BfPkLiteStatus); + + uint? selected = harness.Targets.GetSelectedOrClosestCombatTarget( + autoTarget: true); + + Assert.Equal(monster, selected); + Assert.Equal(monster, harness.Selection.SelectedObjectId); + } + + /// + /// The actual IA-19 invariant: with nothing selected and NO monster in + /// range at all, a nearby compatible-PK player must still be rejected by + /// automatic acquisition — FindClosestHostileMonster filters + /// through the narrow , + /// finds no candidate, and the stale selection is cleared, exactly as + /// retail's IA-19 divergence documents (auto-target never acquires a + /// player even though retail's own AutoTarget would). + /// + [Fact] + public void AutoTargetNeverAcquiresAPlayerWhenNoMonsterIsInRange() + { + var harness = new Harness(); + harness.SetLocalPlayerPvpFlags(SelectedObjectHealthPolicy.BfPkLiteStatus); + const uint pkLitePlayer = 0x7000_0006u; + harness.Add( + pkLitePlayer, + new Vector3(1f, 0f, 0f), + attackable: false, + isPlayer: true, + extraFlags: SelectedObjectHealthPolicy.BfPkLiteStatus); + + uint? selected = harness.Targets.GetSelectedOrClosestCombatTarget( + autoTarget: true); + + Assert.Null(selected); + Assert.Null(harness.Selection.SelectedObjectId); + } + [Fact] public void AutoTargetUsesNearestEligibleLiveHostile() { @@ -107,11 +243,22 @@ public sealed class CombatAttackTargetSourceTests Add(Player, Vector3.Zero, attackable: false, isPlayer: true); } + public void SetLocalPlayerPvpFlags(uint extraFlags) => + Objects.AddOrUpdate(new ClientObject + { + ObjectId = Player, + Name = $"Object {Player:X8}", + Type = ItemType.Creature, + PublicWeenieBitfield = SelectedObjectHealthPolicy.BfPlayer | extraFlags, + }); + public WorldEntity Add( uint guid, Vector3 position, bool attackable, - bool isPlayer = false) + bool isPlayer = false, + uint extraFlags = 0u, + uint petOwnerId = 0u) { Runtime.RegisterLiveEntity(Spawn(guid)); WorldEntity entity = Runtime.MaterializeLiveEntity( @@ -130,12 +277,14 @@ public sealed class CombatAttackTargetSourceTests uint flags = attackable ? SelectedObjectHealthPolicy.BfAttackable : 0u; if (isPlayer) flags |= SelectedObjectHealthPolicy.BfPlayer; + flags |= extraFlags; Objects.AddOrUpdate(new ClientObject { ObjectId = guid, Name = $"Object {guid:X8}", Type = ItemType.Creature, PublicWeenieBitfield = flags, + PetOwnerId = petOwnerId, }); return entity; } diff --git a/tests/AcDream.App.Tests/Combat/CombatCameraTargetSourceTests.cs b/tests/AcDream.App.Tests/Combat/CombatCameraTargetSourceTests.cs index 8c0bc9e1..dac924f9 100644 --- a/tests/AcDream.App.Tests/Combat/CombatCameraTargetSourceTests.cs +++ b/tests/AcDream.App.Tests/Combat/CombatCameraTargetSourceTests.cs @@ -57,6 +57,7 @@ public sealed class CombatCameraTargetSourceTests public string Describe(uint serverGuid) => string.Empty; public bool IsCreature(uint serverGuid) => false; public bool IsHostileMonster(uint serverGuid) => false; + public bool IsAttackableTarget(uint serverGuid) => false; public ClosestCombatTarget? FindClosestHostileMonster() => null; public bool IsUseable(uint serverGuid) => false; public bool IsPickupable(uint serverGuid) => false; diff --git a/tests/AcDream.App.Tests/Interaction/SelectionInteractionControllerTests.cs b/tests/AcDream.App.Tests/Interaction/SelectionInteractionControllerTests.cs index 1bccc07f..cd7a2849 100644 --- a/tests/AcDream.App.Tests/Interaction/SelectionInteractionControllerTests.cs +++ b/tests/AcDream.App.Tests/Interaction/SelectionInteractionControllerTests.cs @@ -30,6 +30,7 @@ public sealed class SelectionInteractionControllerTests public bool Current { get; set; } = true; public bool Creature { get; set; } public bool Hostile { get; set; } + public bool Attackable { get; set; } public bool Useable { get; set; } = true; public bool Pickupable { get; set; } = true; public bool WieldedByPlayer { get; set; } @@ -66,6 +67,7 @@ public sealed class SelectionInteractionControllerTests public string Describe(uint serverGuid) => $"Target {serverGuid:X8}"; public bool IsCreature(uint serverGuid) => Creature; public bool IsHostileMonster(uint serverGuid) => Hostile; + public bool IsAttackableTarget(uint serverGuid) => Attackable; public ClosestCombatTarget? FindClosestHostileMonster() => Closest; public bool IsUseable(uint serverGuid) => Useable; public bool IsPickupable(uint serverGuid) => Pickupable; @@ -1040,6 +1042,44 @@ public sealed class SelectionInteractionControllerTests Assert.False(h.Items.TryGetPendingBackpackPlacement(Target, out _)); } + /// + /// #298: explicit-target admission routes through the wider + /// IsAttackableTarget query (retail ObjectIsAttackable, + /// which admits a compatible-PK player), not the narrower + /// IsHostileMonster that auto-acquisition uses. + /// + [Fact] + public void GetSelectedOrClosestCombatTargetAcceptsExplicitAttackableEvenWhenNotHostileMonster() + { + var h = new Harness(); + h.Query.Attackable = true; + h.Query.Hostile = false; + h.Selection.Select(Target, SelectionChangeSource.Keyboard); + + Assert.Equal( + Target, + h.Controller.GetSelectedOrClosestCombatTarget(autoTarget: false)); + } + + /// + /// #298 / IA-19 guard: when the explicit selection is not an admitted + /// attack target, auto-acquisition falls back to the narrower + /// FindClosestHostileMonster query unchanged. + /// + [Fact] + public void GetSelectedOrClosestCombatTargetFallsBackToAutoAcquisitionWhenExplicitTargetIsNotAttackable() + { + var h = new Harness(); + const uint monster = 0x7000_0099u; + h.Query.Attackable = false; + h.Query.Closest = new ClosestCombatTarget(monster, DistanceSquared: 4f); + h.Selection.Select(Target, SelectionChangeSource.Keyboard); + + Assert.Equal( + monster, + h.Controller.GetSelectedOrClosestCombatTarget(autoTarget: true)); + } + private static WorldSession.EntitySpawn Spawn(uint guid, ushort instance) => new( guid, diff --git a/tests/AcDream.App.Tests/Interaction/WorldSelectionQueryTests.cs b/tests/AcDream.App.Tests/Interaction/WorldSelectionQueryTests.cs index e1c4a0b6..27d819fc 100644 --- a/tests/AcDream.App.Tests/Interaction/WorldSelectionQueryTests.cs +++ b/tests/AcDream.App.Tests/Interaction/WorldSelectionQueryTests.cs @@ -299,6 +299,44 @@ public sealed class WorldSelectionQueryTests Assert.Equal(64f, closest?.DistanceSquared); } + /// + /// #298 follow-up: retail ClientCombatSystem::UpdateTargetTracking + /// @ 0x0056A950 (pc:375691-375696) gates CameraSet::TrackTarget + /// on ObjectIsAttackable @ 0x0056A600 — the SAME wide predicate as + /// ExecuteAttack, not the narrower monster-only + /// IsHostileMonster. A compatible-PK player under explicit + /// selection is a valid camera target; a non-PK player is not. + /// + [Fact] + public void CombatCameraTracksACompatiblePkPlayerButNotAnIncompatibleOne() + { + var h = new Harness(); + h.Objects.AddOrUpdate(new ClientObject + { + ObjectId = Player, + Name = $"Object {Player:X8}", + Type = ItemType.Creature, + PublicWeenieBitfield = SelectedObjectHealthPolicy.BfPlayer + | SelectedObjectHealthPolicy.BfPkLiteStatus, + }); + const uint pkLiteOpponent = 0x7000_0050u; + const uint nonPkPlayer = 0x7000_0051u; + h.Add( + pkLiteOpponent, + new Vector3(2f, 0f, 0f), + ItemType.Creature, + SelectedObjectHealthPolicy.BfPlayer + | SelectedObjectHealthPolicy.BfPkLiteStatus); + h.Add( + nonPkPlayer, + new Vector3(3f, 0f, 0f), + ItemType.Creature, + SelectedObjectHealthPolicy.BfPlayer); + + Assert.NotNull(h.Query.GetCombatCameraTargetPoint(pkLiteOpponent)); + Assert.Null(h.Query.GetCombatCameraTargetPoint(nonPkPlayer)); + } + [Theory] [InlineData(0.59f, true)] [InlineData(0.61f, false)] diff --git a/tests/AcDream.App.Tests/Runtime/CurrentGameRuntimeAdapterTests.cs b/tests/AcDream.App.Tests/Runtime/CurrentGameRuntimeAdapterTests.cs index 3acc470a..0251bf1d 100644 --- a/tests/AcDream.App.Tests/Runtime/CurrentGameRuntimeAdapterTests.cs +++ b/tests/AcDream.App.Tests/Runtime/CurrentGameRuntimeAdapterTests.cs @@ -1181,6 +1181,7 @@ public sealed class CurrentGameRuntimeAdapterTests public string Describe(uint serverGuid) => "Runtime target"; public bool IsCreature(uint serverGuid) => serverGuid == target; public bool IsHostileMonster(uint serverGuid) => serverGuid == target; + public bool IsAttackableTarget(uint serverGuid) => serverGuid == target; public ClosestCombatTarget? FindClosestHostileMonster() => new(target, DistanceSquared: 4f); public bool IsUseable(uint serverGuid) => serverGuid == target; diff --git a/tests/AcDream.Core.Tests/Combat/SelectedObjectHealthPolicyTests.cs b/tests/AcDream.Core.Tests/Combat/SelectedObjectHealthPolicyTests.cs index 5e6e84e9..c33de890 100644 --- a/tests/AcDream.Core.Tests/Combat/SelectedObjectHealthPolicyTests.cs +++ b/tests/AcDream.Core.Tests/Combat/SelectedObjectHealthPolicyTests.cs @@ -93,4 +93,27 @@ public sealed class SelectedObjectHealthPolicyTests Assert.False(SelectedObjectHealthPolicy.ObjectIsAttackable( PlayerId, null, creature.ObjectId, creature)); } + + /// + /// #298 follow-up: this arm (retail else if (esi->pwd._pet_owner == + /// 0) @ 0x0056a683 — falling through to eax = 0; return 0 when + /// it does NOT hold) only became reachable from the App layer once + /// explicit-target admission started calling ObjectIsAttackable + /// directly; CombatTargetPolicy.IsHostileMonster rejects pets + /// earlier and never reaches it. Pin it here at the source predicate: an + /// attackable, owned pet is refused even though its + /// BfAttackable bit is set. + /// + [Fact] + public void ObjectIsAttackable_AttackablePetIsRejected() + { + var player = Obj(PlayerId, flags: SelectedObjectHealthPolicy.BfPlayer); + var pet = Obj( + 0x50000041u, + flags: SelectedObjectHealthPolicy.BfAttackable, + petOwner: PlayerId); + + Assert.False(SelectedObjectHealthPolicy.ObjectIsAttackable( + PlayerId, player, pet.ObjectId, pet)); + } }