-
Notifications
You must be signed in to change notification settings - Fork 17
chore: ISS via standalone descriptor + per-asset bundles #8805
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 13 commits
2782809
be6d9dc
3d2309a
0caf229
9e66ef8
b05ca70
2bcadee
3714605
68909b9
d4d3dd7
75fb3f7
8fb69bc
3b8133b
bfcb556
a996d2c
922340b
c1cd356
5b98d46
9f2c181
2dbbf60
2484cb7
b2d52ad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| using Arch.Core; | ||
| using Arch.System; | ||
| using Arch.SystemGroups; | ||
| using DCL.Diagnostics; | ||
| using ECS.Abstract; | ||
| using ECS.LifeCycle.Components; | ||
| using ECS.Prioritization.Components; | ||
| using ECS.SceneLifeCycle; | ||
| using ECS.StreamableLoading.AssetBundles.InitialSceneState; | ||
| using ECS.StreamableLoading.Common; | ||
| using ECS.StreamableLoading.Common.Components; | ||
|
|
||
| namespace ECS.SceneLifeCycle.SceneDefinition | ||
| { | ||
| /// <summary> | ||
| /// Bridges <see cref="LoadISSDescriptorSystem"/> to <see cref="SceneDefinitionComponent"/>: lazily | ||
| /// spawns the resolver promise for each scene definition and writes the resolved descriptor onto | ||
| /// the component. Global-world systems (UpdateSceneLODInfoSystem, ResolveISSLODSystem) iterate | ||
| /// <see cref="SceneDefinitionComponent.ISSDescriptor"/> directly and don't need to know the | ||
| /// resolver exists. The scene-runtime path (LoadSceneSystemLogicBase) creates its own promise and | ||
| /// gets the same instance via the loader's ongoing-request dedup, so AttachAssetBundle on its side | ||
| /// mutates the same descriptor that ends up on this component. | ||
| /// </summary> | ||
| [UpdateInGroup(typeof(RealmGroup))] | ||
| [LogCategory(ReportCategory.SCENE_LOADING)] | ||
| public partial class ResolveISSDescriptorSystem : BaseUnityLoopSystem | ||
| { | ||
| internal ResolveISSDescriptorSystem(World world) : base(world) { } | ||
|
|
||
| protected override void Update(float t) | ||
| { | ||
| SpawnPromiseQuery(World); | ||
| ConsumePromiseQuery(World); | ||
| } | ||
|
|
||
| [Query] | ||
| [None(typeof(DeleteEntityIntention))] | ||
| private void SpawnPromise(ref SceneDefinitionComponent sceneDefinitionComponent, ref PartitionComponent partition) | ||
| { | ||
| // Skip if already resolved (NONE or otherwise) or a promise is already in flight. | ||
| if (sceneDefinitionComponent.ISSDescriptorResolved) return; | ||
| if (sceneDefinitionComponent.ISSDescriptorPromise.Entity != Entity.Null) return; | ||
|
|
||
| sceneDefinitionComponent.ISSDescriptorPromise = AssetPromise<ISSDescriptor, GetISSDescriptor>.Create( | ||
| World, GetISSDescriptor.For(sceneDefinitionComponent.Definition), partition); | ||
| } | ||
|
|
||
| [Query] | ||
| [None(typeof(DeleteEntityIntention))] | ||
| private void ConsumePromise(ref SceneDefinitionComponent sceneDefinitionComponent) | ||
| { | ||
| if (sceneDefinitionComponent.ISSDescriptorPromise.Entity == Entity.Null) return; | ||
|
|
||
| if (!sceneDefinitionComponent.ISSDescriptorPromise.TryConsume(World, out StreamableLoadingResult<ISSDescriptor> result)) | ||
| return; | ||
|
|
||
| sceneDefinitionComponent.ISSDescriptor = result is { Succeeded: true } ? result.Asset! : ISSDescriptor.NONE; | ||
| sceneDefinitionComponent.ISSDescriptorPromise = AssetPromise<ISSDescriptor, GetISSDescriptor>.NULL; | ||
| sceneDefinitionComponent.ISSDescriptorResolved = true; | ||
| } | ||
| } | ||
| } |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,29 +16,26 @@ public class AssetBundleData : StreamableRefCountData<AssetBundle> | |
| { | ||
| private readonly string AssetBundleName; | ||
|
|
||
| public readonly InitialSceneStateMetadata? InitialSceneStateMetadata; | ||
|
|
||
| private bool AssetBundleUnloaded; | ||
| private Dictionary<string, AssetInfo>? Assets; | ||
| private readonly AssetBundleData[] Dependencies; | ||
|
|
||
| public AssetBundleData(AssetBundle assetBundle, InitialSceneStateMetadata? initialSceneState, Object[] loadedAssets, Type? assetType, AssetBundleData[] dependencies, string version = "", string source = "") | ||
| //TODO: Rehook isISS | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking (unfinished TODO): Wire it: add an |
||
| public AssetBundleData(AssetBundle assetBundle , Object[] loadedAssets, Type? assetType, AssetBundleData[] dependencies, string version = "", string source = "", bool isISS = false) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking (unfinished TODO, 3rd flag): Wire it: add an
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking (unfinished TODO, 4th flag): The PR description says this flag is "plumbed through so ISS bundles aren't unloaded prematurely". The code contradicts that claim. Fix: add an |
||
| : base(assetBundle, ReportCategory.ASSET_BUNDLES) | ||
| { | ||
| InitialSceneStateMetadata = initialSceneState; | ||
|
|
||
| Assets = new Dictionary<string, AssetInfo>(); | ||
|
|
||
| for (var i = 0; i < loadedAssets.Length; i++) | ||
| Assets[loadedAssets[i].name] = new AssetInfo(loadedAssets[i], assetType ?? loadedAssets[i].GetType(), version, source, InitialSceneStateMetadata.HasValue); | ||
| Assets[loadedAssets[i].name] = new AssetInfo(loadedAssets[i], assetType ?? loadedAssets[i].GetType(), version, source, isISS); | ||
|
|
||
| Dependencies = dependencies; | ||
|
|
||
| //Debugging purposes. Test cases may bring a null AB, therefore we need this check | ||
| AssetBundleName = Asset?.name; | ||
|
|
||
| //We cannot unload an AB if its an ISS (Initial Scene State AB). It may be a dependency for dynamically isntanced AB | ||
| if (!InitialSceneStateMetadata.HasValue) | ||
| if (!isISS) | ||
| UnloadAB(); | ||
| } | ||
|
|
||
|
|
@@ -157,10 +154,3 @@ public static TValue FirstValueOrDefaultNonAlloc<TKey, TValue>(this Dictionary<T | |
| } | ||
| } | ||
|
|
||
| public struct InitialSceneStateMetadata | ||
| { | ||
| public List<string> assetHash; | ||
| public List<Vector3> positions; | ||
| public List<Quaternion> rotations; | ||
| public List<Vector3> scales; | ||
| } | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocking regression:
isISSis never passed astruefromLoadAssetBundleSystem.CreateAssetBundleDataAsync(the only call site), so every ISS bundle getsisISS = falseandUnloadAB()is called on it. The original code used!InitialSceneStateMetadata.HasValueto guard the unload — this PR removes that guard without restoring it.The PR description claims this flag is "plumbed through so ISS bundles aren't unloaded prematurely" but the code contradicts that. The
//TODO: Rehook isISScomment confirms it's unfinished.Wire
isISSproperly:LoadAssetBundleSystem.CreateAssetBundleDataAsyncreceives no ISS context right now. A simple fix would be to add anisISSparameter toCreateAssetBundleDataAsync, set by the caller viaGetAssetBundleIntention(a new flag on the intention) or by detecting the bundle hash against the ISS URL.