acdream/docs/ISSUES.md
Erik b153bbe5ad feat(player): #6 fold enchantment buffs into vital max via EnchantmentMath
Ports CEnchantmentRegistry::EnchantAttribute (PDB 0x00594570, see
docs/research/named-retail/acclient_2013_pseudo_c.txt line 416110).
The retail formula:

    real_max = (vital.(ranks+start) + attribute_contribution) * mult_buff + add_buff
    clamp >= 5 if base >= 5 else >= 1

is now applied in LocalPlayerState.GetMaxApprox.

EnchantmentMath.GetMod(activeEnchantments, table, statKey)
  - Family-stacking dedup via SpellTable.Family (only one buff per
    family-bucket wins, by highest spell-id as a generation proxy).
  - Family=0 means "no bucket" — each layer is its own bucket.
  - Returns (Multiplier, Additive) ready to apply.
  - StatKey constants: MaxHealth=1, MaxStamina=3, MaxMana=5
    (verified against named-retail/acclient.h line 37287-37301).

Spellbook.GetVitalMod(statKey) delegates to EnchantmentMath using
its constructor-injected SpellTable.

LocalPlayerState.GetMaxApprox now applies the full formula with
the min-vital floor (matches CreatureVital::GetMaxValue at PDB
0x0058F2DD). When Spellbook is null (back-compat), falls back to
Identity (no buff modification) — existing tests stay green.

GameWindow constructor wires SpellBook -> LocalPlayer so the chain
is complete in the live session.

Architecture in place; data still flat.

Until ISSUES.md #12 lands the wire-format extension that captures
StatMod (type/key/val) on ActiveEnchantmentRecord, the per-enchantment
modifier value isn't aggregated yet — GetMod returns Identity. Once
#12 wires the data, the existing aggregator + formula light up
automatically. Live +Acdream Stam/Mana will keep reading ~95% until
#12 lands.

6 new EnchantmentMathTests cover: empty list returns Identity,
no-table-entries returns Identity, stat-key constants match ACE,
Identity is (1, 0), family-stacking dedup, family=0 (no-bucket).

Total tests: 828 -> 834.

Closes #6 architecturally. Files #12 to track the wire-data follow-up.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 17:55:15 +02:00

