Mirror: Partial atmos refactor (#312)

## Mirror of PR #22521: [Partial atmos
refactor](space-wizards/space-station-14#22521)
from <img src="https://avatars.githubusercontent.com/u/10567778?v=4"
alt="space-wizards" width="22"/>
[space-wizards](https://github.com/space-wizards)/[space-station-14](https://github.com/space-wizards/space-station-14)

###### `18a35e7e83b2b71ee84b054d44d9ed5e595dd618`

PR opened by <img
src="https://avatars.githubusercontent.com/u/60421075?v=4"
width="16"/><a href="https://github.com/ElectroJr"> ElectroJr</a> at
2023-12-15 03:45:42 UTC

---

PR changed 43 files with 891 additions and 635 deletions.

The PR had the following labels:
- Status: Needs Review


---

<details open="true"><summary><h1>Original Body</h1></summary>

> This PR reworks how some parts of atmos code work. Originally it was
just meant to be a performance and bugfix PR, but it has ballooned in
scope. I'm not sure about some of my changes largely because I'm not
sure if some things were an oversight or an intentional decision for
some reason.
> 
> List of changes:
> - The `MolesArchived float[]` field is now read-only
> - It simply gets zeroed whenever the `GasMixture` is set to null
instead of constantly reallocating
> - Airtight query information is now cached in `TileAtmosphere`
> - This means that it should only iterate over anchored entities once
per update.
> - Previously an invalidated atmos tile would cause
`ProcessRevalidate()` to query airtight entities on the same tile six
times by calling a combination of `GridIsTileAirBlocked()`,
`NeedsVacuumFixing()`, and `GridIsTileAirBlocked()`. So this should help
significantly reduce component lookups & entity enumeration.
> - This does change some behaviour. In particular blocked directions
are now only updated if the tile was invalidated prior to the current
atmos-update, and will only ever be updated once per atmos-update.
> - AFAIK this only has an effect if the invalid tile processing is
deferred over multiple ticks, and I don't think it should cause any
issues?
> - Fixes a potential bug, where tiles might not dispose of their
excited group if their direction flags changed.
> - `MapAtmosphereComponent.Mixture` is now always immutable and no
longer nullable
> - I'm not sure why the mixture was nullable before? AFAICT the
component is meaningless if its null?
> - Space "gas" was always immutable, but there was nothing that
required planet atmospheres to be immutable. Requiring that it be
immutable gets rid of the constant gas mixture cloning.
> - I don't know if there was a reason for why they weren't immutable to
begin with.
> - Fixes lungs removing too much air from a gas mixture, resulting in
negative moles.
> - `GasMixture.Moles` is now `[Access]` restricted to the atmosphere
system.
> - This is to prevent people from improperly modifying the gas mixtures
(e.g., lungs), or accidentally modifying immutable mixtures.
> - Fixes an issue where non-grid atmosphere tiles would fail to update
their adjacent tiles, resulting in null reference exception spam
>   - Fixes #21732
>   - Fixes #21210 (probably) 
> - Disconnected atmosphere tiles, i.e., tiles that aren't on or
adjacent to a grid tile, will now get removed from the tile set.
Previously the tile set would just always increase, with tiles never
getting removed.
> - Removes various redundant component and tile-definition queries.
> - Removes some method events in favour of just using methods.
> - Map-exposded tiles now get updated when a map's atmosphere changes
(or the grid moves across maps).
> - Adds a `setmapatmos` command for adding map-wide atmospheres.
> - Fixed (non-planet) map atmospheres rendering over grids.
> 
> ## Media
> 
> This PR also includes changes to the atmos debug overlay, though I've
also split that off into a separate PR to make reviewing easier
(#22520).
> 
> Below is a video showing that atmos still seems to work, and that
trimming of disconnected tiles works:
> 
>
https://github.com/space-wizards/space-station-14/assets/60421075/4da46992-19e6-4354-8ecd-3cd67be4d0ed
> 
> For comparison, here is a video showing how current master works
(disconnected tiles never get removed):
> 
>
https://github.com/space-wizards/space-station-14/assets/60421075/54590777-e11c-41dc-b49d-fd7e53bfeed7
> 
> 🆑
> - fix: Fixed a bug where partially airtight entities (e.g., thin
windows or doors) could let air leak out into space.
> 


</details>

Co-authored-by: SimpleStation14 <Unknown>

Content.Client/Atmos/Overlays/GasTileOverlay.cs

@@ -8,7 +8,6 @@
 using Robust.Client.Graphics;
 using Robust.Client.ResourceManagement;
 using Robust.Shared.Enums;
-using Robust.Shared.Graphics;
 using Robust.Shared.Graphics.RSI;
 using Robust.Shared.Map;
 using Robust.Shared.Map.Components;
@@ -23,7 +22,7 @@ public sealed class GasTileOverlay : Overlay
         private readonly IEntityManager _entManager;
         private readonly IMapManager _mapManager;
 
-        public override OverlaySpace Space => OverlaySpace.WorldSpaceEntities;
+        public override OverlaySpace Space => OverlaySpace.WorldSpaceEntities | OverlaySpace.WorldSpaceBelowWorld;
         private readonly ShaderInstance _shader;
 
         // Gas overlays
@@ -79,7 +78,8 @@ public GasTileOverlay(GasTileOverlaySystem system, IEntityManager entManager, IR
                         var rsi = resourceCache.GetResource<RSIResource>(animated.RsiPath).RSI;
                         var stateId = animated.RsiState;
 
-                        if (!rsi.TryGetState(stateId, out var state)) continue;
+                        if (!rsi.TryGetState(stateId, out var state))
+                            continue;
 
                         _frames[i] = state.GetFrames(RsiDirection.South);
                         _frameDelays[i] = state.GetDelays();
@@ -111,7 +111,8 @@ protected override void FrameUpdate(FrameEventArgs args)
             for (var i = 0; i < _gasCount; i++)
             {
                 var delays = _frameDelays[i];
-                if (delays.Length == 0) continue;
+                if (delays.Length == 0)
+                    continue;
 
                 var frameCount = _frameCounter[i];
                 _timer[i] += args.DeltaSeconds;
@@ -127,7 +128,8 @@ protected override void FrameUpdate(FrameEventArgs args)
             for (var i = 0; i < FireStates; i++)
             {
                 var delays = _fireFrameDelays[i];
-                if (delays.Length == 0) continue;
+                if (delays.Length == 0)
+                    continue;
 
                 var frameCount = _fireFrameCounter[i];
                 _fireTimer[i] += args.DeltaSeconds;
@@ -161,26 +163,10 @@ protected override void Draw(in OverlayDrawArgs args)
             var mapUid = _mapManager.GetMapEntityId(args.MapId);
 
             if (_entManager.TryGetComponent<MapAtmosphereComponent>(mapUid, out var atmos))
-            {
-                var bottomLeft = args.WorldAABB.BottomLeft.Floored();
-                var topRight = args.WorldAABB.TopRight.Ceiled();
-
-                for (var x = bottomLeft.X; x <= topRight.X; x++)
-                {
-                    for (var y = bottomLeft.Y; y <= topRight.Y; y++)
-                    {
-                        var tilePosition = new Vector2(x, y);
-
-                        for (var i = 0; i < atmos.OverlayData.Opacity.Length; i++)
-                        {
-                            var opacity = atmos.OverlayData.Opacity[i];
+                DrawMapOverlay(drawHandle, args, mapUid, atmos);
 
-                            if (opacity > 0)
-                                args.WorldHandle.DrawTexture(_frames[i][_frameCounter[i]], tilePosition, Color.White.WithAlpha(opacity));
-                        }
-                    }
-                }
-            }
+            if (args.Space != OverlaySpace.WorldSpaceEntities)
+                return;
 
             // TODO: WorldBounds callback.
             _mapManager.FindGridsIntersecting(args.MapId, args.WorldAABB, ref gridState,
@@ -265,5 +251,41 @@ protected override void Draw(in OverlayDrawArgs args)
             drawHandle.UseShader(null);
             drawHandle.SetTransform(Matrix3.Identity);
         }
+
+        private void DrawMapOverlay(
+            DrawingHandleWorld handle,
+            OverlayDrawArgs args,
+            EntityUid map,
+            MapAtmosphereComponent atmos)
+        {
+            var mapGrid = _entManager.HasComponent<MapGridComponent>(map);
+
+            // map-grid atmospheres get drawn above grids
+            if (mapGrid && args.Space != OverlaySpace.WorldSpaceEntities)
+                return;
+
+            // Normal map atmospheres get drawn below grids
+            if (!mapGrid && args.Space != OverlaySpace.WorldSpaceBelowWorld)
+                return;
+
+            var bottomLeft = args.WorldAABB.BottomLeft.Floored();
+            var topRight = args.WorldAABB.TopRight.Ceiled();
+
+            for (var x = bottomLeft.X; x <= topRight.X; x++)
+            {
+                for (var y = bottomLeft.Y; y <= topRight.Y; y++)
+                {
+                    var tilePosition = new Vector2(x, y);
+
+                    for (var i = 0; i < atmos.OverlayData.Opacity.Length; i++)
+                    {
+                        var opacity = atmos.OverlayData.Opacity[i];
+
+                        if (opacity > 0)
+                            handle.DrawTexture(_frames[i][_frameCounter[i]], tilePosition, Color.White.WithAlpha(opacity));
+                    }
+                }
+            }
+        }
     }
 }

Content.Client/Mapping/MappingSystem.cs

@@ -83,7 +83,7 @@ private void OnFillActionSlot(FillActionSlotEvent ev)
             if (tileDef is not ContentTileDefinition contentTileDef)
                 return;
 
-            var tileIcon = contentTileDef.IsSpace
+            var tileIcon = contentTileDef.MapAtmosphere
                 ? _spaceIcon
                 : new Texture(contentTileDef.Sprite!.Value);
 

Content.IntegrationTests/Tests/Body/LungTest.cs

@@ -128,7 +128,7 @@ await server.WaitAssertion(() =>
                     metaSys.Update(1.0f);
                     metaSys.Update(1.0f);
                     respSys.Update(2.0f);
-                    Assert.That(GetMapMoles(), Is.EqualTo(startingMoles).Within(0.0001));
+                    Assert.That(GetMapMoles(), Is.EqualTo(startingMoles).Within(0.0002));
                 });
             }
 

Content.IntegrationTests/Tests/Interaction/InteractionTest.Helpers.cs

@@ -1006,15 +1006,10 @@ protected async Task AddAtmosphere(EntityUid? uid = null)
         await Server.WaitPost(() =>
         {
             var atmosSystem = SEntMan.System<AtmosphereSystem>();
-            var atmos = SEntMan.EnsureComponent<MapAtmosphereComponent>(target);
             var moles = new float[Atmospherics.AdjustedNumberOfGases];
             moles[(int) Gas.Oxygen] = 21.824779f;
             moles[(int) Gas.Nitrogen] = 82.10312f;
-            atmosSystem.SetMapAtmosphere(target, false, new GasMixture(2500)
-            {
-                Temperature = 293.15f,
-                Moles = moles,
-            }, atmos);
+            atmosSystem.SetMapAtmosphere(target, false, new GasMixture(moles, Atmospherics.T20C));
         });
     }
 

Content.Server/Atmos/Commands/SetMapAtmosCommand.cs

@@ -0,0 +1,94 @@
+using Content.Server.Administration;
+using Content.Server.Atmos.Components;
+using Content.Server.Atmos.EntitySystems;
+using Content.Shared.Administration;
+using Content.Shared.Atmos;
+using Robust.Shared.Console;
+using Robust.Shared.Map;
+
+namespace Content.Server.Atmos.Commands;
+
+[AdminCommand(AdminFlags.Admin)]
+public sealed class AddMapAtmosCommand : LocalizedCommands
+{
+    [Dependency] private readonly IEntityManager _entities = default!;
+    [Dependency] private readonly IMapManager _map = default!;
+
+    private const string _cmd = "cmd-set-map-atmos";
+    public override string Command => "setmapatmos";
+    public override string Description => Loc.GetString($"{_cmd}-desc");
+    public override string Help => Loc.GetString($"{_cmd}-help");
+
+    public override void Execute(IConsoleShell shell, string argStr, string[] args)
+    {
+        if (args.Length < 2)
+        {
+            shell.WriteLine(Help);
+            return;
+        }
+
+        int.TryParse(args[0], out var id);
+        var map = _map.GetMapEntityId(new MapId(id));
+        if (!map.IsValid())
+        {
+            shell.WriteError(Loc.GetString("cmd-parse-failure-mapid",  ("arg", args[0])));
+            return;
+        }
+
+        if (!bool.TryParse(args[1], out var space))
+        {
+            shell.WriteError(Loc.GetString("cmd-parse-failure-bool", ("arg", args[1])));
+            return;
+        }
+
+        if (space || args.Length < 4)
+        {
+            _entities.RemoveComponent<MapAtmosphereComponent>(map);
+            shell.WriteLine(Loc.GetString($"{_cmd}-removed", ("map", id)));
+            return;
+        }
+
+        if (!float.TryParse(args[2], out var temp))
+        {
+            shell.WriteError(Loc.GetString("cmd-parse-failure-float", ("arg", args[2])));
+            return;
+        }
+
+        var mix = new GasMixture(Atmospherics.CellVolume) {Temperature = Math.Min(temp, Atmospherics.TCMB)};
+        for (var i = 0; i < Atmospherics.TotalNumberOfGases; i++)
+        {
+            if (args.Length == 3 + i)
+                break;
+
+            if (!float.TryParse(args[3+i], out var moles))
+            {
+                shell.WriteError(Loc.GetString("cmd-parse-failure-float", ("arg", args[3+i])));
+                return;
+            }
+
+            mix.AdjustMoles(i, moles);
+        }
+
+        var atmos = _entities.EntitySysManager.GetEntitySystem<AtmosphereSystem>();
+        atmos.SetMapAtmosphere(map, space, mix);
+        shell.WriteLine(Loc.GetString($"{_cmd}-updated", ("map", id)));
+    }
+
+    public override CompletionResult GetCompletion(IConsoleShell shell, string[] args)
+    {
+        if (args.Length == 1)
+            return CompletionResult.FromHintOptions(CompletionHelper.MapIds(_entities), Loc.GetString($"{_cmd}-hint-map"));
+
+        if (args.Length == 2)
+            return CompletionResult.FromHintOptions(new[]{ "false", "true"}, Loc.GetString($"{_cmd}-hint-space"));
+
+        if (!bool.TryParse(args[1], out var space) || space)
+            return CompletionResult.Empty;
+
+        if (args.Length == 3)
+            return CompletionResult.FromHint(Loc.GetString($"{_cmd}-hint-temp"));
+
+        var gas = (Gas) args.Length - 4;
+        return CompletionResult.FromHint(Loc.GetString($"{_cmd}-hint-gas" , ("gas", gas.ToString())));
+    }
+}

Content.Server/Atmos/Components/AirtightComponent.cs

@@ -1,9 +1,10 @@
+using Content.Server.Atmos.EntitySystems;
 using Content.Shared.Atmos;
 using Robust.Shared.Serialization.TypeSerializers.Implementations.Custom;
 
 namespace Content.Server.Atmos.Components
 {
-    [RegisterComponent]
+    [RegisterComponent, Access(typeof(AirtightSystem))]
     public sealed partial class AirtightComponent : Component
     {
         public (EntityUid Grid, Vector2i Tile) LastPosition { get; set; }
@@ -29,6 +30,7 @@ public sealed partial class AirtightComponent : Component
         [DataField("noAirWhenFullyAirBlocked")]
         public bool NoAirWhenFullyAirBlocked { get; set; } = true;
 
+        [Access(Other = AccessPermissions.ReadWriteExecute)]
         public AtmosDirection AirBlockedDirection => (AtmosDirection)CurrentAirBlockedDirection;
     }
 }

Content.Server/Atmos/Components/GridAtmosphereComponent.cs

@@ -28,6 +28,9 @@ public sealed partial class GridAtmosphereComponent : Component
         [IncludeDataField(customTypeSerializer:typeof(TileAtmosCollectionSerializer))]
         public Dictionary<Vector2i, TileAtmosphere> Tiles = new(1000);
 
+        [ViewVariables]
+        public HashSet<TileAtmosphere> MapTiles = new(1000);
+
         [ViewVariables]
         public readonly HashSet<TileAtmosphere> ActiveTiles = new(1000);
 
@@ -80,7 +83,10 @@ public sealed partial class GridAtmosphereComponent : Component
         public readonly HashSet<Vector2i> InvalidatedCoords = new(1000);
 
         [ViewVariables]
-        public readonly Queue<Vector2i> CurrentRunInvalidatedCoordinates = new();
+        public readonly Queue<TileAtmosphere> CurrentRunInvalidatedTiles = new();
+
+        [ViewVariables]
+        public readonly List<TileAtmosphere> PossiblyDisconnectedTiles = new(100);
 
         [ViewVariables]
         public int InvalidatedCoordsCount => InvalidatedCoords.Count;

Content.Server/Atmos/Components/MapAtmosphereComponent.cs

@@ -12,12 +12,14 @@ public sealed partial class MapAtmosphereComponent : SharedMapAtmosphereComponen
     /// <summary>
     ///     The default GasMixture a map will have. Space mixture by default.
     /// </summary>
-    [DataField("mixture"), ViewVariables(VVAccess.ReadWrite)]
-    public GasMixture? Mixture = GasMixture.SpaceGas;
+    [DataField, ViewVariables(VVAccess.ReadWrite)]
+    public GasMixture Mixture = GasMixture.SpaceGas;
 
     /// <summary>
     ///     Whether empty tiles will be considered space or not.
     /// </summary>
-    [DataField("space"), ViewVariables(VVAccess.ReadWrite)]
+    [DataField, ViewVariables(VVAccess.ReadWrite)]
     public bool Space = true;
+
+    public SharedGasTileOverlaySystem.GasOverlayData Overlay;
 }

