From f12c9301d7005a50caabc690e2af622ea2ff2992 Mon Sep 17 00:00:00 2001 From: Mikhail Agapov <118179774+mikhail-dcl@users.noreply.github.com> Date: Thu, 11 Jan 2024 17:45:04 +0100 Subject: [PATCH] Get rid of nullability of AudioClip promise as it was not communicated back to ECS (#257) --- .../Components/AudioSourceComponent.cs | 7 +++---- .../Systems/CleanUpAudioSourceSystem.cs | 17 +++++++---------- .../Systems/StartAudioSourceLoadingSystem.cs | 6 ++---- .../Systems/UpdateAudioSourceSystem.cs | 12 +++++++++--- .../Tests/AudioSourcePluginIntegrationTests.cs | 4 ++-- .../Tests/CreateAudioSourceSystemShould.cs | 8 ++++---- .../Tests/StartAudioClipLoadingSystemShould.cs | 4 ++-- 7 files changed, 29 insertions(+), 29 deletions(-) diff --git a/Explorer/Assets/DCL/SDKComponents/AudioSources/Components/AudioSourceComponent.cs b/Explorer/Assets/DCL/SDKComponents/AudioSources/Components/AudioSourceComponent.cs index 7e9e32ba20..7a1e1d91f5 100644 --- a/Explorer/Assets/DCL/SDKComponents/AudioSources/Components/AudioSourceComponent.cs +++ b/Explorer/Assets/DCL/SDKComponents/AudioSources/Components/AudioSourceComponent.cs @@ -1,5 +1,4 @@ using DCL.ECSComponents; -using DCL.Optimization.Pools; using System; using UnityEngine; using Promise = ECS.StreamableLoading.Common.AssetPromise; @@ -10,16 +9,16 @@ public struct AudioSourceComponent : IDisposable { public readonly PBAudioSource PBAudioSource; - public Promise? ClipPromise; + public Promise ClipPromise; /// /// The final audio source ready for consumption /// public AudioSource Result; - public AudioSourceComponent(PBAudioSource pbAudioSource) + public AudioSourceComponent(PBAudioSource pbAudioSource, Promise promise) { - ClipPromise = null; + ClipPromise = promise; PBAudioSource = pbAudioSource; Result = null; } diff --git a/Explorer/Assets/DCL/SDKComponents/AudioSources/Systems/CleanUpAudioSourceSystem.cs b/Explorer/Assets/DCL/SDKComponents/AudioSources/Systems/CleanUpAudioSourceSystem.cs index afc008bb5c..130e2bde0c 100644 --- a/Explorer/Assets/DCL/SDKComponents/AudioSources/Systems/CleanUpAudioSourceSystem.cs +++ b/Explorer/Assets/DCL/SDKComponents/AudioSources/Systems/CleanUpAudioSourceSystem.cs @@ -17,11 +17,11 @@ namespace DCL.SDKComponents.AudioSources [UpdateInGroup(typeof(CleanUpGroup))] [LogCategory(ReportCategory.AUDIO_SOURCES)] [ThrottlingEnabled] - public partial class CleanUpAudioSourceSystem: BaseUnityLoopSystem, IFinalizeWorldSystem + public partial class CleanUpAudioSourceSystem : BaseUnityLoopSystem, IFinalizeWorldSystem { private ReleaseAudioSourceComponent releaseAudioSourceComponent; - private CleanUpAudioSourceSystem(World world, AudioClipsCache cache,IComponentPoolsRegistry poolsRegistry) : base(world) + private CleanUpAudioSourceSystem(World world, AudioClipsCache cache, IComponentPoolsRegistry poolsRegistry) : base(world) { releaseAudioSourceComponent = new ReleaseAudioSourceComponent(world, cache, poolsRegistry); } @@ -59,7 +59,7 @@ public void FinalizeComponents(in Query query) private readonly AudioClipsCache cache; private readonly IComponentPool componentPool; - public ReleaseAudioSourceComponent(World world, AudioClipsCache cache,IComponentPoolsRegistry poolsRegistry) + public ReleaseAudioSourceComponent(World world, AudioClipsCache cache, IComponentPoolsRegistry poolsRegistry) { this.world = world; this.cache = cache; @@ -69,18 +69,15 @@ public ReleaseAudioSourceComponent(World world, AudioClipsCache cache,IComponent public void Update(ref AudioSourceComponent component) { - if (component.ClipPromise == null) return; // loading not started + GetAudioClipIntention clipIntent = component.ClipPromise.LoadingIntention; + + component.ClipPromise.ForgetLoading(world); if (component.Result == null) // loading in progress - { - component.ClipPromise.Value.ForgetLoading(world); - component.ClipPromise = null; return; - } - cache.Dereference(component.ClipPromise!.Value.LoadingIntention, component.Result.clip); + cache.Dereference(clipIntent, component.Result.clip); componentPool.Release(component.Result); - component.Dispose(); } } diff --git a/Explorer/Assets/DCL/SDKComponents/AudioSources/Systems/StartAudioSourceLoadingSystem.cs b/Explorer/Assets/DCL/SDKComponents/AudioSources/Systems/StartAudioSourceLoadingSystem.cs index b2f2001891..ee86a9eab3 100644 --- a/Explorer/Assets/DCL/SDKComponents/AudioSources/Systems/StartAudioSourceLoadingSystem.cs +++ b/Explorer/Assets/DCL/SDKComponents/AudioSources/Systems/StartAudioSourceLoadingSystem.cs @@ -44,13 +44,11 @@ private void CreateAudioSourceComponentWithPromise(in Entity entity, ref PBAudio if (!frameTimeBudgetProvider.TrySpendBudget()) return; if (!sceneData.TryGetContentUrl(sdkAudioSource.AudioClipUrl, out URLAddress audioClipUrl)) return; - var audioSourceComponent = new AudioSourceComponent(sdkAudioSource); - - audioSourceComponent.ClipPromise = Promise.Create(World, new GetAudioClipIntention + var audioSourceComponent = new AudioSourceComponent(sdkAudioSource, Promise.Create(World, new GetAudioClipIntention { CommonArguments = new CommonLoadingArguments(audioClipUrl), AudioType = sdkAudioSource.AudioClipUrl.ToAudioType(), - }, partitionComponent); + }, partitionComponent)); World.Add(entity, audioSourceComponent); } diff --git a/Explorer/Assets/DCL/SDKComponents/AudioSources/Systems/UpdateAudioSourceSystem.cs b/Explorer/Assets/DCL/SDKComponents/AudioSources/Systems/UpdateAudioSourceSystem.cs index 17e7e9fa25..39146a910c 100644 --- a/Explorer/Assets/DCL/SDKComponents/AudioSources/Systems/UpdateAudioSourceSystem.cs +++ b/Explorer/Assets/DCL/SDKComponents/AudioSources/Systems/UpdateAudioSourceSystem.cs @@ -34,6 +34,7 @@ protected override void Update(float t) { CreateAudioSourceQuery(World); UpdateAudioSourceQuery(World); + // TODO: Handle Volume updates - refer to ECSAudioSourceComponentHandler.cs in unity-renderer and check UpdateAudioSourceVolume() method and its usages } @@ -41,7 +42,7 @@ protected override void Update(float t) [All(typeof(PBAudioSource), typeof(AudioSourceComponent))] private void UpdateAudioSource(ref PBAudioSource sdkAudioSource, ref AudioSourceComponent audioSourceComponent) { - if (sdkAudioSource.IsDirty) + if (sdkAudioSource.IsDirty && audioSourceComponent.Result != null) { audioSourceComponent.Result.ApplyPBAudioSource(sdkAudioSource); sdkAudioSource.IsDirty = false; @@ -51,9 +52,11 @@ private void UpdateAudioSource(ref PBAudioSource sdkAudioSource, ref AudioSource } [Query] - private void CreateAudioSource(ref AudioSourceComponent audioSourceComponent, ref TransformComponent entityTransform) + private void CreateAudioSource(ref PBAudioSource sdkAudioSource, ref AudioSourceComponent audioSourceComponent, ref TransformComponent entityTransform) { - if (NoBudget() || audioSourceComponent.ClipPromise == null || !audioSourceComponent.ClipPromise.Value.TryConsume(World, out StreamableLoadingResult promiseResult)) + if (NoBudget() + || audioSourceComponent.ClipPromise.IsConsumed + || !audioSourceComponent.ClipPromise.TryConsume(World, out StreamableLoadingResult promiseResult)) return; if (audioSourceComponent.Result == null) @@ -61,6 +64,9 @@ private void CreateAudioSource(ref AudioSourceComponent audioSourceComponent, re audioSourceComponent.Result.FromPBAudioSource(promiseResult.Asset, audioSourceComponent.PBAudioSource); + // Reset isDirty as we just applied the PBAudioSource to the AudioSource + sdkAudioSource.IsDirty = false; + Transform transform = audioSourceComponent.Result.transform; transform.SetParent(entityTransform.Transform, false); transform.ResetLocalTRS(); diff --git a/Explorer/Assets/DCL/SDKComponents/AudioSources/Tests/AudioSourcePluginIntegrationTests.cs b/Explorer/Assets/DCL/SDKComponents/AudioSources/Tests/AudioSourcePluginIntegrationTests.cs index f6f4b1b7c9..d719251139 100644 --- a/Explorer/Assets/DCL/SDKComponents/AudioSources/Tests/AudioSourcePluginIntegrationTests.cs +++ b/Explorer/Assets/DCL/SDKComponents/AudioSources/Tests/AudioSourcePluginIntegrationTests.cs @@ -65,10 +65,10 @@ public async Task ShouldCreateAudioSource_WithDownloadedAudioClip_WhenPBAudioSou startLoadingSystem.Update(0); world.TryGet(entity, out AudioSourceComponent audioSourceComponent); - world.Get(audioSourceComponent.ClipPromise!.Value.Entity).SetAllowed(Substitute.For()); + world.Get(audioSourceComponent.ClipPromise.Entity).SetAllowed(Substitute.For()); loadAudioClipSystem.Update(1); - await UniTask.WaitUntil(() => audioSourceComponent.ClipPromise.Value.TryGetResult(world, out _)); + await UniTask.WaitUntil(() => audioSourceComponent.ClipPromise.TryGetResult(world, out _)); updateAudioSourceSystem.Update(2); diff --git a/Explorer/Assets/DCL/SDKComponents/AudioSources/Tests/CreateAudioSourceSystemShould.cs b/Explorer/Assets/DCL/SDKComponents/AudioSources/Tests/CreateAudioSourceSystemShould.cs index 0ca1701898..d7b741c743 100644 --- a/Explorer/Assets/DCL/SDKComponents/AudioSources/Tests/CreateAudioSourceSystemShould.cs +++ b/Explorer/Assets/DCL/SDKComponents/AudioSources/Tests/CreateAudioSourceSystemShould.cs @@ -1,4 +1,5 @@ using Arch.Core; +using DCL.ECSComponents; using DCL.Optimization.PerformanceBudgeting; using DCL.Optimization.Pools; using ECS.Prioritization.Components; @@ -28,13 +29,12 @@ public void SetUp() void CreateComponent() { - component = new AudioSourceComponent(CreatePBAudioSource()); - component.ClipPromise = AssetPromise.Create(world, new GetAudioClipIntention(), PartitionComponent.TOP_PRIORITY); + component = new AudioSourceComponent(CreatePBAudioSource(), AssetPromise.Create(world, new GetAudioClipIntention(), PartitionComponent.TOP_PRIORITY)); } void CreateEntity() { - entity = world.Create(component); + entity = world.Create(component, new PBAudioSource()); AddTransformToEntity(entity); } } @@ -75,7 +75,7 @@ public void NotCreateAudioSourceIfClipNotFinishedLoading() public void CreateAudioSourceFromResolvedPromise() { // Arrange - world.Add(component.ClipPromise!.Value.Entity, new StreamableLoadingResult(TestAudioClip)); + world.Add(component.ClipPromise.Entity, new StreamableLoadingResult(TestAudioClip)); // Act system.Update(0); diff --git a/Explorer/Assets/DCL/SDKComponents/AudioSources/Tests/StartAudioClipLoadingSystemShould.cs b/Explorer/Assets/DCL/SDKComponents/AudioSources/Tests/StartAudioClipLoadingSystemShould.cs index 5cec9ba2c8..0dbb26acbe 100644 --- a/Explorer/Assets/DCL/SDKComponents/AudioSources/Tests/StartAudioClipLoadingSystemShould.cs +++ b/Explorer/Assets/DCL/SDKComponents/AudioSources/Tests/StartAudioClipLoadingSystemShould.cs @@ -64,8 +64,8 @@ public void CreateAudioSourceComponentForPBAudioSource() Assert.That(audioSourceComponent.Result, Is.Null); // Assert promise - Assert.That(audioSourceComponent.ClipPromise!.Value, Is.Not.Null); - AssetPromise promiseValue = audioSourceComponent.ClipPromise.Value; + Assert.That(audioSourceComponent.ClipPromise, Is.Not.EqualTo(AssetPromise.NULL)); + AssetPromise promiseValue = audioSourceComponent.ClipPromise; Assert.That(world.TryGet(promiseValue.Entity, out GetAudioClipIntention intention), Is.True); Assert.That(intention.CommonArguments.URL, Is.EqualTo(pbAudioSource.AudioClipUrl)); Assert.That(intention.AudioType, Is.EqualTo(pbAudioSource.AudioClipUrl.ToAudioType()));