fix(vfx): resolve an entity's cell through one owner so effects follow it

Fixes #282 (plan S2). Adds register row AP-133.

Retail gives a CPhysicsObj exactly ONE cell: ShouldDrawParticles @0x0050fe60
reads this->cell and calls IsInView on it, and set_cell_id @0x0050f4f0 /
change_cell @0x00513390 are the only things that move it. acdream splits that
into ParentCellId (render parent, deliberately null for outdoor dat stabs) and
EffectCellId (the authored landcell those parentless stabs still need) - an
adaptation, now recorded as AP-133.

WorldEntity.EffectCellId documents itself as the stab field, with live and
interior entities using ParentCellId. f24532ad began writing it for live
entities too. Because EntityEffectPoseRegistry resolved EffectCellId FIRST,
that write won - and the audit shows only 3 of 14 cell writers maintain it.
The other 11 do not, including the hottest paths: RemotePhysicsUpdater:239,294
and LiveEntityOrdinaryPhysicsUpdater:107 write ParentCellId every physics tick
from the snapshot, and LocalPlayerProjectionController:79 writes the local
player's cell every frame.

So a moving entity updated its cell constantly while EffectCellId stayed
frozen at whatever cell it materialized in. Its particles and lights kept
being tested against that stale cell and failed IsInView the moment it crossed
a boundary - effects vanishing on a monster that is plainly visible, or
drawing through a wall from a room the viewer cannot see.

The consumers had also drifted into disagreeing: EntityEffectPoseRegistry
preferred EffectCellId while WbDrawDispatcher.TryGetEntityCell and the remote
spawn seed preferred ParentCellId - two answers to "which cell is this in".

- WorldEntity.VisibilityCellId (ParentCellId ?? EffectCellId) is the single
  accessor; all five consumer sites resolve through it, so the precedence
  cannot drift apart again.
- LiveEntityRuntime's three live-entity EffectCellId writes are removed,
  restoring the field to its documented purpose. Its real writers -
  LandblockLoader:80,97 and LandblockBuildFactory:408 - are untouched, and the
  parentless-stab path is pinned by a new test.
- f24532ad's actual fix is preserved: RebucketLiveEntity still installs the
  committed cell, just on the one field live entities use.

LiveEntityLightControllerTests.Refresh_FollowsCurrentTopLevelRootAndCell is
back to moving the entity by ParentCellId alone - its original pre-f24532ad
form - and passes. CanonicalOnlyRebucket_DoesNotOverwriteAuthoritativeFullCell
had its two EffectCellId assertions (added by f24532ad, encoding the defect)
replaced with the corrected contract: ParentCellId set, EffectCellId null,
VisibilityCellId resolving - a stronger assertion, not a relaxed one.

Complete Release solution: 10,836 passed / 4 skipped / 0 failed.

User visual check still outstanding: a monster with an active spell effect
crossing a cell boundary, and a lit static object, indoors and outdoors.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-03 14:02:03 +02:00
parent 97d11e6c7f
commit 3c36b4cc21
10 changed files with 134 additions and 29 deletions

View file

@ -731,10 +731,9 @@ internal sealed class LiveEntityNetworkUpdateController
update.Guid,
entity,
remote.Body.Position,
// Interior live entities carry ParentCellId; outdoor live
// entities carry the outdoor landcell in EffectCellId (see
// WorldEntity's cell-field docs). 0 → helper no-ops.
entity.ParentCellId ?? entity.EffectCellId ?? 0u);
// #282: one owner for "which cell is this in" (see
// WorldEntity.VisibilityCellId). 0 → helper no-ops.
entity.VisibilityCellId ?? 0u);
}
if (!IsCurrentOwner(remote))
return default;

View file