Content.Server/Atmos/EntitySystems/AirtightSystem.cs

@@ -2,15 +2,14 @@
 using Content.Server.Explosion.EntitySystems;
 using Content.Shared.Atmos;
 using JetBrains.Annotations;
-using Robust.Shared.Map;
 using Robust.Shared.Map.Components;
 
 namespace Content.Server.Atmos.EntitySystems
 {
     [UsedImplicitly]
     public sealed class AirtightSystem : EntitySystem
     {
-        [Dependency] private readonly IMapManager _mapManager = default!;
+        [Dependency] private readonly SharedTransformSystem _transform = default!;
         [Dependency] private readonly AtmosphereSystem _atmosphereSystem = default!;
         [Dependency] private readonly ExplosionSystem _explosionSystem = default!;
 
@@ -121,19 +120,16 @@ public void UpdatePosition(Entity<AirtightComponent> ent, TransformComponent? xf
             if (!xform.Anchored || !TryComp(xform.GridUid, out MapGridComponent? grid))
                 return;
 
-            airtight.LastPosition = (xform.GridUid.Value, grid.TileIndicesFor(xform.Coordinates));
-            InvalidatePosition(airtight.LastPosition.Item1, airtight.LastPosition.Item2, airtight.FixVacuum && !airtight.AirBlocked);
+            var indices = _transform.GetGridTilePositionOrDefault((ent, xform), grid);
+            airtight.LastPosition = (xform.GridUid.Value, indices);
+            InvalidatePosition((xform.GridUid.Value, grid), indices);
         }
 
-        public void InvalidatePosition(EntityUid gridId, Vector2i pos, bool fixVacuum = false)
+        public void InvalidatePosition(Entity<MapGridComponent?> grid, Vector2i pos)
         {
-            if (!TryComp(gridId, out MapGridComponent? grid))
-                return;
-
             var query = EntityManager.GetEntityQuery<AirtightComponent>();
-            _explosionSystem.UpdateAirtightMap(gridId, pos, grid, query);
-            // TODO make atmos system use query
-            _atmosphereSystem.InvalidateTile(gridId, pos);
+            _explosionSystem.UpdateAirtightMap(grid, pos, grid, query);
+            _atmosphereSystem.InvalidateTile(grid.Owner, pos);
         }
 
         private AtmosDirection Rotate(AtmosDirection myDirection, Angle myAngle)

Content.Server/Atmos/EntitySystems/AtmosDebugOverlaySystem.cs

@@ -97,22 +97,19 @@ private void OnPlayerStatusChanged(object? sender, SessionStatusEventArgs e)
             }
         }
 
-        private AtmosDebugOverlayData ConvertTileToData(TileAtmosphere? tile)
+        private AtmosDebugOverlayData? ConvertTileToData(TileAtmosphere tile)
         {
-            if (tile == null)
-                return default;
-
             return new AtmosDebugOverlayData(
                 tile.GridIndices,
                 tile.Air?.Temperature ?? default,
                 tile.Air?.Moles,
                 tile.PressureDirection,
                 tile.LastPressureDirection,
-                tile.BlockedAirflow,
+                tile.AirtightData.BlockedDirections,
                 tile.ExcitedGroup?.GetHashCode(),
                 tile.Space,
-                false,
-                false);
+                tile.MapAtmosphere,
+                tile.NoGridTile);
         }
 
         public override void Update(float frameTime)