Split InteractionOutline from GameScreen (#6155)

Looking at making GameScreen in charge of managing the current GameHud and so it should be less of a god class.

Content.Client/DragDrop/DragDropSystem.cs

@@ -1,5 +1,6 @@
+using System;
 using System.Collections.Generic;
-using Content.Client.State;
+using Content.Client.Outline;
 using Content.Client.Viewport;
 using Content.Shared.ActionBlocker;
 using Content.Shared.DragDrop;
@@ -36,6 +37,7 @@ namespace Content.Client.DragDrop
         [Dependency] private readonly IEyeManager _eyeManager = default!;
         [Dependency] private readonly IPlayerManager _playerManager = default!;
         [Dependency] private readonly IPrototypeManager _prototypeManager = default!;
+        [Dependency] private readonly InteractionOutlineSystem _outline = default!;
         [Dependency] private readonly SharedInteractionSystem _interactionSystem = default!;
         [Dependency] private readonly InputSystem _inputSystem = default!;
         [Dependency] private readonly ActionBlockerSystem _actionBlockerSystem = default!;
@@ -201,7 +203,7 @@ namespace Content.Client.DragDrop
                 }
 
                 HighlightTargets();
-                EntityManager.EventBus.RaiseEvent(EventSource.Local, new OutlineToggleMessage(false));
+                _outline.Enabled = false;
 
                 // drag initiated
                 return true;
@@ -254,7 +256,7 @@ namespace Content.Client.DragDrop
                 EntityManager.DeleteEntity(_dragShadow);
             }
 
-            EntityManager.EventBus.RaiseEvent(EventSource.Local, new OutlineToggleMessage(true));
+            _outline.Enabled = true;
             _dragShadow = default;
             _draggables.Clear();
             _dragger = default;
@@ -307,7 +309,17 @@ namespace Content.Client.DragDrop
                 return false;
             }
 
-            var entities = GameScreenBase.GetEntitiesUnderPosition(_stateManager, args.Coordinates);
+            IList<EntityUid> entities;
+
+            if (_stateManager.CurrentState is GameScreen screen)
+            {
+                entities = screen.GetEntitiesUnderPosition(args.Coordinates);
+            }
+            else
+            {
+                entities = Array.Empty<EntityUid>();
+            }
+
             var outOfRange = false;
 
             foreach (var entity in entities)

Content.Client/Outline/InteractionOutlineSystem.cs

@@ -0,0 +1,116 @@
+using Content.Client.ContextMenu.UI;
+using Content.Client.Interactable;
+using Content.Client.Interactable.Components;
+using Content.Client.Viewport;
+using Content.Shared.CCVar;
+using Robust.Client.Graphics;
+using Robust.Client.Input;
+using Robust.Client.Player;
+using Robust.Client.State;
+using Robust.Client.UserInterface;
+using Robust.Client.UserInterface.CustomControls;
+using Robust.Shared.Configuration;
+using Robust.Shared.GameObjects;
+using Robust.Shared.IoC;
+
+namespace Content.Client.Outline;
+
+public sealed class InteractionOutlineSystem : EntitySystem
+{
+    [Dependency] private readonly IConfigurationManager _configManager = default!;
+    [Dependency] private readonly IEyeManager _eyeManager = default!;
+    [Dependency] private readonly IInputManager _inputManager = default!;
+    [Dependency] private readonly IPlayerManager _playerManager = default!;
+    [Dependency] private readonly IStateManager _stateManager = default!;
+    [Dependency] private readonly IUserInterfaceManager _uiManager = default!;
+
+    public bool Enabled = true;
+
+    private EntityUid? _lastHoveredEntity;
+
+    public override void FrameUpdate(float frameTime)
+    {
+        base.FrameUpdate(frameTime);
+
+        // If there is no local player, there is no session, and therefore nothing to do here.
+        var localPlayer = _playerManager.LocalPlayer;
+        if (localPlayer == null)
+            return;
+
+        // TODO InteractionOutlineComponent
+        // BUG: The logic that gets the renderScale here assumes that the entity is only visible in a single
+        // viewport. The entity will be highlighted in ALL viewport where it is visible, regardless of which
+        // viewport is being used to hover over it. If these Viewports have very different render scales, this may
+        // lead to extremely thick outlines in the other viewports. Fixing this probably requires changing how the
+        // hover outline works, so that it only highlights the entity in a single viewport.
+
+        // GameScreen is still in charge of what entities are visible under a specific cursor position.
+        // Potentially change someday? who knows.
+        var currentState = _stateManager.CurrentState;
+
+        if (currentState is not GameScreen screen) return;
+
+        EntityUid? entityToClick = null;
+        var renderScale = 1;
+        if (_uiManager.CurrentlyHovered is IViewportControl vp)
+        {
+            var mousePosWorld = vp.ScreenToMap(_inputManager.MouseScreenPosition.Position);
+            entityToClick = screen.GetEntityUnderPosition(mousePosWorld);
+
+            if (vp is ScalingViewport svp)
+            {
+                renderScale = svp.CurrentRenderScale;
+            }
+        }
+        else if (_uiManager.CurrentlyHovered is EntityMenuElement element)
+        {
+            entityToClick = element.Entity;
+            // TODO InteractionOutlineComponent
+            // Currently we just take the renderscale from the main viewport. In the future, when the bug mentioned
+            // above is fixed, the viewport should probably be the one that was clicked on to open the entity menu
+            // in the first place.
+            renderScale = _eyeManager.MainViewport.GetRenderScale();
+        }
+
+        var inRange = false;
+        if (localPlayer.ControlledEntity != null && entityToClick != null)
+        {
+            inRange = localPlayer.InRangeUnobstructed(entityToClick.Value, ignoreInsideBlocker: true);
+        }
+
+        InteractionOutlineComponent? outline;
+
+        if (!Enabled || !_configManager.GetCVar(CCVars.OutlineEnabled))
+        {
+            if (entityToClick != null && TryComp(entityToClick, out outline))
+            {
+                outline.OnMouseLeave(); //Prevent outline remains from persisting post command.
+            }
+
+            return;
+        }
+
+        if (entityToClick == _lastHoveredEntity)
+        {
+            if (entityToClick != null && TryComp(entityToClick, out outline))
+            {
+                outline.UpdateInRange(inRange, renderScale);
+            }
+
+            return;
+        }
+
+        if (_lastHoveredEntity != null && !Deleted(_lastHoveredEntity) &&
+            TryComp(_lastHoveredEntity, out outline))
+        {
+            outline.OnMouseLeave();
+        }
+
+        _lastHoveredEntity = entityToClick;
+
+        if (_lastHoveredEntity != null && TryComp(_lastHoveredEntity, out outline))
+        {
+            outline.OnMouseEnter(inRange, renderScale);
+        }
+    }
+}

