FIX: Navigation not being confined to multiplayer playerRoot (case 1306361, #1443).

Author: Rene Damm

Assets/Tests/InputSystem/Plugins/UITests.cs

@@ -42,6 +42,7 @@ internal class UITests : CoreTestsFixture
     private struct TestObjects
     {
         public Camera camera;
+        public Canvas canvas;
         public InputSystemUIInputModule uiModule;
         public TestEventSystem eventSystem;
         public GameObject parentGameObject;
@@ -102,7 +103,7 @@ private static TestObjects CreateUIScene()
     // Set up a InputSystemUIInputModule with a full roster of actions and inputs
     // and then see if we can generate all the various events expected by the UI
     // from activity on input devices.
-    private static TestObjects CreateTestUI(Rect viewport = default, bool noFirstSelected = false, string namePrefix = "")
+    private static TestObjects CreateTestUI(Rect viewport = default, bool noFirstSelected = false, string namePrefix = "", bool makeSelectable = false)
     {
         var objects = new TestObjects();
 
@@ -126,6 +127,7 @@ private static TestObjects CreateTestUI(Rect viewport = default, bool noFirstSel
         canvasObject.AddComponent<GraphicRaycaster>();
         canvasObject.AddComponent<TrackedDeviceRaycaster>();
         canvas.worldCamera = objects.camera;
+        objects.canvas = canvas;
 
         // Set up a GameObject hierarchy that we send events to. In a real setup,
         // this would be a hierarchy involving UI components.
@@ -141,13 +143,17 @@ private static TestObjects CreateTestUI(Rect viewport = default, bool noFirstSel
         leftChildGameObject.AddComponent<Image>();
         objects.leftChildReceiver = leftChildGameObject.AddComponent<UICallbackReceiver>();
         objects.leftGameObject = leftChildGameObject;
+        if (makeSelectable)
+            leftChildGameObject.AddComponent<Selectable>();
 
         var rightChildGameObject = new GameObject(namePrefix + "Right Child");
         rightChildGameObject.SetActive(false);
         var rightChildTransform = rightChildGameObject.AddComponent<RectTransform>();
         rightChildGameObject.AddComponent<Image>();
         objects.rightChildReceiver = rightChildGameObject.AddComponent<UICallbackReceiver>();
         objects.rightGameObject = rightChildGameObject;
+        if (makeSelectable)
+            rightChildGameObject.AddComponent<Selectable>();
 
         parentTransform.SetParent(canvasObject.transform, worldPositionStays: false);
         leftChildTransform.SetParent(parentTransform, worldPositionStays: false);
@@ -2451,15 +2457,29 @@ public IEnumerator UI_CanOperateMultiplayerUIGloballyUsingMouse()
 
 #endif
 
-    [UnityTest]
-    [Category("UI")]
     // Check that two players can have separate UI and control it using separate gamepads, using
     // MultiplayerEventSystem.
+    [UnityTest]
+    [Category("UI")]
     public IEnumerator UI_CanOperateMultiplayerUILocallyUsingGamepads()
     {
         // Create devices.
         var gamepads = new[] { InputSystem.AddDevice<Gamepad>(), InputSystem.AddDevice<Gamepad>() };
-        var players = new[] { CreateTestUI(new Rect(0, 0, 0.5f, 1)), CreateTestUI(new Rect(0.5f, 0, 0.5f, 1)) };
+
+        // Create scene with side-by-side split-screen.
+        var players = new[]
+        {
+            CreateTestUI(new Rect(0, 0, 0.5f, 1), namePrefix: "Player1", makeSelectable: true), // Left
+            CreateTestUI(new Rect(0.5f, 0, 0.5f, 1), namePrefix: "Player2", makeSelectable: true) // Right
+        };
+
+        // Offset player #2's canvas by moving its camera such that the resulting UI will be to the *right*
+        // of that of player #1. This is important as it will, by default, make player #2's UI navigatable
+        // from player #1 using the navigation logic in Selectable.
+        var screenWidthInWorldSpace =
+            Mathf.Abs(players[1].camera.ScreenToWorldPoint(new Vector3(0, 0, players[1].canvas.planeDistance)).x -
+                players[1].camera.ScreenToWorldPoint(new Vector3(Screen.width, 0, players[1].canvas.planeDistance)).x);
+        players[1].camera.transform.Translate(new Vector3(screenWidthInWorldSpace, 0, 0));
 
         for (var i = 0; i < 2; i++)
         {
@@ -2469,9 +2489,9 @@ public IEnumerator UI_CanOperateMultiplayerUILocallyUsingGamepads()
             // Create actions.
             var map = new InputActionMap("map");
             asset.AddActionMap(map);
-            var moveAction = map.AddAction("move", type: InputActionType.PassThrough);
-            var submitAction = map.AddAction("submit", type: InputActionType.PassThrough);
-            var cancelAction = map.AddAction("cancel", type: InputActionType.PassThrough);
+            var moveAction = map.AddAction("move", type: InputActionType.Value);
+            var submitAction = map.AddAction("submit", type: InputActionType.Button);
+            var cancelAction = map.AddAction("cancel", type: InputActionType.Button);
 
             // Create bindings.
             moveAction.AddBinding(gamepads[i].leftStick);
@@ -2483,58 +2503,83 @@ public IEnumerator UI_CanOperateMultiplayerUILocallyUsingGamepads()
             players[i].uiModule.submit = InputActionReference.Create(submitAction);
             players[i].uiModule.cancel = InputActionReference.Create(cancelAction);
 
-            players[i].leftChildReceiver.moveTo = players[i].rightGameObject;
-            players[i].rightChildReceiver.moveTo = players[i].leftGameObject;
-
             // Enable the whole thing.
             map.Enable();
         }
 
-        // We need to wait a frame to let the underlying canvas update and properly order the graphics images for raycasting.
         yield return null;
 
         Assert.That(players[0].eventSystem.currentSelectedGameObject, Is.SameAs(players[0].leftGameObject));
         Assert.That(players[1].eventSystem.currentSelectedGameObject, Is.SameAs(players[1].leftGameObject));
 
-        // Reset initial selection
         players[0].leftChildReceiver.events.Clear();
         players[1].leftChildReceiver.events.Clear();
 
-        // Check Player 0 Move Axes
-        InputSystem.QueueDeltaStateEvent(gamepads[0].leftStick, new Vector2(1.0f, 0.0f));
-
+        // Move right on player #1's gamepad.
+        Set(gamepads[0].leftStick, Vector2.right);
         yield return null;
 
+        // Player #1 should have moved from left to right object.
         Assert.That(players[0].eventSystem.currentSelectedGameObject, Is.SameAs(players[0].rightGameObject));
         Assert.That(players[1].eventSystem.currentSelectedGameObject, Is.SameAs(players[1].leftGameObject));
 
-        Assert.That(players[0].leftChildReceiver.events, Has.Count.EqualTo(2));
-        Assert.That(players[0].leftChildReceiver.events[0].type, Is.EqualTo(EventType.Move));
-        Assert.That(players[0].leftChildReceiver.events[1].type, Is.EqualTo(EventType.Deselect));
-        players[0].leftChildReceiver.events.Clear();
+        Assert.That(players[0].leftChildReceiver.events,
+            EventSequence(
+                OneEvent("type", EventType.Move),
+                OneEvent("type", EventType.Deselect)));
+        Assert.That(players[0].rightChildReceiver.events,
+            EventSequence(
+                OneEvent("type", EventType.Select)));
 
-        Assert.That(players[0].rightChildReceiver.events, Has.Count.EqualTo(1));
-        Assert.That(players[0].rightChildReceiver.events[0].type, Is.EqualTo(EventType.Select));
+        players[0].leftChildReceiver.events.Clear();
         players[0].rightChildReceiver.events.Clear();
 
+        // No change for player #2.
+        Assert.That(players[1].leftChildReceiver.events, Is.Empty);
+        Assert.That(players[1].rightChildReceiver.events, Is.Empty);
+
+        // https://fogbugz.unity3d.com/f/cases/1306361/
+        // Move right on player #1's gamepad AGAIN. This should *not* cross
+        // over to player #2's UI but should instead not result in any selection change.
+        Set(gamepads[0].leftStick, Vector2.zero);
         yield return null;
+        Set(gamepads[0].leftStick, Vector2.right);
+        yield return null;
+
+        Assert.That(players[0].eventSystem.currentSelectedGameObject, Is.SameAs(players[0].rightGameObject));
+        Assert.That(players[1].eventSystem.currentSelectedGameObject, Is.SameAs(players[1].leftGameObject));
+
+        Assert.That(players[0].leftChildReceiver.events, Is.Empty);
+        Assert.That(players[0].rightChildReceiver.events,
+            EventSequence(
+                OneEvent("type", EventType.Move))); // OnMove will still get called to *attempt* a move.
+
+        players[0].leftChildReceiver.events.Clear();
+        players[0].rightChildReceiver.events.Clear();
+
+        // No change for player #2.
+        Assert.That(players[1].leftChildReceiver.events, Is.Empty);
+        Assert.That(players[1].rightChildReceiver.events, Is.Empty);
+
+        Set(gamepads[0].leftStick, Vector2.zero);
 
         // Check Player 0 Submit
         PressAndRelease(gamepads[0].buttonSouth);
-
         yield return null;
 
-        Assert.That(players[0].rightChildReceiver.events, Has.Count.EqualTo(1));
-        Assert.That(players[0].rightChildReceiver.events[0].type, Is.EqualTo(EventType.Submit));
+        Assert.That(players[0].rightChildReceiver.events,
+            EventSequence(OneEvent("type", EventType.Submit)));
+        Assert.That(players[1].leftChildReceiver.events, Is.Empty);
+
         players[0].rightChildReceiver.events.Clear();
 
         // Check Player 1 Submit
         PressAndRelease(gamepads[1].buttonSouth);
-
         yield return null;
 
-        Assert.That(players[1].leftChildReceiver.events, Has.Count.EqualTo(1));
-        Assert.That(players[1].leftChildReceiver.events[0].type, Is.EqualTo(EventType.Submit));
+        Assert.That(players[1].leftChildReceiver.events,
+            EventSequence(OneEvent("type", EventType.Submit)));
+        Assert.That(players[0].rightChildReceiver.events, Is.Empty);
     }
 
     [UnityTest]
@@ -3718,7 +3763,6 @@ public override string ToString()
         }
 
         public List<Event> events = new List<Event>();
-        public GameObject moveTo;
 
         public void OnPointerClick(PointerEventData eventData)
         {
@@ -3757,8 +3801,6 @@ public void OnPointerMove(PointerEventData eventData)
         public void OnMove(AxisEventData eventData)
         {
             events.Add(new Event(EventType.Move, CloneAxisEventData(eventData)));
-            if (moveTo != null)
-                EventSystem.current.SetSelectedGameObject(moveTo, eventData);
         }
 
         public void OnSubmit(BaseEventData eventData)

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -34,6 +34,8 @@ however, it has to be formatted properly to pass verification tests.
 - Fixed calling `IsPressed` on an entire device returning `true` ([case 1374024](https://issuetracker.unity3d.com/issues/inputcontrol-dot-ispressed-always-returns-true-when-using-new-input-system)).
 - Fixed `InputSystem.RegisterLayoutOverride` resulting in the layout that overrides are being applied to losing the connection to its base layout ([case 1377719](https://fogbugz.unity3d.com/f/cases/1377719/)).
 - Fixed `Touch.activeTouches` still registering touches after the app loses focus ([case 1364017](https://issuetracker.unity3d.com/issues/input-system-new-input-system-registering-active-touches-when-app-loses-focus)).
+- Fixed `MultiplayerEventSystem` not preventing keyboard and gamepad/joystick navigation from one player's UI moving to another player's UI ([case 1306361](https://issuetracker.unity3d.com/issues/input-system-ui-input-module-lets-the-player-navigate-across-other-canvases)).
+  * This fix relies on a `CanvasGroup` being injected into each `playerRoot` and the `interactable` property of the group being toggled back and forth depending on which part of the UI is being updated.
 - Fixed `InputTestFixture` incorrectly running input updates out of sync with the player loop ([case 1341740](https://issuetracker.unity3d.com/issues/buttoncontrol-dot-waspressedthisframe-is-false-when-using-inputtestfixture-dot-press)).
   * This had effects such as `InputAction.WasPressedThisFrame()` returning false expectedly.
 

Packages/com.unity.inputsystem/InputSystem/Plugins/UI/MultiplayerEventSystem.cs

@@ -1,6 +1,6 @@
 #if PACKAGE_DOCS_GENERATION || UNITY_INPUT_SYSTEM_ENABLE_UI
-using UnityEngine;
 using UnityEngine.EventSystems;
+using UnityEngine.InputSystem.Utilities;
 
 namespace UnityEngine.InputSystem.UI
 {
@@ -19,14 +19,92 @@ public class MultiplayerEventSystem : EventSystem
         [Tooltip("If set, only process mouse events for any game objects which are children of this game object.")]
         [SerializeField] private GameObject m_PlayerRoot;
 
+        /// <summary>
+        /// The root object of the UI hierarchy that belongs to the given player.
+        /// </summary>
+        /// <remarks>
+        /// This can either be an entire <c>Canvas</c> or just part of the hierarchy of
+        /// a specific <c>Canvas</c>.
+        ///
+        /// Note that if the given <c>GameObject</c> has a <c>CanvasGroup</c> component on it, its
+        /// <c>interactable</c> property will be toggled back and forth by <see cref="MultiplayerEventSystem"/>.
+        /// If no such component exists on the <c>GameObject</c>, one will be added automatically.
+        ///
+        /// Only the <c>CanvasGroup</c> corresponding to the <see cref="MultiplayerEventSystem"/> that is currently
+        /// executing its <see cref="Update"/> method (or did so last) will have <c>interactable</c> set to true.
+        /// In other words, only the UI hierarchy corresponding to the player that is currently running a UI
+        /// update (or that did so last) can be interacted with.
+        /// </remarks>
         public GameObject playerRoot
         {
             get => m_PlayerRoot;
-            set => m_PlayerRoot = value;
+            set
+            {
+                m_PlayerRoot = value;
+                InitializeCanvasGroup();
+            }
+        }
+
+        private CanvasGroup m_CanvasGroup;
+        private bool m_CanvasGroupWasAddedByUs;
+
+        private static int s_MultiplayerEventSystemCount;
+        private static MultiplayerEventSystem[] s_MultiplayerEventSystems;
+
+        protected override void OnEnable()
+        {
+            base.OnEnable();
+
+            ArrayHelpers.AppendWithCapacity(ref s_MultiplayerEventSystems, ref s_MultiplayerEventSystemCount, this);
+
+            InitializeCanvasGroup();
+        }
+
+        private void InitializeCanvasGroup()
+        {
+            if (m_PlayerRoot != null)
+            {
+                m_CanvasGroup = m_PlayerRoot.GetComponent<CanvasGroup>();
+                if (m_CanvasGroup == null)
+                {
+                    m_CanvasGroup = m_PlayerRoot.AddComponent<CanvasGroup>();
+                    m_CanvasGroupWasAddedByUs = true;
+                }
+                else
+                    m_CanvasGroupWasAddedByUs = false;
+            }
+            else
+            {
+                m_CanvasGroup = null;
+            }
+        }
+
+        protected override void OnDisable()
+        {
+            var index = s_MultiplayerEventSystems.IndexOfReference(this);
+            if (index != -1)
+                s_MultiplayerEventSystems.EraseAtWithCapacity(ref s_MultiplayerEventSystemCount, index);
+
+            if (m_CanvasGroupWasAddedByUs)
+                Destroy(m_CanvasGroup);
+
+            m_CanvasGroup = default;
+            m_CanvasGroupWasAddedByUs = default;
+
+            base.OnDisable();
         }
 
         protected override void Update()
         {
+            for (var i = 0; i < s_MultiplayerEventSystemCount; ++i)
+            {
+                var system = s_MultiplayerEventSystems[i];
+                if (system.m_PlayerRoot == null)
+                    continue;
+
+                system.m_CanvasGroup.interactable = system == this;
+            }
+
             var originalCurrent = current;
             current = this; // in order to avoid reimplementing half of the EventSystem class, just temporarily assign this EventSystem to be the globally current one
             try