Fix T-ray scanner PVS issues (#6554)

Co-authored-by: metalgearsloth <31366439+metalgearsloth@users.noreply.github.com>

Content.Client/SubFloor/TrayScannerSystem.cs

@@ -1,19 +1,14 @@
-using System.Collections.Generic;
 using System.Linq;
 using Content.Shared.SubFloor;
 using Robust.Shared.Containers;
-using Robust.Shared.GameObjects;
+using Robust.Shared.Map;
-using Robust.Shared.GameStates;
-using Robust.Shared.IoC;
-using Robust.Shared.Log;
-using Robust.Shared.Maths;
 using Robust.Shared.Utility;
 
 namespace Content.Client.SubFloor;
 
-public class TrayScannerSystem : SharedTrayScannerSystem
+public sealed class TrayScannerSystem : SharedTrayScannerSystem
 {
-    [Dependency] private IEntityLookup _entityLookup = default!;
+    [Dependency] private IMapManager _mapManager = default!;
     [Dependency] private SubFloorHideSystem _subfloorSystem = default!;
     [Dependency] private SharedContainerSystem _containerSystem = default!;
 
@@ -28,7 +23,7 @@ public class TrayScannerSystem : SharedTrayScannerSystem
 
     public void OnComponentShutdown(EntityUid uid, TrayScannerComponent scanner, ComponentShutdown args)
     {
-        _subfloorSystem.ToggleSubfloorEntities(scanner.RevealedSubfloors, false, uid, _visualizerKeys);
+        _subfloorSystem.SetEntitiesRevealed(scanner.RevealedSubfloors, uid, false, _visualizerKeys);
         _invalidScanners.Add(uid);
     }
 
@@ -66,6 +61,29 @@ public class TrayScannerSystem : SharedTrayScannerSystem
         if (_invalidScanners.List != null) _invalidScanners.List.Clear();
     }
 
+    /// <summary>
+    ///     When a subfloor entity gets anchored (which includes spawning & coming into PVS range), Check for nearby scanners.
+    /// </summary>
+    public override void OnSubfloorAnchored(EntityUid uid, SubFloorHideComponent? hideComp = null, TransformComponent? xform = null)
+    {
+        if (!Resolve(uid, ref hideComp, ref xform))
+            return;
+        
+        var pos = xform.MapPosition;
+
+        foreach (var entity in _activeScanners)
+        {
+            if (!TryComp(entity, out TrayScannerComponent? scanner))
+                continue;
+
+            if (!Transform(entity).MapPosition.InRange(pos, scanner.Range))
+                continue;
+
+            hideComp.RevealedBy.Add(entity);
+            scanner.RevealedSubfloors.Add(uid);
+        }
+    }
+
     /// <summary>
     ///     Updates a T-Ray scanner. Should be called on immediate
     ///     state change (turned on/off), or during the update
@@ -84,9 +102,9 @@ public class TrayScannerSystem : SharedTrayScannerSystem
         // set all the known subfloor to invisible,
         // and return false so it's removed from
         // the active scanner list
-        if (!scanner.Toggled)
+        if (!scanner.Toggled || transform.MapID == MapId.Nullspace)
         {
-            _subfloorSystem.ToggleSubfloorEntities(scanner.RevealedSubfloors, false, uid, _visualizerKeys);
+            _subfloorSystem.SetEntitiesRevealed(scanner.RevealedSubfloors, uid, false, _visualizerKeys);
             scanner.LastLocation = Vector2.Zero;
             scanner.RevealedSubfloors.Clear();
             return false;
@@ -130,36 +148,56 @@ public class TrayScannerSystem : SharedTrayScannerSystem
         // is still technically on
         if (flooredPos == Vector2.Zero)
         {
-            _subfloorSystem.ToggleSubfloorEntities(scanner.RevealedSubfloors, false, uid, _visualizerKeys);
+            _subfloorSystem.SetEntitiesRevealed(scanner.RevealedSubfloors, uid, false, _visualizerKeys);
             scanner.RevealedSubfloors.Clear();
             return true;
         }
 
+        // MAYBE redo this. Currently different players can see different entities
+        //
+        // Here we avoid the entity lookup & return early if the scanner's position hasn't appreciably changed. However,
+        // if a new player enters PVS-range, they will update the in-range entities on their end and use that to set
+        // LastLocation. This means that different players can technically see different entities being revealed by the
+        // same scanner. The correct fix for this is probably just to network the revealed entity set.... But I CBF
+        // doing that right now....
         if (flooredPos == scanner.LastLocation
             || (float.IsNaN(flooredPos.X) && float.IsNaN(flooredPos.Y)))
             return true;
 
         scanner.LastLocation = flooredPos;
 
-        // get all entities in range by uid
+        //  Update entities in Range
-        // but without using LINQ
         HashSet<EntityUid> nearby = new();
+        var coords = transform.MapPosition;
+        var worldBox = Box2.CenteredAround(coords.Position, (scanner.Range * 2, scanner.Range * 2));
 
-        foreach (var entityInRange in _entityLookup.GetEntitiesInRange(uid, scanner.Range))
+        // For now, limiting to the scanner's own grid. We could do a grid-lookup, but then what do we do if one grid
-            if (FilterAnchored(entityInRange)) nearby.Add(entityInRange);
+        // flies away, while the scanner's local-position remains unchanged?
+        if (_mapManager.TryGetGrid(transform.GridID, out var grid))
+        {
+            foreach (var entity in grid.GetAnchoredEntities(worldBox))
+            {
+                if (!Transform(entity).MapPosition.InRange(coords, scanner.Range))
+                    continue;
+
+                if (!TryComp(entity, out SubFloorHideComponent? hideComp))
+                    continue; // Not a hide-able entity.
+
+                nearby.Add(entity);
+
+                if (scanner.RevealedSubfloors.Add(entity))
+                    _subfloorSystem.SetEntityRevealed(entity, uid, true, hideComp, _visualizerKeys);
+            }
+        }
 
         // get all the old elements that are no longer detected
-        scanner.RevealedSubfloors.ExceptWith(nearby);
+        HashSet<EntityUid> missing = new(scanner.RevealedSubfloors.Except(nearby));
 
-        // hide all of them, since they're no longer needed
+        // remove those from the list
-        _subfloorSystem.ToggleSubfloorEntities(scanner.RevealedSubfloors, false, uid, _visualizerKeys);
+        scanner.RevealedSubfloors.ExceptWith(missing);
-        scanner.RevealedSubfloors.Clear();
 
-        // set the revealedsubfloor set to the new nearby set
+        // and hide them
-        scanner.RevealedSubfloors.UnionWith(nearby);
+        _subfloorSystem.SetEntitiesRevealed(missing, uid, false, _visualizerKeys);
-
-        // show all the new subfloor
-        _subfloorSystem.ToggleSubfloorEntities(scanner.RevealedSubfloors, true, uid, _visualizerKeys);
 
         return true;
     }
@@ -169,10 +207,4 @@ public class TrayScannerSystem : SharedTrayScannerSystem
         SubFloorVisuals.SubFloor,
         TrayScannerTransparency.Key
     };