@ -54,7 +54,7 @@ public sealed class EntityEffectPoseRegistry :
Matrix4x4.CreateFromQuaternion(entity.Rotation)
* Matrix4x4.CreateTranslation(entity.Position),
partLocal,
entity.EffectCellId ?? entity.ParentCellId ?? 0u,
entity.VisibilityCellId ?? 0u,
availability);
}
@ -79,11 +79,11 @@ public sealed class EntityEffectPoseRegistry :
else
{
changed = record.RootWorld != rootWorld
|| record.CellId != (entity.EffectCellId ?? entity.ParentCellId ?? 0u);
|| record.CellId != (entity.VisibilityCellId ?? 0u);
}
record.RootWorld = rootWorld;
record.CellId = entity.EffectCellId ?? entity.ParentCellId ?? 0u;
record.CellId = entity.VisibilityCellId ?? 0u;
if (entity.IndexedPartTransforms.Count > 0)
{
changed |= CopyParts(
@ -160,7 +160,14 @@ public sealed class EntityEffectPoseRegistry :
return false;
Matrix4x4 rootWorld = Matrix4x4.CreateFromQuaternion(entity.Rotation)
* Matrix4x4.CreateTranslation(entity.Position);
uint cellId = entity.EffectCellId ?? entity.ParentCellId ?? 0u;
// #282: resolve through the one owner. Reading EffectCellId first was
// inverted relative to every other consumer
// (WbDrawDispatcher.TryGetEntityCell,
// LiveEntityNetworkUpdateController's spawn seed), and for a live
// entity it pinned effects to the materialization cell - only 3 of 14
// cell writers maintain EffectCellId, while ParentCellId is updated
// every physics tick.
uint cellId = entity.VisibilityCellId ?? 0u;
if (record.RootWorld == rootWorld && record.CellId == cellId)
return true;

View file

@ -435,14 +435,13 @@ public sealed partial class WbDrawDispatcher : IDisposable
private static bool TryGetEntityCell(WorldEntity entity, out uint cell)
{
if (entity.ParentCellId is uint parent)
// #282: one owner for "which cell is this in" - see
// WorldEntity.VisibilityCellId. This site already had the correct
// precedence; routing it through the accessor keeps it from drifting
// apart from the effect/particle path again.
if (entity.VisibilityCellId is uint resolved)
{
cell = parent;
return true;
}
if (entity.EffectCellId is uint effect)
{
cell = effect;
cell = resolved;
return true;
}
cell = 0;

View file

@ -851,8 +851,12 @@ public sealed class LiveEntityRuntime : ILiveEntityRadarSource
// CObjCell read by ShouldDrawParticles at the same edge; retaining
// the prior sidecar cell makes newly-created spell particles fail
// IsInView as soon as the player crosses an outdoor landcell.
// #282: a live entity's cell is ParentCellId alone. EffectCellId
// exists only for outdoor dat stabs/building shells, which keep a
// null render parent (LandblockLoader:80,97) - writing it here
// made it win WorldEntity.VisibilityCellId for live entities,
// which the 11 non-rebucketing per-tick writers cannot maintain.
entity.ParentCellId = spatialCellOrLandblockId;
entity.EffectCellId = spatialCellOrLandblockId;
}
Exception? spatialNotificationFailure = null;
uint priorRebucketingGuid = _rebucketingGuid;
@ -1097,8 +1101,9 @@ public sealed class LiveEntityRuntime : ILiveEntityRadarSource
entity.SetPosition(projection.WorldPosition);
entity.Rotation = projection.Orientation;
// #282: live entities carry ParentCellId only; EffectCellId is the
// outdoor dat stab / building-shell field (LandblockLoader:80,97).
entity.ParentCellId = token.ExactCellId;
entity.EffectCellId = token.ExactCellId;
return RebucketLiveEntityPresentationOnly(
record.ServerGuid,
record,
@ -1234,8 +1239,9 @@ public sealed class LiveEntityRuntime : ILiveEntityRadarSource
entity.SetPosition(projection.WorldPosition);
entity.Rotation = projection.Orientation;
// #282: live entities carry ParentCellId only; EffectCellId is the
// outdoor dat stab / building-shell field (LandblockLoader:80,97).
entity.ParentCellId = token.ExactCellId;
entity.EffectCellId = token.ExactCellId;
record.IsSpatiallyProjected = true;
Exception? spatialNotificationFailure = null;

View file

@ -101,6 +101,26 @@ public sealed class WorldEntity
/// </summary>
public uint? EffectCellId { get; set; }
/// <summary>
/// #282: the ONE cell this entity occupies, as retail models it. A
/// <c>CPhysicsObj</c> has a single <c>cell</c>; <c>ShouldDrawParticles</c>
/// @0x0050fe60 reads that same field and calls <c>IsInView</c> on it, and
/// <c>set_cell_id</c> @0x0050f4f0 / <c>change_cell</c> @0x00513390 are the
/// only things that move it. Our split into
/// <see cref="ParentCellId"/> + <see cref="EffectCellId"/> is an
/// adaptation for outdoor dat stabs, which keep a null render parent yet
/// still need a landcell for particle gating.
///
/// Resolve through here, never by reading one field or re-deriving the
/// precedence at a call site. Live entities carry
/// <see cref="ParentCellId"/>, kept current by every per-tick physics and
/// network writer; stabs and building shells carry only
/// <see cref="EffectCellId"/>. Reading <see cref="EffectCellId"/> FIRST is
/// what stranded live entities' particles and lights on their
/// materialization cell, because only 3 of 14 cell writers maintain it.
/// </summary>
public uint? VisibilityCellId => ParentCellId ?? EffectCellId;
/// <summary>
/// True when this entity originates from <c>LandBlockInfo.Buildings[]</c>
/// (the dat array that carries building shells: cottage walls, smithy walls,