254 lines
19 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# acdream — known issues + small deferred features
Rolling tactical list. What goes here:
- **Bugs**: user-visible defects we've observed but haven't fixed yet.
- **Small deferred features**: work that fits in one or two commits.
Anything larger should be a named Phase in the [roadmap](plans/2026-04-11-roadmap.md).
What does NOT go here:
- Large multi-commit work → add a Phase to the roadmap instead.
- Ideas / wishlist → `docs/plans/`.
- Design questions → open a `docs/research/*.md` note.
## Conventions
- Sequential integer IDs (`#1`, `#2`, …). Commits that close an issue reference the ID in the message (e.g. `fix #3: periodic TimeSync parsing`).
- `Status` is `OPEN`, `IN-PROGRESS`, or `DONE`. DONE items move to the **Recently closed** section at the bottom with closed-date + commit SHA.
- Every session: scan OPEN issues at start; promote/close anything we touched during the session before ending.
- Promoting to a Phase: mark as `DONE (promoted to Phase X)` + commit SHA where the Phase entry landed.
## Template
Copy this block when adding a new issue:
```
## #NN — Short title
**Status:** OPEN
**Severity:** HIGH | MEDIUM | LOW
**Filed:** YYYY-MM-DD
**Component:** e.g. sky, physics, net, ui
**Description:** One paragraph — what's wrong or what's missing.
**Root cause / status:** What we know so far. Empty if unknown.
**Files:** Path references with approximate line numbers.
**Research:** Links to `docs/research/*.md` if applicable.
**Acceptance:** How we'll know it's fixed.
```
---
# Active issues
## #1 — Rain falls only to horizon, not to the player's feet
**Status:** OPEN
**Severity:** MEDIUM
**Filed:** 2026-04-25
**Component:** weather / particles
**Description:** During Rainy DayGroups, rain particles are visible in the upper sky band but fade out before reaching the camera / ground level. Retail's rain falls all the way past the camera to the terrain.
**Root cause / status:** Unknown. Likely one of: (a) particle emitter volume too short in Z, (b) particle lifetime shorter than the time it takes to traverse emitter-top → ground, (c) emitter anchored in world-space so particles escape the player's reference frame as they fall, (d) camera-relative spawn origin is offset too high above the player.
**Files:**
- `src/AcDream.App/Rendering/GameWindow.cs``UpdateWeatherParticles` (~line 4591)
- `src/AcDream.Core/Vfx/ParticleSystem.cs` — emitter spawn config + lifetime integration
**Research:** `docs/research/deepdives/r12-weather-daynight.md` (rain mechanism — but does not pin volume / lifetime values).
**Acceptance:** Standing at 9,115 in Holtburg during a Rainy DayGroup, rain drops visibly fall all the way from the sky band past the camera to the ground level.
---
## #2 — Lightning visual not wired (dat-baked PES triggers)
**Status:** OPEN
**Severity:** MEDIUM
**Filed:** 2026-04-25
**Component:** weather / sky / vfx
**Description:** Retail's Rainy DayGroup in the Dereth Region dat contains 12+ `SkyObject` entries with non-zero `PesObjectId` and narrow visibility windows (570 ms at keyframe-boundary moments) that drive PhysicsScript-authored flash + thunder effects. We render the sky meshes but ignore the PES path, so no lightning flashes appear during storms. The fragment-shader flash bump on `uFogParams.z` is already wired in `sky.frag` — only the CPU-side PES→runner wire is missing.
**Root cause / status:** Research complete. Implementation is: in `SkyRenderer.Render`, detect visibility-window entry on any SkyObject with `obj.PesObjectId != 0`, call `PhysicsScriptRunner.Play(pesObjectId, ownerId: sky-owner, anchorPos: camera)`, and route any `SetFlash` / `Sound` hooks from the script into `uFogParams.z` + audio.
**Files:**
- `src/AcDream.App/Rendering/Sky/SkyRenderer.cs` — add per-SkyObject PES dispatch inside the visibility loop
- `src/AcDream.Core/Vfx/PhysicsScriptRunner.cs` — already shipped (Phase 6a); exposes `Play(scriptId, entityId, anchorWorldPos)`
- `src/AcDream.Core/Lighting/SceneLightingUbo.cs``FogParams.Z` is the flash slot; needs a sink that bumps it and decays
- `src/AcDream.App/Rendering/Shaders/sky.frag` — flash bump already wired (`rgb += flash * vec3(1.5, 1.5, 1.8)`)
**Research:**
- `docs/research/2026-04-23-lightning-real.md` (decompile trace + dat discovery)
- `docs/research/2026-04-23-physicsscript.md` (runtime semantics)
- `docs/research/2026-04-23-lightning-crossfade.md` (crossfade mechanism)
**Acceptance:** During a Rainy DayGroup's storm window, visible flashes appear in the sky at the dat-scripted moments, the fragment-shader flash bump briefly brightens the scene, and (later, once thunder audio is wired) a thunder clap plays with a short propagation delay.
---
## #3 — Client clock drifts from retail after ~10 minutes (periodic TimeSync missing)
**Status:** OPEN
**Severity:** MEDIUM
**Filed:** 2026-04-25
**Component:** net / sky
**Description:** Our `WorldTimeService.DayFraction` syncs with the server once at login via `ConnectRequest + TimeSync`, then advances from the local wall-clock. Retail receives periodic `TimeSync` refreshes (header flag `0x1000000`) carrying a fresh `PortalYearTicks double` and re-anchors its clock. Without those, acdream's keyframe state drifts from retail's over 10+ minutes — observed during the 2026-04-24 sky-color debug sessions where retail was at DayFraction 0.976 while acdream was at 0.634.
**Root cause / status:** Mechanism is well-understood (see research). `WorldTimeService.SyncFromServer(double)` already exists — we just need to detect the periodic flag in the packet header and call it whenever a fresh tick arrives.
**Files:**
- `src/AcDream.Core.Net/WorldSession.cs` — header-flag parsing; currently only the initial sync is consumed
- `src/AcDream.Core/World/WorldTimeService.cs``SyncFromServer(double ticks)` ready; needs caller wiring
**Research:** `docs/research/deepdives/r12-weather-daynight.md` §TimeSync (line ~563). References retail packet-header flag `0x1000000` carrying `PortalYearTicks double`.
**Acceptance:** Probe retail via `tools/RetailTimeProbe` and acdream's ACDREAM_DUMP_SKY log at the same wall-clock moment after a 20-minute session without re-login; `abs(acdream.DayFraction - retail.DayFraction) < 0.01`.
---
## #12 — Capture full Enchantment wire payload (StatMod) on ActiveEnchantmentRecord
**Status:** OPEN
**Severity:** LOW (architecture for buff aggregation is in place via #6 / Phase G; this is the wire-data extension that lights it up)
**Filed:** 2026-04-25
**Component:** net / spells
**Description:** `ParseMagicUpdateEnchantment` currently reads only the first 16 bytes of the 0x02C2 payload (`SpellId, LayerId, Duration, CasterGuid`) and ignores the trailing fields including `_smod` (the StatMod triad). `EnchantmentMath.GetMod` consequently returns `(1.0, 0.0)` for every stat key — the architecture is wired (see `Spellbook.GetVitalMod`, `LocalPlayerState.GetMaxApprox`) but no actual buff modifier flows through. Until this lands, the `+Acdream` Stam/Mana percent will continue to read ~95% (vs. retail's 100%) when buffs are active.
**Root cause / status:** Holtburger `crates/holtburger-protocol/src/messages/magic/types.rs` documents the full 60-64 byte Enchantment wire format: `u16 spell_id, u16 layer, u16 spell_category, u16 has_spell_set_id, u32 power_level, f64 start_time, f64 duration, u32 caster_guid, f32 degrade_modifier, f32 degrade_limit, f64 last_time_degraded, u32 stat_mod_type, u32 stat_mod_key, f32 stat_mod_value, [u32 spell_set_id]?`. Note the wire format starts with `u16` not `u32` for spell_id / layer / category — our existing parser may be reading garbage. Verify against actual ACE traffic via `ACDREAM_DUMP_VITALS=1`.
**Files:**
- `src/AcDream.Core.Net/Messages/GameEvents.cs` — extend `ParseMagicUpdateEnchantment` + `EnchantmentSummary`.
- `src/AcDream.Core/Spells/Spellbook.cs` — extend `ActiveEnchantmentRecord` with `StatModType`, `StatModKey`, `StatModValue`; extend `OnEnchantmentAdded` arity.
- `src/AcDream.Core/Spells/EnchantmentMath.cs` — uncomment the StatMod-aware aggregator inside `GetMod`.
**Research:** holtburger `messages/magic/types.rs:40` (`Enchantment::unpack`); `docs/research/named-retail/acclient.h` line 37406+ (`Enchantment` + `StatMod`).
**Acceptance:** With `+Acdream` logged in and standard self-buffs active, `LocalPlayer.StaminaPercent` rises from ~95% to a percent that matches retail's reading. New tests cover: parse round-trip with synthetic Enchantment payload; StatMod aggregation in `EnchantmentMath` (multiplicative + additive + family-stacking).
---
---
## #7 — PlayerDescription parser stops after spells (options/inventory/equipped not extracted)
**Status:** OPEN
**Severity:** LOW (Issue #5 needed only the early sections; later panels will need the rest)
**Filed:** 2026-04-25
**Component:** net / player-state
**Description:** Current `PlayerDescriptionParser` walks through `Attribute / Skill / Spell` vector-flag blocks per holtburger but stops before the trailing options / shortcuts / hotbars / desired_comps / spellbook_filters / options2 / gameplay_options / inventory / equipped sections. Future panels (hotbar, inventory, character options) need that data; the parser will need extension. Holtburger's events.rs:462-625 has the full layout; the messy parts are `gameplay_options` (variable-length opaque blob requiring heuristic skip) and `desired_comps`.
**Root cause / status:** Just a scope decision — port-the-easy-bits-first approach. Reference shape lives in holtburger's full `unpack`; the only complex piece is `find_inventory_start_after_gameplay_options` (heuristic alignment search).
**Files:**
- `src/AcDream.Core.Net/Messages/PlayerDescriptionParser.cs` — extend `Parsed` record + walker.
- `tests/AcDream.Core.Net.Tests/PlayerDescriptionParserTests.cs` — add coverage with synthetic payloads of the trailing sections.
**Research:** holtburger `crates/holtburger-protocol/src/messages/player/events.rs:462-625` (full unpacker including the heuristic `find_inventory_start_after_gameplay_options`).
**Acceptance:** All sections of a real-world PlayerDescription parse to completion — verified via a packet capture or by feeding synthetic test fixtures covering every flag combination.
---
## #4 — Sky horizon-glow disabled (fog-mix skipped on sky meshes)
**Status:** OPEN
**Severity:** LOW (aesthetic feature-parity, not regression from pre-session state)
**Filed:** 2026-04-25
**Component:** sky
**Description:** Phase 8.1 (commit `593b76f`) disabled the fog-mix on sky meshes to fix the "entire dome swallowed by fog color" regression. Dereth's keyframe `FogEnd` values (02400 m) are calibrated for terrain; sky meshes are authored at radii 105014271 m so every sky pixel was past `FogEnd`, saturated to `uFogColor`, destroying stars / moon / dome texture. Disabling the mix restored visibility but we lost retail's horizon-glow effect (gradient from clear zenith to fog-tinted horizon band at dusk/dawn).
**Root cause / status:** Three competing hypotheses, none pinned down: (a) retail uses a **different** fog range for sky than terrain; (b) retail applies fog with an **elevation-angle** weighting rather than linear distance; (c) retail's sky meshes **don't participate** in the global fog and the "horizon glow" comes from a different atmospheric-scatter path. Need to identify retail's actual sky-fog behaviour before re-enabling with correct parameters.
**Files:**
- `src/AcDream.App/Rendering/Shaders/sky.frag` — line ~55, `rgb = mix(uFogColor.rgb, rgb, vFogFactor)` currently commented out
- `src/AcDream.App/Rendering/Shaders/sky.vert` — lines 109-114, `vFogFactor` computation
**Research:** `docs/research/2026-04-23-sky-fog.md`. Partial; doesn't pin the sky-specific fog path.
**Acceptance:** At dusk in Holtburg, the sky dome shows a clear zenith and a warm fog-tinted horizon band that matches retail's appearance, with stars / moon / sun / clouds all still visible at their correct brightnesses elsewhere in the frame.
---
---
# Recently closed
## #6 — [DONE 2026-04-25 architecture; data follow-up #12] Vital max ignores enchantment buffs + vitae
**Closed:** 2026-04-25
**Commit:** `feat(player): #6 fold enchantment buffs into vital max via EnchantmentMath`
**Resolution:** Ported `CEnchantmentRegistry::EnchantAttribute` (PDB `0x00594570`) as `EnchantmentMath.GetMod(IEnumerable<ActiveEnchantmentRecord>, SpellTable, statKey)` returning `(Multiplier, Additive)`. Family-stacking dedup via `SpellTable.Family` (only one buff per family bucket wins, by highest spell-id as a generation proxy). `Spellbook.GetVitalMod(statKey)` delegates. `LocalPlayerState.GetMaxApprox` reworked to apply `(unbuffed × mult) + add` with retail's min-vital clamp (`>= 5` if base ≥ 5 else `>= 1`, matches `CreatureVital::GetMaxValue` at PDB `0x0058F2DD`). Stat-key constants (`MaxHealth=1`, `MaxStamina=3`, `MaxMana=5`) verified against `docs/research/named-retail/acclient.h` line 37287-37301.
**Architecture in place; data still flat.** Until ISSUES.md #12 lands the wire-format extension that captures `StatMod (type/key/val)` on `ActiveEnchantmentRecord`, the per-enchantment modifier value isn't aggregated yet — `EnchantmentMath.GetMod` returns `Identity (1.0, 0.0)` for every stat key. Once #12 wires the data, the existing aggregator + formula light up automatically. Live `+Acdream` Stam/Mana percent will continue to read ~95% until #12 lands.
6 new EnchantmentMathTests cover: empty list returns Identity, no-table-entries returns Identity, stat-key constants match ACE enum, Identity is `(1, 0)`, family-stacking dedup, family=0 (no-bucket) treated as separate.
---
## #11 — [DONE 2026-04-25] Spell metadata loader (spells.csv → SpellTable)
**Closed:** 2026-04-25
**Commit:** `feat(spells): #11 SpellTable — hydrate metadata from spells.csv at startup`
**Resolution:** Added `SpellMetadata` record + `SpellTable` CSV loader (hand-rolled RFC 4180-ish parser for the quoted Description column with embedded commas). Wired into `Spellbook` constructor as optional metadata source; `Spellbook.TryGetMetadata(spellId, out)` returns the static record when found. `GameWindow` loads `data/spells.csv` from bin output at construction (file copied via `<None Include>` in `AcDream.App.csproj` from `docs/research/data/spells.csv`). Falls back to `SpellTable.Empty` + console warning if the file is missing (e.g. tooling contexts). 10 new tests covering: empty table, header-only, simple row, quoted description with commas, blank lines skipped, bad spell-id rows skipped, lookup hit/miss, RFC 4180 escaped-quote parsing.
---
## #9 — [DONE 2026-04-25] Address-correction sweep on `acclient_function_map.md`
**Closed:** 2026-04-25
**Commit:** `docs(research): #9 sweep acclient_function_map.md against PDB symbols`
**Resolution:** Wrote `tools/pdb-extract/check_function_map.py` that cross-checks 63 hand-curated entries against `docs/research/named-retail/symbols.json`. Findings: **zero entries matched address-and-name exactly** (confirms ~0x800-0xC10 byte delta vs the binary that produced our Ghidra chunks — different build revision). 38 entries corrected by PDB name lookup; 25 entries either lack PDB symbol records (inlined / non-public) or had wrong class assignments (e.g. `0x5387C0` claimed as `CTransition::find_collisions` was actually `CPolygon::polygon_hits_sphere`). Updated `acclient_function_map.md` with corrected addresses, kept legacy addresses in a "Was" column for traceability, added a top-of-file sweep summary.
---
## #10 — [DONE 2026-04-25] Wire `KillerNotification (0x01AD)`
**Closed:** 2026-04-25
**Commit:** `docs(issues): #8/#9/#11 filed; #10 wired (KillerNotification)`
**Resolution:** Orphan parser at `GameEvents.ParseKillerNotification` existed but was never registered for dispatch in `GameEventWiring.cs`. Added a `combat.OnKillerNotification(victimName, victimGuid)` method on `CombatState` that fires a new `KillLanded` event, then registered the handler. One-line dispatch + 12-line CombatState method + one regression test fixture in `GameEventWiringTests`.
---
## #8 — [DONE 2026-04-25] pdb-extract tool: PDB → symbols.json + types.json
**Closed:** 2026-04-25
**Commit:** `tools(pdb-extract): #8 PDB -> symbols.json + types.json sidecar`
**Resolution:** Pure-Python (no deps) MSF 7.00 PDB parser at `tools/pdb-extract/pdb_extract.py`. Reads `refs/acclient.pdb` (Sept 2013 EoR build), extracts S_PUB32 records from the symbol stream + named class/struct types from TPI, and writes JSON sidecars to `docs/research/named-retail/`:
- `symbols.json` — 18,366 named functions (`address` + demangled `name` + raw `mangled`)
- `types.json` — 5,371 named class/struct records (`name` + `size` + `kind`)
Best-effort MSVC C++ demangler handles the common `?Method@Class@@<sig>` patterns + ctors (`??0`) + dtors (`??1`); operator overloads and vtables left mangled. Spot-check verified: `CEnchantmentRegistry::EnchantAttribute` resolves to `0x00594570` exactly as the discovery agent reported. Runtime <1s.
Regen workflow: `py tools/pdb-extract/pdb_extract.py refs/acclient.pdb`. The committed JSON outputs are stable + ~3 MB combined; ripgrep/jq on them is faster than re-parsing.
---
## #5 — [DONE 2026-04-25] VitalsPanel stamina/mana bars always null
**Closed:** 2026-04-25
**Commit:** `feat(player): #5 PlayerDescription parser — Stam/Mana via attribute block`
**Resolution:** First attempt (commit `d42bf57`) used `AppraiseInfoParser` for `PlayerDescription (0x0013)` wrong wire format. ACE source confirmed via `GameEventPlayerDescription.WriteEventBody`: PlayerDescription is hand-written (DescriptionPropertyFlag-driven property hashtables, vector flags, attribute block, skills, spells, options/inventory tail) distinct from `IdentifyObjectResponse (0x00C9)`'s `AppraiseInfo.Write`. Pivoted to a real port: new `PlayerDescriptionParser.cs` that walks property hashtables (Int32/Int64/Bool/Double/String/Did/Iid + Position) gated on the property flags, then reads vector flags + has_health + the attribute block where vitals 7/8/9 carry `ranks/start/xp/current`. Also redesigned `LocalPlayerState` to track per-vital snapshots (replacing the sentinel-API of attempt 1) plus per-attribute snapshots, with `GetMaxApprox` applying the retail formula `vital.(ranks+start) + attribute_contribution` (Endurance/2 for Health, Endurance for Stamina, Self for Mana). Live verified: `+Acdream` shows three bars; ~95% reading on Stam/Mana traced to active buff multipliers (filed as #6). Wire-port also added `PrivateUpdateVital (0x02E7)` + `PrivateUpdateVitalCurrent (0x02E9)` for delta updates per holtburger `UpdateVital`. ~700 LOC C#, 30+ new tests.
<!--
Example:
## #0 — [DONE 2026-04-24 · 593b76f] Sky cube edges visible as cross in daytime sky
**Closed:** 2026-04-24
**Commit:** `593b76f sky(phase-8.1): CLAUDE_TO_EDGE on static sky meshes`
**Resolution:** Switched to `GL_CLAMP_TO_EDGE` wrap mode for static sky
meshes; scrolling cloud layers kept `GL_REPEAT`. The 5 dome walls were
sampling opposite-edge pixels via UV wrap + LINEAR filtering, producing
visible seam lines that formed a cube outline across the view.
-->