-
-    private bool FilterAnchored(EntityUid uid)
-    {
-        return EntityManager.TryGetComponent<TransformComponent>(uid, out var transform)
-            && transform.Anchored;
-    }
 }

Content.Server/SubFloor/TrayScannerSystem.cs

@@ -2,7 +2,7 @@ using Content.Shared.SubFloor;
 
 namespace Content.Server.SubFloor;
 
-public class TrayScannerSystem : SharedTrayScannerSystem
+public sealed class TrayScannerSystem : SharedTrayScannerSystem
 {
     public override void Initialize()
     {

Content.Shared/SubFloor/SharedTrayScannerSystem.cs

@@ -1,9 +1,5 @@
-using System;
 using Content.Shared.Interaction;
-using Robust.Shared.GameObjects;
 using Robust.Shared.GameStates;
-using Robust.Shared.IoC;
-using Robust.Shared.Log;
 using Robust.Shared.Serialization;
 
 namespace Content.Shared.SubFloor;
@@ -43,8 +39,6 @@ public abstract class SharedTrayScannerSystem : EntitySystem
 
         scanner.Toggled = state;
         scanner.Dirty();
-
-        // RaiseLocalEvent(uid, new TrayScannerToggleEvent(scanner.Toggled));
     }
 
     private void OnTrayScannerGetState(EntityUid uid, TrayScannerComponent scanner, ref ComponentGetState args)
@@ -59,6 +53,10 @@ public abstract class SharedTrayScannerSystem : EntitySystem
 
         ToggleTrayScanner(uid, state.Toggled, scanner);
     }