Content.Client/State/OutlineToggleMessage.cs

@@ -1,14 +0,0 @@
-using Robust.Shared.GameObjects;
-
-namespace Content.Client.State
-{
-    public sealed class OutlineToggleMessage : EntityEventArgs
-    {
-        public bool Enabled { get; }
-
-        public OutlineToggleMessage(bool enabled)
-        {
-            Enabled = enabled;
-        }
-    }
-}

Content.Client/Viewport/GameScreenBase.cs

@@ -1,21 +1,13 @@
 using System;
 using System.Collections.Generic;
-using System.Collections.Immutable;
 using System.Linq;
 using Content.Client.Clickable;
-using Content.Client.ContextMenu.UI;
-using Content.Client.Interactable;
-using Content.Client.Interactable.Components;
-using Content.Client.State;
-using Content.Shared.CCVar;
 using Robust.Client.GameObjects;
-using Robust.Client.Graphics;
 using Robust.Client.Input;
 using Robust.Client.Player;
 using Robust.Client.State;
 using Robust.Client.UserInterface;
 using Robust.Client.UserInterface.CustomControls;
-using Robust.Shared.Configuration;
 using Robust.Shared.Containers;
 using Robust.Shared.GameObjects;
 using Robust.Shared.Input;
@@ -29,121 +21,27 @@ namespace Content.Client.Viewport
     // OH GOD.
     // Ok actually it's fine.
     // Instantiated dynamically through the StateManager, Dependencies will be resolved.
