Skip to content

Commit 346d5fd

Browse files
author
Miguel Cartier
committed
fix(ui-service)!: implement Phase 2 high priority fixes
- Add null checks for AnimationDelayer clips - Add exception handling in UnloadUi and RemoveUiSet methods - Optimize ReadOnly properties to eliminate GC allocations - Fix CloseAllUi to avoid modification during iteration - Add CancellationToken support to async methods - Replace Task.Delay with UniTask.Delay for proper Unity patterns BREAKING CHANGE: IUiAssetLoader and IPresenterDelayer interfaces now use UniTask and CancellationToken
1 parent 9353170 commit 346d5fd

File tree

6 files changed

+125
-97
lines changed

6 files changed

+125
-97
lines changed

‎Runtime/DelayUiPresenter.cs‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
using UnityEngine;
1+
using Cysharp.Threading.Tasks;
2+
using UnityEngine;
23

34
namespace GameLovers.UiService
45
{
@@ -20,13 +21,13 @@ private void OnValidate()
2021
/// <inheritdoc />
2122
protected override void OnOpened()
2223
{
23-
_ = _delayer.OpenWithDelay(OnOpenedCompleted);
24+
_delayer.OpenWithDelay(OnOpenedCompleted).Forget();
2425
}
2526

2627
/// <inheritdoc />
2728
protected override void OnClosed()
2829
{
29-
_ = _delayer.CloseWithDelay(OnClosedCompleted);
30+
_delayer.CloseWithDelay(OnClosedCompleted).Forget();
3031
}
3132

3233
/// <summary>
@@ -79,13 +80,13 @@ private void OnValidate()
7980
/// <inheritdoc />
8081
protected override void OnOpened()
8182
{
82-
_ = _delayer.OpenWithDelay(OnOpenedCompleted);
83+
_delayer.OpenWithDelay(OnOpenedCompleted).Forget();
8384
}
8485

8586
/// <inheritdoc />
8687
protected override void OnClosed()
8788
{
88-
_ = _delayer.CloseWithDelay(OnClosedCompleted);
89+
_delayer.CloseWithDelay(OnClosedCompleted).Forget();
8990
}
9091

9192
/// <summary>

‎Runtime/Delayers/AnimationDelayer.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@ public class AnimationDelayer : PresenterDelayerBase
1616
[SerializeField] protected AnimationClip _outroAnimationClip;
1717

1818
/// <inheritdoc />
19-
public override float OpenDelayInSeconds => _introAnimationClip.length;
19+
public override float OpenDelayInSeconds => _introAnimationClip?.length ?? 0f;
2020

2121
/// <inheritdoc />
22-
public override float CloseDelayInSeconds => _outroAnimationClip.length;
22+
public override float CloseDelayInSeconds => _outroAnimationClip?.length ?? 0f;
2323

2424
private void OnValidate()
2525
{

‎Runtime/Delayers/PresenterDelayerBase.cs‎

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
using System;
2-
using System.Threading.Tasks;
1+
using Cysharp.Threading.Tasks;
2+
using System;
33
using UnityEngine;
44

55
namespace GameLovers.UiService
@@ -19,21 +19,21 @@ public interface IPresenterDelayer
1919
/// </summary>
2020
float CloseDelayInSeconds { get; }
2121

22-
/// <summary>
23-
/// Gets the Task of the current's delay process
24-
/// </summary>
25-
Task CurrentDelayTask { get; }
22+
/// <summary>
23+
/// Gets the UniTask of the current's delay process
24+
/// </summary>
25+
UniTask CurrentDelayTask { get; }
2626
}
2727

2828
/// <inheritdoc />
2929
public abstract class PresenterDelayerBase : MonoBehaviour, IPresenterDelayer
3030
{
31-
/// <inheritdoc />
32-
public abstract float OpenDelayInSeconds { get; }
33-
/// <inheritdoc />
34-
public abstract float CloseDelayInSeconds { get; }
35-
/// <inheritdoc />
36-
public Task CurrentDelayTask { get; protected set; }
31+
/// <inheritdoc />
32+
public abstract float OpenDelayInSeconds { get; }
33+
/// <inheritdoc />
34+
public abstract float CloseDelayInSeconds { get; }
35+
/// <inheritdoc />
36+
public UniTask CurrentDelayTask { get; protected set; }
3737

3838
/// <summary>
3939
/// Called when the presenter's opening delay starts.
@@ -45,33 +45,33 @@ protected virtual void OnOpenStarted() { }
4545
/// </summary>
4646
protected virtual void OnCloseStarted() { }
4747

48-
internal async Task OpenWithDelay(Action onOpenedCompleted)
49-
{
50-
OnOpenStarted();
48+
internal async UniTask OpenWithDelay(Action onOpenedCompleted)
49+
{
50+
OnOpenStarted();
5151

52-
CurrentDelayTask = Task.Delay(Mathf.RoundToInt(OpenDelayInSeconds * 1000));
52+
CurrentDelayTask = UniTask.Delay(TimeSpan.FromSeconds(OpenDelayInSeconds));
5353

54-
await CurrentDelayTask;
54+
await CurrentDelayTask;
5555

56-
if (gameObject != null)
57-
{
58-
onOpenedCompleted();
59-
}
56+
if (gameObject != null)
57+
{
58+
onOpenedCompleted();
6059
}
60+
}
6161

62-
internal async Task CloseWithDelay(Action onCloseCompleted)
63-
{
64-
OnCloseStarted();
62+
internal async UniTask CloseWithDelay(Action onCloseCompleted)
63+
{
64+
OnCloseStarted();
6565

66-
CurrentDelayTask = Task.Delay(Mathf.RoundToInt(CloseDelayInSeconds * 1000));
66+
CurrentDelayTask = UniTask.Delay(TimeSpan.FromSeconds(CloseDelayInSeconds));
6767

68-
await CurrentDelayTask;
68+
await CurrentDelayTask;
6969

70-
if (gameObject != null)
71-
{
72-
gameObject.SetActive(false);
73-
onCloseCompleted();
74-
}
70+
if (gameObject != null)
71+
{
72+
gameObject.SetActive(false);
73+
onCloseCompleted();
7574
}
75+
}
7676
}
7777
}

‎Runtime/IUiService.cs‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
using Cysharp.Threading.Tasks;
22
using System;
33
using System.Collections.Generic;
4-
using System.Threading.Tasks;
4+
using System.Threading;
55
using UnityEngine;
66

77
// ReSharper disable CheckNamespace
@@ -110,9 +110,10 @@ public interface IUiService
110110
/// </summary>
111111
/// <typeparam name="T">The type of UI to load.</typeparam>
112112
/// <param name="openAfter">Whether to open the UI after loading.</param>
113+
/// <param name="cancellationToken">Cancellation token to cancel the operation.</param>
113114
/// <returns>A task that completes with the loaded UI.</returns>
114115
/// <exception cref="KeyNotFoundException">Thrown if the service does not contain a UI configuration for the specified type.</exception>
115-
async UniTask<T> LoadUiAsync<T>(bool openAfter = false) where T : UiPresenter => (await LoadUiAsync(typeof(T), openAfter)) as T;
116+
async UniTask<T> LoadUiAsync<T>(bool openAfter = false, CancellationToken cancellationToken = default) where T : UiPresenter => (await LoadUiAsync(typeof(T), openAfter, cancellationToken)) as T;
116117

117118
/// <summary>
118119
/// Loads the UI of the specified type asynchronously.
@@ -121,9 +122,10 @@ public interface IUiService
121122
/// </summary>
122123
/// <param name="type">The type of UI to load.</param>
123124
/// <param name="openAfter">Whether to open the UI after loading.</param>
125+
/// <param name="cancellationToken">Cancellation token to cancel the operation.</param>
124126
/// <returns>A task that completes with the loaded UI.</returns>
125127
/// <exception cref="KeyNotFoundException">Thrown if the service does not contain a UI configuration for the specified type.</exception>
126-
UniTask<UiPresenter> LoadUiAsync(Type type, bool openAfter = false);
128+
UniTask<UiPresenter> LoadUiAsync(Type type, bool openAfter = false, CancellationToken cancellationToken = default);
127129

128130
/// <summary>
129131
/// Loads all UI presenters from the specified UI set asynchronously.
@@ -168,34 +170,38 @@ public interface IUiService
168170
/// Opens a UI presenter asynchronously, loading its assets if necessary.
169171
/// </summary>
170172
/// <typeparam name="T">The type of UI presenter to open.</typeparam>
173+
/// <param name="cancellationToken">Cancellation token to cancel the operation.</param>
171174
/// <returns>A task that completes when the UI presenter is opened.</returns>
172-
async UniTask<T> OpenUiAsync<T>() where T : UiPresenter => (await OpenUiAsync(typeof(T))) as T;
175+
async UniTask<T> OpenUiAsync<T>(CancellationToken cancellationToken = default) where T : UiPresenter => (await OpenUiAsync(typeof(T), cancellationToken)) as T;
173176

174177
/// <summary>
175178
/// Opens a UI presenter asynchronously, loading its assets if necessary.
176179
/// </summary>
177180
/// <param name="type">The type of UI presenter to open.</param>
181+
/// <param name="cancellationToken">Cancellation token to cancel the operation.</param>
178182
/// <returns>A task that completes when the UI presenter is opened.</returns>
179-
UniTask<UiPresenter> OpenUiAsync(Type type);
183+
UniTask<UiPresenter> OpenUiAsync(Type type, CancellationToken cancellationToken = default);
180184

181185
/// <summary>
182186
/// Opens a UI presenter asynchronously, loading its assets if necessary, and sets its initial data.
183187
/// </summary>
184188
/// <typeparam name="T">The type of UI presenter to open.</typeparam>
185189
/// <typeparam name="TData">The type of initial data to set.</typeparam>
186190
/// <param name="initialData">The initial data to set.</param>
191+
/// <param name="cancellationToken">Cancellation token to cancel the operation.</param>
187192
/// <returns>A task that completes when the UI presenter is opened.</returns>
188-
async UniTask<T> OpenUiAsync<T, TData>(TData initialData)
193+
async UniTask<T> OpenUiAsync<T, TData>(TData initialData, CancellationToken cancellationToken = default)
189194
where T : class, IUiPresenterData
190-
where TData : struct => await OpenUiAsync(typeof(T), initialData) as T;
195+
where TData : struct => await OpenUiAsync(typeof(T), initialData, cancellationToken) as T;
191196

192197
/// <summary>
193198
/// Opens a UI presenter asynchronously, loading its assets if necessary, and sets its initial data.
194199
/// </summary>
195200
/// <param name="type">The type of UI presenter to open.</param>
196201
/// <param name="initialData">The initial data to set.</param>
202+
/// <param name="cancellationToken">Cancellation token to cancel the operation.</param>
197203
/// <returns>A task that completes when the UI presenter is opened.</returns>
198-
UniTask<UiPresenter> OpenUiAsync<TData>(Type type, TData initialData) where TData : struct;
204+
UniTask<UiPresenter> OpenUiAsync<TData>(Type type, TData initialData, CancellationToken cancellationToken = default) where TData : struct;
199205

200206
/// <summary>
201207
/// Closes a UI presenter and optionally destroys its assets.

‎Runtime/UiAssetLoader.cs‎

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using Cysharp.Threading.Tasks;
2+
using System.Threading;
23
using UnityEngine;
34
using UnityEngine.AddressableAssets;
45
using UnityEngine.ResourceManagement.AsyncOperations;
@@ -13,13 +14,14 @@ namespace GameLovers.UiService
1314
/// </summary>
1415
public interface IUiAssetLoader
1516
{
16-
/// <summary>
17-
/// Instantiates the prefab asynchronously with the given <paramref name="config"/> and <paramref name="parent"/>.
18-
/// </summary>
19-
/// <param name="config">The UI configuration to instantiate.</param>
20-
/// <param name="parent">The parent transform to instantiate the prefab under.</param>
21-
/// <returns>A task that completes with the instantiated prefab game object.</returns>
22-
UniTask<GameObject> InstantiatePrefab(UiConfig config, Transform parent);
17+
/// <summary>
18+
/// Instantiates the prefab asynchronously with the given <paramref name="config"/> and <paramref name="parent"/>.
19+
/// </summary>
20+
/// <param name="config">The UI configuration to instantiate.</param>
21+
/// <param name="parent">The parent transform to instantiate the prefab under.</param>
22+
/// <param name="cancellationToken">Cancellation token to cancel the operation.</param>
23+
/// <returns>A task that completes with the instantiated prefab game object.</returns>
24+
UniTask<GameObject> InstantiatePrefab(UiConfig config, Transform parent, CancellationToken cancellationToken = default);
2325

2426
/// <summary>
2527
/// Unloads the given <paramref name="asset"/> from the game memory
@@ -30,27 +32,27 @@ public interface IUiAssetLoader
3032
/// <inheritdoc />
3133
public class UiAssetLoader : IUiAssetLoader
3234
{
33-
/// <inheritdoc />
34-
public async UniTask<GameObject> InstantiatePrefab(UiConfig config, Transform parent)
35+
/// <inheritdoc />
36+
public async UniTask<GameObject> InstantiatePrefab(UiConfig config, Transform parent, CancellationToken cancellationToken = default)
37+
{
38+
var operation = Addressables.InstantiateAsync(config.AddressableAddress, new InstantiationParameters(parent, false));
39+
40+
if(config.LoadSynchronously)
3541
{
36-
var operation = Addressables.InstantiateAsync(config.AddressableAddress, new InstantiationParameters(parent, false));
37-
38-
if(config.LoadSynchronously)
39-
{
40-
operation.WaitForCompletion();
41-
}
42-
else
43-
{
44-
await operation.Task;
45-
}
46-
47-
if (operation.Status != AsyncOperationStatus.Succeeded)
48-
{
49-
throw operation.OperationException;
50-
}
51-
52-
return operation.Result;
42+
operation.WaitForCompletion();
5343
}
44+
else
45+
{
46+
await operation.ToUniTask(cancellationToken: cancellationToken);
47+
}
48+
49+
if (operation.Status != AsyncOperationStatus.Succeeded)
50+
{
51+
throw operation.OperationException;
52+
}
53+
54+
return operation.Result;
55+
}
5456

5557
/// <inheritdoc />
5658
public void UnloadAsset(GameObject asset)

0 commit comments

Comments
 (0)