Fix occluders for moved grid (#1878)

* Refactor occluders

* Copy pasting

* Reduce bounds

* Clear system updates on shutdown

Robust.Client/GameObjects/ClientComponentFactory.cs

@@ -25,6 +25,7 @@ namespace Robust.Client.GameObjects
             RegisterClass<InputComponent>();
             RegisterClass<SpriteComponent>();
             RegisterClass<ClientOccluderComponent>();
+            RegisterClass<OccluderTreeComponent>();
             RegisterClass<EyeComponent>();
             RegisterClass<AppearanceComponent>();
             RegisterClass<AppearanceTestComponent>();

Robust.Client/Graphics/Clyde/Clyde.LightRendering.cs

@@ -5,6 +5,7 @@ using OpenToolkit.Graphics.OpenGL4;
 using Robust.Client.GameObjects;
 using Robust.Client.ResourceManagement;
 using Robust.Shared.GameObjects;
+using Robust.Shared.IoC;
 using Robust.Shared.Log;
 using Robust.Shared.Map;
 using Robust.Shared.Maths;
@@ -773,24 +774,13 @@ namespace Robust.Client.Graphics.Clyde
                 var ii = 0;
                 var imi = 0;
 
-                foreach (var gridId in _mapManager.FindGridIdsIntersecting(map, expandedBounds, true))
+                foreach (var comp in occluderSystem.GetOccluderTrees(map, expandedBounds))
                 {
-                    if (!occluderSystem.TryGetOccluderTreeForGrid(map, gridId, out var occluderTree)) continue;
+                    // TODO: I know this doesn't work with rotated grids but when I come back to these I'm adding tests
+                    // because rotation bugs are common.
+                    var treeBounds = expandedBounds.Translated(-comp.Owner.Transform.WorldPosition);
 
-                    Box2 gridBounds;
-
-                    if (gridId == GridId.Invalid)
-                    {
-                        gridBounds = expandedBounds;
-                    }
-                    else
-                    {
-                        // TODO: Ideally this would clamp to the outer border of what we can see
-                        var grid = _mapManager.GetGrid(gridId);
-                        gridBounds = expandedBounds.Translated(-grid.WorldPosition);
-                    }
-
-                    occluderTree.QueryAabb((in OccluderComponent sOccluder) =>
+                    comp.Tree.QueryAabb((in OccluderComponent sOccluder) =>
                     {
                         var occluder = (ClientOccluderComponent)sOccluder;
                         var transform = occluder.Owner.Transform;
@@ -919,7 +909,7 @@ namespace Robust.Client.Graphics.Clyde
                         ami += 4;
 
                         return true;
-                    }, gridBounds);
+                    }, treeBounds);
                 }
 
                 _occlusionDataLength = ii;

Robust.Client/Graphics/Clyde/Clyde.cs