+
+    public virtual void OnSubfloorAnchored(EntityUid uid, SubFloorHideComponent? hideComp = null, TransformComponent? xform = null)
+    {
+    }
 }
 
 [Serializable, NetSerializable]

Content.Shared/SubFloor/SubFloorHideComponent.cs

@@ -1,11 +1,5 @@
-using System;
-using System.Collections.Generic;
-using Robust.Shared.GameObjects;
 using Robust.Shared.GameStates;
-using Robust.Shared.Players;
 using Robust.Shared.Serialization;
-using Robust.Shared.Serialization.Manager.Attributes;
-using Robust.Shared.ViewVariables;
 
 namespace Content.Shared.SubFloor
 {
@@ -17,6 +11,7 @@ namespace Content.Shared.SubFloor
     /// <seealso cref="P:Content.Shared.Maps.ContentTileDefinition.IsSubFloor" />
     [NetworkedComponent]
     [RegisterComponent]
+    [Friend(typeof(SubFloorHideSystem))]
     public sealed class SubFloorHideComponent : Component
     {
         /// <summary>
@@ -26,16 +21,25 @@ namespace Content.Shared.SubFloor
         [DataField("enabled")]
         public bool Enabled { get; set; } = true;
 
+        /// <summary>
+        ///     Whether the entity's current position has a "Floor-type" tile above its current position.
+        /// </summary>
+        public bool IsUnderCover { get; set; } = false;
+
+        /*
+         * An un-anchored hiding entity would require listening to on-move events in case it moves into a sub-floor
+         * tile. Also T-Ray scanner headaches.
         /// <summary>
         ///     This entity needs to be anchored to be hid when not in subfloor.
         /// </summary>
         [ViewVariables(VVAccess.ReadWrite)]
         [DataField("requireAnchored")]
         public bool RequireAnchored { get; set; } = true;
+        */
 
         public override ComponentState GetComponentState()
         {
-            return new SubFloorHideComponentState(Enabled, RequireAnchored);
+            return new SubFloorHideComponentState(Enabled);
         }
 
         /// <summary>
@@ -50,25 +54,16 @@ namespace Content.Shared.SubFloor
         /// </summary>
         [ViewVariables]
         public HashSet<EntityUid> RevealedBy { get; set; } = new();
-
-        /// <summary>
-        ///     Whether or not this entity was revealed with or without
-        ///     an entity.
-        /// </summary>
-        [ViewVariables]
-        public bool RevealedWithoutEntity { get; set; }
     }
 
     [Serializable, NetSerializable]
-    public class SubFloorHideComponentState : ComponentState
+    public sealed class SubFloorHideComponentState : ComponentState
     {
         public bool Enabled { get; }
-        public bool RequireAnchored { get; }
 
-        public SubFloorHideComponentState(bool enabled, bool requireAnchored)
+        public SubFloorHideComponentState(bool enabled)
         {
             Enabled = enabled;
-            RequireAnchored = requireAnchored;
         }
     }
 }

Content.Shared/SubFloor/SubFloorHideSystem.cs

@@ -1,16 +1,9 @@
-using System;
-using System.Collections.Generic;
 using Content.Shared.Interaction;
 using Content.Shared.Maps;
 using JetBrains.Annotations;
-using Robust.Shared.GameObjects;
 using Robust.Shared.GameStates;
-using Robust.Shared.IoC;
-using Robust.Shared.Log;
 using Robust.Shared.Map;
-using Robust.Shared.Maths;
 using Robust.Shared.Serialization;
