Skip to content

Commit af6da23

Browse files
refactor: drop impossible-situation guard around tips.Release()
Both ISceneTipsProvider implementations resolve synchronously, so tips is always assigned before any close path can run (review feedback). Also clears the warning-ratchet findings in the touched files (nullable test fields, redundant usings).
1 parent 6dde43d commit af6da23

3 files changed

Lines changed: 13 additions & 21 deletions

File tree

Explorer/Assets/DCL/SceneLoadingScreens/ISceneTipsProvider.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
using Cysharp.Threading.Tasks;
22
using System.Threading;
3-
using UnityEngine;
43

54
namespace DCL.SceneLoadingScreens
65
{

Explorer/Assets/DCL/SceneLoadingScreens/SceneLoadingScreenController.cs

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -137,14 +137,7 @@ protected override void OnViewClose()
137137
audioMixerVolumesController.UnmuteGroup(AudioMixerExposedParam.Chat_Volume);
138138

139139
viewInstance!.ClearTips();
140-
141-
// Tips is null until the first load completes: a close racing that load must not
142-
// release a default instance, and a later close must not release the same tips twice.
143-
if (tips.Tips != null)
144-
{
145-
tips.Release();
146-
tips = default;
147-
}
140+
tips.Release();
148141
}
149142

150143
protected override async UniTask WaitForCloseIntentAsync(CancellationToken ct)

Explorer/Assets/DCL/SceneLoadingScreens/Tests/SceneLoadingScreenControllerInputBlockShould.cs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
using System.Threading;
1616
using System.Threading.Tasks;
1717
using UnityEditor;
18-
using UnityEngine;
1918
using UnityEngine.Audio;
2019
using UnityEngine.TestTools;
2120
using Object = UnityEngine.Object;
@@ -53,11 +52,11 @@ public class SceneLoadingScreenControllerInputBlockShould
5352
// suppression is still active, instead of leaking into an unrelated later test.
5453
private const int FLUSH_FRAME_COUNT = 30;
5554

56-
private World world;
55+
private World? world;
5756
private SingleInstanceEntity inputMapEntity;
58-
private IInputBlock inputBlock;
59-
private SceneLoadingScreenView viewInstance;
60-
private AudioMixerVolumesController audioMixerVolumesController;
57+
private IInputBlock? inputBlock;
58+
private SceneLoadingScreenView? viewInstance;
59+
private AudioMixerVolumesController? audioMixerVolumesController;
6160
private bool originalIgnoreFailingMessages;
6261

6362
[OneTimeSetUp]
@@ -119,7 +118,7 @@ public void TearDown()
119118
if (viewInstance != null)
120119
Object.DestroyImmediate(viewInstance.gameObject);
121120

122-
world.Dispose();
121+
world!.Dispose();
123122

124123
// Reset the static field so later tests in the same run aren't left with a stale in-memory
125124
// prefs instance (mirrors the reset half of the same established pattern).
@@ -203,10 +202,11 @@ public async Task ReleaseInputBlockWhenCloseRacesTheInitialTipsLoadAsync()
203202
}
204203
catch (NullReferenceException)
205204
{
206-
// Pre-existing, separate defect (review.md finding 1): unpatched OnViewClose() calls
207-
// tips.Release() on a still-default `tips` and throws. That defect is not what is under
208-
// test here - what matters is whether the input block was released before that
209-
// statement could run at all, which is asserted below regardless of this exception.
205+
// An artifact of the never-resolving tips mock above: OnViewClose() calls tips.Release()
206+
// on a still-default `tips` and throws. The real ISceneTipsProvider implementations
207+
// resolve synchronously, so in production `tips` is always assigned before any close.
208+
// It is not what is under test here - what matters is whether the input block was
209+
// released before that statement could run at all, asserted below regardless.
210210
}
211211

212212
Assert.That(ActiveKinds(), Is.EqualTo(ALL_KINDS),
@@ -221,7 +221,7 @@ public async Task ReleaseInputBlockWhenCloseRacesTheInitialTipsLoadAsync()
221221
}
222222

223223
private SceneLoadingScreenController CreateController(ISceneTipsProvider tipsProvider) =>
224-
new (() => viewInstance, tipsProvider, TimeSpan.Zero, audioMixerVolumesController, inputBlock);
224+
new (() => viewInstance!, tipsProvider, TimeSpan.Zero, audioMixerVolumesController!, inputBlock!);
225225

226226
private static SceneLoadingScreenController.Params CompletedParams()
227227
{
@@ -231,7 +231,7 @@ private static SceneLoadingScreenController.Params CompletedParams()
231231
}
232232

233233
private InputMapComponent.Kind ActiveKinds() =>
234-
inputMapEntity.GetInputMapComponent(world).Active;
234+
inputMapEntity.GetInputMapComponent(world!).Active;
235235

236236
private static InputMapComponent.Kind AllKinds()
237237
{

0 commit comments

Comments
 (0)