@@ -24,6 +24,7 @@ namespace Robust.Client.Graphics.Clyde
     internal sealed partial class Clyde : IClydeInternal, IClydeAudio, IPostInjectInit
     {
         [Dependency] private readonly IClydeTileDefinitionManager _tileDefinitionManager = default!;
+        [Dependency] private readonly IEntityLookup _lookup = default!;
         [Dependency] private readonly IEyeManager _eyeManager = default!;
         [Dependency] private readonly ILightManager _lightManager = default!;
         [Dependency] private readonly ILogManager _logManager = default!;

Robust.Server/GameObjects/ServerComponentFactory.cs

@@ -24,6 +24,7 @@ namespace Robust.Server.GameObjects
             RegisterClass<CollisionWakeComponent>();
             RegisterClass<ContainerManagerComponent>();
             RegisterClass<OccluderComponent>();
+            RegisterClass<OccluderTreeComponent>();
             RegisterClass<SpriteComponent>();
             RegisterClass<AppearanceComponent>();
             RegisterClass<SnapGridComponent>();

Robust.Shared/GameObjects/Components/Light/OccluderComponent.cs

@@ -1,8 +1,6 @@
 using System;
 using Robust.Shared.GameStates;
-using Robust.Shared.Map;
 using Robust.Shared.Maths;
-using Robust.Shared.Prototypes;
 using Robust.Shared.Players;
 using Robust.Shared.Serialization;
 using Robust.Shared.Serialization.Manager.Attributes;
@@ -20,6 +18,8 @@ namespace Robust.Shared.GameObjects
         [DataField("boundingBox")]
         private Box2 _boundingBox = new(-0.5f, -0.5f, 0.5f, 0.5f);
 
+        internal OccluderTreeComponent? Tree = null;
+
         [ViewVariables(VVAccess.ReadWrite)]
         public Box2 BoundingBox
         {
@@ -28,15 +28,10 @@ namespace Robust.Shared.GameObjects
             {
                 _boundingBox = value;
                 Dirty();
-                BoundingBoxChanged();
+                Owner.EntityManager.EventBus.RaiseLocalEvent(Owner.Uid, new OccluderUpdateEvent(this));
             }
         }
 
-        private void BoundingBoxChanged()
-        {
-            Owner.EntityManager.EventBus.RaiseEvent(EventSource.Local, new OccluderBoundingBoxChangedMessage(this));
-        }
-
         [ViewVariables(VVAccess.ReadWrite)]
         public virtual bool Enabled
         {
@@ -47,30 +42,19 @@ namespace Robust.Shared.GameObjects
                     return;
 
                 _enabled = value;
+                if (_enabled)
+                {
+                    Owner.EntityManager.EventBus.RaiseLocalEvent(Owner.Uid, new OccluderAddEvent(this));
+                }
+                else
+                {
+                    Owner.EntityManager.EventBus.RaiseLocalEvent(Owner.Uid, new OccluderRemoveEvent(this));
+                }
+
                 Dirty();
             }
         }
 
-        protected override void Startup()
-        {
-            base.Startup();
-
-            EntitySystem.Get<OccluderSystem>().AddOrUpdateEntity(Owner, Owner.Transform.Coordinates);
-        }
-
-        protected override void Shutdown()
-        {
-            base.Shutdown();
-
-            var transform = Owner.Transform;
-            var map = transform.MapID;
-            if (map != MapId.Nullspace)
-            {
-                Owner.EntityManager.EventBus.RaiseEvent(EventSource.Local,
-                    new OccluderTreeRemoveOccluderMessage(this, map, transform.GridID));
-            }
-        }
-
         public override ComponentState GetComponentState(ICommonSession player)
         {
             return new OccluderComponentState(Enabled, BoundingBox);
@@ -102,14 +86,4 @@ namespace Robust.Shared.GameObjects
             }
         }
     }
-
-    internal struct OccluderBoundingBoxChangedMessage
-    {
-        public OccluderComponent Occluder;
-
-        public OccluderBoundingBoxChangedMessage(OccluderComponent occluder)
-        {
-            Occluder = occluder;
-        }
-    }
 }

Robust.Shared/GameObjects/Components/Light/OccluderTreeComponent.cs

@@ -0,0 +1,14 @@
+using Robust.Shared.Physics;
+
+namespace Robust.Shared.GameObjects
+{
+    /// <summary>
+    /// Stores the relevant occluder children of this entity.
+    /// </summary>
+    public sealed class OccluderTreeComponent : Component
+    {
+        public override string Name => "OccluderTree";
+
+        internal DynamicTree<OccluderComponent> Tree { get; set; } = default!;
+    }
+}

Robust.Shared/GameObjects/Systems/OccluderSystem.cs

@@ -1,6 +1,5 @@
 using System;
 using System.Collections.Generic;
-using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using Robust.Shared.IoC;
 using Robust.Shared.Map;