-using Robust.Shared.ViewVariables;
 
 namespace Content.Shared.SubFloor
 {
@@ -18,10 +11,11 @@ namespace Content.Shared.SubFloor
     ///     Entity system backing <see cref="SubFloorHideComponent"/>.
     /// </summary>
     [UsedImplicitly]
-    public class SubFloorHideSystem : EntitySystem
+    public sealed class SubFloorHideSystem : EntitySystem
     {
         [Dependency] private readonly IMapManager _mapManager = default!;
         [Dependency] private readonly ITileDefinitionManager _tileDefinitionManager = default!;
+        [Dependency] private readonly SharedTrayScannerSystem _trayScannerSystem = default!;
 
         private bool _showAll;
 
@@ -63,32 +57,20 @@ namespace Content.Shared.SubFloor
         public void SetEnabled(SubFloorHideComponent subFloor, bool enabled)
         {
             subFloor.Enabled = enabled;
-            subFloor.Dirty();
+            Dirty(subFloor);
-            UpdateEntity(subFloor.Owner);
+            UpdateAppearance(subFloor.Owner);
-        }
-
-        public void SetRequireAnchoring(SubFloorHideComponent subFloor, bool requireAnchored)
-        {
-            subFloor.RequireAnchored = requireAnchored;
-            subFloor.Dirty();
-            UpdateEntity(subFloor.Owner);
         }
 
         private void OnInteractionAttempt(EntityUid uid, SubFloorHideComponent component, InteractUsingEvent args)
         {
-            if (!EntityManager.TryGetComponent(uid, out TransformComponent? transform))
+            // TODO make this use an interact attempt event or something. Handling an InteractUsing is not going to work in general.
-                return;
+            args.Handled = component.IsUnderCover;
-
-            if (_mapManager.TryGetGrid(transform.GridID, out var grid)
-                && !IsSubFloor(grid, grid.TileIndicesFor(transform.Coordinates)))
-            {
-                args.Handled = true;
-            }
         }
 
         private void OnSubFloorStarted(EntityUid uid, SubFloorHideComponent component, ComponentStartup _)
         {
-            UpdateEntity(uid);
+            UpdateFloorCover(uid, component);
+            UpdateAppearance(uid, component);
             EntityManager.EnsureComponent<CollideOnAnchorComponent>(uid);
         }
 
@@ -99,14 +81,26 @@ namespace Content.Shared.SubFloor
                 return;
 
             // Regardless of whether we're on a subfloor or not, unhide.
-            UpdateEntity(uid, true);
+            component.IsUnderCover = false;
-            EntityManager.RemoveComponent<CollideOnAnchorComponent>(uid);
+            UpdateAppearance(uid, component);
         }
 
         private void HandleAnchorChanged(EntityUid uid, SubFloorHideComponent component, ref AnchorStateChangedEvent args)
         {
-            // We do this directly instead of calling UpdateEntity.
+            if (args.Anchored)
-            UpdateEntity(uid);
+            {
+                var xform = Transform(uid);
+                _trayScannerSystem.OnSubfloorAnchored(uid, component, xform);
+                UpdateFloorCover(uid, component, xform);
+
+                if (component.IsUnderCover)
+                    UpdateAppearance(uid, component);
+            }
+            else if (component.IsUnderCover)
+            {
+                component.IsUnderCover = false;
+                UpdateAppearance(uid, component);
+            }
         }
 
         private void HandleComponentState(EntityUid uid, SubFloorHideComponent component, ref ComponentHandleState args)
@@ -115,8 +109,7 @@ namespace Content.Shared.SubFloor
                 return;
 
             component.Enabled = state.Enabled;
-            component.RequireAnchored = state.RequireAnchored;
+            UpdateAppearance(uid, component);
-            UpdateEntity(uid);
         }
 
         private void MapManagerOnTileChanged(object? sender, TileChangedEventArgs e)
@@ -132,102 +125,95 @@ namespace Content.Shared.SubFloor
             }
         }
 
