Skip to content

Commit 6dde43d

Browse files
eordanoclaude
andcommitted
fix: unblock camera/avatar input after paste or Alt-Tab focus loss
The input block applied while text input is active was never released when focus was lost mid-input (clipboard paste dialog, Alt-Tab, or spawning into a World), leaving camera and avatar movement permanently locked until restart. Fixes #9502 Includes a regression test that fails without this fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 8cf2529 commit 6dde43d

3 files changed

Lines changed: 288 additions & 1 deletion

File tree

Explorer/Assets/DCL/SceneLoadingScreens/SceneLoadingScreenController.cs

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ public partial class SceneLoadingScreenController : ControllerBase<SceneLoadingS
3131
private SceneTips tips;
3232
private CancellationTokenSource? tipsRotationCancellationToken;
3333
private CancellationTokenSource? tipsFadeCancellationToken;
34+
private bool inputsBlocked;
3435

3536
// IntVariable causes deadlock occasionally.
3637
// There is a similar issue reported on the forum:https://discussions.unity.com/t/deadlock-freezing-issue-with-localizationsettings-stringdatabase-getlocalizedstring-under-multiple-concurrent-calls-on-mobile/1566794
@@ -122,6 +123,12 @@ protected override void OnViewShow()
122123
protected override void OnViewClose()
123124
{
124125
base.OnViewClose();
126+
127+
// The blocked inputs must be restored on every close path, so the release runs first,
128+
// before any statement that can throw: the fade-out is skipped when the close intent
129+
// is cancelled or fails, while OnViewClose is guaranteed by the MVC teardown.
130+
UnblockUnwantedInputs();
131+
125132
tipsRotationCancellationToken?.SafeCancelAndDispose();
126133
tipsFadeCancellationToken?.SafeCancelAndDispose();
127134

@@ -130,7 +137,14 @@ protected override void OnViewClose()
130137
audioMixerVolumesController.UnmuteGroup(AudioMixerExposedParam.Chat_Volume);
131138

132139
viewInstance!.ClearTips();
133-
tips.Release();
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+
}
134148
}
135149