@@ -11,54 +10,88 @@ namespace Robust.Shared.GameObjects
 {
     public abstract class OccluderSystem : EntitySystem
     {
-
         [Dependency] private readonly IMapManagerInternal _mapManager = default!;
 
-        private readonly Dictionary<MapId, Dictionary<GridId, DynamicTree<OccluderComponent>>> _gridTrees =
-                     new();
+        private const float TreeGrowthRate = 256;
 
-        private readonly List<(OccluderComponent Occluder, EntityCoordinates Coordinates)> _occluderAddQueue =
-                     new();
-
-        private readonly List<(OccluderComponent Occluder, EntityCoordinates Coordinates)> _occluderRemoveQueue =
-            new();
-
-        internal bool TryGetOccluderTreeForGrid(MapId mapId, GridId gridId, [NotNullWhen(true)] out DynamicTree<OccluderComponent>? gridTree)
-        {
-            gridTree = null;
-
-            if (!_gridTrees.TryGetValue(mapId, out var grids))
-                return false;
-
-            if (!grids.TryGetValue(gridId, out gridTree))
-                return false;
-
-            return true;
-        }
+        private Queue<OccluderEvent> _updates = new(64);
 
         public override void Initialize()
         {
             base.Initialize();
 
             _mapManager.MapCreated += OnMapCreated;
-            _mapManager.MapDestroyed += OnMapDestroyed;
-            _mapManager.OnGridCreated += OnGridCreated;
-            _mapManager.OnGridRemoved += OnGridRemoved;
 
-            SubscribeLocalEvent<MoveEvent>(EntMoved);
-            SubscribeLocalEvent<EntParentChangedMessage>(EntParentChanged);
-            SubscribeLocalEvent<OccluderBoundingBoxChangedMessage>(OccluderBoundingBoxChanged);
-            SubscribeLocalEvent<OccluderTreeRemoveOccluderMessage>(RemoveOccluder);
+            SubscribeLocalEvent<GridInitializeEvent>(HandleGridInit);
+            SubscribeLocalEvent<OccluderTreeComponent, ComponentInit>(HandleOccluderTreeInit);
+            SubscribeLocalEvent<OccluderComponent, ComponentInit>(HandleOccluderInit);
+            SubscribeLocalEvent<OccluderComponent, ComponentShutdown>(HandleOccluderShutdown);
+
+            SubscribeLocalEvent<OccluderComponent, MoveEvent>(EntMoved);
+            SubscribeLocalEvent<OccluderComponent, EntParentChangedMessage>(EntParentChanged);
+            SubscribeLocalEvent<OccluderEvent>(ev => _updates.Enqueue(ev));
+        }
+
+        internal IEnumerable<OccluderTreeComponent> GetOccluderTrees(MapId mapId, Box2 worldAABB)
+        {
+            if (mapId == MapId.Nullspace) yield break;
+
+            foreach (var grid in _mapManager.FindGridsIntersecting(mapId, worldAABB))
+            {
+                yield return EntityManager.GetEntity(grid.GridEntityId).GetComponent<OccluderTreeComponent>();
+            }
+
+            yield return _mapManager.GetMapEntity(mapId).GetComponent<OccluderTreeComponent>();
+        }
+
+        private void HandleOccluderInit(EntityUid uid, OccluderComponent component, ComponentInit args)
+        {
+            if (!component.Enabled) return;
+            _updates.Enqueue(new OccluderAddEvent(component));
+        }
+
+        private void HandleOccluderShutdown(EntityUid uid, OccluderComponent component, ComponentShutdown args)
+        {
+            if (!component.Enabled) return;
+            _updates.Enqueue(new OccluderRemoveEvent(component));
+        }
+
+        private void HandleOccluderTreeInit(EntityUid uid, OccluderTreeComponent component, ComponentInit args)
+        {
+            var capacity = (int) Math.Min(256, Math.Ceiling(component.Owner.Transform.ChildCount / TreeGrowthRate) * TreeGrowthRate);
+
+            component.Tree = new DynamicTree<OccluderComponent>(ExtractAabbFunc, capacity: capacity);
+        }
+
+        private void HandleGridInit(GridInitializeEvent ev)
+        {
+            EntityManager.GetEntity(ev.EntityUid).EnsureComponent<OccluderTreeComponent>();
+        }
+
+        private OccluderTreeComponent? GetOccluderTree(OccluderComponent component)
+        {
+            var entity = component.Owner;
+
+            if (entity.Transform.MapID == MapId.Nullspace) return null;
+
+            var parent = entity.Transform.Parent?.Owner;
+
+            while (true)
+            {
+                if (parent == null) break;
+
+                if (parent.TryGetComponent(out OccluderTreeComponent? comp)) return comp;
+                parent = parent.Transform.Parent?.Owner;
+            }
+
+            return null;
         }
 
         public override void Shutdown()
         {
             base.Shutdown();
-
             _mapManager.MapCreated -= OnMapCreated;
-            _mapManager.MapDestroyed -= OnMapDestroyed;
-            _mapManager.OnGridCreated -= OnGridCreated;
-            _mapManager.OnGridRemoved -= OnGridRemoved;
+            _updates.Clear();
         }
 
         public override void FrameUpdate(float frameTime)
@@ -73,143 +106,59 @@ namespace Robust.Shared.GameObjects
 
         private void UpdateTrees()
         {
-            // Only care about stuff parented to a grid I think?
-            foreach (var (occluder, coordinates) in _occluderRemoveQueue)
+            while (_updates.TryDequeue(out var occluderUpdate))
             {
-                if (coordinates.TryGetParent(EntityManager, out var parent) &&
-                    parent.HasComponent<MapGridComponent>())
-                {
-                    var gridTree = _gridTrees[parent.Transform.MapID][parent.Transform.GridID];
+                OccluderTreeComponent? tree;
+                var component = occluderUpdate.Component;
 
-                    gridTree.Remove(occluder);
+                switch (occluderUpdate)
+                {
+                    case OccluderAddEvent:
+                        if (component.Tree != null) break;
+                        tree = GetOccluderTree(component);
+                        if (tree == null) break;
+                        component.Tree = tree;
+                        tree.Tree.Add(component);
+                        break;
+                    case OccluderUpdateEvent:
+                        var oldTree = component.Tree;
+                        tree = GetOccluderTree(component);
+                        if (oldTree != tree)
+                        {
+                            oldTree?.Tree.Remove(component);
+                            tree?.Tree.Add(component);
+                            component.Tree = tree;
+                            break;
+                        }
+
+                        tree?.Tree.Update(component);
+
+                        break;
+                    case OccluderRemoveEvent:
+                        tree = component.Tree;
+                        tree?.Tree.Remove(component);
+                        break;
+                    default:
+                        throw new ArgumentOutOfRangeException($"No implemented occluder update for {occluderUpdate.GetType()}");
                 }
             }
-
-            _occluderRemoveQueue.Clear();
-
-            foreach (var (occluder, coordinates) in _occluderAddQueue)
-            {
-                if (occluder.Deleted) continue;
-
-                if (coordinates.TryGetParent(EntityManager, out var parent) &&
-                    parent.HasComponent<MapGridComponent>() || occluder.Owner.Transform.GridID == GridId.Invalid)
-                {
-                    parent ??= EntityManager.GetEntity(occluder.Owner.Transform.ParentUid);
-                    var gridTree = _gridTrees[parent.Transform.MapID][parent.Transform.GridID];
-
-                    gridTree.AddOrUpdate(occluder);
-                }
-            }
-
-            _occluderAddQueue.Clear();
         }
 
-        // If the Transform is removed BEFORE the Occluder,
-        // then the MapIdChanged code will handle and remove it (because MapId gets set to nullspace).
-        // Otherwise these will still have their past MapId and that's all we need..
-        private void RemoveOccluder(OccluderTreeRemoveOccluderMessage ev)
+        private void EntMoved(EntityUid uid, OccluderComponent component, MoveEvent args)
         {
-            _gridTrees[ev.MapId][ev.GridId].Remove(ev.Occluder);
+            _updates.Enqueue(new OccluderUpdateEvent(component));
         }
 
-        private void OccluderBoundingBoxChanged(OccluderBoundingBoxChangedMessage ev)
+        private void EntParentChanged(EntityUid uid, OccluderComponent component, EntParentChangedMessage args)
         {
-            QueueUpdateOccluder(ev.Occluder, ev.Occluder.Owner.Transform.Coordinates);
-        }
-
-        private void EntMoved(MoveEvent ev)
-        {
-            ev.OldPosition.TryGetParent(EntityManager, out var oldParent);
-            ev.NewPosition.TryGetParent(EntityManager, out var newParent);
-
-            if (oldParent?.Uid != newParent?.Uid)
-                RemoveEntity(ev.Sender, ev.OldPosition);
-
-            AddOrUpdateEntity(ev.Sender, ev.NewPosition);
-        }
-
-        private void EntParentChanged(EntParentChangedMessage message)
-        {
-            if (!message.Entity.TryGetComponent(out OccluderComponent? occluder))
-                return;
-
-            // Really only care if it's a map or grid
-            if (message.OldParent != null && message.OldParent.TryGetComponent(out MapGridComponent? oldGrid))
-            {
-                var map = message.OldParent.Transform.MapID;
-                if (_gridTrees[map].TryGetValue(oldGrid.GridIndex, out var tree))
-                {
-                    tree.Remove(occluder);
-                }
-            }
-
-            var newParent = EntityManager.GetEntity(message.Entity.Transform.ParentUid);
-
-            newParent.TryGetComponent(out MapGridComponent? newGrid);
-            var newGridIndex = newGrid?.GridIndex ?? GridId.Invalid;
-            var newMap = newParent.Transform.MapID;
-
-            if (!_gridTrees.TryGetValue(newMap, out var newMapGrids))
-            {
-                newMapGrids = new Dictionary<GridId, DynamicTree<OccluderComponent>>();
-                _gridTrees[newMap] = newMapGrids;
-            }
-
-            if (!newMapGrids.TryGetValue(newGridIndex, out var newTree))
-            {
-                newTree = new DynamicTree<OccluderComponent>(ExtractAabbFunc);
-                newMapGrids[newGridIndex] = newTree;
-            }
-
-            newTree.AddOrUpdate(occluder);
-        }
-
-        private void RemoveEntity(IEntity entity, EntityCoordinates coordinates)
-        {
-            if (entity.TryGetComponent(out OccluderComponent? occluder))
-            {
-                QueueRemoveOccluder(occluder, coordinates);
-            }
-        }
-
-        internal void AddOrUpdateEntity(IEntity entity, EntityCoordinates coordinates)
-        {
-            if (entity.TryGetComponent(out OccluderComponent? occluder))
-            {
-                QueueUpdateOccluder(occluder, coordinates);
-            }
-            // Do we even need the children update? Coz they be slow af and allocate a lot.
-            // If you do end up adding children back in then for the love of GOD check if the entity has a mapgridcomponent
-        }
-
-        private void OnMapDestroyed(object? sender, MapEventArgs e)
-        {
-            _gridTrees.Remove(e.Map);
+            _updates.Enqueue(new OccluderUpdateEvent(component));
         }
 
         private void OnMapCreated(object? sender, MapEventArgs e)
         {
-            if (e.Map == MapId.Nullspace)
-                return;
+            if (e.Map == MapId.Nullspace) return;
 
-            _gridTrees[e.Map] = new Dictionary<GridId, DynamicTree<OccluderComponent>>();
-        }
-
-        private void OnGridRemoved(MapId mapId, GridId gridId)
-        {
-            foreach (var (_, gridIds) in _gridTrees)
-            {
-                if (gridIds.Remove(gridId))
-                    break;
-            }
-        }
-
-        private void OnGridCreated(MapId mapId, GridId gridId)
-        {
-            if (!_gridTrees.TryGetValue(mapId, out var gridTree))
-                return;
-
-            gridTree.Add(gridId, new DynamicTree<OccluderComponent>(ExtractAabbFunc));
+            _mapManager.GetMapEntity(e.Map).EnsureComponent<OccluderTreeComponent>();
         }
 
         private static Box2 ExtractAabbFunc(in OccluderComponent o)
@@ -217,28 +166,26 @@ namespace Robust.Shared.GameObjects
             return o.BoundingBox.Translated(o.Owner.Transform.LocalPosition);
         }
 
-        private void QueueUpdateOccluder(OccluderComponent occluder, EntityCoordinates coordinates)
-        {
-            _occluderAddQueue.Add((occluder, coordinates));
-        }
-
-        private void QueueRemoveOccluder(OccluderComponent occluder, EntityCoordinates coordinates)
-        {
-            _occluderRemoveQueue.Add((occluder, coordinates));
-        }
-
-        public IEnumerable<RayCastResults> IntersectRayWithPredicate(MapId originMapId, in Ray ray, float maxLength,
+        public IEnumerable<RayCastResults> IntersectRayWithPredicate(MapId mapId, in Ray ray, float maxLength,
             Func<IEntity, bool>? predicate = null, bool returnOnFirstHit = true)
         {
-            if (originMapId == MapId.Nullspace) return Enumerable.Empty<RayCastResults>();
+            if (mapId == MapId.Nullspace) return Enumerable.Empty<RayCastResults>();
             var list = new List<RayCastResults>();
-            var worldBox = new Box2();
 
-            foreach (var gridId in _mapManager.FindGridIdsIntersecting(originMapId, worldBox, true))
+            var endPoint = ray.Position + ray.Direction * maxLength;
+            var worldBox = new Box2(Vector2.ComponentMin(ray.Position, endPoint), Vector2.ComponentMax(ray.Position, endPoint));
+
+            foreach (var comp in GetOccluderTrees(mapId, worldBox))
             {
-                var gridTree = _gridTrees[originMapId][gridId];
+                var transform = comp.Owner.Transform;
+                var treePos = transform.WorldPosition;
+                var treeRot = transform.WorldRotation;
 
-                gridTree.QueryRay(ref list,
+                var relativePos = new Angle(-treeRot.Theta).RotateVec(ray.Position - treePos);
+
+                var treeRay = new Ray(relativePos, new Angle(-treeRot).RotateVec(ray.Direction));
+
+                comp.Tree.QueryRay(ref list,
                     (ref List<RayCastResults> state, in OccluderComponent value, in Vector2 point, float distFromOrigin) =>
                     {
                         if (distFromOrigin > maxLength)
@@ -253,24 +200,35 @@ namespace Robust.Shared.GameObjects
                         var result = new RayCastResults(distFromOrigin, point, value.Owner);
                         state.Add(result);
                         return !returnOnFirstHit;
-                    }, ray);
+                    }, treeRay);
             }
 
             return list;
         }
     }
 
-    internal readonly struct OccluderTreeRemoveOccluderMessage
+    internal sealed class OccluderAddEvent : OccluderEvent
     {
-        public readonly OccluderComponent Occluder;
-        public readonly MapId MapId;
-        public readonly GridId GridId;
+        public OccluderAddEvent(OccluderComponent component) : base(component) {}
+    }
 
-        public OccluderTreeRemoveOccluderMessage(OccluderComponent occluder, MapId mapId, GridId gridId)
+    internal sealed class OccluderUpdateEvent : OccluderEvent
+    {
+        public OccluderUpdateEvent(OccluderComponent component) : base(component) {}
+    }
+
+    internal sealed class OccluderRemoveEvent : OccluderEvent
+    {
+        public OccluderRemoveEvent(OccluderComponent component) : base(component) {}
+    }
+
+    internal abstract class OccluderEvent : EntityEventArgs
+    {
+        public OccluderComponent Component { get; }
+
+        public OccluderEvent(OccluderComponent component)
         {
-            Occluder = occluder;
-            MapId = mapId;
-            GridId = gridId;
+            Component = component;
         }
     }
 }