-        private bool IsSubFloor(IMapGrid grid, Vector2i position)
+        /// <summary>
+        ///     Update whether a given entity is currently covered by a floor tile.
+        /// </summary>
+        private void UpdateFloorCover(EntityUid uid, SubFloorHideComponent? component = null, TransformComponent? xform = null)
         {
+            if (!Resolve(uid, ref component, ref xform))
+                return;
+
+            if (xform.Anchored && _mapManager.TryGetGrid(xform.GridID, out var grid))
+                component.IsUnderCover = HasFloorCover(grid, grid.TileIndicesFor(xform.Coordinates));
+            else
+                component.IsUnderCover = false;
+
+            // Update normally.
+            UpdateAppearance(uid, component);
+        }
+
+        private bool HasFloorCover(IMapGrid grid, Vector2i position)
+        {
+            // TODO Redo this function. Currently wires on an asteroid are always "below the floor"
             var tileDef = (ContentTileDefinition) _tileDefinitionManager[grid.GetTileRef(position).Tile.TypeId];
-            return tileDef.IsSubFloor;
+            return !tileDef.IsSubFloor;
         }
 
         private void UpdateAll()
         {
             foreach (var comp in EntityManager.EntityQuery<SubFloorHideComponent>(true))
             {
-                UpdateEntity(comp.Owner);
+                UpdateAppearance(comp.Owner, comp);
             }
         }
 
         private void UpdateTile(IMapGrid grid, Vector2i position)
         {
-            var isSubFloor = IsSubFloor(grid, position);
+            var covered = HasFloorCover(grid, position);
 
             foreach (var uid in grid.GetAnchoredEntities(position))
             {
-                if(EntityManager.HasComponent<SubFloorHideComponent>(uid))
+                if (!TryComp(uid, out SubFloorHideComponent? hideComp))
-                    UpdateEntity(uid, isSubFloor);
+                    continue;
+
+                if (hideComp.IsUnderCover == covered)
+                    continue;
+
+                hideComp.IsUnderCover = covered;
+                UpdateAppearance(uid, hideComp);
             }
         }
 
-        private void UpdateEntity(EntityUid uid)
+        /// <summary>
+        ///     This function is used by T-Ray scanners or other sub-floor revealing entities to toggle visibility.
+        /// </summary>
+        public void SetEntitiesRevealed(IEnumerable<EntityUid> entities, EntityUid revealer, bool visible, IEnumerable<object>? appearanceKeys = null)
         {
-            var transform = EntityManager.GetComponent<TransformComponent>(uid);
+            foreach (var uid in entities)
-
-            if (!_mapManager.TryGetGrid(transform.GridID, out var grid))
             {
-                // Not being on a grid counts as no subfloor, unhide this.
+                SetEntityRevealed(uid, revealer, visible);
-                UpdateEntity(uid, true);
+            }
+        }
+
+        /// <summary>
+        ///     This function is used by T-Ray scanners or other sub-floor revealing entities to toggle visibility.
+        /// </summary>
+        public void SetEntityRevealed(EntityUid uid, EntityUid revealer, bool visible,
+            SubFloorHideComponent? hideComp = null,
+            IEnumerable<object>? appearanceKeys = null)
+        {
+            if (!Resolve(uid, ref hideComp))
+                return;
+
+            if (visible)
+            {
+                if (hideComp.RevealedBy.Add(revealer) && hideComp.RevealedBy.Count == 1)
+                    UpdateAppearance(uid, hideComp, appearanceKeys);
+
                 return;
             }
 
-            // Update normally.
+            if (hideComp.RevealedBy.Remove(revealer) && hideComp.RevealedBy.Count == 0)
-            bool isSubFloor = IsSubFloor(grid, grid.TileIndicesFor(transform.Coordinates));
+                UpdateAppearance(uid, hideComp, appearanceKeys);
-            UpdateEntity(uid, isSubFloor);
         }
 
