From 6c44dd9665128f2c344e9028223553b26fec7346 Mon Sep 17 00:00:00 2001 From: metalgearsloth <31366439+metalgearsloth@users.noreply.github.com> Date: Thu, 20 May 2021 18:57:29 +1000 Subject: [PATCH] RenderingTreeSystem cleanup (#1759) * RenderingTreeSystem cleanup 10% less bad but 100% still boilerplate * Slight changies * Even better * New shit just got made * Apply revews --- .../Components/Light/PointLightComponent.cs | 21 +- .../Components/Renderable/SpriteComponent.cs | 14 +- .../EntitySystems/RenderingTreeSystem.cs | 392 +++++++++++------- .../GameObjects/EntitySystems/SpriteSystem.cs | 23 +- .../EntMapIdChangedMessage.cs | 2 +- 5 files changed, 285 insertions(+), 167 deletions(-) diff --git a/Robust.Client/GameObjects/Components/Light/PointLightComponent.cs b/Robust.Client/GameObjects/Components/Light/PointLightComponent.cs index d4b68b1b4a..352e3af5e0 100644 --- a/Robust.Client/GameObjects/Components/Light/PointLightComponent.cs +++ b/Robust.Client/GameObjects/Components/Light/PointLightComponent.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using Robust.Client.Graphics; using Robust.Client.ResourceManagement; using Robust.Shared.Animations; @@ -167,7 +168,7 @@ namespace Robust.Client.GameObjects set { _radius = MathF.Max(value, 0.01f); // setting radius to 0 causes exceptions, so just use a value close enough to zero that it's unnoticeable. - Owner.EntityManager.EventBus.RaiseEvent(EventSource.Local, new PointLightRadiusChangedMessage(this)); + Owner.EntityManager.EventBus.RaiseEvent(EventSource.Local, new PointLightRadiusChangedEvent(this)); } } @@ -179,6 +180,18 @@ namespace Robust.Client.GameObjects Mask = null; } + /// + /// What MapId we are intersecting for RenderingTreeSystem. + /// + [ViewVariables] + internal MapId IntersectingMapId { get; set; } = MapId.Nullspace; + + /// + /// What grids we're on for RenderingTreeSystem. + /// + [ViewVariables] + internal List IntersectingGrids = new(); + void ISerializationHooks.AfterDeserialization() { if (_maskPath != null) @@ -230,7 +243,7 @@ namespace Robust.Client.GameObjects if (map != MapId.Nullspace) { Owner.EntityManager.EventBus.RaiseEvent(EventSource.Local, - new RenderTreeRemoveLightMessage(this, map)); + new RenderTreeRemoveLightEvent(this, map)); } } @@ -248,11 +261,11 @@ namespace Robust.Client.GameObjects } } - public struct PointLightRadiusChangedMessage + public class PointLightRadiusChangedEvent : EntityEventArgs { public PointLightComponent PointLightComponent { get; } - public PointLightRadiusChangedMessage(PointLightComponent pointLightComponent) + public PointLightRadiusChangedEvent(PointLightComponent pointLightComponent) { PointLightComponent = pointLightComponent; } diff --git a/Robust.Client/GameObjects/Components/Renderable/SpriteComponent.cs b/Robust.Client/GameObjects/Components/Renderable/SpriteComponent.cs index 4daf3c351b..783fef877c 100644 --- a/Robust.Client/GameObjects/Components/Renderable/SpriteComponent.cs +++ b/Robust.Client/GameObjects/Components/Renderable/SpriteComponent.cs @@ -125,6 +125,18 @@ namespace Robust.Client.GameObjects [DataField("directional")] private bool _directional = true; + /// + /// What MapId we are intersecting for RenderingTreeSystem. + /// + [ViewVariables] + internal MapId IntersectingMapId { get; set; } = MapId.Nullspace; + + /// + /// What grids we're on for RenderingTreeSystem. + /// + [ViewVariables] + internal List IntersectingGrids { get; } = new(); + [DataField("layerDatums")] private List LayerDatums { @@ -1355,7 +1367,7 @@ namespace Robust.Client.GameObjects if (map != MapId.Nullspace) { Owner.EntityManager.EventBus.RaiseEvent(EventSource.Local, - new RenderTreeRemoveSpriteMessage(this, map)); + new RenderTreeRemoveSpriteEvent(this, map)); } } diff --git a/Robust.Client/GameObjects/EntitySystems/RenderingTreeSystem.cs b/Robust.Client/GameObjects/EntitySystems/RenderingTreeSystem.cs index e12f69c642..eaeb03447f 100644 --- a/Robust.Client/GameObjects/EntitySystems/RenderingTreeSystem.cs +++ b/Robust.Client/GameObjects/EntitySystems/RenderingTreeSystem.cs @@ -1,4 +1,6 @@ using System.Collections.Generic; +using System.Drawing; +using System.Linq; using JetBrains.Annotations; using Robust.Client.Physics; using Robust.Shared.GameObjects; @@ -6,6 +8,7 @@ using Robust.Shared.IoC; using Robust.Shared.Map; using Robust.Shared.Maths; using Robust.Shared.Physics; +using Robust.Shared.Utility; namespace Robust.Client.GameObjects { @@ -47,145 +50,176 @@ namespace Robust.Client.GameObjects _mapManager.OnGridCreated += MapManagerOnGridCreated; _mapManager.OnGridRemoved += MapManagerOnGridRemoved; - SubscribeLocalEvent(EntMapIdChanged); - SubscribeLocalEvent(EntMoved); - SubscribeLocalEvent(EntParentChanged); - SubscribeLocalEvent(PointLightRadiusChanged); - SubscribeLocalEvent(RemoveSprite); - SubscribeLocalEvent(RemoveLight); + SubscribeLocalEvent(SpriteMapChanged); + SubscribeLocalEvent(SpriteMoved); + SubscribeLocalEvent(SpriteParentChanged); + SubscribeLocalEvent(RemoveSprite); + + SubscribeLocalEvent(LightMapChanged); + SubscribeLocalEvent(LightMoved); + SubscribeLocalEvent(LightParentChanged); + SubscribeLocalEvent(PointLightRadiusChanged); + SubscribeLocalEvent(RemoveLight); } + // For the RemoveX methods + // If the Transform is removed BEFORE the Sprite/Light, + // 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.. + + #region SpriteHandlers + private void SpriteMapChanged(EntityUid uid, SpriteComponent component, EntMapIdChangedMessage args) + { + QueueSpriteUpdate(component); + } + + private void SpriteMoved(EntityUid uid, SpriteComponent component, MoveEvent args) + { + QueueSpriteUpdate(component); + } + + private void SpriteParentChanged(EntityUid uid, SpriteComponent component, EntParentChangedMessage args) + { + QueueSpriteUpdate(component); + } + + private void RemoveSprite(EntityUid uid, SpriteComponent component, RenderTreeRemoveSpriteEvent args) + { + ClearSprite(component); + } + + private void ClearSprite(SpriteComponent component) + { + if (_gridTrees.TryGetValue(component.IntersectingMapId, out var gridTrees)) + { + foreach (var gridId in component.IntersectingGrids) + { + if (!gridTrees.TryGetValue(gridId, out var tree)) continue; + tree.SpriteTree.Remove(component); + } + } + + component.IntersectingGrids.Clear(); + } + + private void QueueSpriteUpdate(SpriteComponent component) + { + if (component.TreeUpdateQueued) return; + + component.TreeUpdateQueued = true; + _spriteQueue.Add(component); + + foreach (var child in component.Owner.Transform.Children) + { + QueueSpriteUpdate(child.Owner); + } + } + + private void QueueSpriteUpdate(IEntity entity) + { + if (!entity.TryGetComponent(out SpriteComponent? spriteComponent)) return; + QueueSpriteUpdate(spriteComponent); + + foreach (var child in entity.Transform.Children) + { + QueueSpriteUpdate(child.Owner); + } + } + #endregion + + #region LightHandlers + private void LightMapChanged(EntityUid uid, PointLightComponent component, EntMapIdChangedMessage args) + { + QueueLightUpdate(component); + } + + private void LightMoved(EntityUid uid, PointLightComponent component, MoveEvent args) + { + QueueLightUpdate(component); + } + + private void LightParentChanged(EntityUid uid, PointLightComponent component, EntParentChangedMessage args) + { + QueueLightUpdate(component); + } + + private void PointLightRadiusChanged(EntityUid uid, PointLightComponent component, PointLightRadiusChangedEvent args) + { + QueueLightUpdate(component); + } + + private void RemoveLight(EntityUid uid, PointLightComponent component, RenderTreeRemoveLightEvent args) + { + ClearLight(component); + } + + private void ClearLight(PointLightComponent component) + { + if (_gridTrees.TryGetValue(component.IntersectingMapId, out var gridTrees)) + { + foreach (var gridId in component.IntersectingGrids) + { + if (!gridTrees.TryGetValue(gridId, out var tree)) continue; + tree.LightTree.Remove(component); + } + } + + component.IntersectingGrids.Clear(); + } + + private void QueueLightUpdate(PointLightComponent component) + { + if (component.TreeUpdateQueued) return; + + component.TreeUpdateQueued = true; + _lightQueue.Add(component); + + foreach (var child in component.Owner.Transform.Children) + { + QueueLightUpdate(child.Owner); + } + } + + private void QueueLightUpdate(IEntity entity) + { + if (!entity.TryGetComponent(out PointLightComponent? lightComponent)) return; + QueueLightUpdate(lightComponent); + + foreach (var child in entity.Transform.Children) + { + QueueLightUpdate(child.Owner); + } + } + #endregion + public override void Shutdown() { base.Shutdown(); - _mapManager.MapCreated -= MapManagerOnMapCreated; _mapManager.MapDestroyed -= MapManagerOnMapDestroyed; _mapManager.OnGridCreated -= MapManagerOnGridCreated; _mapManager.OnGridRemoved -= MapManagerOnGridRemoved; } - // For these next 2 methods (the Remove* ones): - // If the Transform is removed BEFORE the Sprite/Light, - // 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 RemoveLight(RenderTreeRemoveLightMessage ev) - { - foreach (var gridId in _mapManager.FindGridIdsIntersecting(ev.Map, MapTrees.LightAabbFunc(ev.Light), true)) - { - _gridTrees[ev.Map][gridId].LightTree.Remove(ev.Light); - } - } - - private void RemoveSprite(RenderTreeRemoveSpriteMessage ev) - { - foreach (var gridId in _mapManager.FindGridIdsIntersecting(ev.Map, MapTrees.SpriteAabbFunc(ev.Sprite), true)) - { - _gridTrees[ev.Map][gridId].SpriteTree.Remove(ev.Sprite); - } - } - - private void PointLightRadiusChanged(PointLightRadiusChangedMessage ev) - { - QueueUpdateLight(ev.PointLightComponent); - } - - private void EntParentChanged(EntParentChangedMessage ev) - { - UpdateEntity(ev.Entity); - } - - private void EntMoved(MoveEvent ev) - { - UpdateEntity(ev.Sender); - } - - private void UpdateEntity(IEntity entity) - { - if (entity.TryGetComponent(out SpriteComponent? spriteComponent)) - { - if (!spriteComponent.TreeUpdateQueued) - { - spriteComponent.TreeUpdateQueued = true; - - _spriteQueue.Add(spriteComponent); - } - } - - if (entity.TryGetComponent(out PointLightComponent? light)) - { - QueueUpdateLight(light); - } - - foreach (var child in entity.Transform.ChildEntityUids) - { - UpdateEntity(EntityManager.GetEntity(child)); - } - } - - private void QueueUpdateLight(PointLightComponent light) - { - if (!light.TreeUpdateQueued) - { - light.TreeUpdateQueued = true; - - _lightQueue.Add(light); - } - } - - private void EntMapIdChanged(EntMapIdChangedMessage ev) - { - // Nullspace is a valid map ID for stuff to have but we also aren't gonna bother indexing it. - // So that's why there's a GetValueOrDefault. - var oldMapTrees = _gridTrees.GetValueOrDefault(ev.OldMapId); - var newMapTrees = _gridTrees.GetValueOrDefault(ev.Entity.Transform.MapID); - - // TODO: MMMM probably a better way to do this. - if (ev.Entity.TryGetComponent(out SpriteComponent? sprite)) - { - if (oldMapTrees != null) - { - foreach (var (_, gridTree) in oldMapTrees) - { - gridTree.SpriteTree.Remove(sprite); - } - } - - var bounds = MapTrees.SpriteAabbFunc(sprite); - - foreach (var gridId in _mapManager.FindGridIdsIntersecting(ev.Entity.Transform.MapID, bounds, true)) - { - var gridBounds = gridId == GridId.Invalid - ? bounds : bounds.Translated(-_mapManager.GetGrid(gridId).WorldPosition); - - newMapTrees?[gridId].SpriteTree.AddOrUpdate(sprite, gridBounds); - } - } - - if (ev.Entity.TryGetComponent(out PointLightComponent? light)) - { - if (oldMapTrees != null) - { - foreach (var (_, gridTree) in oldMapTrees) - { - gridTree.LightTree.Remove(light); - } - } - - var bounds = MapTrees.LightAabbFunc(light); - - foreach (var gridId in _mapManager.FindGridIdsIntersecting(ev.Entity.Transform.MapID, bounds, true)) - { - var gridBounds = gridId == GridId.Invalid - ? bounds : bounds.Translated(-_mapManager.GetGrid(gridId).WorldPosition); - - newMapTrees?[gridId].LightTree.AddOrUpdate(light, gridBounds); - } - } - } - private void MapManagerOnMapDestroyed(object? sender, MapEventArgs e) { + foreach (var (_, gridTree) in _gridTrees[e.Map]) + { + foreach (var comp in gridTree.LightTree) + { + comp.IntersectingGrids.Clear(); + } + + foreach (var comp in gridTree.SpriteTree) + { + comp.IntersectingGrids.Clear(); + } + + // Just in case? + gridTree.LightTree.Clear(); + gridTree.SpriteTree.Clear(); + } + _gridTrees.Remove(e.Map); } @@ -209,47 +243,109 @@ namespace Robust.Client.GameObjects private void MapManagerOnGridRemoved(MapId mapId, GridId gridId) { + var gridTree = _gridTrees[mapId][gridId]; + + foreach (var sprite in gridTree.SpriteTree) + { + sprite.IntersectingGrids.Remove(gridId); + } + + foreach (var light in gridTree.LightTree) + { + light.IntersectingGrids.Remove(gridId); + } + + // Clear in case + gridTree.LightTree.Clear(); + gridTree.SpriteTree.Clear(); _gridTrees[mapId].Remove(gridId); } public override void FrameUpdate(float frameTime) { - foreach (var queuedUpdateSprite in _spriteQueue) + foreach (var sprite in _spriteQueue) { - var map = queuedUpdateSprite.Owner.Transform.MapID; - if (map == MapId.Nullspace) + var mapId = sprite.Owner.Transform.MapID; + + // If we're on a new map then clear the old one. + if (sprite.IntersectingMapId != mapId) { - continue; + ClearSprite(sprite); } - var mapTree = _gridTrees[map]; + sprite.IntersectingMapId = mapId; - foreach (var gridId in _mapManager.FindGridIdsIntersecting(map, - MapTrees.SpriteAabbFunc(queuedUpdateSprite), true)) + if (mapId == MapId.Nullspace) continue; + + var mapTree = _gridTrees[mapId]; + var aabb = MapTrees.SpriteAabbFunc(sprite); + var intersectingGrids = _mapManager.FindGridIdsIntersecting(mapId, aabb, true).ToList(); + + // Remove from old + foreach (var gridId in sprite.IntersectingGrids) { - mapTree[gridId].SpriteTree.AddOrUpdate(queuedUpdateSprite); + if (intersectingGrids.Contains(gridId)) continue; + mapTree[gridId].SpriteTree.Remove(sprite); } - queuedUpdateSprite.TreeUpdateQueued = false; + // Rebuild in the update below + sprite.IntersectingGrids.Clear(); + + // Update / add to new + foreach (var gridId in intersectingGrids) + { + var translated = aabb.Translated(gridId == GridId.Invalid + ? Vector2.Zero + : -_mapManager.GetGrid(gridId).WorldPosition); + + mapTree[gridId].SpriteTree.AddOrUpdate(sprite, translated); + + sprite.IntersectingGrids.Add(gridId); + } + + sprite.TreeUpdateQueued = false; } - foreach (var queuedUpdateLight in _lightQueue) + foreach (var light in _lightQueue) { - var map = queuedUpdateLight.Owner.Transform.MapID; - if (map == MapId.Nullspace) + var mapId = light.Owner.Transform.MapID; + + // If we're on a new map then clear the old one. + if (light.IntersectingMapId != mapId) { - continue; + ClearLight(light); } - var mapTree = _gridTrees[map]; + light.IntersectingMapId = mapId; - foreach (var gridId in _mapManager.FindGridIdsIntersecting(map, - MapTrees.LightAabbFunc(queuedUpdateLight), true)) + if (mapId == MapId.Nullspace) continue; + + var mapTree = _gridTrees[mapId]; + var aabb = MapTrees.LightAabbFunc(light); + var intersectingGrids = _mapManager.FindGridIdsIntersecting(mapId, aabb, true).ToList(); + + // Remove from old + foreach (var gridId in intersectingGrids) { - mapTree[gridId].LightTree.AddOrUpdate(queuedUpdateLight); + if (intersectingGrids.Contains(gridId)) continue; + mapTree[gridId].LightTree.Remove(light); } - queuedUpdateLight.TreeUpdateQueued = false; + // Rebuild in the update below + light.IntersectingGrids.Clear(); + + // Update / add to new + foreach (var gridId in intersectingGrids) + { + var translated = aabb.Translated(gridId == GridId.Invalid + ? Vector2.Zero + : -_mapManager.GetGrid(gridId).WorldPosition); + + mapTree[gridId].LightTree.AddOrUpdate(light, translated); + light.IntersectingGrids.Add(gridId); + } + + light.TreeUpdateQueued = false; } _spriteQueue.Clear(); @@ -284,9 +380,9 @@ namespace Robust.Client.GameObjects } } - internal struct RenderTreeRemoveSpriteMessage + internal class RenderTreeRemoveSpriteEvent : EntityEventArgs { - public RenderTreeRemoveSpriteMessage(SpriteComponent sprite, MapId map) + public RenderTreeRemoveSpriteEvent(SpriteComponent sprite, MapId map) { Sprite = sprite; Map = map; @@ -296,9 +392,9 @@ namespace Robust.Client.GameObjects public MapId Map { get; } } - internal struct RenderTreeRemoveLightMessage + internal class RenderTreeRemoveLightEvent : EntityEventArgs { - public RenderTreeRemoveLightMessage(PointLightComponent light, MapId map) + public RenderTreeRemoveLightEvent(PointLightComponent light, MapId map) { Light = light; Map = map; diff --git a/Robust.Client/GameObjects/EntitySystems/SpriteSystem.cs b/Robust.Client/GameObjects/EntitySystems/SpriteSystem.cs index a3c1708e78..828ce300ee 100644 --- a/Robust.Client/GameObjects/EntitySystems/SpriteSystem.cs +++ b/Robust.Client/GameObjects/EntitySystems/SpriteSystem.cs @@ -16,11 +16,17 @@ namespace Robust.Client.GameObjects [Dependency] private readonly IEyeManager _eyeManager = default!; [Dependency] private readonly IMapManager _mapManager = default!; + private RenderingTreeSystem _treeSystem = default!; + + public override void Initialize() + { + base.Initialize(); + _treeSystem = Get(); + } + /// public override void FrameUpdate(float frameTime) { - var renderTreeSystem = EntitySystemManager.GetEntitySystem(); - // So we could calculate the correct size of the entities based on the contents of their sprite... // Or we can just assume that no entity is larger than 10x10 and get a stupid easy check. var pvsBounds = _eyeManager.GetWorldViewport().Enlarged(5); @@ -33,18 +39,9 @@ namespace Robust.Client.GameObjects foreach (var gridId in _mapManager.FindGridIdsIntersecting(currentMap, pvsBounds, true)) { - Box2 gridBounds; + var gridBounds = gridId == GridId.Invalid ? pvsBounds : pvsBounds.Translated(-_mapManager.GetGrid(gridId).WorldPosition); - if (gridId == GridId.Invalid) - { - gridBounds = pvsBounds; - } - else - { - gridBounds = pvsBounds.Translated(-_mapManager.GetGrid(gridId).WorldPosition); - } - - var mapTree = renderTreeSystem.GetSpriteTreeForMap(currentMap, gridId); + var mapTree = _treeSystem.GetSpriteTreeForMap(currentMap, gridId); mapTree.QueryAabb(ref frameTime, (ref float state, in SpriteComponent value) => { diff --git a/Robust.Shared/GameObjects/EntitySystemMessages/EntMapIdChangedMessage.cs b/Robust.Shared/GameObjects/EntitySystemMessages/EntMapIdChangedMessage.cs index 0c8e07e443..57b231b053 100644 --- a/Robust.Shared/GameObjects/EntitySystemMessages/EntMapIdChangedMessage.cs +++ b/Robust.Shared/GameObjects/EntitySystemMessages/EntMapIdChangedMessage.cs @@ -2,7 +2,7 @@ using Robust.Shared.Map; namespace Robust.Shared.GameObjects { - public readonly struct EntMapIdChangedMessage + public class EntMapIdChangedMessage : EntityEventArgs { public EntMapIdChangedMessage(IEntity entity, MapId oldMapId) {