From bfc5e473654a1686bec298f932e38d558ddee702 Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 24 Jul 2026 11:52:31 +0200 Subject: [PATCH] docs(issues): file #236-#244 - deferred P2 findings from the 2026-07-24 audit review Nine tactical entries for verified findings the review chose not to fix inline: UiText per-frame re-wrap (#236), Transition per-resolve allocation graph (#237, pooling gated on Slice I lifetime tests), EquippedChildRenderController double-tick + ToArray snapshots (#238), inbound per-packet allocation chain (#239, Slice H-c scope), RetailAnimationLoader unbounded cache (#240), InteriorEntityPartition landblock frustum cull reusing the diagnostics AABB results (#241), publication third-dictionary + per-attempt re-sort (#242), _boundsCache bound (#243), SequencerFactory unguarded Setup probe + [dat-miss] under-lock logging (#244). Co-Authored-By: Claude Fable 5 --- docs/ISSUES.md | 232 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 232 insertions(+) diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 4a71252e..2cc0ad39 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -250,6 +250,238 @@ hooks, collision, landing, and movement-wire cadence remain unchanged. --- +## #236 — UiText re-runs full word-wrap shaping every visible frame + +**Status:** OPEN +**Severity:** MEDIUM +**Filed:** 2026-07-24 +**Component:** ui (retained) + +**Description:** `UiText.LinesProvider` is polled unconditionally on every +draw, and the appraisal / character-info / effects wire-ups re-run full +word-wrap from scratch per poll (string split + per-glyph measure walks + +concatenation), including re-shaping fixed captured strings +(`AppraisalUiController.cs:750, 814`) and rebuilding the entire character +report per frame (`CharacterController.cs:99`). Real CPU + GC cost on the +most commonly opened panels, multiplying with uncapped FPS. + +**Root cause / status:** Found by the 2026-07-24 audit review (missed by the +audit itself). The correct pattern already exists in-tree: +`ChatWindowController.GetTranscriptLines` caches on revision + wrap width + +font identity. Apply it to the appraisal/indicator/character paths. Mapped +to plan Slice H-a. + +**Files:** `src/AcDream.App/UI/UiText.cs:39,454`; +`src/AcDream.App/UI/Layout/IndicatorDetailText.cs:14-36`; +`src/AcDream.App/UI/Layout/ItemAppraisalReport.cs:137-183`; +`src/AcDream.App/UI/Layout/CharacterStatController.cs:297,306`. + +--- + +## #237 — PhysicsEngine allocates a fresh Transition graph per resolve call + +**Status:** OPEN +**Severity:** MEDIUM +**Filed:** 2026-07-24 +**Component:** physics + +**Description:** Every `ResolveWithTransition` call allocates +`new Transition()` → 7+ heap objects (ObjectInfo, SpherePath, CollisionInfo, +three Sphere[] pairs). Callers: local player per quantum, every moving +NPC/remote, projectiles, and the camera probe — per-tick GC pressure that +scales with active-mover count (combat/crowds), a scenario the audit's +dwell-heavy route under-measured. + +**Root cause / status:** 2026-07-24 audit review finding. Pooling is only +allowed under plan Slice I's identity/lifetime-test condition (the adjacent +`LiveEntityAnimationScheduler` scratch-reuse pattern is the precedent) — +Transition state must be provably reset-complete between movers or a pooled +graph corrupts collision state, which is worse than the allocation. + +**Files:** `src/AcDream.Core/Physics/PhysicsEngine.cs:995`; +`src/AcDream.Core/Physics/TransitionTypes.cs:360-367,694-698`. + +--- + +## #238 — EquippedChildRenderController ticks twice per frame with six ToArray snapshots + +**Status:** OPEN +**Severity:** MEDIUM +**Filed:** 2026-07-24 +**Component:** render / live entities + +**Description:** `Tick()` runs twice per host frame +(`LiveObjectFrameController.cs:204` pre-network and `:241` post-network via +the spatial reconciler), and each run's `RetryPendingProjectionTransitions` +takes up to six `.ToArray()` dictionary snapshots for iteration safety — +up to 12 short-lived arrays per frame while any equip transition is in +flight, plus the full parent-first pose-composition walk running twice for +every character with a visible equipped item. + +**Root cause / status:** 2026-07-24 audit review finding. The class already +has the allocation-free pattern (`AttachmentUpdateOrder`, +`_pendingProjectionChildrenScratch`). The double-tick needs its own look: +if the second tick exists for post-network reconciliation ordering, keep +the ordering and skip the redundant recomposition instead. Plan Slice H-a. + +**Files:** `src/AcDream.App/Rendering/EquippedChildRenderController.cs:265-310,1421-1429`; +`src/AcDream.App/Update/LiveObjectFrameController.cs:204,241`. + +--- + +## #239 — Inbound net path allocates 3-4 arrays/objects per packet + +**Status:** OPEN +**Severity:** LOW +**Filed:** 2026-07-24 +**Component:** net + +**Description:** Every inbound datagram allocates: the `UdpClient.Receive` +byte[], a second full copy into `Packet.BodyBytes` +(`PacketCodec.cs:58,64`), per-packet `Packet` + `List` + +`PacketHeaderOptional` objects, and a third per-FRAGMENT copy in +`MessageFragment.TryParse` (`MessageFragment.cs:41`) — scaling with +fragment count, which dominates during active play. The audit named only +the outbound `ToArray()` copy. + +**Root cause / status:** 2026-07-24 audit review finding. In scope for plan +Slice H-c (requires the §0.3 freeze authorization); invariants pinned in +the plan amendment (single outstanding receive, arrival order, +ack-per-packet). + +**Files:** `src/AcDream.Core.Net/NetClient.cs:63-78`; +`src/AcDream.Core.Net/Packets/PacketCodec.cs:58,64`; +`src/AcDream.Core.Net/Packets/MessageFragment.cs:41`. + +--- + +## #240 — RetailAnimationLoader caches every parsed Animation forever + +**Status:** OPEN +**Severity:** MEDIUM +**Filed:** 2026-07-24 +**Component:** content / animation + +**Description:** `RetailAnimationLoader._cache` +(`ConcurrentDictionary>`) has no bound or eviction; +each entry holds per-part per-keyframe transform + hook data (tens of KB). +A long-uptime session (or 30-bot fleet) that encounters progressively more +creature/object motion tables grows this monotonically — same class as the +audio caches bounded on 2026-07-24, and structurally incapable of the +plateau behavior the audit measured for entities/particles. + +**Root cause / status:** 2026-07-24 audit review finding. Bound with the +same LRU/byte-budget shape as `DatSoundCache` (2026-07-24) / +`BoundedDatObjectCache`. Note sequencers retain references to loaded +animations — eviction must not invalidate an Animation a live sequencer +still holds (they keep the object alive; the cache only drops its own +reference). + +**Files:** `src/AcDream.Content/Vfx/RetailAnimationLoader.cs:19-53`. + +--- + +## #241 — InteriorEntityPartition never uses its per-landblock AABBs to cull + +**Status:** OPEN +**Severity:** MEDIUM +**Filed:** 2026-07-24 +**Component:** render + +**Description:** `Partition` receives per-landblock `AabbMin`/`AabbMax` but +walks every near-tier landblock's full entity list every frame without a +landblock-level frustum test — while `WorldSceneDiagnosticsController` +computes exactly that AABB-vs-frustum answer every frame one call away and +uses it only for a diagnostics/checkpoint counter. Reusing it as a pre-cull +would skip most of the dense-town entity walk for landblocks outside the +frustum. + +**Root cause / status:** 2026-07-24 audit review finding (the audit flagged +the whole-world walk as P2; the free cull input was the missed half). Keep +the retail dynamics contract: out-of-flood dynamics are dropped by the +per-dynamic viewcone CULL in `DrawDynamicsLast`, not by set membership — +the landblock pre-cull must sit upstream of, not replace, that. Plan +Slice H-a / G. + +**Files:** `src/AcDream.App/Rendering/InteriorEntityPartition.cs:108-144`; +`src/AcDream.App/Rendering/WorldSceneDiagnosticsController.cs:169-201`. + +--- + +## #242 — Static publication rebuilds a third dictionary and re-sorts per completion attempt + +**Status:** OPEN +**Severity:** LOW +**Filed:** 2026-07-24 +**Component:** streaming + +**Description:** Beyond the two dictionaries + hashset + two sorts the audit +counted per landblock publication, `CompletePublication` also rebuilds +`_activeByLandblock[canonical]` wholesale via `ToDictionary`, and its +`OrderBy(...).ToArray()` re-runs on every completion ATTEMPT while the +plugin cursor persists across retries — per-retry amplification during +portal bursts. + +**Root cause / status:** 2026-07-24 audit review finding. Fold into the plan +Slice C/E cursor-receipt work: sort once at publication construction, store +the ordered array on the publication object. + +**Files:** `src/AcDream.App/Streaming/LandblockStaticPresentationPublisher.cs:261-296`. + +--- + +## #243 — ObjectMeshManager._boundsCache grows unbounded + +**Status:** OPEN +**Severity:** LOW +**Filed:** 2026-07-24 +**Component:** render + +**Description:** `_boundsCache` is an unbounded `ConcurrentDictionary` +memoizing GetBounds results for the session's lifetime — small per entry, +but the one unbounded container in a class where every other cache is +deliberately budgeted, and another no-plateau curve for long-uptime +sessions. + +**Root cause / status:** 2026-07-24 audit review finding. Bound with the +existing entry-count LRU shape; plan Slice D's accounting covers it. + +**Files:** `src/AcDream.App/Rendering/Wb/ObjectMeshManager.cs` (`_boundsCache`, +insert site ~1757). + +--- + +## #244 — SequencerFactory probes Setup without a DID guard or catch; [dat-miss] logs under the database lock + +**Status:** OPEN +**Severity:** MEDIUM +**Filed:** 2026-07-24 +**Component:** composition / content + +**Description:** Two residuals adjacent to the audit's ResolveActivation +finding: (1) `SequencerFactory` runs the identical unguarded +`Get(SourceGfxObjOrSetupId)` probe with NO try/catch — a live entity +carrying a GfxObj-typed source id would crash the spawn path outright +instead of degrading (live weenies conventionally carry Setup ids, which is +why it has never fired); a dead duplicate exists in +`RenderBootstrap.cs:179-206` (ui-studio only). (2) Non-throwing failed +probes hit the dated TEMP `[dat-miss]` `Console.WriteLine` +(`DatCollectionAdapter.cs:196-206`, "strip with fix, 2026-06-09") — +synchronous console I/O while holding `_databaseLock`, serializing all +readers of that dat. + +**Root cause / status:** 2026-07-24 audit review finding. Plan Slice C's +typed DID metadata removes the probe pattern; the Slice C amendment also +requires narrowing `ResolveActivation`'s blanket catch. The `[dat-miss]` +tripwire should be resolved (root-caused or demoted to a rate-limited +diagnostic) in the same change. + +**Files:** `src/AcDream.App/Composition/LivePresentationComposition.cs:200-225`; +`src/AcDream.Content/DatCollectionAdapter.cs:196-206`; +`src/AcDream.App/Rendering/RenderBootstrap.cs:179-206`. + +--- + ## #233 — Live skill-credit resolver used an ACE shortcut, not retail's formula **Status:** DONE (2026-07-22, GameWindow Slice 8 Checkpoint C)