-        // Toggles an enumerable set of entities to display.
+        public void UpdateAppearance(EntityUid uid, SubFloorHideComponent? hideComp = null, IEnumerable<object>? appearanceKeys = null)
-        public void ToggleSubfloorEntities(IEnumerable<EntityUid> entities, bool visible, EntityUid? uid = null, IEnumerable<object>? appearanceKeys = null)
         {
-            foreach (var entity in entities)
+            if (!Resolve(uid, ref hideComp))
-            {
+                return;
-                if (!EntityManager.HasComponent<SubFloorHideComponent>(entity))
-                    continue;
 
-                UpdateEntity(entity, visible, uid, appearanceKeys);
+            var revealedWithoutEntity = ShowAll || !hideComp.IsUnderCover;
-            }
+            var revealed = revealedWithoutEntity || hideComp.RevealedBy.Count != 0;
-        }
-
-        private void UpdateEntity(EntityUid uid, bool subFloor, EntityUid? revealedUid = null, IEnumerable<object>? appearanceKeys = null)
-        {
-            bool revealedWithoutEntity = false;
-
-            if (EntityManager.TryGetComponent(uid, out SubFloorHideComponent? subFloorHideComponent))
-            {
-                // We only need to query the subfloor component to check if it's enabled or not when we're not on subfloor.
-                // Getting components is expensive, after all.
-                if (!subFloor)
-                {
-                    // If the component isn't enabled, then subfloor will always be true, and the entity will be shown.
-                    if (!subFloorHideComponent.Enabled)
-                    {
-                        subFloor = true;
-                    }
-                    // We only need to query the TransformComp if the SubfloorHide is enabled and requires anchoring.
-                    else if (subFloorHideComponent.RequireAnchored && EntityManager.TryGetComponent(uid, out TransformComponent? transformComponent))
-                    {
-                        // If we require the entity to be anchored but it's not, this will set subfloor to true, unhiding it.
-                        subFloor = !transformComponent.Anchored;
-                    }
-                }
-
-                // If this was revealed by anything, we need to add it into the
-                // component's set of entities that reveal it
-                //
-                if (revealedUid != null)
-                {
-                    if (subFloor) subFloorHideComponent.RevealedBy.Add((EntityUid) revealedUid);
-                    else          subFloorHideComponent.RevealedBy.Remove((EntityUid) revealedUid);
-                }
-                else
-                {
-                    subFloorHideComponent.RevealedWithoutEntity = subFloor;
-                }
-
-                subFloor = subFloorHideComponent.RevealedBy.Count != 0 || subFloorHideComponent.RevealedWithoutEntity;
-                revealedWithoutEntity = subFloorHideComponent.RevealedWithoutEntity;
-            }
-
-
-            // Whether to show this entity as visible, visually.
-            var subFloorVisible = ShowAll || subFloor;
 
             // if there are no keys given,
             // or if the subfloor is already revealed,
@@ -238,15 +224,15 @@ namespace Content.Shared.SubFloor
             // should only apply if the visualizer is underneath a subfloor
             if (appearanceKeys == null || revealedWithoutEntity) appearanceKeys = _defaultVisualizerKeys;
 
-            ShowSubfloorSprite(uid, subFloorVisible, appearanceKeys);
+            ShowSubfloorSprite(uid, revealed, appearanceKeys);
         }
 
-        private void ShowSubfloorSprite(EntityUid uid, bool subFloorVisible, IEnumerable<object> appearanceKeys)
+        private void ShowSubfloorSprite(EntityUid uid, bool revealed, IEnumerable<object> appearanceKeys)
         {
             // Show sprite
             if (EntityManager.TryGetComponent(uid, out SharedSpriteComponent? spriteComponent))
             {
-                spriteComponent.Visible = subFloorVisible;
+                spriteComponent.Visible = revealed;
             }
 
             // Set an appearance data value so visualizers can use this as needed.
@@ -257,10 +243,10 @@ namespace Content.Shared.SubFloor
                     switch (key)
                     {
                         case Enum enumKey:
-                            appearanceComponent.SetData(enumKey, subFloorVisible);
+                            appearanceComponent.SetData(enumKey, revealed);
                             break;
                         case string stringKey:
-                            appearanceComponent.SetData(stringKey, subFloorVisible);
+                            appearanceComponent.SetData(stringKey, revealed);
                             break;
                     }
                 }

Content.Shared/SubFloor/TrayScannerComponent.cs

@@ -22,7 +22,7 @@ public class TrayScannerComponent : Component
 
     // range of the scanner itself
     [DataField("range")]
-    public float Range { get; set; } = 0.5f;
+    public float Range { get; set; } = 2f;
 
     // exclude entities that are not the set
     // of entities in range & entities already revealed