-    public partial class GameScreenBase : Robust.Client.State.State, IEntityEventSubscriber
+    public class GameScreenBase : State, IEntityEventSubscriber
     {
-        [Dependency] protected readonly IClientEntityManager EntityManager = default!;
+        [Dependency] private readonly IInputManager _inputManager = default!;
-        [Dependency] protected readonly IInputManager InputManager = default!;
+        [Dependency] private readonly IPlayerManager _playerManager = default!;
-        [Dependency] protected readonly IPlayerManager PlayerManager = default!;
+        [Dependency] private readonly IEntitySystemManager _entitySystemManager = default!;
-        [Dependency] protected readonly IEntitySystemManager EntitySystemManager = default!;
+        [Dependency] private readonly IGameTiming _timing = default!;
-        [Dependency] protected readonly IGameTiming Timing = default!;
+        [Dependency] private readonly IMapManager _mapManager = default!;
-        [Dependency] protected readonly IMapManager MapManager = default!;
         [Dependency] protected readonly IUserInterfaceManager UserInterfaceManager = default!;
-        [Dependency] protected readonly IConfigurationManager ConfigurationManager = default!;
         [Dependency] private readonly IEntityManager _entityManager = default!;
-        [Dependency] private readonly IEyeManager _eyeManager = default!;
 
-        private IEventBus _eventBus => _entityManager.EventBus;
+        private ClickableEntityComparer _comparer = default!;
-
-        private EntityUid? _lastHoveredEntity;
-
-        private bool _outlineEnabled = true;
 
         public override void Startup()
         {
-            InputManager.KeyBindStateChanged += OnKeyBindStateChanged;
+            _inputManager.KeyBindStateChanged += OnKeyBindStateChanged;
-            _eventBus.SubscribeEvent<OutlineToggleMessage>(EventSource.Local, this, HandleOutlineToggle);
+            _comparer = new ClickableEntityComparer(_entityManager);
         }
 
         public override void Shutdown()
         {
-            InputManager.KeyBindStateChanged -= OnKeyBindStateChanged;
+            _inputManager.KeyBindStateChanged -= OnKeyBindStateChanged;
-            _eventBus.UnsubscribeEvent<OutlineToggleMessage>(EventSource.Local, this);
-        }
-
-        private void HandleOutlineToggle(OutlineToggleMessage message)
-        {
-            _outlineEnabled = message.Enabled;
-        }
-
-        /// <summary>
-        ///     Highlight the currently hovered entity.
-        /// </summary>
-        public override void FrameUpdate(FrameEventArgs e)
-        {
-            base.FrameUpdate(e);
-
-            // If there is no local player, there is no session, and therefore nothing to do here.
-            var localPlayer = PlayerManager.LocalPlayer;
-            if (localPlayer == null)
-                return;
-
-            // TODO InteractionOutlineComponent
-            // BUG: The logic that gets the renderScale here assumes that the entity is only visible in a single
-            // viewport. The entity will be highlighted in ALL viewport where it is visible, regardless of which
-            // viewport is being used to hover over it. If these Viewports have very different render scales, this may
-            // lead to extremely thick outlines in the other viewports. Fixing this probably requires changing how the
-            // hover outline works, so that it only highlights the entity in a single viewport.
-
-            EntityUid? entityToClick = null;
-            var renderScale = 1;
-            if (UserInterfaceManager.CurrentlyHovered is IViewportControl vp)
-            {
-                var mousePosWorld = vp.ScreenToMap(InputManager.MouseScreenPosition.Position);
-                entityToClick = GetEntityUnderPosition(mousePosWorld);
-
-                if (vp is ScalingViewport svp)
-                {
-                    renderScale = svp.CurrentRenderScale;
-                }
-            }
-            else if (UserInterfaceManager.CurrentlyHovered is EntityMenuElement element)
-            {
-                entityToClick = element.Entity;
-                // TODO InteractionOutlineComponent
-                // Currently we just take the renderscale from the main viewport. In the future, when the bug mentioned
-                // above is fixed, the viewport should probably be the one that was clicked on to open the entity menu
-                // in the first place.
-                renderScale = _eyeManager.MainViewport.GetRenderScale();
-            }
-
-            var inRange = false;
-            if (localPlayer.ControlledEntity != null && entityToClick != null)
-            {
-                inRange = localPlayer.InRangeUnobstructed(entityToClick.Value, ignoreInsideBlocker: true);
-            }
-
-            InteractionOutlineComponent? outline;
-            if(!_outlineEnabled || !ConfigurationManager.GetCVar(CCVars.OutlineEnabled))
-            {
-                if(entityToClick != null && _entityManager.TryGetComponent(entityToClick, out outline))
-                {
-                    outline.OnMouseLeave(); //Prevent outline remains from persisting post command.
-                }
-                return;
-            }
-
-            if (entityToClick == _lastHoveredEntity)
-            {
-                if (entityToClick != null && _entityManager.TryGetComponent(entityToClick, out outline))
-                {
-                    outline.UpdateInRange(inRange, renderScale);
-                }
-
-                return;
-            }
-
-            if (_lastHoveredEntity != null && !_entityManager.Deleted(_lastHoveredEntity) &&
-                _entityManager.TryGetComponent(_lastHoveredEntity, out outline))
-            {
-                outline.OnMouseLeave();
-            }
-
-            _lastHoveredEntity = entityToClick;
-
-            if (_lastHoveredEntity != null && _entityManager.TryGetComponent(_lastHoveredEntity, out outline))
-            {
-                outline.OnMouseEnter(inRange, renderScale);
-            }
         }
 
         public EntityUid? GetEntityUnderPosition(MapCoordinates coordinates)
@@ -154,7 +52,7 @@ namespace Content.Client.Viewport
 
         public IList<EntityUid> GetEntitiesUnderPosition(EntityCoordinates coordinates)
         {
-            return GetEntitiesUnderPosition(coordinates.ToMap(EntityManager));
+            return GetEntitiesUnderPosition(coordinates.ToMap(_entityManager));
         }
 
         public IList<EntityUid> GetEntitiesUnderPosition(MapCoordinates coordinates)
@@ -163,12 +61,14 @@ namespace Content.Client.Viewport
             var entities = IoCManager.Resolve<IEntityLookup>().GetEntitiesIntersecting(coordinates.MapId,
                 Box2.CenteredAround(coordinates.Position, (1, 1)));
 
+            var containerSystem = _entitySystemManager.GetEntitySystem<SharedContainerSystem>();
+
             // Check the entities against whether or not we can click them
             var foundEntities = new List<(EntityUid clicked, int drawDepth, uint renderOrder)>();
             foreach (var entity in entities)
             {
                 if (_entityManager.TryGetComponent<ClickableComponent?>(entity, out var component)
-                    && !entity.IsInContainer()
+                    && !containerSystem.IsEntityInContainer(entity)
                     && component.CheckClick(coordinates.Position, out var drawDepthClicked, out var renderOrder))
                 {
                     foundEntities.Add((entity, drawDepthClicked, renderOrder));
@@ -178,32 +78,13 @@ namespace Content.Client.Viewport
             if (foundEntities.Count == 0)
                 return Array.Empty<EntityUid>();
 
-            foundEntities.Sort(new ClickableEntityComparer(_entityManager));
+            foundEntities.Sort(_comparer);
             // 0 is the top element.
             foundEntities.Reverse();
             return foundEntities.Select(a => a.clicked).ToList();
         }
 
-        /// <summary>
+        private sealed class ClickableEntityComparer : IComparer<(EntityUid clicked, int depth, uint renderOrder)>
-        /// Gets all entities intersecting the given position.
-        ///
-        /// Static alternative to GetEntitiesUnderPosition to cut out
-        /// some of the boilerplate needed to get state manager and check the current state.
-        /// </summary>
-        /// <param name="stateManager">state manager to use to get the current game screen</param>
-        /// <param name="coordinates">coordinates to check</param>
-        /// <returns>the entities under the position, empty list if none found</returns>
-        public static IList<EntityUid> GetEntitiesUnderPosition(IStateManager stateManager, EntityCoordinates coordinates)
-        {
-            if (stateManager.CurrentState is GameScreenBase gameScreenBase)
-            {
-                return gameScreenBase.GetEntitiesUnderPosition(coordinates);
-            }
-
-            return ImmutableList<EntityUid>.Empty;
-        }
-
-        internal class ClickableEntityComparer : IComparer<(EntityUid clicked, int depth, uint renderOrder)>
         {
             private readonly IEntityManager _entities;
 
@@ -249,12 +130,12 @@ namespace Content.Client.Viewport
         protected virtual void OnKeyBindStateChanged(ViewportBoundKeyEventArgs args)
         {
             // If there is no InputSystem, then there is nothing to forward to, and nothing to do here.
-            if(!EntitySystemManager.TryGetEntitySystem(out InputSystem? inputSys))
+            if(!_entitySystemManager.TryGetEntitySystem(out InputSystem? inputSys))
                 return;
 
             var kArgs = args.KeyEventArgs;
             var func = kArgs.Function;
-            var funcId = InputManager.NetworkBindMap.KeyFunctionID(func);
+            var funcId = _inputManager.NetworkBindMap.KeyFunctionID(func);
 
             EntityCoordinates coordinates = default;
             EntityUid? entityToClick = null;
@@ -263,16 +144,16 @@ namespace Content.Client.Viewport
                 var mousePosWorld = vp.ScreenToMap(kArgs.PointerLocation.Position);
                 entityToClick = GetEntityUnderPosition(mousePosWorld);
 
-                coordinates = MapManager.TryFindGridAt(mousePosWorld, out var grid) ? grid.MapToGrid(mousePosWorld) :
+                coordinates = _mapManager.TryFindGridAt(mousePosWorld, out var grid) ? grid.MapToGrid(mousePosWorld) :
-                    EntityCoordinates.FromMap(MapManager, mousePosWorld);
+                    EntityCoordinates.FromMap(_mapManager, mousePosWorld);
             }
 
-            var message = new FullInputCmdMessage(Timing.CurTick, Timing.TickFraction, funcId, kArgs.State,
+            var message = new FullInputCmdMessage(_timing.CurTick, _timing.TickFraction, funcId, kArgs.State,
                 coordinates , kArgs.PointerLocation,
                 entityToClick ?? default); // TODO make entityUid nullable
 
             // client side command handlers will always be sent the local player session.
-            var session = PlayerManager.LocalPlayer?.Session;
+            var session = _playerManager.LocalPlayer?.Session;
             if (inputSys.HandleInputCommand(session, func, message))
             {
                 kArgs.Handle();