decentraland / decentraland/unity-explorer
AvatarModifierArea removal can leave an avatar permanently hidden (survives scene reload)
- Dominant language
- C#
- Stars
- 23
- Forks
- 17
- Avg merge
- 2d 14h
- Merged PRs (30d)
- 94
Description
Removing an `AvatarModifierArea` component while a player is inside the volume can leave that player's avatar **permanently hidden**. The hidden state survives a scene reload, and once it happens the area entity can never hide or unhide anyone again.
## Reproducing
A scene that creates an entity with `AvatarModifierArea{ modifiers: [AMT_HIDE_AVATARS], area: (16,8,16), excludeIds: [] }` covering the parcel, then calls `AvatarModifierArea.deleteFrom(entity)` while the local player stands inside. The entity itself stays alive. Cycle that on/off a few times, ideally with vsync off so the framerate is well above the physics rate.
Observed:
1. Hide works. After a few cycles, an unhide fails and the avatar stays hidden with no area present.
2. From then on, re-adding the component does **nothing** — it no longer hides either.
3. A scene reload alone does not restore the avatar.
4. After a reload, a fresh hide → unhide cycle does restore it.
Reproduced on macOS against a local `sdk-commands start` preview.
## Mechanism
Two systems react to `PBAvatarModifierArea` disappearing, and they race:
| System | Group | Does |
| --- | --- | --- |
| `AvatarModifierAreaHandlerSystem.HandleComponentRemoval` (`:154`) | `SyncedInitializationFixedUpdateThrottledGroup` | iterates `CurrentEntitiesInside` → `ShowAvatar` → clears `HiddenByModifierArea`, then removes `AvatarModifierAreaComponent` |
| `SDKEntityTriggerAreaCleanupSystem.HandleComponentRemoval` (`:41`) | `CleanUpGroup` | `TryRelease` to the pool, then `World.Remove` |
The unhide handler requires `ref SDKEntityTriggerAreaComponent` and a populated occupants list. The cleanup handler destroys both: pool release runs `SDKEntityTriggerArea.Dispose()` (`:38-47`), which moves occupants into `exitedEntitiesToBeProcessed` — nothing drains that on this path, since the only drain is in `UpdateAvatarModifierArea`, which requires `ref PBAvatarModifierArea` and has already stopped matching — and clears `currentEntitiesInside`.
The two groups run on different cadences: `FixedUpdateThrottledGroup.Update` gates on `Time.fixedTime > processedFixedTime` (~50 Hz), while `CleanUpGroup` is `[UpdateInGroup(typeof(SyncedPreRenderingSystemGroup))]`, i.e. per render frame. Above the physics rate there are frames in which cleanup runs and the modifier-area system does not, so whether a given cycle recovers is effectively a coin flip — which matches the intermittent onset.
**Why it then latches.** `SetupAvatarModifierArea` (`:74`) is gated `[None(typeof(SDKEntityTriggerAreaComponent), typeof(AvatarModifierAreaComponent))]`. When cleanup wins, `AvatarModifierAreaComponent` is left orphaned on the entity — only `HandleComponentRemoval` removes it, and that can no longer match — so re-adding `PBAvatarModifierArea` never builds a new trigger area. Both hide and unhide are dead for that entity until it is destroyed.
**Why a reload doesn't fix it.** `AvatarShapeComponent.HiddenByModifierArea` lives in the **global** world and has only two writers, `ShowAvatar` (`:184`) and `HideAvatar` (`:201`). The struct default only applies when a new `AvatarShapeComponent` is created, which for the main player happens once in `AvatarLoaderSystem.CreateMainPlayerAvatarShapeFromProfile`; `ApplyProfileToAvatarShape` doesn't touch it. `AvatarShapeVisibilitySystem.cs:212` re-applies it every frame. So the scene-world state is torn down by a reload while the stuck bit is not — hence step 3, and hence step 4 once a working cycle finally calls `ShowAvatar`.
**Both safety nets fail for the same reason.** `ResetAffectedEntities` (`:58`), the `IFinalizeWorldSystem` path, also takes `ref SDKEntityTriggerAreaComponent` and reads the same already-cleared occupants list.
## Secondary effects
- **The scene restriction sticks on.** `SceneRestriction.CreateAvatarHidden(REMOVED)` is pushed only from `ShowAvatar` (`:186-190`), so the "avatar hidden" indicator never clears. Worse, `localAvatarTransform` is a plain field on the scene-world system instance: even a later successful `ShowAvatar` in a reloaded scene finds it `null`, fails the identity check, and still never pushes `REMOVED`. `ownAvatarEntity` / `EnableAvatarInteraction` (`:218-224`) have the same shape.
- **Leak.** The orphaned `AvatarModifierAreaComponent` is never `Dispose()`d, so its pooled `HashSetPool` is leaked once per stuck cycle.
## Why tests don't catch it
`AvatarModifierAreaHandlerSystemShould.HandleComponentRemoveCorrectly` (`:386-435`) asserts exactly this scenario and passes — but `Setup()` (`:47-66`) constructs only `AvatarModifierAreaHandlerSystem`. `SDKEntityTriggerAreaCleanupSystem` is never added to the test world, so the racing system that causes the bug isn't present.
## Suggested direction
The ownership boundary looks like the real issue: the component that the unhide path depends on is deleted by a system that doesn't know about that dependency. Options, roughly in order of directness:
- Have `SDKEntityTriggerAreaCleanupSystem` skip entities that still carry a consumer component (`AvatarModifierAreaComponent`, `CameraModeAreaComponent`, …), leaving teardown to the consumer's own removal handler.
- Or drain `exitedEntitiesToBeProcessed` on the teardown path so the occupants survive the release.
- Or make the unhide independent of the trigger component — e.g. track affected entities on `AvatarModifierAreaComponent` itself, so removal can restore them without needing the area object.
Either way, `ResetAffectedEntities` and `HandleComponentRemoval` should probably not both be gated on a component owned by another system, and adding `SDKEntityTriggerAreaCleanupSystem` to the existing test would cover the regression.
## Notes
The intra-frame ordering of `InitializationSystemGroup` vs `PreRenderingSystemGroup` was not verified from source (Arch.SystemGroups isn't vendored in the checkout) — the race is inferred from the group attributes and `FixedUpdateThrottledGroup.Update`, and is consistent with the intermittent onset observed. The latch and the two safety-net failures are read directly off the query predicates.
Contributor guide
Assessment
This issue has not been assessed yet.