From 9b97102c673f02f327a35af2ec01d436df209118 Mon Sep 17 00:00:00 2001 From: Erik Date: Sun, 12 Jul 2026 09:19:38 +0200 Subject: [PATCH] fix(combat): persist death and retarget on kill Keep the velocity-only NPC adaptation inside the locomotion family so authoritative Dead motion remains persistent. Route selection clears through a dedicated combat target controller that reacquires the closest eligible creature when retail Auto Target conditions apply. Co-Authored-By: Codex --- .../retail-divergence-register.md | 2 +- docs/plans/2026-04-11-roadmap.md | 2 +- docs/plans/2026-05-12-milestones.md | 7 +- ...-07-12-death-and-auto-target-pseudocode.md | 60 ++++++++++++++ .../Combat/CombatTargetController.cs | 72 ++++++++++++++++ src/AcDream.App/Rendering/GameWindow.cs | 45 ++++++---- .../Physics/ServerControlledLocomotion.cs | 15 ++++ src/AcDream.Core/Selection/SelectionState.cs | 1 + .../Combat/CombatTargetControllerTests.cs | 82 +++++++++++++++++++ .../ServerControlledLocomotionTests.cs | 14 ++++ 10 files changed, 281 insertions(+), 19 deletions(-) create mode 100644 docs/research/2026-07-12-death-and-auto-target-pseudocode.md create mode 100644 src/AcDream.App/Combat/CombatTargetController.cs create mode 100644 tests/AcDream.App.Tests/Combat/CombatTargetControllerTests.cs diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index c5efe336..07a6929b 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -183,7 +183,7 @@ AP-94..AP-112 for the confirmed retail-UI completion gaps. | AP-75 | **Adapter-boundary `adjust_motion` + locomotion velocity/omega synthesis**: `SetCycle` still (a) remaps TurnLeft/SideStepLeft/WalkBackward to their mirrors with negated speed BEFORE dispatch (retail adjusts in `CMotionInterp` — R3 scope; GameWindow's local-player path passes raw ids), and (b) post-dispatch overwrites sequence velocity (low-bytes 05/06/07/0F/10) and omega (0D/0E, only when dat-silent) with the retail locomotion constants — retail drives BODY velocity from `get_state_velocity`, not the sequence accumulators (R2-Q4 carry-over of the pre-Q4 adapter tail) | `src/AcDream.Core/Physics/AnimationSequencer.cs` (`SetCycle` head remap + tail synthesis) | (a) preserves unadjusted GameWindow callers until R3-W6 unifies the local player onto MotionInterpreter; (b) preserves the remote-DR and local Option-B velocity consumers until R6 root motion drives the body (sibling of IA-3) | (a) none while callers stay in the known set; (b) a dat whose locomotion MotionData carries a REAL velocity different from the constants gets overwritten — exotic-creature speed skew (same class as IA-3's risk) | `CMotionInterp::adjust_motion` @305343; `get_state_velocity` 0x00528960; retire (a) R3-W6, (b) R6 | | AP-76 | **Remote rotation from the ObservedOmega side-channel**: the R2-Q5 sink callbacks (`MotionTableDispatchSink.TurnApplied/TurnStopped`) seed `RemoteMotion.ObservedOmega = (0,0,-(pi/2)*signedTurnSpeed)` on wire turns and zero it on turn-stops; GameWindow's per-tick step applies ObservedOmega (preferring it over sequence omega) to the remote body's orientation. Retail rotates the body from the SEQUENCE omega inside the per-tick apply_physics chain (CSequence::apply_physics -> CPhysicsObj omega integration) (carried verbatim from the deleted RemoteMotionSink; H17) | `src/AcDream.App/Rendering/GameWindow.cs` (sink callbacks in OnLiveMotionUpdated + the omegaToApply step in TickAnimations) | Same angular rate retail derives (pi/2 rad/s x turn speed); starts rotation the same tick as the wire turn without waiting for R6's per-tick order; UpdatePosition orientation snaps bound any drift | If the dat's turn modifier omega differs from the pi/2 constant, remote rotation rate diverges until an orientation snap; double-application risk if R6 lands apply_physics-driven rotation without deleting this seam | retire in R6 (retail per-tick order: apply_physics drives remote rotation) | | AP-77 | **`apply_current_movement`'s interpreted-branch tail (`ApplyCurrentMovementInterpreted`) writes body velocity DIRECTLY via `get_state_velocity`/`set_local_velocity` when grounded, instead of dispatching through an `IInterpretedMotionSink`** — retail's real `apply_interpreted_movement` (0x00528600) drives velocity indirectly through `DoInterpretedMotion`'s animation-table backend (the SAME function the funnel's `ApplyInterpretedMovement` already uses WITH a sink); this dual-dispatch call site (`HitGround`/`LeaveGround`/`ReportExhaustion`/hold-key toggles/`SetWeenieObject`/`SetPhysicsObject`) has no sink threaded through it | `src/AcDream.Core/Physics/MotionInterpreter.cs` (`ApplyCurrentMovementInterpreted`) | Direct continuation of the pre-R3-W4 `apply_current_movement` approximation (R3-W4 plan explicitly keeps this shape rather than relocating it — "the direct grounded-velocity write MOVES to the controller-side call site unchanged"); correct for the grounded, no-animation-table-yet state acdream is in today | A MotionTable whose locomotion cycle bakes a DIFFERENT velocity than `get_state_velocity`'s constants would silently diverge from what the animation actually plays, since this path never touches the animation backend at all | `CMotionInterp::apply_interpreted_movement` 0x00528600; retire when a sink is threaded through `apply_current_movement`'s callers (R6 root motion) | -| AP-80 | **PlanFromVelocity survives for velocity-only NPC cycles** (M16): UpdatePosition-derived speed picks Ready/Walk/Run cycles for server-controlled creatures whose UMs never arrive (scripted-path NPCs); retail derives every cycle from motion messages through the motion tables (R4-V4 note; pre-existing mechanism, row added per the V4 plan) | `src/AcDream.Core/Physics/ServerControlledLocomotion.cs` (`PlanFromVelocity`); consumer `GameWindow.ApplyServerControlledVelocityCycle` | Some ACE entities move by position updates alone — without this, they slide in T-pose; constants (StopSpeed 0.2, RunThreshold 1.25) tuned against live ACE traffic | Cycle-pick thresholds are acdream inventions — a creature intended to walk fast may show run legs near the threshold | retire in R6 (root motion + full per-tick order) | +| AP-80 | **PlanFromVelocity survives for velocity-only NPC cycles** (M16): UpdatePosition-derived speed picks Ready/Walk/Run cycles for server-controlled creatures whose UMs never arrive (scripted-path NPCs); retail derives every cycle from motion messages through the motion tables. The adaptation is now structurally limited to replacing Ready/Walk/Run-family states, so authoritative actions/substates (especially Dead) always win. | `src/AcDream.Core/Physics/ServerControlledLocomotion.cs` (`PlanFromVelocity`, `CanApplyVelocityCycle`); consumer `GameWindow.ApplyServerControlledVelocityCycle` | Some ACE entities move by position updates alone — without this, they slide in T-pose; constants (StopSpeed 0.2, RunThreshold 1.25) tuned against live ACE traffic | Cycle-pick thresholds are acdream inventions — a creature intended to walk fast may show run legs near the threshold | retire in R6 (root motion + full per-tick order) | | AP-81 | **Remote-DR gravity toggled via the Gravity STATE bit**: the jump handler sets `Body.State \|= Gravity` at VectorUpdate and both landing blocks clear it after `HitGround()`; retail keeps GRAVITY set for the object's whole life and gates gravity ACCELERATION on the Contact transient (`calc_acceleration`) (pre-existing K-fix9/K-fix15 mechanism, row added during #161 — which also fixed the ordering so `Motion.HitGround()`'s verbatim `state&0x400` gate runs BEFORE the clear) | `src/AcDream.App/Rendering/GameWindow.cs` (VectorUpdate jump handler + the two landing blocks) | The DR tick integrates gravity only for airborne remotes; the flag dance delivers exactly that without porting the full contact-gated `calc_acceleration` chain; the #161 ordering fix keeps the retail HitGround contract satisfied | Any NEW call into `Motion.HitGround`/`LeaveGround` placed after the clear silently no-ops on the gravity gate (the #161 leg-2 class); grounded remotes carry a non-retail state word (probes comparing state bits vs retail mislead) | `CPhysicsObj::calc_acceleration` (contact-gated); `set_on_walkable` 0x00511310; retire in R6 (contact-gated accel + persistent GRAVITY) | | AP-82 | **StickyManager deep-overlap back-off sign pin**: when the stick-gap overlap exceeds one tick's step (`speed×quantum < \|dist\|`, `dist < 0`), acdream applies `delta = −(speed×quantum)` (rate-limited back-off); ACE's literal port keeps `+delta` there — a runaway that steers INTO the target with equilibrium at centers-coincident. The BN mush (0x00555554-0x00555597) is unreadable on exactly this compare; the pin is refuted-by-evidence against ACE-literal: #171 gate-3 probe showed 1661 deep-overlap ticks all steering inward (monsters converged to centerDist≈0 — "monster inside the player") while retail side-by-side on the same ACE shows separation. ACE servers essentially never reach the branch (quantum ≥1/30 → threshold ~1 m; render-rate quanta → ~0.13 m) | `src/AcDream.Core/Physics/Motion/StickyManager.cs` (`AdjustOffset` delta clamp; conformance `StickyManagerTests.AdjustOffset_DeepOverlap_BacksOff_RateLimited`) | Minimal interpretation consistent with the mush structure AND observed retail; identical to ACE-literal in every shallow/outside case | If retail's true deep-overlap behavior differs (e.g. no movement at all), our back-off rate diverges in that rare state; verify via cdb `StickyManager::adjust_offset` trace with a forced overlap when convenient | `StickyManager::adjust_offset` 0x00555430 (x87 mush); ACE StickyManager.cs:117-121 (the literal branch this pin overrides) | | AP-85 | **Point-light pool = single 128-cap player-nearest list, optionally FILTERED by LAST FRAME's rendered visible-cell set, vs retail's dual pools (7 dynamic + 40 static, degrade-scaled) collected from a DBObj-load/flush-bounded resident registry** (A7.L1, 2026-07-09 — third revision, Town Network starvation fix #79/#93/#176/#177): retail's `CEnvCell::visible_cell_table` (`add_visible_cell` 0x0052de40) is populated ON DEMAND as cells are approached/seen (`DBObj::Get`-loads) and pruned by `flush_cells` — so a real dungeon's per-frame candidate set stays small (naturally proximity-bounded) even though the collection walk itself (`add_dynamic_lights` 0x0052d410) is "the whole resident table, not a re-flood." acdream's `_all` list instead registers at LANDBLOCK-granularity load/unload (a whole single-landblock dungeon streams as ONE unit), so for the Town Network (463 registered fixtures, one landblock) `_all` is effectively "everything ever loaded in this dungeon," not a proximity-bounded set — wide enough that the player-nearest-128 cap alone let a straight-line-closer-but-wall-disconnected corridor's fixtures out-rank the player's own room, starving it. Fix: `BuildPointLightSnapshot(playerWorldPos, visibleCells)` takes an optional candidacy FILTER — a light joins the pool iff `CellId==0` (cell-less, always in) or `visibleCells.Contains(CellId)` — narrowing candidates to the frame's actual visible cells BEFORE the existing dynamics-first player-nearest cap runs; `GameWindow` feeds LAST FRAME's already-rendered `RetailPViewFrameResult.DrawableCells` (one frame / ~16 ms latency, chosen specifically to avoid re-threading a mid-`DrawInside` callback — the exact mechanism, `c500912b`, that caused the #176 seam-floor flicker regression when it re-flooded an independent CAMERA-seeded set mid-frame). The distance-sort anchor stays the PLAYER (unchanged from the prior revision) — only candidacy narrows. Remaining deviation: this is a RENDER-visibility approximation of retail's true on-demand-load/flush RESIDENCY bound, with one frame of latency, not a port of the DBObj-load/flush mechanism itself; and the pool is still ONE 128-cap list vs retail's separate 7-dynamic/40-static degrade-scaled pools | `src/AcDream.Core/Lighting/LightManager.cs` (`BuildPointLightSnapshot`, `MaxGlobalLights`); `src/AcDream.App/Rendering/GameWindow.cs` (`_lightPoolVisibleCells`/`_lightPoolVisibleCellsValid`); pins `PointSnapshot_HubScaleLightCount_ObjectSelectionIsCameraInvariant`, `PointSnapshot_OverCap_DynamicsNeverEvictedByNearerStatics`, `PointSnapshot_ResidentCollection_CellTagDoesNotFilter`, `BuildPointLightSnapshot_VisibleCellScoping_RoomLightsSurviveOverEuclideanCloserInvisibleCell`, `BuildPointLightSnapshot_VisibleCellScoping_CellLessLightAlwaysIncluded` | The render already computes a visible-cell set every frame for drawing (single source of truth, no duplicate flood) — reusing it as a candidacy filter approximates retail's proximity-bounded residency without porting DBObj on-demand load/flush; one-frame latency is imperceptible at normal camera speeds and structurally differs from the reverted mechanism (no independent re-flood mid-frame) | On a portal crossing, the FIRST indoor frame after re-entry (or after any outdoor-only frame) is unscoped (fail-open) — one frame may show slightly wider pool composition than steady-state; a room with >7 resident dynamics still shows them all (retail trims to 7 player-nearest) — slightly purpler wedge than retail; adopt the dual pools + degrade caps + true DBObj-bounded residency in later A7-arc work | `insert_light` 0x0054d1b0 (player-sorted, capped); `add_visible_cell` 0x0052de40 (on-demand-load resident registry + flush); `add_dynamic_lights` 0x0052d410 (whole-table walk); caller 0x00452d30; `calc_point_light` 0x0059c8b0 (static 1/d³ curve — A7 fix #2) | diff --git a/docs/plans/2026-04-11-roadmap.md b/docs/plans/2026-04-11-roadmap.md index c309de47..b4f04e45 100644 --- a/docs/plans/2026-04-11-roadmap.md +++ b/docs/plans/2026-04-11-roadmap.md @@ -497,7 +497,7 @@ behavior. Estimated 17–26 days focused work, 3–5 weeks calendar. - **Wave 4.4e implemented — exact missile ammo number (live gate pending).** Pure Core reproduces retail's thrown-weapon-vs-separate-ammo resolution over the ordered player equipment list and its zero-to-one count normalization. Relevant equipment/stack events update authored missile indicator `0x10000194` with the DAT font. Warning-free Release build and 4,754-pass / 5-skip suite are green; AP-101 is retired. - **M2 held-object parenting shipped and live-gated 2026-07-11.** The combat toggle now ports `GetDefaultCombatMode` over ordered equipped contents, so a bow requests Missile instead of the old hardcoded Melee. CreateObject preserves parent/placement/timestamp fields, `0xF749` ParentEvent is handled, and `EquippedChildRenderController` renders the weapon as a separate child composed from the animated hand part + holding frame + child placement frame. Live gate passed: bow selected missile stance, rendered in-hand, followed animation, unequipped cleanly, and melee remained correct. App Release builds with zero warnings; the full 4,765-pass / 5-skip suite is green. AP-111 is retired; research: `docs/research/2026-07-11-combat-default-and-parent-event-pseudocode.md`. - **M2 local attack receive funnel implemented 2026-07-11; live gate pending.** Retail `ExecuteAttack` only sends the request; ACE chooses the concrete melee/missile action and returns it in a non-autonomous mt-0 `UpdateMotion`. The local branch now runs that state through the same constructor-defaulted `MoveToInterpretedState` funnel and 15-bit action-stamp gate as remotes, then applies sticky/long-jump tails. The old local-only direct `Commands[]` replay is deleted. Shared conversion lives in `InboundInterpretedMotionFactory`; research: `docs/research/2026-07-11-local-combat-motion-pseudocode.md`. -- **M2 basic retail combat bar implemented 2026-07-11; corrective live visual gate pending.** Production mounts authored `gmCombatUI` LayoutDesc `0x21000073`, shows it only for Melee/Missile, and routes mouse plus keyboard through one `CombatAttackController`. Corrections now include the right-to-left red charge meter, silent control-only `AttackDone(ActionCancelled)`, persistent Dead cycles despite zero-velocity position updates, and target-frame Keep in View that preserves manual viewer-offset orbit. Matching retail x86 recovered exact 1.0-second normal and 0.8-second dual-wield power-up times. Research: `docs/research/2026-07-11-retail-combat-bar-pseudocode.md`, `docs/research/2026-07-11-combat-target-camera-pseudocode.md`, `docs/research/2026-07-11-combat-live-gate-corrections-pseudocode.md`; AP-24/AP-95 retired, AP-110 narrowed, AP-112 records the remaining advanced/Recklessness and command-interpreter seams. +- **M2 basic retail combat bar implemented 2026-07-11; corrective live visual gate pending.** Production mounts authored `gmCombatUI` LayoutDesc `0x21000073`, shows it only for Melee/Missile, and routes mouse plus keyboard through one `CombatAttackController`. Corrections include the right-to-left red charge meter, silent control-only `AttackDone(ActionCancelled)`, target-frame Keep in View with manual orbit, and persistent corpse motion: the AP-80 velocity-only NPC adaptation can now replace only Ready/Walk/Run, never authoritative Dead/actions. `CombatTargetController` ports the selection-cleared AutoTarget consumer, so a selected creature's authoritative Dead motion clears it and selects the nearest eligible creature when enabled. Research: `docs/research/2026-07-11-retail-combat-bar-pseudocode.md`, `docs/research/2026-07-11-combat-target-camera-pseudocode.md`, `docs/research/2026-07-12-death-and-auto-target-pseudocode.md`; AP-24/AP-95 retired, AP-80 narrowed, AP-110 narrowed, AP-112 records the remaining advanced/Recklessness and command-interpreter seams. - **✓ SHIPPED — Character window** (`LayoutDesc 0x2100002E`, `CharacterStatController`, 2026-06-26, same branch). **Visually user-confirmed 2026-06-26 — Attributes tab reads as retail.** Three tabs, header (name/heritage/PK), large-gold level number (dat font, `largeDatFont` 18px), "Total Experience (XP):" + "XP for next level:" captions, 9-row attribute list (icons + right-aligned values + Health/Stamina/Mana vitals), click-to-select (top/bottom selection bars + footer State-B "{Attr}: {value}" / "Experience To Raise: Infinity!" + affordability-gated raise triangles), centered footer. User noted "still needs some polish for later" — deferred to Issue #158. - **✓ SHIPPED — D.5.4 — Client object/item data model (foundation).** Shipped 2026-06-18 (`b506f53`..`a33e897`, 11 commits). Renamed `ItemRepository`→`ClientObjectTable` / `ItemInstance`→`ClientObject`; broadened the table to hold EVERY server object (retail `weenie_object_table` shape). `CreateObject` is now the canonical merge-upsert (`ClientObjectTable.Ingest`, retail `SetWeenieDesc` semantics) via a new Core.Net `ObjectTableWiring` (off GameWindow); `DeleteObject` evicts; `PlayerDescription` is a membership manifest (`RecordMembership`); live container-membership index (`GetContents`, retail `object_inventory_table`). `_liveEntityInfoByGuid` retired (selection/describe resolve from the one table). Root fix: the old enrich-existing-only `EnrichItem` dropped `CreateObject`s for items with no `PlayerDescription` stub — live-Coldeve 4/6 hotbar slots blank; items are now created, not dropped. **Crux resolved:** retail is TWO tables (`object_table` + `weenie_object_table`), NOT one — acdream's `WorldEntity` (3D system) + `ClientObjectTable` (data/UI) split was already architecturally faithful; the fix was the ingestion path, not a table unification. 2671 tests green. - **Roadmap correction (2026-07-10):** the completion order is now the architecture-first campaign in `docs/superpowers/plans/2026-07-10-retail-ui-fidelity-completion.md`. Retail `gmToolbarUI` is object-only: preserve `ShortCutData.index_`, `objectID_`, and `spellID_`, but do not invent spell glyphs on this bar. `PlayerModule::favorite_spells_[8]` feeds separate spell bars. diff --git a/docs/plans/2026-05-12-milestones.md b/docs/plans/2026-05-12-milestones.md index e7f21637..413a7320 100644 --- a/docs/plans/2026-05-12-milestones.md +++ b/docs/plans/2026-05-12-milestones.md @@ -463,9 +463,10 @@ include dungeons. press/hold/release attack-request owner across DAT buttons and keybindings. Retail x86 resolves full charge to 1.0 s (0.8 s dual wield), also retiring the guessed jump-charge timing. The corrective gate ports the right-to-left red - meter, silent AttackDone control status, persistent corpse motion, and - target-frame Keep in View with retained manual orbit. See - `docs/research/2026-07-11-combat-live-gate-corrections-pseudocode.md`. + meter, silent AttackDone control status, target-frame Keep in View with + retained manual orbit, persistent corpse motion protected from the AP-80 + velocity adaptation, and post-death nearest-target acquisition when Auto + Target is enabled. See `docs/research/2026-07-12-death-and-auto-target-pseudocode.md`. - **L.1b** — Command router + motion-state cleanup (prereq for L.1c). **Freeze on landing:** diff --git a/docs/research/2026-07-12-death-and-auto-target-pseudocode.md b/docs/research/2026-07-12-death-and-auto-target-pseudocode.md new file mode 100644 index 00000000..f1fcd8d9 --- /dev/null +++ b/docs/research/2026-07-12-death-and-auto-target-pseudocode.md @@ -0,0 +1,60 @@ +# Persistent death and post-death Auto Target pseudocode + +Sources: Sept 2013 EoR named retail client. + +- `ClientCombatSystem::AutoTarget` (`0x0056BC80`) +- `ClientCombatSystem::RecvNotice_SelectionChanged` (`0x0056BD80`) +- `CPlayerSystem::SelectNext` (`0x0055F9A0`) +- `MovementManager::unpack_movement` (`0x00524440`) +- `CPhysicsObj::MoveOrTeleport` (`0x00516330`) + +## Animation authority + +```text +on UpdateMotion(interpretedState): + pass the state through MovementManager and CMotionInterp + Dead is a persistent substate selected by the motion table + +on UpdatePosition(position, velocity): + update the physics pose and velocity + do not select Ready, Walk, Run, or any other animation +``` + +acdream retains a documented AP-80 adaptation for ACE actors that move only +through position snapshots: velocity may choose a visible locomotion cycle. +That adaptation is now restricted to replacing only Ready/Walk/Run-family +states. An authoritative action or substate such as attack, hit reaction, or +Dead always wins. This preserves the adaptation without allowing a late +position delta to make a corpse stand again. + +## Auto Target after the selected creature dies + +```text +when authoritative Dead motion is applied to the selected object: + clear selected object + publish SelectionChanged + +RecvNotice_SelectionChanged(): + update target tracking + if selected object is zero + and combat mode is Melee or Missile + and Auto Target is enabled: + AutoTarget() + +AutoTarget(): + if a recent valid last-attacked object exists: + select it + else: + SelectNext(forward=true, includeCurrent=true, COMPASS_ITEM) + +SelectNext(... COMPASS_ITEM) while in Melee/Missile: + reject non-attackable, fellowship, hidden, and out-of-radar candidates + rank eligible candidates by the retail weighted spatial distance + select the best candidate +``` + +The current M2 world-state projection has no retail `lastAttacked` instance-id +quality, so its existing closest eligible creature scan implements the +fallback branch. The important lifecycle behavior is exact: authoritative +death clears selection, and the SelectionChanged consumer reacquires only +when Auto Target is enabled in Melee/Missile combat. diff --git a/src/AcDream.App/Combat/CombatTargetController.cs b/src/AcDream.App/Combat/CombatTargetController.cs new file mode 100644 index 00000000..cdd739ac --- /dev/null +++ b/src/AcDream.App/Combat/CombatTargetController.cs @@ -0,0 +1,72 @@ +using AcDream.Core.Combat; +using AcDream.Core.Physics; +using AcDream.Core.Selection; + +namespace AcDream.App.Combat; + +/// +/// Owns combat-target lifecycle around the shared selection state. +/// +/// +/// Retail ClientCombatSystem::RecvNotice_SelectionChanged +/// (0x0056BD80) invokes AutoTarget when selection becomes zero while +/// Auto Target is enabled in Melee/Missile combat. Authoritative Dead motion +/// makes the selected combat object unavailable and enters that same notice +/// path. +/// +public sealed class CombatTargetController : IDisposable +{ + private readonly CombatState _combat; + private readonly SelectionState _selection; + private readonly Func _autoTarget; + private readonly Func _selectClosestTarget; + private bool _disposed; + + public CombatTargetController( + CombatState combat, + SelectionState selection, + Func autoTarget, + Func selectClosestTarget) + { + _combat = combat ?? throw new ArgumentNullException(nameof(combat)); + _selection = selection ?? throw new ArgumentNullException(nameof(selection)); + _autoTarget = autoTarget ?? throw new ArgumentNullException(nameof(autoTarget)); + _selectClosestTarget = selectClosestTarget + ?? throw new ArgumentNullException(nameof(selectClosestTarget)); + + _selection.Changed += OnSelectionChanged; + } + + /// + /// Called after an accepted UpdateMotion has reached the entity's motion + /// table. Dead is a persistent substate, so CurrentMotion is the + /// authoritative availability signal rather than a damage prediction. + /// + public void OnMotionApplied(uint objectId, uint currentMotion) + { + if (currentMotion != MotionCommand.Dead + || _selection.SelectedObjectId != objectId) + return; + + _selection.Clear( + SelectionChangeSource.System, + SelectionChangeReason.CombatTargetDied); + } + + public void Dispose() + { + if (_disposed) return; + _disposed = true; + _selection.Changed -= OnSelectionChanged; + } + + private void OnSelectionChanged(SelectionTransition transition) + { + if (transition.SelectedObjectId is not null + || !_autoTarget() + || !CombatInputPlanner.SupportsTargetedAttack(_combat.CurrentMode)) + return; + + _selectClosestTarget(); + } +} diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 914dbb50..dafd9546 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -761,6 +761,7 @@ public sealed class GameWindow : IDisposable private AcDream.App.UI.UiHost? _uiHost; private AcDream.App.UI.RetailUiRuntime? _retailUiRuntime; private AcDream.App.Combat.CombatAttackController? _combatAttackController; + private AcDream.App.Combat.CombatTargetController? _combatTargetController; private AcDream.App.UI.ItemInteractionController? _itemInteractionController; private readonly AcDream.Core.Items.StackSplitQuantityState _stackSplitQuantity = new(); // Phase D.2b Sub-phase C Slice 2 — the 3-D doll viewport: an off-screen RTT renderer, the UiViewport @@ -1991,6 +1992,11 @@ public sealed class GameWindow : IDisposable == AcDream.Core.Combat.CombatInputPlanner.DualWieldCombatStyle, playerReadyForAttack: IsPlayerReadyForCombatAttack, autoRepeatAttack: () => _persistedGameplay.AutoRepeatAttack); + _combatTargetController = new AcDream.App.Combat.CombatTargetController( + Combat, + _selection, + autoTarget: () => _persistedGameplay.AutoTarget, + selectClosestTarget: () => SelectClosestCombatTarget(showToast: false)); // Phase D.2b retail-look retained UI (ACDREAM_RETAIL_UI=1). if (_options.RetailUi) @@ -5125,6 +5131,12 @@ public sealed class GameWindow : IDisposable } } + // Authoritative Dead motion invalidates a selected combat target. + // The controller clears shared selection, whose SelectionChanged + // consumer ports retail's post-clear AutoTarget behavior. + _combatTargetController?.OnMotionApplied( + update.Guid, ae.Sequencer.CurrentMotion); + // CRITICAL: when we enter a locomotion cycle (Walk/Run/etc), // stamp the _remoteLastMove timestamp to "now". Without this, // the stop-detection loop in TickAnimations sees the previous @@ -5286,12 +5298,6 @@ public sealed class GameWindow : IDisposable $"[setstate] guid=0x{parsed.Guid:X8} entityId=0x{registryKey:X8} state=0x{parsed.PhysicsState:X8} instSeq={parsed.InstanceSequence} stateSeq={parsed.StateSequence}")); } - private static bool IsRemoteLocomotion(uint motion) - { - uint low = motion & 0xFFu; - return low is 0x05 or 0x06 or 0x07 or 0x0F or 0x10; - } - private void ApplyServerControlledVelocityCycle( uint serverGuid, AnimatedEntity ae, @@ -5328,11 +5334,16 @@ public sealed class GameWindow : IDisposable return; } + uint currentMotion = ae.Sequencer.CurrentMotion; + // AP-80 is a position-only compatibility adaptation, not an + // animation authority. Restrict it to Ready/Walk/Run so late death + // position deltas cannot replace the authoritative Dead substate. + if (!AcDream.Core.Physics.ServerControlledLocomotion + .CanApplyVelocityCycle(currentMotion)) + return; + var plan = AcDream.Core.Physics.ServerControlledLocomotion .PlanFromVelocity(velocity); - uint currentMotion = ae.Sequencer.CurrentMotion; - if (!plan.IsMoving && !IsRemoteLocomotion(currentMotion)) - return; uint style = ae.Sequencer.CurrentStyle != 0 ? ae.Sequencer.CurrentStyle @@ -9949,11 +9960,10 @@ public sealed class GameWindow : IDisposable // stop signal flows the same way as any other motion change: // alt releases W → MoveToState(Ready) → ACE broadcasts // UpdateMotion(Ready) + UpdatePosition with zero velocity. - // On our side, both are handled: UpdateMotion routes through - // MotionInterpreter.DoInterpretedMotion which zeroes the - // interpreter's state, and UpdatePosition's HasVelocity~0 hits - // MotionInterpreter.StopCompletely. Anything else is server - // buggery (packet loss, ACE bug) — don't guess client-side. + // On our side, only UpdateMotion changes interpreted state; + // UpdatePosition updates physics pose/velocity and never selects an + // animation. Anything else is server buggery (packet loss, ACE bug) + // — don't guess client-side. var now = System.DateTime.UtcNow; foreach (var kv in _animatedEntities) @@ -12575,6 +12585,11 @@ public sealed class GameWindow : IDisposable if (!_entitiesByServerGuid.ContainsKey(guid)) return false; + if (_entitiesByServerGuid.TryGetValue(guid, out var entity) + && _animatedEntities.TryGetValue(entity.Id, out var animated) + && animated.Sequencer?.CurrentMotion == AcDream.Core.Physics.MotionCommand.Dead) + return false; + return (LiveItemType(guid) & AcDream.Core.Items.ItemType.Creature) != 0; } @@ -13616,6 +13631,8 @@ public sealed class GameWindow : IDisposable // UI resource is still alive. UiHost disposes controllers before its renderer. _retailUiRuntime?.Dispose(); _retailUiRuntime = null; + _combatTargetController?.Dispose(); + _combatTargetController = null; _combatAttackController?.Dispose(); _combatAttackController = null; _uiHost = null; diff --git a/src/AcDream.Core/Physics/ServerControlledLocomotion.cs b/src/AcDream.Core/Physics/ServerControlledLocomotion.cs index 6444b3da..9bfa3f03 100644 --- a/src/AcDream.Core/Physics/ServerControlledLocomotion.cs +++ b/src/AcDream.Core/Physics/ServerControlledLocomotion.cs @@ -61,6 +61,21 @@ public static class ServerControlledLocomotion true); } + /// + /// Limits the AP-80 position-velocity adaptation to the locomotion state + /// family it exists to supply. Retail UpdatePosition never selects a + /// motion at all; therefore an authoritative action/substate such as Dead + /// must never be replaced by this adaptation. + /// + public static bool CanApplyVelocityCycle(uint currentMotion) + => currentMotion == MotionCommand.Ready || IsLocomotion(currentMotion); + + public static bool IsLocomotion(uint motion) + { + uint low = motion & 0xFFu; + return low is 0x05 or 0x06 or 0x07 or 0x0F or 0x10; + } + public readonly record struct LocomotionCycle( uint Motion, float SpeedMod, diff --git a/src/AcDream.Core/Selection/SelectionState.cs b/src/AcDream.Core/Selection/SelectionState.cs index de3d51be..e9581011 100644 --- a/src/AcDream.Core/Selection/SelectionState.cs +++ b/src/AcDream.Core/Selection/SelectionState.cs @@ -19,6 +19,7 @@ public enum SelectionChangeReason Selected, Cleared, SelectedObjectRemoved, + CombatTargetDied, PreviousSelection, } diff --git a/tests/AcDream.App.Tests/Combat/CombatTargetControllerTests.cs b/tests/AcDream.App.Tests/Combat/CombatTargetControllerTests.cs new file mode 100644 index 00000000..1ef586bf --- /dev/null +++ b/tests/AcDream.App.Tests/Combat/CombatTargetControllerTests.cs @@ -0,0 +1,82 @@ +using AcDream.App.Combat; +using AcDream.Core.Combat; +using AcDream.Core.Physics; +using AcDream.Core.Selection; + +namespace AcDream.App.Tests.Combat; + +public sealed class CombatTargetControllerTests +{ + [Fact] + public void DeadSelectedTarget_AutoTargetEnabled_SelectsClosestReplacement() + { + var combat = new CombatState(); + combat.SetCombatMode(CombatMode.Melee); + var selection = new SelectionState(); + selection.Select(0x50000001u, SelectionChangeSource.World); + int calls = 0; + using var controller = new CombatTargetController( + combat, + selection, + autoTarget: () => true, + selectClosestTarget: () => + { + calls++; + selection.Select(0x50000002u, SelectionChangeSource.System); + return 0x50000002u; + }); + + controller.OnMotionApplied(0x50000001u, MotionCommand.Dead); + + Assert.Equal(1, calls); + Assert.Equal(0x50000002u, selection.SelectedObjectId); + } + + [Fact] + public void DeadSelectedTarget_AutoTargetDisabled_LeavesSelectionClear() + { + var combat = new CombatState(); + combat.SetCombatMode(CombatMode.Melee); + var selection = new SelectionState(); + selection.Select(0x50000001u, SelectionChangeSource.World); + int calls = 0; + using var controller = new CombatTargetController( + combat, selection, () => false, () => { calls++; return null; }); + + controller.OnMotionApplied(0x50000001u, MotionCommand.Dead); + + Assert.Equal(0, calls); + Assert.Null(selection.SelectedObjectId); + } + + [Fact] + public void DeadSelectedTarget_NonCombatMode_DoesNotAutoTarget() + { + var combat = new CombatState(); + var selection = new SelectionState(); + selection.Select(0x50000001u, SelectionChangeSource.World); + int calls = 0; + using var controller = new CombatTargetController( + combat, selection, () => true, () => { calls++; return null; }); + + controller.OnMotionApplied(0x50000001u, MotionCommand.Dead); + + Assert.Equal(0, calls); + Assert.Null(selection.SelectedObjectId); + } + + [Fact] + public void DeadUnselectedObject_DoesNotDisturbCurrentTarget() + { + var combat = new CombatState(); + combat.SetCombatMode(CombatMode.Missile); + var selection = new SelectionState(); + selection.Select(0x50000002u, SelectionChangeSource.World); + using var controller = new CombatTargetController( + combat, selection, () => true, () => throw new InvalidOperationException()); + + controller.OnMotionApplied(0x50000001u, MotionCommand.Dead); + + Assert.Equal(0x50000002u, selection.SelectedObjectId); + } +} diff --git a/tests/AcDream.Core.Tests/Physics/ServerControlledLocomotionTests.cs b/tests/AcDream.Core.Tests/Physics/ServerControlledLocomotionTests.cs index 9ea2ca04..9b494254 100644 --- a/tests/AcDream.Core.Tests/Physics/ServerControlledLocomotionTests.cs +++ b/tests/AcDream.Core.Tests/Physics/ServerControlledLocomotionTests.cs @@ -52,4 +52,18 @@ public sealed class ServerControlledLocomotionTests Assert.Equal(MotionCommand.RunForward, plan.Motion); Assert.Equal(ServerControlledLocomotion.MaxSpeedMod, plan.SpeedMod); } + + [Theory] + [InlineData(MotionCommand.Ready)] + [InlineData(MotionCommand.WalkForward)] + [InlineData(MotionCommand.RunForward)] + public void CanApplyVelocityCycle_AllowsOnlyLocomotionFamily(uint motion) + => Assert.True(ServerControlledLocomotion.CanApplyVelocityCycle(motion)); + + [Theory] + [InlineData(MotionCommand.Dead)] + [InlineData(0x10000058u)] // ThrustMed action + [InlineData(0x10000051u)] // Twitch1 hit reaction + public void CanApplyVelocityCycle_PreservesAuthoritativeNonLocomotion(uint motion) + => Assert.False(ServerControlledLocomotion.CanApplyVelocityCycle(motion)); }