Docs: 2026-08-13 session — combat freeze diagnosis + Track B allocation pass
Session log for the "freezes on a kill" diagnosis (the cause is the camera, not CPU: shake is integrated by the follow filter, the hold is frame-counted, and the hit + kill packages stack on the lethal frame) and for the Track B allocation work the operator selected. Gotchas archive gains nine dated entries, incl. two that changed how the work was ranked: GC.GetTotalMemory quantises to 4 KB and is useless for allocation attribution (use ProfilerRecorder), and an editor-only IMGUI dev tool self-spawns into Game.unity and dominated every editor-side measurement. CLAUDE.md: one new pooling/per-frame-cost line, paid for under the net-zero rule by dropping the DR-051 build-palette text (that code died with the purge), the two purge enumerations now carried by their DRs, and a duplicated ACES clause. 40183 bytes, under the 40960 budget. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,194 @@
|
||||
---
|
||||
date: 2026-08-13
|
||||
topic: Combat "freeze on kill" diagnosis + Track B (allocation / pooling)
|
||||
status: shipped (code uncommitted at time of writing)
|
||||
---
|
||||
|
||||
# 2026-08-13 — Combat freeze diagnosis + Track B allocation pass
|
||||
|
||||
Operator report: *"on a kill the combat feels like it freezes up the game and overall doesn't feel smooth."*
|
||||
|
||||
## Part 1 — Diagnosis (measured, not inferred)
|
||||
|
||||
Instrumented Play with an `EditorApplication.update` guardian injecting **real lethal `DamageEvent`s**
|
||||
server-side (the true death path — setting `Health.Current = 0` directly does NOT work, because
|
||||
`HealthApplyDamageSystem` skips any entity whose `DamageEvent` buffer is empty and so never stamps `Dying`).
|
||||
|
||||
**Ruled out by measurement:**
|
||||
|
||||
| Hypothesis | Verdict |
|
||||
|---|---|
|
||||
| CPU hitch on kill | **No.** Median frame 12.2 ms, p99 20.1 ms; a kill costs ~15 ms. |
|
||||
| Wave respawn stalls on the last kill | **No.** Enemies drip back ~1 per 29 frames, no spike. |
|
||||
| Corpses block movement for the 0.9 s `Dying` window | **No.** Enemies carry **no `PhysicsCollider` at all**. |
|
||||
| `Time.timeScale` abuse | **No.** The codebase is disciplined about this. |
|
||||
|
||||
**What it actually is — the camera.** With a *stationary* player (so ideal camera motion is exactly zero,
|
||||
making every measured value an artifact), one kill produces:
|
||||
|
||||
```
|
||||
f51 shake=0.301 hold=2 fovKick=2.20 dev=0.111 <- kill lands
|
||||
f52 step=0.283 hold=1 dev=0.269 <- 28 cm camera jump in ONE frame
|
||||
f58 dev=0.447 <- peak, 45 cm off-frame
|
||||
f75 dev=0.199 <- still 20 cm off, 24 frames later
|
||||
```
|
||||
|
||||
Three root causes, all in `PrototypeCameraRig.LateUpdate`:
|
||||
|
||||
1. **Shake is integrated by the follow filter.** `basePos = Vector3.Lerp(transform.position, desired, k)` reads
|
||||
back a `transform.position` that already contains last frame's random shake, so the smoother treats shake as
|
||||
real positional error and corrects only ~9 %/frame (`FollowSharpness 8` at 12 ms → k≈0.09). Shake is never
|
||||
subtracted, only slowly lerped out while new shake is added. In sustained combat the camera random-walks
|
||||
around its ideal framing and never settles. **This is the "doesn't feel smooth".**
|
||||
2. **The hit package and the kill package both fire on the lethal frame** — measured `shake = 0.301`
|
||||
(`HitShakeRemote 0.10` + `KillShake 0.20` stacking) and `fovKick = 2.20` (the hit punch winning over the
|
||||
kill's 1.0). One kill fires 2 SFX + 4 particle bursts + 2 light flashes + a damage number + 2 FOV punches +
|
||||
a shake + a hold + a rumble, simultaneously.
|
||||
3. **`PrototypeCameraRig.Hold()` freezes the follow, counted in FRAMES.** `HitStopMaxFrames = 2` on every kill,
|
||||
`FinisherHoldFrames = 7`. The comment claims "~117 ms" but that assumes 60 fps — it is 49 ms at 144 fps and
|
||||
233 ms at 30 fps. The hold branch also does `basePos = transform.position`, permanently baking in the shake
|
||||
offset (that is the 0.283-unit single-frame step at f52).
|
||||
|
||||
Plus a **one-off 41 ms spike on the first kill** (+44 KB) — first-use warmup of the death VFX/audio path;
|
||||
later kills cost ~15 ms.
|
||||
|
||||
A full remediation plan was presented (Track A camera, Track B allocation, Track C feel). **The operator chose
|
||||
Track B.** Track A and C remain open — see *Next session*.
|
||||
|
||||
## Part 2 — Track B shipped
|
||||
|
||||
### Measurement instrument matters (a retracted result)
|
||||
|
||||
The first attribution run used `System.GC.GetTotalMemory(false)` deltas per frame and produced a per-system
|
||||
table that was **entirely noise** — that API quantises to 4 KB pages, so every per-system median came back as 0
|
||||
or exactly one page and the "savings" column was negative nonsense. **Discarded.** `ProfilerRecorder(
|
||||
ProfilerCategory.Memory, "GC Allocated In Frame")` is the correct instrument and gives real bytes/frame.
|
||||
|
||||
The corrected bulk measurement then reframed the whole track:
|
||||
|
||||
| Condition | median B/frame | p90 |
|
||||
|---|---|---|
|
||||
| All on | 15 151 | 49 073 |
|
||||
| **All 29 `ProjectM.Client` systems OFF** | **11 586** | 12 125 |
|
||||
|
||||
With *every* client system disabled the editor still allocates ~11.6 KB/frame. Our entire presentation layer is
|
||||
only ~10–25 % of the median — but it owns the **spiky tail** (p90 49 KB → 12 KB with our systems off), and that
|
||||
tail is what triggers collections in bursts. A design-review critic also found the reason the floor is so high:
|
||||
`PixelArtDevControls` is an **editor-only** dev tool that self-spawns via `[RuntimeInitializeOnLoadMethod]` +
|
||||
`DontDestroyOnLoad` into **Game.unity**, and its `OnGUI` draws a `GUI.Button` on every IMGUI pass *before* its
|
||||
early-out. It is a MonoBehaviour, so disabling ECS systems never touched it. It does not ship — but it polluted
|
||||
every editor-side allocation number taken this session.
|
||||
|
||||
### B1 — pooled one-shot SFX (`OneShotAudioPool.cs`, new)
|
||||
|
||||
All 21 one-shot call sites funnel through the single line `FeedbackFx.PlayClip` →
|
||||
`AudioSource.PlayClipAtPoint`, which allocates a `GameObject` + `AudioSource` **per call** and schedules a
|
||||
delayed `Destroy` — ~20–33 calls/s in light combat. Replaced with a 32-voice ring; `PlayClip`'s signature is
|
||||
unchanged so **all 20 consuming call sites compile untouched**.
|
||||
|
||||
Parity traps the design review caught before they shipped:
|
||||
- **`spatialBlend = 1`** — a fresh `AudioSource` defaults to **2D**; `PlayClipAtPoint` is the only thing setting
|
||||
it. Missing it would silently make every combat cue non-positional.
|
||||
- **`dopplerLevel = 0`** — pooled voices *teleport* between events; at the stock `1` a 20 m jump pitch-bends the
|
||||
clip. This bug cannot exist when the source is created at the position and never moves.
|
||||
- **`DontDestroyOnLoad`** — `WorldLauncher` does `LoadScene(..., Single)` while the client world is alive; a
|
||||
scene-parented pool would be destroyed mid-session and every SFX would go silently dead.
|
||||
- **`[RuntimeInitializeOnLoadMethod(SubsystemRegistration)]` reset** — statics survive fast-enter-playmode but
|
||||
the `UnityEngine.Object`s they point at do not; session two would hold an array of destroyed voices and throw
|
||||
on the first cue.
|
||||
- Recycle deadline from the actual `clip.length` (cues span 0.05 s–0.45 s), steal-oldest when saturated,
|
||||
`pitch` reset per rent, `GameVolume.Sfx` read at play time (never cached, never double-applying master).
|
||||
|
||||
### B2 — pooled authored VFX (`CombatFeedbackSystem`)
|
||||
|
||||
Per-impact `Object.Instantiate`/`Destroy` replaced with a per-prefab pool (`RentVfx`/`FillVfx`/`ReturnVfx`).
|
||||
Traps handled, all from the risk pass:
|
||||
- Component arrays cached **per instance**, not per prefab (component refs are instance-scoped; arrays captured
|
||||
off the prefab asset would drive the asset).
|
||||
- `main.stopAction = None` forced at fill — a prefab set to `Destroy`/`Disable` would silently drain the pool.
|
||||
- Instances fill under an **inactive** root so `Awake`/`Start` never run, which is what makes `DestroyImmediate`
|
||||
in `StripCosmetic` safe (a deferred `Destroy` would hand out an instance still carrying a live Rigidbody +
|
||||
Collider for one frame).
|
||||
- `StripCosmetic` now disables **all** MonoBehaviours, not two name substrings — a pooled instance re-runs
|
||||
`OnEnable` on every rent, so a surviving helper would re-arm each time.
|
||||
- Transform (position/rotation/**scale**/parent) rewritten per rent; `ps.Clear(true)` before `Play` (a
|
||||
world-space system would re-show the previous burst); `TrailRenderer.Clear()` after the reposition.
|
||||
- `Rented` flag = at-most-once guard against a double `Return` aliasing one instance to two callers.
|
||||
- Returned by the prefab stored **on the record**, never a re-read of `VFXConfig` (an inspector swap mid-play
|
||||
would file it under the wrong effect). In-flight cap `MaxActiveVfx = 40` unchanged; separate retained cap
|
||||
`VfxPerPrefabRetain = 10`.
|
||||
|
||||
### B3 — per-frame allocation
|
||||
|
||||
- `BuildSlashInto` + `BuildDangerMesh`: four arrays each (~1.7 KB / ~840 B) allocated on **every** call — the
|
||||
first runs twice a frame while a swing arc is alive, the second once per winding enemy per frame. Hoisted to
|
||||
scratch fields; UVs/triangles are argument-independent so they now upload to a mesh only on its first fill.
|
||||
- `HudSystem`, `AbilityBarSystem`: unguarded per-frame `Label.text` builds gated on a changed value.
|
||||
- `CombatFeedbackSystem.AnimateNumbers`: legacy `TextMesh` bakes colour into vertex colours, so every colour
|
||||
write rebuilds the text mesh — fade quantised to 12 steps.
|
||||
- `EnemyHealthBarSystem`: per-bar uGUI writes epsilon-gated (an `anchorMax` write triggers
|
||||
`OnRectTransformDimensionsChange`).
|
||||
- `EnemyHitFlashSystem` / `NodeFeedbackSystem`: stopped playing back an **empty** `EntityCommandBuffer` (a
|
||||
structural-change sync point) every frame.
|
||||
- **`AudioClip` leak, closed across all seven clip-owning systems.** An `AudioClip.Create`d clip is a standalone
|
||||
`UnityEngine.Object` — destroying a system's FX-root does **not** take it with it, so every client-world
|
||||
teardown leaked its native audio buffer (`MusicSystem` alone ~6.8 MB, `AmbientAudioSystem` ~2 MB). Helper
|
||||
promoted to `FeedbackFx.DestroyClip(ref AudioClip)`.
|
||||
|
||||
## Validation
|
||||
|
||||
- **L1** console clean (0 errors) against the session baseline.
|
||||
- **L2** EditMode **304/304 green** (304 is the expected post-purge count, matching 2026-08-07).
|
||||
- **L3 live**, the structural proofs that are immune to editor noise:
|
||||
- **`oneShotAudioObjectsSeen = 0`** — `PlayClipAtPoint`'s signature `"One shot audio"` GameObject never
|
||||
appeared once across 270 frames of combat with kills.
|
||||
- VFX pool filled to its `VfxPerPrefabRetain = 10` cap and **stabilised** (bounded reuse, not churn).
|
||||
- Audio verified live end-to-end: real cues (`husk_hit`, `strike`, `growl_grunt` ×2) serviced through the
|
||||
ring within 2 s of a non-lethal injected hit.
|
||||
- Clean-run frame time **median 8.6 ms / p99 13.2 ms** (pre-change runs measured 12.2–13.3 ms / 20–21 ms).
|
||||
Caveat: the two harnesses are not byte-identical (the pre-change one built a string per frame), so treat
|
||||
the direction as solid and the exact delta as indicative.
|
||||
- Allocation medians stayed inside the editor noise band — expected, given our layer is a minority of it.
|
||||
**A player build is required for a true allocation number.**
|
||||
|
||||
### A false negative worth remembering
|
||||
|
||||
An intermediate audio check reported `framesWithAudio = 0/280` and looked exactly like a silent-audio
|
||||
regression. It was a **dead window** — the previous run had killed every enemy, so nothing was cueing. Direct
|
||||
probing (`OneShotAudioPool.Play` → `isPlaying = true`, plus `s_freeAt` showing four voices serviced in the last
|
||||
0.4 s) disproved it. *Rule: before believing a "feature is dead" measurement, prove the stimulus actually
|
||||
occurred.*
|
||||
|
||||
## Reviews
|
||||
|
||||
- **Pre-code audit** (4 lenses + completeness critic + refactor-risk verifier, 6/6 agents, 0 failures): produced
|
||||
the parity spec and the trap list above. Caught `spatialBlend`, Doppler, `LoadScene(Single)`, and the
|
||||
play-enter static reset **before** they shipped, plus the `PixelArtDevControls` measurement contaminant.
|
||||
- **Post-impl diff review** (3 lenses + 19 adversarial verifiers, 22/22 agents, 0 failures): **19 findings → 5
|
||||
confirmed**, all fixed:
|
||||
1. **Real regression I introduced** — `Mathf.RoundToInt` rounds **half-to-even** while `ToString("0.0")`
|
||||
rounds **half-away-from-zero**, so my ability-bar gate key and the string it guarded disagreed at
|
||||
midpoints; the label latched a stale, too-high reading and skipped a tenth on every cooldown. Fixed by
|
||||
single-sourcing the text from the quantised integer and switching to `CeilToInt` (a countdown should never
|
||||
read lower than the true remainder), with the branch bool cached so the format still switches at 597 ticks.
|
||||
2. The clip-leak fix was **half-closed** — only `CombatFeedbackSystem`'s ten clips. Completed across all seven
|
||||
owners.
|
||||
3–5. Two duplicated comment blocks left by structured edits, and a doc-accuracy nit on the ring-size rationale.
|
||||
|
||||
## Notes / loose ends
|
||||
|
||||
- **`Assets/_Project/Shaders/PixelOutline.mat` is modified in the working tree and is NOT mine to commit.**
|
||||
`PixelArtDevControls` writes straight to the shared material asset, and `_MasterEnabled` flipped 0→1 with
|
||||
posterize/normal-edges off during the Play sessions. Left untouched in case the toggle was deliberate;
|
||||
excluded from the Track B commits. It is a real look change — decide before committing.
|
||||
- **CLAUDE.md is at 39 850 / 40 960 bytes.** A condensation pass is due; this session's long-form lessons went
|
||||
to the gotchas archive rather than inline.
|
||||
|
||||
## Next session
|
||||
|
||||
1. **Track A — the camera** (this is what the operator's report actually describes, and it is still unfixed):
|
||||
split the base/shake channels, make the hold time-based and shake-safe, de-stack the lethal frame, prewarm
|
||||
the first-kill VFX. Open fork: keep the camera hold at all, or drop it and carry impact on FOV + shake.
|
||||
2. **Track C — feel**: enemies have **no collider** (you walk through them); FOV pumps on every hit; the 0.9 s
|
||||
corpse window wants re-judging once the camera is fixed.
|
||||
3. Measure allocation in a **player build** to get a number free of editor contamination.
|
||||
@@ -553,3 +553,66 @@ long forms are preserved here.
|
||||
shaking."* An `EditorApplication.update` trace quantified it instantly (y 1.157↔13.559, 7.43 units/tick; after
|
||||
the fix span 0.051, max step 0.004). Gate such effects on the presence of the owning component
|
||||
(`GetComponent<PrototypeCameraRig>()`), never on a scene name.
|
||||
|
||||
## 2026-08-13 — Combat-freeze diagnosis + Track B (allocation / pooling)
|
||||
|
||||
Session log: [[2026-08-13_Combat_Freeze_Diagnosis_Track_B_Allocation]]. Payment for the CLAUDE.md line added
|
||||
this session (net-zero rule): the six items below stay here, one condensed pointer lives inline.
|
||||
|
||||
1. **`GC.GetTotalMemory(false)` is USELESS for per-frame allocation attribution — it quantises to 4 KB pages.**
|
||||
A 29-system toggle sweep produced medians of exactly `0` or `4096` and a "savings" column of pure noise; I
|
||||
nearly shipped a refactor ranked off it. The correct instrument is
|
||||
`ProfilerRecorder.StartNew(ProfilerCategory.Memory, "GC Allocated In Frame")`, which reports real
|
||||
bytes/frame. Retract and re-measure rather than reasoning over a quantised signal.
|
||||
|
||||
2. **Attribute allocation by BULK toggle before ranking per-system work.** Disabling all 29 `ProjectM.Client`
|
||||
systems at once dropped the median from ~15.1 KB to ~11.6 KB/frame — i.e. the whole presentation layer was
|
||||
only ~10–25 % of the editor-side number, while owning the spiky tail (p90 49 KB → 12 KB). Per-system deltas
|
||||
were inside the noise; only the bulk delta was legible. Rank work off the bulk number, not the per-item one.
|
||||
|
||||
3. **★ An editor-only IMGUI dev tool can dominate your allocation measurement.** `PixelArtDevControls`
|
||||
self-spawns via `[RuntimeInitializeOnLoadMethod(AfterSceneLoad)]` + `DontDestroyOnLoad` into **Game.unity**
|
||||
(not just DevSandbox) and its `OnGUI` draws a `GUI.Button` on EVERY IMGUI pass *before* its `if (!_open)
|
||||
return`. It is a MonoBehaviour, so an ECS-system toggle sweep never touches it. Exclude it before
|
||||
attributing any editor-measured allocation to the game.
|
||||
|
||||
4. **★ A value-CHANGE gate must be single-sourced with the value it renders.** Gating a label on
|
||||
`Mathf.RoundToInt(x)` while the string is built by `ToString("0.0")` is a live bug: `Mathf.RoundToInt` is
|
||||
round-half-to-**EVEN**, custom numeric formats round half-**AWAY-from-zero**. They disagree at midpoints, so
|
||||
the gate latches and the label holds a stale, too-high reading and visibly skips a value. Derive the rendered
|
||||
text FROM the quantised key, never re-derive it from the raw input. For a countdown prefer `CeilToInt` so the
|
||||
readout never reads lower than the true remainder. (Caught by the post-impl review, not by 304 green tests.)
|
||||
|
||||
5. **Pooling an `AudioSource` is not a drop-in for `AudioSource.PlayClipAtPoint` — three properties bite.**
|
||||
(a) a fresh `AudioSource` defaults to `spatialBlend = 0` (**2D**); PlayClipAtPoint is the only thing setting
|
||||
it to 1, so a naive pool silently makes every cue non-positional and no test catches it. (b) `dopplerLevel`
|
||||
defaults to 1 and a pooled voice **teleports** between events — a 20 m jump pitch-bends the clip, a failure
|
||||
that cannot exist when the source is created at the position and never moves; set 0. (c) `playOnAwake`
|
||||
defaults true, which is harmless for a one-frame object and wrong for a long-lived one. Also: the pool root
|
||||
needs `DontDestroyOnLoad` (WorldLauncher does `LoadScene(..., Single)` while the client world is alive) and a
|
||||
`[RuntimeInitializeOnLoadMethod(SubsystemRegistration)]` reset, or session two holds destroyed voices.
|
||||
|
||||
6. **Pooling a ParticleSystem prefab: `SetActive(false)/(true)` does NOT reset it.** Rent order is transform →
|
||||
`SetActive(true)` → `ps.Clear(true); ps.Play(true)` (a world-space system otherwise re-shows the previous
|
||||
burst at its OLD positions) and `TrailRenderer.Clear()` AFTER the reposition (else a streak draws from the
|
||||
last despawn point). Force `main.stopAction = None` at fill — a prefab set to `Destroy` silently drains the
|
||||
pool. Cache component arrays **per instance** (component refs are instance-scoped; arrays captured off the
|
||||
prefab asset drive the asset). Fill under an **inactive** root so `Awake`/`Start` never run, which is what
|
||||
makes `DestroyImmediate` of the Rigidbody/Collider safe — a deferred `Destroy` would hand out an instance
|
||||
still carrying them for one frame. Guard `Return` with a `Rented` flag: a double return aliases one instance
|
||||
to two callers.
|
||||
|
||||
7. **An `AudioClip.Create`d clip is NOT owned by the GameObject that played it.** Destroying a system's FX-root
|
||||
in `OnDestroy` leaves every procedural clip alive — seven presentation systems here leaked their native audio
|
||||
buffers on every client-world teardown (`MusicSystem` ~6.8 MB, `AmbientAudioSystem` ~2 MB), growing unbounded
|
||||
across menu → game → menu cycles. Shared helper: `FeedbackFx.DestroyClip(ref AudioClip)`.
|
||||
|
||||
8. **Before believing a "the feature is dead" measurement, prove the STIMULUS occurred.** A 280-frame watch
|
||||
reported `framesWithAudio = 0` and looked exactly like a silent-audio regression from the new pool. It was a
|
||||
dead window — the prior run had killed every enemy, so nothing was cueing. Direct probing (a synthetic
|
||||
`PlayClip` → `isPlaying = true`, plus `s_freeAt` showing four voices serviced within 0.4 s) disproved it in
|
||||
one call. The same false-negative class as the 07-21 auto-recast retraction.
|
||||
|
||||
9. **Injecting a kill: append a lethal `DamageEvent`, never write `Health.Current = 0`.**
|
||||
`HealthApplyDamageSystem` early-`continue`s on an empty `DamageEvent` buffer, so a direct Health write is
|
||||
never seen by the death branch and no `Dying` is ever stamped.
|
||||
|
||||
Reference in New Issue
Block a user