Fixing performance issue with Proximity Detector (#23557)

* fixing access levels and removed strings from data defs

* Fixing proximity detector performance

- Re-added component filtering for the close entity search.
- Changed criteria functionality to only allow for searching for entities with all specified comps (matching any would be too unperformant)

Content.Shared/Beeper/Components/BeeperComponent.cs

@@ -16,25 +16,25 @@ public sealed partial class BeeperComponent : Component
     /// <summary>
     /// Whether or not it's on.
     /// </summary>
-    [DataField("enabled")]
+    [DataField, AutoNetworkedField]
     public bool Enabled = true;
 
     /// <summary>
     /// How much to scale the interval by (< 0 = min, > 1 = max)
     /// </summary>
-    [DataField("intervalScaling"), ViewVariables(VVAccess.ReadWrite), AutoNetworkedField]
+    [DataField, ViewVariables(VVAccess.ReadWrite), AutoNetworkedField]
     public FixedPoint2 IntervalScaling = 0;
 
     /// <summary>
     /// The maximum interval between beeps.
     /// </summary>
-    [DataField("maxBeepInterval"), ViewVariables(VVAccess.ReadWrite), AutoNetworkedField]
+    [DataField, ViewVariables(VVAccess.ReadWrite), AutoNetworkedField]
     public TimeSpan MaxBeepInterval = TimeSpan.FromSeconds(1.5f);
 
     /// <summary>
     /// The minimum interval between beeps.
     /// </summary>
-    [DataField("minBeepInterval"), ViewVariables(VVAccess.ReadWrite), AutoNetworkedField]
+    [DataField, ViewVariables(VVAccess.ReadWrite), AutoNetworkedField]
     public TimeSpan MinBeepInterval = TimeSpan.FromSeconds(0.25f);
 
     /// <summary>
@@ -55,12 +55,12 @@ public sealed partial class BeeperComponent : Component
     /// <summary>
     /// Is the beep muted
     /// </summary>
-    [DataField("muted"), ViewVariables(VVAccess.ReadWrite), AutoNetworkedField]
+    [DataField, ViewVariables(VVAccess.ReadWrite), AutoNetworkedField]
     public bool IsMuted = false;
 
     /// <summary>
     /// The sound played when the locator beeps.
     /// </summary>
-    [DataField("beepSound"), ViewVariables(VVAccess.ReadWrite), AutoNetworkedField]
+    [DataField, ViewVariables(VVAccess.ReadWrite), AutoNetworkedField]
     public SoundSpecifier? BeepSound;
 }

Content.Shared/ProximityDetection/Components/ProximityDetectorComponent.cs

@@ -13,10 +13,14 @@ public sealed partial class ProximityDetectorComponent : Component
     /// <summary>
     /// Whether or not it's on.
     /// </summary>
-    [DataField("enabled"), AutoNetworkedField, ViewVariables(VVAccess.ReadWrite)]
+    [DataField, AutoNetworkedField, ViewVariables(VVAccess.ReadWrite)]
     public bool Enabled = true;
 
-    [DataField("criteria", required: true), AutoNetworkedField, ViewVariables(VVAccess.ReadWrite)]
+    /// <summary>
+    /// The criteria used to filter entities
+    /// Note: RequireAll is only supported for tags, all components are required to count as a match!
+    /// </summary>
+    [DataField( required: true), AutoNetworkedField, ViewVariables(VVAccess.ReadWrite)]
     public EntityWhitelist Criteria = default!;
 
     /// <summary>
@@ -35,11 +39,11 @@ public sealed partial class ProximityDetectorComponent : Component
     /// <summary>
     /// The farthest distance to search for targets
     /// </summary>
-    [DataField("range"), ViewVariables(VVAccess.ReadWrite), AutoNetworkedField]
+    [DataField, ViewVariables(VVAccess.ReadWrite), AutoNetworkedField]
     public FixedPoint2 Range = 10f;
 
     public float AccumulatedFrameTime;
 
-    [DataField("updateRate"), ViewVariables(VVAccess.ReadWrite), AutoNetworkedField]
+    [DataField, ViewVariables(VVAccess.ReadWrite), AutoNetworkedField]
     public float UpdateRate = 0.3f;
 }

