chore: iss via standalone descriptor (descriptor-only, bundle deferred)#8870
chore: iss via standalone descriptor (descriptor-only, bundle deferred)#8870dalkia wants to merge 16 commits into
Conversation
…lution Reworks the **Initial Scene State (ISS)** loading path so a scene's preview can be served by a JSON descriptor (`<sceneId>_InitialSceneState.json` on the LOD manifest bucket) listing per-asset hashes + transforms, with each asset fetched as its own AB. The legacy single-AB bundle path is kept structurally but currently disabled at the loader level — to be revived in a follow-up. ### Highlights - **`ISSDescriptor` is an ECS component on each scene entity** (class semantics). Constructed in `Uninitialized` state for scenes that may have ISS, or `None` for opt-outs (PX, static-pointer/LSD, smart-wearable previews). The resolver mutates state in place via `MarkResolved` so cached references in `OrderedDataManaged` and `GetSceneFacadeIntention` see the resolved state without a refetch. - **Radius-gated resolution.** `ResolveSceneStateByIncreasingRadiusSystem` spawns the resolver promise on the first SHOWING_LOD / SHOWING_SCENE transition for a scene with `Uninitialized` descriptor, then bails for one tick until `ResolveISSDescriptorSystem` consumes the promise. No descriptor resolution work for scenes that never come into range. - **Descriptor mode (new): `ResolveISSLODSystem`** spawns one AB promise per descriptor entry, applies the transform, counts failures so `AllAssetsInstantiated` settles even on 404s. Bridge handoff to the SDK runtime is reservation-based (`ISSDescriptor.TryReserveBridgeSlot`) capped at the descriptor's per-hash multiplicity, so SDK-instantiated copies of the same hash created at runtime never pollute the bridge. - **Digest-aware GLTF cache reuse.** Both LOD and SDK runtime paths key the GLTF container cache by `hash@digest` via `AssetBundleManifestVersionExtensions.ComposeCacheKey`, so bridged assets round-trip cleanly between the two without spawning duplicate instances. Per-asset AB promises populate `GetAssetBundleIntention.DepsDigest` from the manifest, making the `(Hash, DepsDigest)` cache key match the SDK runtime's — eliminates the "asset bundle already loaded" Unity error from parallel loads of the same physical bundle. - **`SupportsISS()` on `AssetBundleManifestVersion`** — mirrors `SupportsDepsDigests()`. Cached version check; gates the descriptor lookup to v49+ manifests. - **Disk-cached descriptors** with text-editor-readable JSON format. NONE results are not persisted to disk (decorator skip). ### Files of interest - `Explorer/Assets/DCL/Infrastructure/ECS/StreamableLoading/AssetBundles/InitialSceneState/ISSDescriptor.cs` — descriptor + bridge state + JSON DTOs + in-place mutation - `Explorer/Assets/DCL/Infrastructure/ECS/StreamableLoading/AssetBundles/InitialSceneState/LoadISSDescriptorSystem.cs` — `LoadSystemBase<ISSDescriptor, GetISSDescriptor>` hitting the LOD bucket - `Explorer/Assets/DCL/Infrastructure/ECS/SceneLifeCycle/IncreasingRadius/ResolveSceneStateByIncreasingRadiusSystem.cs` — the gate that spawns the resolver promise on first LOD/Scene transition - `Explorer/Assets/DCL/Infrastructure/ECS/SceneLifeCycle/SceneDefinition/Systems/ResolveISSDescriptorSystem.cs` — consumes the promise, mutates the entity's descriptor in place - `Explorer/Assets/DCL/LOD/Systems/ResolveISSLODSystem.cs` — descriptor mode + digest-aware cache reuse + bridge slot reservation ### Removed - `EarlySceneRequestSystem` + `EarlyAssetBundleRequestSystem` and their flag types. The prewarm chain only made sense when the ISS descriptor was resolved synchronously during scene-definition loading; with lazy resolution there's no longer a deterministic point to usefully prefetch the bundle. A follow-up PR will revive the prewarm in a form that fits the lazy pipeline. ### Temporarily forced - Descriptor mode is the only path enabled in `LoadISSDescriptorSystem` while we validate the per-asset descriptor pipeline end-to-end. `IsBundleReachableAsync` and the Bundle-mode plumbing are kept commented for a follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The squash from chore/new-iss to chore/iss-v2 lost this single line. Unity's runtime AB tracker keeps the AssetBundle registered until UnloadAB is called, so a second fetch of the same URL (LOD path → SDK runtime, or any other cross-path reuse) hits "AssetBundle ... can't be loaded because another AssetBundle with the same files is already loaded." Calling UnloadAB right after the Object[] is extracted lets the file handle go but keeps the loaded assets — matching the pre-merge behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Windows and Mac build successful in Unity Cloud! You can find a link to the downloadable artifact below. |
…03 spam - CreateSceneEntity now takes the ISSDescriptor explicitly; each caller picks the right starting state for its scene type. - LoadFixedPointersSystem passes Uninitialized — Worlds (fixed-realm) scenes go through the same AB pipeline as Genesis and may have ISS descriptors. - LoadStaticPointersSystem / portable-experience / smart-wearable paths pass None — those aren't deployed through the regular AB pipeline. - MockSceneData.ISSDescriptor returns the NONE singleton instead of null. - LoadISSDescriptorSystem.TryLoadDescriptorAsync re-enables suppressErrors on the GetAsync — descriptor-missing is the expected case for non-ISS scenes and was spamming the [SCENE_LOADING] log every realm load. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Introduces ISSDescriptorResolution (readonly struct: state + assets) as the TAsset of LoadISSDescriptorSystem and the disk-cache value type. The per-entity ISSDescriptor class now owns only the runtime concerns (bridge slots, MarkResolved) and takes a resolution to apply. The loader no longer hands out a half-initialized class instance that exists solely to be unpacked by the resolver. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
PR #8870, run #26555773261 Builds: Windows change, Windows baseline, macOS change, macOS baseline Framework 13 i7
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
PR #8870, run #26557791876 Builds: Windows change, Windows baseline, macOS change, macOS baseline Framework 13 i7
|
NickKhalow
left a comment
There was a problem hiding this comment.
Solid work! Please address the comments below
| for (var i = 0; i < source.Count; i++) assets.Add(source[i]); | ||
|
|
||
| string json = JsonUtility.ToJson(new ISSDescriptorMetadata { assets = assets }); | ||
| byte[] payload = Encoding.UTF8.GetBytes(json); |
There was a problem hiding this comment.
Why do we need UTF8 representation? The data can be stored as UTF16 without allocating additional buffers. MemoryIterator can read directly on str bytes
There was a problem hiding this comment.
Stepping further, we could store the blob without serializing it into json. Please add a note for the potential optimization
There was a problem hiding this comment.
Not sure how would I address this one. Are you reffering on doing a simple Encoding.Unicode.GetBytes(json) and Encoding.Unicode.GetString(data.Memory.Span), respectively?
There was a problem hiding this comment.
Good question, I'm referring to pinning the string and reading it via char*
unsafe
{
fixed (char* p = json)
{
byte* bytes = (byte*)p;
int byteCount = json.Length * sizeof(char);
}
}
| catch (OperationCanceledException) { throw; } | ||
| catch (Exception) | ||
| { | ||
| // No descriptor at the expected path — treat as no ISS. |
There was a problem hiding this comment.
Hm, it catches all the exceptions. There could be network or parsing errors as well. Is that intended it will swallow them too?
There was a problem hiding this comment.
For now, I dont think a retry mechanism is needed. Its okay for any given exception to consinder the ISS as failed
There was a problem hiding this comment.
Thanks for clarification, I would add Log|Warning/Error| at least for UNITY_EDITOR || DEBUG. WDYT?
There was a problem hiding this comment.
Keep in mind that this is already in the ResolveISSSystem.cs
It wont be swallowed, but rather surface on the system responsible for the resolve
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Resolution wrapper carried a State field whose only resolved value was Descriptor (None lives only on the per-entity component). Caching the State conflicts with Bundle-mode coming back live in a follow-up, so the field was already dead weight. Dropping the wrapper makes that structural: the cache TAsset is now ISSDescriptorMetadata (the literal JSON shape), the disk serializer collapses to a thin JsonUtility wrapper, and ISSDescriptor.MarkResolved takes the asset list directly. Failure / no-ISS uses a separate ISSDescriptor.MarkAsNone() so the descriptor's State transition is explicit at each call site. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
PR #8870, run #26617613074 Builds: Windows change, Windows baseline, macOS change, macOS baseline Framework 13 i7
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| $"Failed to get ISS LOD for {sceneLODInfo.id}, will try to do the old LOD"); | ||
| } | ||
| // We just consumed one bridged copy — free up its slot so a future SDK cleanup of the same hash can re-bridge. | ||
| issDescriptor.ReleaseBridgeSlot(entry.hash); |
There was a problem hiding this comment.
[WARNING] Spurious bridge-release warning on first LOD load (unchanged)
ReleaseBridgeSlot is called unconditionally for every gltfCache.TryGet hit. On the first LOD load of a scene — before any SDK run has ever called TryReserveBridgeSlot — bridgedCount is empty, so n > 0 is false and the "no outstanding reservation" warning fires.
The comment says "We just consumed one bridged copy" but a cache hit on first load doesn't mean a slot was ever reserved. Consider guarding:
if (gltfCache.TryGet(cacheKey, out var asset))
{
// Only release if a bridge slot was actually reserved (SDK→LOD bridge path).
// On first-ever LOD load the cache may have the asset from a prior context but no reservation.
issDescriptor.ReleaseBridgeSlot(entry.hash); // silently no-ops if count is 0; no warning needed
PositionAsset(...);
continue;
}Or change ReleaseBridgeSlot to silently clamp at 0 without warning (remove the log call), since it is also hit legitimately on cache-warm second loads.
|
PR #8870, run #26619640424 Builds: Windows change, Windows baseline, macOS change, macOS baseline Framework 13 i7
|
|
PR #8870, run #26620771499 Builds: Windows change, Windows baseline, macOS change, macOS baseline Framework 13 i7
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
🔍 Claude reviewed this PR and found no blocking issues, but assessed it as complex — human DEV review is still required before merging. |
|
PR #8870, run #26657522104 Builds: Windows change, Windows baseline, macOS change, macOS baseline Framework 13 i7
|
|
Claude finished @dalkia's task in 3m 20s —— View job Claude finished @dalkia's task —— View job Tasks
Code ReviewWhat this PR fixes: Migrates the ISS descriptor from being embedded inside a shared Sync Commit ReviewThe latest trigger is The two ISS-specific commits ( Full Issue Tracker
No New IssuesThe merge commit is unrelated to ISS. All previously identified blocking issues are resolved. No new blocking issues introduced. REVIEW_RESULT: PASS ✅ |
|
PR #8870, run #26665063028 Builds: Windows change, Windows baseline, macOS change, macOS baseline Framework 13 i7
|
Pull Request Description
What is "Initial Scene State (ISS)"?
When you walk through Decentraland and a scene comes into view, the runtime needs to fetch, instantiate, and render every asset in that scene before you see anything. For a complex parcel that can take seconds — the scene "pops in" once it's ready. ISS is a streaming optimization that gives the player something to look at while the real scene loads: a lightweight, pre-baked snapshot of the scene's static geometry that the LOD pipeline can show immediately, then seamlessly hand off to the SDK runtime once the real scene is ready.
How it relates to the existing LOD pipeline
Decentraland already has a distance-based LOD system: each scene has multiple progressively-simpler versions baked (LOD_0 and LOD_1). Until ISS, LOD_0 (the closest, highest-quality version) was a separate copy that the client downloaded in full, instantiated as its own GameObjects, and then threw away and re-downloaded the same assets again once the player walked close enough to trigger the real SDK scene — so we paid the bandwidth and instantiation cost twice for the same geometry.
ISS replaces LOD_0 with a smarter representation: instead of duplicating the scene's assets in a separate LOD bundle, the descriptor points at the same per-asset bundles the SDK runtime will load anyway, plus the transforms needed to show them at preview time. The LOD path and the SDK runtime now share the underlying GameObjects via a "bridge" — when the LOD instance is no longer needed (the player walked close enough), it transitions into the running scene instead of being destroyed and re-created. No double download, no double instantiation.
LOD_1+ are unchanged and still used as-is for far-distance fallback. They're produced by the new LOD generator: https://github.com/decentraland/lod-generator-unity
How the descriptor reaches the client
For each scene the bake pipeline can publish a small JSON descriptor next to the asset bundles. The descriptor lists which GLTF assets to show as the "initial state" and where to place them (position / rotation / scale). The client fetches the descriptor as soon as the scene starts coming into range, instantiates the listed assets at LOD distance, and reuses them via the bridge described above when the SDK scene starts.
The two paths an ISS descriptor can take:
staticscene_<sceneId><platform>AB.What does this PR change?
The old ISS shipped the descriptor inside the single bundled AB (the
staticscene_*one). To read the descriptor, the client had to download and open that whole AB. With ISS evolving toward shared GameObjects between LOD and SDK runtime, two things changed:staticscene_*bundles correctly. So for now the per-asset bundles produced by the converter for the regular scene load are what gets served; the shared bundle is deferred to a follow-up PR + bake-side coordination.Most of the diff in this PR is the consequence of those two shifts: the descriptor moves from embedded-in-AB to a freestanding JSON loader with its own disk cache, an ECS component on each scene entity that the radius gate consults lazily, and a digest-aware shared GLTF cache so the LOD path and the SDK runtime hit the same underlying assets without colliding in Unity's AB tracker.
The Bundle-mode plumbing is structurally kept (state enum, dormant queries) so reviving it later is a small targeted change.
Highlights
ISSDescriptoris an ECS component on each scene entity (class with mutable internal state — same reference for the scene's lifetime). Constructed inUninitializedfor scenes that may have ISS, orNonefor opt-outs (PX / static-pointer / smart-wearable previews). The resolver mutates state in place viaMarkResolved, so cached references inOrderedDataManagedandGetSceneFacadeIntentionsee the resolved state without a re-fetch — noWorld.Getin the gate hot path.ResolveSceneStateByIncreasingRadiusSystemattaches the resolver promise on the firstSHOWING_LOD/SHOWING_SCENEtransition for anUninitializedscene, then bails for one tick untilResolveISSDescriptorSystemconsumes the promise. Scenes that never come into range never trigger a descriptor lookup.ResolveISSLODSystem(descriptor mode) spawns one AB promise per descriptor entry, applies the transform, counts failures viaAddFailedAssetsoAllAssetsInstantiatedsettles even when individual assets 404. Bridge handoff to the SDK runtime is reservation-based (ISSDescriptor.TryReserveBridgeSlot) capped at the descriptor's per-hash multiplicity, so SDK-instantiated copies of the same hash at runtime never pollute the bridge.hash@digestviaAssetBundleManifestVersionExtensions.ComposeCacheKey, so bridged assets round-trip cleanly between the two without spawning duplicate instances. Per-asset AB promises also populateGetAssetBundleIntention.DepsDigestfrom the manifest, making the(Hash, DepsDigest)cache key match the SDK runtime's — this eliminated the "asset bundle already loaded" Unity error from parallel loads of the same physical bundle.SupportsISS()onAssetBundleManifestVersionmirrorsSupportsDepsDigests(). Cached version-number check; gates the descriptor lookup so pre-v49 manifests skip the network round-trip.Noneresults are not persisted (decorator skips).Files of interest
Explorer/Assets/DCL/Infrastructure/ECS/StreamableLoading/AssetBundles/InitialSceneState/ISSDescriptor.cs— descriptor + bridge state + JSON DTOs +MarkResolved(in-place mutation)Explorer/Assets/DCL/Infrastructure/ECS/StreamableLoading/AssetBundles/InitialSceneState/LoadISSDescriptorSystem.cs—LoadSystemBase<ISSDescriptor, GetISSDescriptor>hitting the LOD bucketExplorer/Assets/DCL/Infrastructure/ECS/SceneLifeCycle/IncreasingRadius/ResolveSceneStateByIncreasingRadiusSystem.cs— the gate that spawns the resolver promise on first LOD/Scene transitionExplorer/Assets/DCL/Infrastructure/ECS/SceneLifeCycle/SceneDefinition/Systems/ResolveISSDescriptorSystem.cs— consumes the promise, mutates the entity's descriptor in placeExplorer/Assets/DCL/LOD/Systems/ResolveISSLODSystem.cs— descriptor mode + digest-aware cache reuse + bridge slot reservationRemoved:
EarlySceneRequestSystem+EarlyAssetBundleRequestSystemRemoved until we decide how to use them effectively just with the descriptor. For cognitive and scope reasons, this will be done on a later PR.
Deferred to a follow-up PR: Bundle mode
The legacy single-bundle ISS path (one shared
staticscene_<sceneID><platform>AB containing every listed asset) is structurally kept but disabled at the loader level.LoadISSDescriptorSystem.IsBundleReachableAsyncis commented out and every ISS-capable scene resolves directly toIISSDescriptor.State.Descriptorfor now.ISSDescriptor.SupportsBundle()returnsfalseso theResolveISSLODSystembundle branch is dormant. The plumbing (Bundle state enum value, bundle-mode query, helper functions) is left in place so re-enabling is a one-line restore in a later PR.Doing this in two steps keeps the cognitive load of the current review focused on the descriptor pipeline; bundle mode will come back with its own targeted PR + bake-side coordination.
Important: AB Converter doesn't bake ISS bundles yet
The asset bundle converter doesn't currently have ISS bundle baking enabled. This PR can be exercised end-to-end against v49+ manifests for the descriptor-only path — but if you point it at a realm whose scenes were not baked with descriptor JSONs in the LOD bucket, you'll just see scenes resolve to
None(no ISS) and fall through to the legacy LOD path. The descriptor-mode happy path requires both the descriptor JSON and per-asset bundles to exist at the expected URLs.Test Instructions
Steps (standard run):
metaforge explorer run XXXX # ← replace with this PR numberTest Steps
Genesis Plaza (zone) — ISS happy path.
Spawn into Genesis Plaza on
zoneand explore: walk around, move outward and back, change directions.When it stabilizes, go back to 0,0. Repeat the previous steps, all should work
Regression — every other scene-loading path. Sanity-check that nothing else broke:
org— descriptors don't exist there yet, so every scene should fall through to the legacy LOD path (one 403 per scene from the LOD bucket is expected and now suppressed in the log; behavior should otherwise be identical to main).State.None).org. The world should still load.Quality Checklist
Code Review Reference
Please review our Branch & PR Standards before submitting.
🤖 Generated with Claude Code