From 0d8b82772106590e2ca3c1278df0d50f7389f143 Mon Sep 17 00:00:00 2001 From: Erik Date: Sat, 20 Jun 2026 15:51:55 +0200 Subject: [PATCH] =?UTF-8?q?fix(lighting):=20indoor=20interiors=20now=20lit?= =?UTF-8?q?=20=E2=80=94=20landblock-key=20bug=20+=20viewer=20light=20+=20w?= =?UTF-8?q?eenie=20fixtures?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The whole "indoor interiors read dark/flat vs retail" saga was ONE root-cause bug: EnvCellRenderer.GetCellLightSet derived the landblock key as `cellId & 0xFFFF0000` (0xXXYY0000), but landblocks are keyed by the streaming id 0xXXYYFFFF. The lookup missed for EVERY cell, so SelectForObject never ran and every EnvCell wall received ZERO point lights — torches, lanterns, the viewer light, all of it. Confirmed by a [cell-light] probe: inBounds=False selected=0 across 1M+ cell draws; after the fix inBounds=True selected=3-4. User-confirmed the interiors now look like retail. Three faithful additions that were blocked by the key bug (and only show now): - Viewer light (LightManager.UpdateViewerLight): retail's SmartBox::set_viewer (0x00452c40) adds a white fill light at the player every frame via add_dynamic_light — the dominant interior fill (no sun indoors). acdream had NO dynamic lights at all. Params from the cdb capture: intensity 2.25, falloff 10, white, offset (0,0,2). Indoor-only via the AP-43 gate. - Weenie fixture lights (OnLiveEntitySpawnedLocked): server-spawned lanterns/ braziers carry Setup.Lights but the dat-static registration never saw CreateObject entities. Register on spawn; unregister on despawn (UnregisterOwner made unconditional). Register row AP-44. - IndoorObjectReceivesTorches now excludes the 0xFFFF landblock marker (it is not an EnvCell) — fixes WbDrawDispatcherIndoorFlagTests.LandblockId_OutdoorFlag0 (a #142 verification miss). Divergence register: AP-44 (weenie light spawn-position, no movement tracking), AP-47 (acdream's 128-light/camera-independent cap keeps interiors always-lit vs retail's 40-nearest-to-player budget that pops in on approach — intentional, user-preferred). Investigation: docs/research/2026-06-20-indoor-torch-lantern-lighting-investigation.md Core 1505 / App 476 green. Visual gate: user-confirmed "looks like retail now." Co-Authored-By: Claude Opus 4.8 (1M context) --- .../retail-divergence-register.md | 2 + ...or-torch-lantern-lighting-investigation.md | 75 +++++++++++++++++++ src/AcDream.App/Rendering/GameWindow.cs | 48 ++++++++++-- .../Rendering/Wb/EnvCellRenderer.cs | 9 ++- .../Rendering/Wb/WbDrawDispatcher.cs | 4 +- src/AcDream.Core/Lighting/LightManager.cs | 54 +++++++++++++ 6 files changed, 185 insertions(+), 7 deletions(-) create mode 100644 docs/research/2026-06-20-indoor-torch-lantern-lighting-investigation.md diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index 8ea89e11..b1f7a65e 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -146,6 +146,8 @@ accepted-divergence entries (#96, #49, #50). | AP-42 | `UiMenu` item model is flat (label + opaque payload, single-level popup); retail `UIElement_Menu::MakePopup @0x46d310` supports hierarchical nested submenus via recursive popup chain | `src/AcDream.App/UI/UiMenu.cs` | The chat talk-focus menu is single-level (14 rows, 2 columns, no submenu); hierarchy is latent and unreachable through the chat window — no behavioral difference in the current usage | A future menu with nested submenus would render flat (only the top-level items drawn, no drill-down) | `UIElement_Menu::MakePopup` @0x46d310 | | AP-45 | `PublicUpdatePropertyInt (0x02CE)` sequence byte parsed-past but not honored; last update wins (no freshness check against sequence number) | `src/AcDream.Core.Net/Messages/PublicUpdatePropertyInt.cs` | Loopback ACE rarely reorders; latest-wins matches `PrivateUpdateVital`/`UpdatePosition`'s existing non-sequence behavior. Sequence tracking added when needed alongside TS-26. | A reordered 0x02CE on a real network could apply a stale UiEffects value — item icon temporarily shows the wrong effect state, corrected on next update | `PublicUpdatePropertyInt` sequence byte (ACE GameMessagePublicUpdatePropertyInt) | | AP-46 | Health-meter gate approximation: retail shows the health meter for `IsPlayer() || pet_owner || ClientCombatSystem::ObjectIsAttackable()` (full PK/faction logic); acdream's `GameWindow.IsHealthBarTarget` uses the server PWD bits `BF_ATTACKABLE (0x10)` OR `BF_PLAYER (0x8)` | `src/AcDream.App/Rendering/GameWindow.cs` (`IsHealthBarTarget`) → `SelectedObjectController` | The PWD `BF_ATTACKABLE`/`BF_PLAYER` bits distinguish monsters + players (bar) from friendly/vendor NPCs (name-only) for the M1.5 dev loop; the pet case and the full ObjectIsAttackable PK/faction refinement (free-PK, PK-vs-PK, PKLite) are not ported | A PK/faction edge (e.g. a hostile-flagged player whose `BF_ATTACKABLE` is unset, or a pet) could show/hide the bar where retail differs — no impact on the non-PK PvE dev loop | `ClientCombatSystem::ObjectIsAttackable` acclient_2013_pseudo_c.txt:375385; `BF_ATTACKABLE` acclient.h:6437 | +| AP-44 | Server-spawned weenie fixtures (lanterns, braziers, glowing items carrying `Setup.Lights`) register their lights at their SPAWN-frame world position via `RegisterOwnedLight` in `OnLiveEntitySpawnedLocked` (mirroring the dat-static path in `ApplyLoadedTerrainLocked`); the light does NOT follow the object if it later moves. Torn down on despawn/respawn by the now-unconditional `UnregisterOwner` in `RemoveLiveEntityByServerGuid`. | `src/AcDream.App/Rendering/GameWindow.cs` (`OnLiveEntitySpawnedLocked` weenie-light block; `RemoveLiveEntityByServerGuid`) | Closes the prior gap where ONLY dat-baked statics registered lights, so server-placed lanterns cast nothing; the overwhelming majority of light-bearing weenies (wall lanterns, braziers, candelabra) are stationary, so spawn-frame placement matches retail; retail's `add_dynamic_light` re-places the light from the object frame each frame | A moving light-bearing weenie (e.g. a torch-carrying NPC) leaves its light at the spawn position instead of following — rare; the faithful fix is to re-place on `OnLivePositionUpdated` | retail object-borne lights `insert_light` 0x0054d1b0 / `add_dynamic_light` 0x0054d420 (per-frame object frame) | +| AP-47 | acdream keeps the 128 nearest-to-CAMERA point lights live (`MaxGlobalLights=128`, `BuildPointLightSnapshot`) and selects per cell CAMERA-INDEPENDENTLY (by the cell's own bounds), so a building interior stays lit at any distance within a town; retail keeps only the 40 nearest-to-PLAYER static lights (`Render::max_static_lights=0x28`, distance-sorted replace-farthest `insert_light`) and re-bakes a cell when its live light set changes, so distant interiors are baked dark and "light up" only as the player approaches and their torches enter the live 40. INTENTIONAL — acdream's always-lit interiors are the preferred behavior (no 1999-era light-budget pop-in); user-confirmed 2026-06-20. | `src/AcDream.Core/Lighting/LightManager.cs` (`MaxGlobalLights=128`, `BuildPointLightSnapshot`, `SelectForObject`) | The retail pop-in is a fixed-function light-budget artifact, not an intended aesthetic; revert to retail by clamping the global set to 40 + distance-to-player sort if ever desired | Distant town interiors are lit in acdream where retail's are dark until approached — a deliberate, user-preferred divergence | `Render::max_static_lights` 0x28; `insert_light` 0x0054d1b0 (distance-sorted, replace-farthest); bake re-trigger `SetStaticLightingVertexColors` cache `burnedInStaticLights != num_static_lights` | --- diff --git a/docs/research/2026-06-20-indoor-torch-lantern-lighting-investigation.md b/docs/research/2026-06-20-indoor-torch-lantern-lighting-investigation.md new file mode 100644 index 00000000..e79a2fdc --- /dev/null +++ b/docs/research/2026-06-20-indoor-torch-lantern-lighting-investigation.md @@ -0,0 +1,75 @@ +# A7 indoor torch/lantern lighting — why acdream interiors read dark (investigation) + +**Date:** 2026-06-20 **Mode:** report-only investigation (no code changed) +**Trigger:** after #142 sun-gate (`ef5049f`) the Agent of Arcanum interior +(`0xA9B40171/0172`) still reads dark; user: "retail uses torches **and lanterns** +for lighting indoors." + +## Retail oracle — the static-light bake (decomp, confirmed) + +`SetStaticLightingVertexColors` (0x0059cfe0), per EnvCell mesh, per vertex: + +``` +for each vertex V: + acc = (0,0,0) // from BLACK; no ambient/sun here + for i in 0 .. world_lights.num_static_lights: // ALL lights — NO per-vertex/cell cap + L = sorted_static_lights[i] + if L.type==0: calc_point_light(V, acc, L) // 0x0059c8b0 + else: acc += dot(N,dir)*scale*L.color + acc = clamp(acc, 0, 1) // per-channel, AFTER summing all + V.diffuse = encode(acc) +``` + +`calc_point_light` (0x0059c8b0): `range = falloff × static_light_factor(1.3)`; +back-face/range gated; `angular = dot_n/dist` (<1 m) else `dot_n/(dist²·dist)`; +`scale = angular·(1−dist/range)·intensity`; per-channel cap `min(scale·c, c)`. + +**Global cap:** `max_static_lights = 0x28 = 40` (`insert_light` 0x0054d1b0 keeps a +distance-sorted, replace-farthest list). So the bake sees up to **40 nearest static +lights — and sums ALL of them per vertex.** There is no 8-light cap on the static +wall bake. (The 8-light cap is the D3D *hardware* path for *dynamic objects*, +`minimize_object_lighting` 0x0054d480 — a different path.) + +## acdream's torch path — three real divergences + +| # | Divergence | Site | Effect | +|---|---|---|---| +| Bug A | **Weenie fixture lights never registered.** `RegisterOwnedLight` is called ONLY from `ApplyLoadedTerrainLocked` for dat-baked landblock + EnvCell statics (Setup `0x02xxxxxx`, `Lights.Count>0`). Server `CreateObject` weenies (`OnLiveEntitySpawnedLocked`) never register lights. | `GameWindow.cs:6733-6758` (registers); `GameWindow.cs:2696/3275` (weenie path — no register) | If a lantern/torch is a server weenie, its light is **entirely absent**. | +| Bug B | **Per-cell 8-light cap.** `GetCellLightSet`→`SelectForObject` keeps the **8 nearest-to-cell-center** lights (`MaxLightsPerObject=8`); 9th+ dropped. Retail sums up to 40. | `EnvCellRenderer.cs:1044`; `LightManager.cs:234-278` | Dense cells (dungeon corridors, fixture clusters) lose lights. **Not binding** for the Arcanum (only ~3 in range). | +| Bug C | **128 global camera pre-filter.** `BuildPointLightSnapshot` keeps the 128 nearest-**to-camera** lights (`MaxGlobalLights=128`), drops the rest before per-cell selection. Retail has no global per-frame cap on statics. | `LightManager.cs:179,208-211` | `registeredLights=129` ⇒ already at the cap; drops farthest-from-camera. Hurts far lights, not the near interior. | + +Bug A·partial: the per-vertex bake is also approximated as **one 8-light set per +whole cell** (uniform across all the cell's vertices), vs retail's true per-vertex +bake — a fidelity gap (bright cluster smears across the cell), secondary to the +source-count gaps above. + +The per-light shader formula (`pointContribution`) + the `min(pointAcc,1.0)` torch +clamp **match** retail's `calc_point_light` + final saturate — magnitude per light +is faithful. The gap is **missing / capped light SOURCES**, not per-light math. + +## Ranked hypotheses (for the Agent of Arcanum specifically) + +1. **H1 — the lanterns are server weenies ⇒ Bug A drops their light** (best fit + for the "lanterns" hint). The cell shows only ~3 *dat* torches; lanterns absent. + *Falsify:* pick a lantern in-game — guid `0x80…`/`0x7…` = weenie ⇒ confirmed. +2. **H4 — per-cell (not per-vertex) bake / magnitude** — if the Arcanum has no + weenie fixtures, the 3 dat torches are all there is and the per-cell-uniform bake + under-fills vs retail's per-vertex bake. +3. **H2/H3 (8-cap / 128-cap)** — real retail divergences, but **not binding** for + this building (≤3 lights in range, near camera). They matter for dense dungeons. + +## Fix plan (faithful, per hypothesis) + +- **H1 (likely):** register `Setup.Lights` for light-bearing weenies in + `OnLiveEntitySpawnedLocked`, mirroring `ApplyLoadedTerrainLocked:6742-6758` + (guard `setup.Lights.Count>0`); unregister on despawn (`UnregisterByOwner`). + Faithful: retail registers object lights regardless of static/dynamic origin. +- **H2:** raise the per-cell static bake cap from 8 toward retail's 40 (the cap is + retail-correct only for the *dynamic-object hardware* path, not the wall bake). +- **H3:** make the per-cell selection independent of the 128 camera pre-filter for + statics (retail's bake has no global count cap). +- **H4:** move the cell bake toward per-vertex (longer-horizon A7 LightBake work). + +## What this is NOT +NOT a #142 sun-gate regression and NOT an ambient bug (ambient matches retail; +it's just dim rainy-twilight). It is missing / capped light **sources**. diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 95191d49..e427a88b 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -2708,7 +2708,7 @@ public sealed class GameWindow : IDisposable // For a respawn, drop the previous rendering state here before we // build the new one. `_entitiesByServerGuid` is the canonical map, // its value is the live WorldEntity we need to dispose. - RemoveLiveEntityByServerGuid(spawn.Guid, logDelete: false); + RemoveLiveEntityByServerGuid(spawn.Guid); // When requested, log every spawn that arrives so we can inventory what the server // sends (including the ones we can't render yet). The Name field @@ -3261,6 +3261,34 @@ public sealed class GameWindow : IDisposable if (liveBounds.TryGet(out var liveBMin, out var liveBMax)) entity.SetLocalBounds(liveBMin, liveBMax); + // A7 indoor lighting: server-spawned weenie fixtures (lanterns, + // braziers, glowing items) carry their light in Setup.Lights exactly + // like dat-baked statics, but the static registration in + // ApplyLoadedTerrainLocked never sees CreateObject entities — so an + // interior's lanterns cast no light and the room reads dark. Register + // them here, mirroring that path (GameWindow.cs ~6742). Owned by + // entity.Id, so RemoveLiveEntityByServerGuid's UnregisterOwner tears + // them down on despawn/respawn. Retail registers object-borne lights + // regardless of static-vs-dynamic origin (insert_light 0x0054d1b0). + // The light is placed at the spawn frame and does NOT follow a moving + // light-bearer yet (register row AP-44) — fine for stationary fixtures, + // which is the common case. _dats is non-null here (checked above). + if (_lightingSink is not null + && (entity.SourceGfxObjOrSetupId & 0xFF000000u) == 0x02000000u) + { + var liteSetup = _dats.Get(entity.SourceGfxObjOrSetupId); + if (liteSetup is not null && liteSetup.Lights.Count > 0) + { + var loaded = AcDream.Core.Lighting.LightInfoLoader.Load( + liteSetup, + ownerId: entity.Id, + entityPosition: entity.Position, + entityRotation: entity.Rotation); + foreach (var ls in loaded) + _lightingSink.RegisterOwnedLight(ls); + } + } + var snapshot = new AcDream.Plugin.Abstractions.WorldEntitySnapshot( Id: entity.Id, SourceId: entity.SourceGfxObjOrSetupId, @@ -3540,7 +3568,7 @@ public sealed class GameWindow : IDisposable private void OnLiveEntityDeleted(AcDream.Core.Net.Messages.DeleteObject.Parsed delete) { - if (RemoveLiveEntityByServerGuid(delete.Guid, logDelete: true) + if (RemoveLiveEntityByServerGuid(delete.Guid) && Environment.GetEnvironmentVariable("ACDREAM_DUMP_MOTION") == "1") { Console.WriteLine( @@ -3722,7 +3750,7 @@ public sealed class GameWindow : IDisposable } } - private bool RemoveLiveEntityByServerGuid(uint serverGuid, bool logDelete) + private bool RemoveLiveEntityByServerGuid(uint serverGuid) { if (!_entitiesByServerGuid.TryGetValue(serverGuid, out var existingEntity)) return false; @@ -3742,8 +3770,12 @@ public sealed class GameWindow : IDisposable if (_selectedGuid == serverGuid) SelectedGuid = null; - if (logDelete) - _lightingSink?.UnregisterOwner(existingEntity.Id); + // A7 indoor lighting: release this entity's owned lights on EVERY + // removal, including the respawn-dedup path (former logDelete=false). + // A respawned weenie fixture would otherwise leak its old light set and + // double-register the new one. (Was gated on logDelete — harmless only + // while live weenies registered no lights, which is no longer true.) + _lightingSink?.UnregisterOwner(existingEntity.Id); return true; } @@ -8120,6 +8152,12 @@ public sealed class GameWindow : IDisposable // consumes binding=1 reads the same data for the rest of the // frame — terrain, static mesh, instanced mesh, sky. UpdateSunFromSky(kf, playerInsideCell); + // A7 indoor lighting: position retail's viewer fill light at the player + // (SmartBox::set_viewer 0x00452c40) before the snapshot is built. It is + // the primary interior fill (no sun indoors) and is indoor-only via the + // AP-43 gate. playerViewPos is the player render position (or camPos when + // there is no player), matching retail's player/viewer branch. + Lighting.UpdateViewerLight(playerViewPos); Lighting.Tick(camPos); // Fix B (A7 #3): build this frame's point-light snapshot and hand it to diff --git a/src/AcDream.App/Rendering/Wb/EnvCellRenderer.cs b/src/AcDream.App/Rendering/Wb/EnvCellRenderer.cs index bf80e9e0..a599b814 100644 --- a/src/AcDream.App/Rendering/Wb/EnvCellRenderer.cs +++ b/src/AcDream.App/Rendering/Wb/EnvCellRenderer.cs @@ -1049,8 +1049,15 @@ public sealed unsafe class EnvCellRenderer : IDisposable System.Array.Fill(set, -1); var snap = _pointSnapshot; + // Landblocks are keyed by the streaming landblock id 0xXXYYFFFF + // (GameWindow: (x<<24)|(y<<16)|0xFFFF), NOT 0xXXYY0000 — so the landblock + // key is (cellId & 0xFFFF0000) | 0xFFFF. The old `cellId & 0xFFFF0000` key + // (0xXXYY0000) NEVER matched a registered landblock, so this lookup always + // missed: SelectForObject never ran and every EnvCell wall received ZERO + // point lights (the entire "indoor torches/lanterns don't light the room" + // bug — confirmed by the [cell-light] probe: inBounds=False for every cell). if (snap is { Count: > 0 } && - _landblocks.TryGetValue(cellId & 0xFFFF0000u, out var lb) && + _landblocks.TryGetValue((cellId & 0xFFFF0000u) | 0xFFFFu, out var lb) && lb.EnvCellBounds.TryGetValue(cellId, out var b)) { Vector3 center = (b.Min + b.Max) * 0.5f; diff --git a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs index d40f57e8..d2375a3e 100644 --- a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs +++ b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs @@ -2107,7 +2107,9 @@ public sealed unsafe class WbDrawDispatcher : IDisposable /// Render::useSunlight == 0, which is true only in the indoor draw stage. /// internal static bool IndoorObjectReceivesTorches(uint? parentCellId) - => parentCellId.HasValue && (parentCellId.Value & 0xFFFFu) >= 0x0100u; + => parentCellId.HasValue + && (parentCellId.Value & 0xFFFFu) >= 0x0100u + && (parentCellId.Value & 0xFFFFu) != 0xFFFFu; // 0xFFFF = landblock marker, not an EnvCell → outdoor /// /// Fix B: append the current entity's 8-slot light set to a group's diff --git a/src/AcDream.Core/Lighting/LightManager.cs b/src/AcDream.Core/Lighting/LightManager.cs index 95ea1edf..57d9789b 100644 --- a/src/AcDream.Core/Lighting/LightManager.cs +++ b/src/AcDream.Core/Lighting/LightManager.cs @@ -43,6 +43,7 @@ public sealed class LightManager private readonly List _all = new(); private readonly LightSource?[] _active = new LightSource?[MaxActiveLights]; private int _activeCount; + private LightSource? _viewerLight; // retail SmartBox::viewer_light (see UpdateViewerLight) /// Current cell ambient state applied to everything. public CellAmbientState CurrentAmbient { get; set; } @@ -86,6 +87,7 @@ public sealed class LightManager _all.Clear(); Array.Clear(_active); _activeCount = 0; + _viewerLight = null; // re-created + re-registered by the next UpdateViewerLight } /// @@ -212,6 +214,58 @@ public sealed class LightManager } } + // ── Viewer light — retail SmartBox::set_viewer (0x00452c40) ────────────── + // Retail adds a white fill light pinned to the player EVERY frame via + // Render::add_dynamic_light. It is the dominant INTERIOR fill: the outdoor + // stage runs useSunlightSet(1) (sun only — dynamics are NOT enabled), but the + // interior stage runs minimize_envcell_lighting, which enables the dynamic + // lights, so EnvCell walls + indoor objects are lit by this viewer light while + // the player is inside. acdream registered NO dynamic lights at all, so + // interiors had only flat ambient and read dark/cool vs retail's lit rooms. + // + // It rides the existing point-light path: registered in _all ⇒ included in + // BuildPointLightSnapshot ⇒ selected by SelectForObject for nearby cells / + // objects. The AP-43 indoor gate (WbDrawDispatcher.IndoorObjectReceivesTorches) + // already restricts per-object point lights to EnvCell-parented objects, and + // only EnvCellRenderer selects per-cell lights, so the viewer light lights + // ONLY indoor draws — matching retail's interior-stage-only dynamic enable. + // Terrain has no point-light path, so it is unaffected. + // + // Params from the live cdb capture (reference_retail_ambient_values.md): + // intensity 2.25, falloff 10, colour white, offset (0,0,2) above the player + // (SmartBox::set_viewer player branch). Dynamic lights use rangeAdjust 1.5 + // (config_hardware_light 0x0059ad30) ⇒ Range = 10 × 1.5 = 15 m. + public const float ViewerLightIntensity = 2.25f; // GRV SmartBox.ViewerLightIntensity + public const float ViewerLightFalloff = 10f; // GRV SmartBox.ViewerLightFalloff + private const uint ViewerLightOwnerId = 0xFFFFFFFFu; // sentinel; never an entity id + + /// + /// Reposition the always-on viewer fill light at the player (offset +2 m up), + /// registering it on first call. Call once per frame BEFORE + /// / . Mirrors retail's + /// per-frame SmartBox::set_viewer add_dynamic_light; here the light lives + /// in _all and is repositioned so the existing snapshot + per-object + /// selection light the cell around the player. Indoor-only via the AP-43 gate + /// (see the note above). + /// + public void UpdateViewerLight(Vector3 playerWorldPos) + { + if (_viewerLight is null) + { + _viewerLight = new LightSource + { + Kind = LightKind.Point, + ColorLinear = Vector3.One, // white (1,1,1) + Intensity = ViewerLightIntensity, + Range = ViewerLightFalloff * 1.5f, // dynamic rangeAdjust 1.5 + OwnerId = ViewerLightOwnerId, + IsLit = true, + }; + _all.Add(_viewerLight); + } + _viewerLight.WorldPosition = playerWorldPos + new Vector3(0f, 0f, 2f); + } + /// /// Select up to point/spot lights from /// that reach the object sphere