Content.Shared/ProximityDetection/Systems/ProximityDetectionSystem.cs

@@ -1,4 +1,5 @@
 using Content.Shared.ProximityDetection.Components;
+using Content.Shared.Tag;
 using Robust.Shared.Network;
 
 namespace Content.Shared.ProximityDetection.Systems;
@@ -9,6 +10,7 @@ public sealed class ProximityDetectionSystem : EntitySystem
 {
     [Dependency] private readonly EntityLookupSystem _entityLookup = default!;
     [Dependency] private readonly SharedTransformSystem _transform = default!;
+    [Dependency] private readonly TagSystem _tagSystem = default!;
     [Dependency] private readonly INetManager _net = default!;
 
     //update is only run on the server
@@ -17,18 +19,27 @@ public sealed class ProximityDetectionSystem : EntitySystem
     {
         SubscribeLocalEvent<ProximityDetectorComponent, EntityPausedEvent>(OnPaused);
         SubscribeLocalEvent<ProximityDetectorComponent, EntityUnpausedEvent>(OnUnpaused);
+        SubscribeLocalEvent<ProximityDetectorComponent, ComponentInit>(OnCompInit);
+
     }
 
-    protected void OnPaused(EntityUid owner, ProximityDetectorComponent component, EntityPausedEvent args)
+    private void OnCompInit(EntityUid uid, ProximityDetectorComponent component, ComponentInit args)
+    {
+        if (component.Criteria.RequireAll)
+            return;
+        Log.Debug("DetectorComponent only supports requireAll = false for tags. All components are required for a match!");
+    }
+
+    private void OnPaused(EntityUid owner, ProximityDetectorComponent component, EntityPausedEvent args)
     {
         SetEnable_Internal(owner,component,false);
     }
 
-    protected void OnUnpaused(EntityUid owner, ProximityDetectorComponent detector, ref EntityUnpausedEvent args)
+    private void OnUnpaused(EntityUid owner, ProximityDetectorComponent detector, ref EntityUnpausedEvent args)
     {
         SetEnable_Internal(owner, detector,true);
     }
-    protected internal void SetEnable(EntityUid owner, bool enabled, ProximityDetectorComponent? detector = null)
+    public void SetEnable(EntityUid owner, bool enabled, ProximityDetectorComponent? detector = null)
     {
         if (!Resolve(owner, ref detector) || detector.Enabled == enabled)
             return;
@@ -52,12 +63,12 @@ public sealed class ProximityDetectionSystem : EntitySystem
         }
     }
 
-    protected internal bool GetEnable(EntityUid owner, ProximityDetectorComponent? detector = null)
+    public bool GetEnable(EntityUid owner, ProximityDetectorComponent? detector = null)
     {
         return Resolve(owner, ref detector, false) && detector.Enabled;
     }
 
-    protected void SetEnable_Internal(EntityUid owner,ProximityDetectorComponent detector, bool enabled)
+    private void SetEnable_Internal(EntityUid owner,ProximityDetectorComponent detector, bool enabled)
     {
         detector.Enabled = enabled;
         var noDetectEvent = new ProximityTargetUpdatedEvent(detector, detector.TargetEnt, detector.Distance);
@@ -72,7 +83,7 @@ public sealed class ProximityDetectionSystem : EntitySystem
         RunUpdate_Internal(owner, detector);
     }
 
-    protected void ForceUpdate(EntityUid owner, ProximityDetectorComponent? detector = null)
+    public void ForceUpdate(EntityUid owner, ProximityDetectorComponent? detector = null)
     {
         if (!Resolve(owner, ref detector))
             return;
@@ -80,17 +91,37 @@ public sealed class ProximityDetectionSystem : EntitySystem
     }
 
 