136150
protected override async UniTask WaitForCloseIntentAsync(CancellationToken ct)
@@ -287,11 +301,17 @@ private async UniTaskVoid RotateTipsOverTimeAsync(TimeSpan frequency, Cancellati
287301

288302
private void BlockUnwantedInputs()
289303
{
304+
if (inputsBlocked) return;
305+
306+
inputsBlocked = true;
290307
inputBlock.Disable(InputMapComponent.BLOCK_USER_INPUT);
291308
}
292309

293310
private void UnblockUnwantedInputs()
294311
{
312+
if (!inputsBlocked) return;
313+
314+
inputsBlocked = false;
295315
inputBlock.Enable(InputMapComponent.BLOCK_USER_INPUT);
296316
}
297317
}
Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
1+
using Arch.Core;
2+
using Cysharp.Threading.Tasks;
3+
using DCL.Audio;
4+
using DCL.Input;
5+
using DCL.Input.Component;
6+
using DCL.Prefs;
7+
using DCL.Utilities;
8+
using ECS.Abstract;
9+
using MVC;
10+
using NSubstitute;
11+
using NUnit.Framework;
12+
using System;
13+
using System.Collections.Generic;
14+
using System.Reflection;
15+
using System.Threading;
16+
using System.Threading.Tasks;
17+
using UnityEditor;
18+
using UnityEngine;
19+
using UnityEngine.Audio;
20+
using UnityEngine.TestTools;
21+
using Object = UnityEngine.Object;
22+
23+
namespace DCL.SceneLoadingScreens.Tests
24+
{
25+
/// <summary>
26+
/// Regression coverage for #9502: camera and avatar input stayed permanently blocked after the
27+
/// scene loading screen closed on a cancelled outer token (teleport superseded/aborted mid-load,
28+
/// e.g. spawning into a World via a coordinate link, or a long Alt-Tab stalling the fade).
29+
/// <see cref="InputMapComponent.BlockInput" />/<see cref="InputMapComponent.UnblockInput" /> are
30+
/// refcounted with no external audit or reset, so an acquire in <c>OnBeforeViewShow</c> that is
31+
/// never matched by a release on an abnormal close leaks the block forever - every subsequent
32+
/// "unblock" (e.g. a chat blur) only brings the counter from 2 back to 1, not to 0.
33+
/// See bugreports-early-aug/camera-avatar-locked-after-paste-alttab/{report.md,review.md}.
34+
/// </summary>
35+
public class SceneLoadingScreenControllerInputBlockShould
36+
{
37+
private const string VIEW_PREFAB_PATH = "Assets/DCL/SceneLoadingScreens/Assets/SceneLoadingScreen.prefab";
38+
private const string AUDIO_MIXER_PATH = "Assets/DCL/Audio/Prefabs/GeneralAudioMixer.mixer";
39+
40+
private static readonly InputMapComponent.Kind ALL_KINDS = AllKinds();
41+
private static readonly InputMapComponent.Kind BLOCKED_BY_LOADING_SCREEN = BlockUserInputMask();
42+
43+
// SceneLoadingScreenController.UpdateLocalizedTextAsync() is fired via .Forget() from both
44+
// OnViewInstantiated and OnViewShow - a detached UniTaskVoid this test never has a handle to
45+
// await. In a bare EditMode harness (no active localization catalog) its AsyncOperationHandle
46+
// resolves to Failed a few Editor ticks later and logs "cannot load localized text" - unrelated
47+
// to the input-block bug under test. Nothing else in the process pumps the Editor loop once a
48+
// test's own awaits are done, so without an explicit flush that continuation can resolve
49+
// arbitrarily far in the future (another fixture entirely) - past any ignoreFailingMessages
50+
// window scoped only to [SetUp]/[TearDown] or even [OneTimeSetUp]/[OneTimeTearDown]. The fix is
51+
// two-part: suppress for the whole fixture AND explicitly pump bounded frames at the tail of
52+
// each test (FlushDeferredViewLogsAsync) so the log - if it fires at all - fires HERE, while
53+
// suppression is still active, instead of leaking into an unrelated later test.
54+
private const int FLUSH_FRAME_COUNT = 30;
55+
56+
private World world;
57+
private SingleInstanceEntity inputMapEntity;
58+
private IInputBlock inputBlock;
59+
private SceneLoadingScreenView viewInstance;
60+
private AudioMixerVolumesController audioMixerVolumesController;
61+
private bool originalIgnoreFailingMessages;
62+
63+
[OneTimeSetUp]
64+
public void OneTimeSetUp()
65+
{
66+
// The view's fire-and-forget localized-text refresh is unrelated to the input-block bug
67+
// under test and can log in a bare Editor test context (no active localization init) -
68+
// including from late async continuations that land between tests, so the suppression
69+
// must span the whole fixture, not a single test.
70+
originalIgnoreFailingMessages = LogAssert.ignoreFailingMessages;
71+
LogAssert.ignoreFailingMessages = true;
72+
}
73+
74+
[OneTimeTearDown]
75+
public void OneTimeTearDown()
76+
{
77+
LogAssert.ignoreFailingMessages = originalIgnoreFailingMessages;
78+
}
79+
80+
// Pumps bounded Editor frames so any pending fire-and-forget continuation (see the comment
81+
// above FLUSH_FRAME_COUNT) gets a chance to actually resolve and log before this test returns,
82+
// rather than resolving later while some unrelated test/fixture is running. Tolerant by design:
83+
// the message may fire 0..N times here (or not at all) - LogAssert.ignoreFailingMessages is what
84+
// makes that acceptable, not an expectation that it must occur.
85+
private static async UniTask FlushDeferredViewLogsAsync()
86+
{
87+
for (var i = 0; i < FLUSH_FRAME_COUNT; i++)
88+
await UniTask.Yield();
89+
}
90+
91+
[SetUp]
92+
public void SetUp()
93+
{
94+
// SceneLoadingScreenController's ctor eagerly constructs a PersistentSetting<int>, which reads
95+
// DCLPlayerPrefs.GetInt off the static dclPrefs backing field - never populated in a bare EditMode
96+
// test process (RuntimeInitializeOnLoadMethod only fires in Play/Runtime). Inject an in-memory
97+
// implementation via reflection, the established pattern for this exact gap (see
98+
// ChatReactionRecentsServiceShould/HomeMarkerControllerShould).
99+
var dclPrefsField = typeof(DCLPlayerPrefs).GetField("dclPrefs", BindingFlags.NonPublic | BindingFlags.Static);
100+
dclPrefsField!.SetValue(null, new InMemoryDCLPlayerPrefs());
101+
102+
world = World.Create();
103+
world.Create(new InputMapComponent(ALL_KINDS));
104+
inputMapEntity = world.CacheInputMap();
105+
inputBlock = new ECSInputBlock(world);
106+
107+
var viewPrefab = AssetDatabase.LoadAssetAtPath<SceneLoadingScreenView>(VIEW_PREFAB_PATH);
108+
Assert.IsNotNull(viewPrefab, $"Could not load the real loading-screen prefab from {VIEW_PREFAB_PATH}");
109+
viewInstance = Object.Instantiate(viewPrefab);
110+
111+
var audioMixer = AssetDatabase.LoadAssetAtPath<AudioMixer>(AUDIO_MIXER_PATH);
112+
Assert.IsNotNull(audioMixer, $"Could not load the real audio mixer from {AUDIO_MIXER_PATH}");
113+
audioMixerVolumesController = new AudioMixerVolumesController(audioMixer);
114+
}
115+
116+
[TearDown]
117+
public void TearDown()
118+
{
119+
if (viewInstance != null)
120+
Object.DestroyImmediate(viewInstance.gameObject);
121+
122+
world.Dispose();
123+
124+
// Reset the static field so later tests in the same run aren't left with a stale in-memory
125+
// prefs instance (mirrors the reset half of the same established pattern).
126+
var dclPrefsField = typeof(DCLPlayerPrefs).GetField("dclPrefs", BindingFlags.NonPublic | BindingFlags.Static);
127+
dclPrefsField!.SetValue(null, null);
128+
}
129+
130+
[Test]
131+
public async Task ReleaseInputBlockWhenCloseIntentIsCancelledAsync()
132+
{
133+
// The framework resets LogAssert state at test start, wiping any fixture/SetUp-scoped
134+
// suppression - the flag must be raised inside the test body itself.
135+
LogAssert.ignoreFailingMessages = true;
136+
137+
ISceneTipsProvider tipsProvider = Substitute.For<ISceneTipsProvider>();
138+
139+
tipsProvider.GetAsync(Arg.Any<CancellationToken>())
140+
.Returns(UniTask.FromResult(new SceneTips(TimeSpan.Zero, false, new List<SceneTips.Tip>())));
141+
142+
SceneLoadingScreenController controller = CreateController(tipsProvider);
143+
144+
using var cts = new CancellationTokenSource();
145+
cts.Cancel();
146+
147+
// OnBeforeViewShow blocks input unconditionally on every show. The outer token is already
148+
// cancelled, so WaitForCloseIntentAsync's "if (!ct.IsCancellationRequested) await FadeOutAsync(ct)"
149+
// guard is never entered and the only release the unpatched code has (the last statement of
150+
// FadeOutAsync) never runs - this is leak path 1 from report.md ("outer token cancelled").
151+
await controller.LaunchViewLifeCycleAsync(new CanvasOrdering(CanvasOrdering.SortingLayer.Overlay, 0), CompletedParams(), cts.Token);
152+
153+
Assert.That(ActiveKinds(), Is.EqualTo(ALL_KINDS & ~BLOCKED_BY_LOADING_SCREEN),
154+
"input should be blocked right after showing the loading screen");
155+
156+
// The MVC teardown (MVCManager.ShowOverlayAsync's finally) always calls HideViewAsync once the
157+
// view has started showing, regardless of whether WaitForCloseIntentAsync threw, was cancelled,
158+
// or returned normally - so OnViewClose is guaranteed to run here exactly as it does in production.
159+
await ((IController)controller).HideViewAsync(CancellationToken.None);
160+
161+
Assert.That(ActiveKinds(), Is.EqualTo(ALL_KINDS),
162+
"BLOCK_USER_INPUT must be released when the loading screen closes on a cancelled token - " +
163+
"unpatched, this leaks +1 on the refcount forever (#9502)");
164+
165+
// Let the OnViewInstantiated/OnViewShow localized-text refreshes settle before TearDown
166+
// destroys viewInstance, so any log they produce fires inside this test, not later.
167+
await FlushDeferredViewLogsAsync();
168+
}
169+
170+
[Test]
171+
public async Task ReleaseInputBlockWhenCloseRacesTheInitialTipsLoadAsync()
172+
{
173+
// The framework resets LogAssert state at test start, wiping any fixture/SetUp-scoped
174+
// suppression - the flag must be raised inside the test body itself.
175+
LogAssert.ignoreFailingMessages = true;
176+
177+
ISceneTipsProvider tipsProvider = Substitute.For<ISceneTipsProvider>();
178+
179+
// The tips load never resolves during this test, so `tips` stays at its default value
180+
// (Tips == null) for the whole run - exactly the close-races-the-initial-load scenario from
181+
// review.md ("earliest-cancel variant... cold addressables make the tips window largest on
182+
// first show") that made the first patch attempt's placement of the release - after
183+
// tips.Release() - still leak, because tips.Release() throws on a default SceneTips before
184+
// reaching it.
185+
tipsProvider.GetAsync(Arg.Any<CancellationToken>()).Returns(UniTask.Never<SceneTips>(CancellationToken.None));
186+
187+
SceneLoadingScreenController controller = CreateController(tipsProvider);
188+
189+
// Deliberately not awaited: LaunchViewLifeCycleAsync runs synchronously through
190+
// OnBeforeViewShow/OnViewShow and suspends inside LoadTipsAsync (awaiting a promise that
191+
// never completes), so by the time control returns here the block has already been
192+
// acquired and WaitForCloseIntentAsync is still in flight - matching the real
193+
// MVCManager.ShowOverlayAsync race where the teardown's finally can call HideViewAsync while
194+
// the orphaned lifecycle task is still suspended.
195+
UniTask launch = controller.LaunchViewLifeCycleAsync(new CanvasOrdering(CanvasOrdering.SortingLayer.Overlay, 0), CompletedParams(), CancellationToken.None);
196+
197+
Assert.That(ActiveKinds(), Is.EqualTo(ALL_KINDS & ~BLOCKED_BY_LOADING_SCREEN),
198+
"input should be blocked right after showing the loading screen");
199+
200+
try
201+
{
202+
await ((IController)controller).HideViewAsync(CancellationToken.None);
203+
}
204+
catch (NullReferenceException)
205+
{
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.
210+
}
211+
212+
Assert.That(ActiveKinds(), Is.EqualTo(ALL_KINDS),
213+
"BLOCK_USER_INPUT must be released even when OnViewClose races the initial tips load - " +
214+
"unpatched, this leaks +1 on the refcount forever (#9502)");
215+
216+
launch.Forget();
217+
218+
// Let the OnViewInstantiated/OnViewShow localized-text refreshes settle before TearDown
219+
// destroys viewInstance, so any log they produce fires inside this test, not later.
220+
await FlushDeferredViewLogsAsync();
221+
}
222+
223+
private SceneLoadingScreenController CreateController(ISceneTipsProvider tipsProvider) =>
224+
new (() => viewInstance, tipsProvider, TimeSpan.Zero, audioMixerVolumesController, inputBlock);
225+
226+
private static SceneLoadingScreenController.Params CompletedParams()
227+
{
228+
AsyncLoadProcessReport report = AsyncLoadProcessReport.Create(CancellationToken.None);
229+
report.SetProgress(1f);
230+
return new SceneLoadingScreenController.Params(report);
231+
}
232+
233+
private InputMapComponent.Kind ActiveKinds() =>
234+
inputMapEntity.GetInputMapComponent(world).Active;
235+
236+
private static InputMapComponent.Kind AllKinds()
237+
{
238+
InputMapComponent.Kind all = InputMapComponent.Kind.None;
239+
240+
foreach (InputMapComponent.Kind kind in InputMapComponent.VALUES)
241+
all |= kind;
242+
243+
return all;
244+
}
245+
246+
private static InputMapComponent.Kind BlockUserInputMask()
247+
{
248+
InputMapComponent.Kind mask = InputMapComponent.Kind.None;
249+
250+
foreach (InputMapComponent.Kind kind in InputMapComponent.BLOCK_USER_INPUT)
251+
mask |= kind;
252+
253+
return mask;
254+
}
255+
}
256+
}

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

Lines changed: 11 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)