-    protected void RunUpdate_Internal(EntityUid owner,ProximityDetectorComponent detector)
+    private void RunUpdate_Internal(EntityUid owner,ProximityDetectorComponent detector)
     {
         if (!_net.IsServer) //only run detection checks on the server!
             return;
         var xformQuery = GetEntityQuery<TransformComponent>();
         var xform = xformQuery.GetComponent(owner);
         List<(EntityUid TargetEnt, float Distance)> detections = new();
-        foreach (var ent in _entityLookup.GetEntitiesInRange(_transform.GetMapCoordinates(owner, xform),
+
-                     detector.Range.Float()))
+        if (detector.Criteria.Components == null)
         {
-            if (!detector.Criteria.IsValid(ent, EntityManager))
+            Log.Error($"ProximityDetectorComponent on {ToPrettyString(owner)} must use at least 1 component as a filter in criteria!");
+            throw new ArgumentException($"ProximityDetectorComponent on {ToPrettyString(owner)} must use at least 1 component as a filter in criteria!");
+        }
+        var firstCompType = EntityManager.ComponentFactory.GetRegistration(detector.Criteria.Components[0]).Type;
+        var foundEnts = _entityLookup.GetEntitiesInRange(firstCompType,_transform.GetMapCoordinates(owner, xform), detector.Range.Float());
+
+        var tagSearchEnabled = detector.Criteria.Tags is {Count: > 0};
+
+        CheckForAllComponentsPresent(detector, ref foundEnts, tagSearchEnabled);
+
+        if (foundEnts.Count == 0)
+        {
+            UpdateTargetFromClosest(owner, detector, detections);
+            return;
+        }
+
+        foreach (var ent in foundEnts)
+        {
+            if (tagSearchEnabled && ent.Comp is TagComponent tags && (detector.Criteria.RequireAll
+                    ? _tagSystem.HasAllTags(tags, detector.Criteria.Tags!)
+                    : _tagSystem.HasAnyTag(tags, detector.Criteria.Tags!)))
                 continue;
             var distance = (_transform.GetWorldPosition(xform, xformQuery) - _transform.GetWorldPosition(ent, xformQuery)).Length();
             if (CheckDetectConditions(ent, distance, owner, detector))
@@ -101,14 +132,44 @@ public sealed class ProximityDetectionSystem : EntitySystem
         UpdateTargetFromClosest(owner, detector, detections);
     }
 
-    protected bool CheckDetectConditions(EntityUid targetEntity, float dist, EntityUid owner, ProximityDetectorComponent detector)
+    private void CheckForAllComponentsPresent(ProximityDetectorComponent detector, ref HashSet<Entity<IComponent>> foundEnts, bool tagSearchEnabled)
+    {
+        var validEnts = new HashSet<Entity<IComponent>>(foundEnts.Count);
+        for (var i = 1; i < detector.Criteria.Components!.Length; i++)
+        {
+            validEnts.Clear();
+            var compType = EntityManager.ComponentFactory.GetRegistration(detector.Criteria.Components[i]).Type;
+            foreach (var ent in foundEnts)
+            {
+                if (!HasComp(ent, compType))
+                    continue;
+                validEnts.Add(ent);
+            }
+            (foundEnts, validEnts) = (validEnts, foundEnts);
+        }
+        validEnts.Clear();
+        if (tagSearchEnabled)
+        {
+            foreach (var ent in foundEnts)
+            {
+                if (!HasComp<TagComponent>(ent))
+                    continue;
+                validEnts.Add(ent);
+            }
+            (foundEnts, validEnts) = (validEnts, foundEnts);
+            validEnts.Clear();
+        }
+    }
+
+
+    private bool CheckDetectConditions(EntityUid targetEntity, float dist, EntityUid owner, ProximityDetectorComponent detector)
     {
         var detectAttempt = new ProximityDetectionAttemptEvent(false, dist, (owner, detector));
         RaiseLocalEvent(targetEntity, ref detectAttempt);
         return !detectAttempt.Cancel;
     }
 
-    protected void UpdateTargetFromClosest(EntityUid owner, ProximityDetectorComponent detector, List<(EntityUid TargetEnt, float Distance)> detections)
+    private void UpdateTargetFromClosest(EntityUid owner, ProximityDetectorComponent detector, List<(EntityUid TargetEnt, float Distance)> detections)
     {
         if (detections.Count == 0)
         {