Refactor grid or map methods (space-wizards#5124)

* Rename TryGetMapOrGridCoordinates to make it clearer it gets grid first

* Add terminating or deleted checks to TryGetGridOrMapCoordinates

* Add comment to check if TerminatingOrDeleted check is necessary

* Reorganize AttachToGridOrMap to match TryGetGridOrMapCoordinates

* Move validation to method

* Replace internals with TryGetGridOrMapCoordinates

* Explicitly set coordinates type

* Format

* Change name back for now

* Don't duplicate `TerminatingOrDeleted()` check

* Don't call `GetInvWorldMatrix` for the map

* Don't check `TerminatingOrDeleted(uid)` in `TryGetMapOrGridCoordinates()`

* Fix parenting to terminating grid

* Fix matrix error

---------

Co-authored-by: ElectroJr <leonsfriedrich@gmail.com>
Co-authored-by: metalgearsloth <31366439+metalgearsloth@users.noreply.github.com>
Co-authored-by: PJB3005 <pieterjan.briers+git@gmail.com>

Author: 4 people

Robust.Shared/GameObjects/Systems/SharedGridTraversalSystem.cs

@@ -100,7 +100,7 @@ public void CheckTraversal(EntityUid entity, TransformComponent xform, EntityUid
         if (_mapManager.TryFindGridAt(map, mapPos, out var gridUid, out _))
         {
             // Some minor duplication here with AttachParent but only happens when going on/off grid so not a big deal ATM.
-            if (gridUid != xform.GridUid)
+            if (gridUid != xform.GridUid && !TerminatingOrDeleted(gridUid))
                 _transform.SetParent(entity, xform, gridUid);
             return;
         }
@@ -110,4 +110,3 @@ public void CheckTraversal(EntityUid entity, TransformComponent xform, EntityUid
             _transform.SetParent(entity, xform, map);
     }
 }
-

Robust.Shared/GameObjects/Systems/SharedTransformSystem.Component.cs

@@ -1413,27 +1413,21 @@ public Matrix3x2 GetInvWorldMatrix(TransformComponent component, EntityQuery<Tra
     /// </summary>
     public void AttachToGridOrMap(EntityUid uid, TransformComponent? xform = null)
     {
+        // TODO make this log an error?
+        // SetCoordinates already does this when trying to move entities mid-deletion.
         if (TerminatingOrDeleted(uid))
             return;
 
-        if (!XformQuery.Resolve(uid, ref xform))
+        if (!XformQuery.Resolve(uid, ref xform, false))
             return;
 
-        if (!xform.ParentUid.IsValid() || xform.ParentUid == xform.GridUid)
+        if (!xform.ParentUid.IsValid())
             return;
 
-        EntityUid newParent;
-        var oldPos = GetWorldPosition(xform);
-        if (_mapManager.TryFindGridAt(xform.MapID, oldPos, out var gridUid, out _)
-            && !TerminatingOrDeleted(gridUid))
-        {
-            newParent = gridUid;
-        }
-        else if (_map.TryGetMap(xform.MapID, out var mapEnt) && !TerminatingOrDeleted(mapEnt))
-        {
-            newParent = mapEnt.Value;
-        }
-        else
+        if (xform.ParentUid == xform.GridUid)
+            return;
+
+        if (!TryGetMapOrGridCoordinates(uid, out var coordinates, xform))
         {
             if (!_mapQuery.HasComp(uid))
                 Log.Warning($"Failed to attach entity to map or grid. Entity: ({ToPrettyString(uid)}). Trace: {Environment.StackTrace}");
@@ -1442,32 +1436,50 @@ public void AttachToGridOrMap(EntityUid uid, TransformComponent? xform = null)
             return;
         }
 
-        if (newParent == xform.ParentUid || newParent == uid)
+        if (coordinates.Value.EntityId == xform.ParentUid || coordinates.Value.EntityId == uid)
             return;
 
-        var newPos = Vector2.Transform(oldPos, GetInvWorldMatrix(newParent));
-        SetCoordinates(uid, xform, new(newParent, newPos));
+        SetCoordinates(uid, xform, coordinates.Value);
     }
 
-    public bool TryGetMapOrGridCoordinates(EntityUid uid, [NotNullWhen(true)] out EntityCoordinates? coordinates, TransformComponent? xform = null)
+    /// <summary>
+    /// Attempts to get the current coordinates of the provided entity, relative to either a grid currently at that location, or the map.
+    /// </summary>
+    /// <remarks>
+    /// This will not return coordinates if the map or grid are currently being deleted.
+    /// </remarks>
+    /// <param name="uid">The entity to get the coordinates of.</param>
+    /// <param name="coordinates">The returned coordinates on the grid or map.</param>
+    /// <param name="xform">The transform component of the <paramref name="uid"/>.</param>
+    /// <returns>Whether the grid or map coordinates were found and <paramref name="coordinates"/> was set.</returns>
+    public bool TryGetMapOrGridCoordinates(
+        EntityUid uid,
+        [NotNullWhen(true)] out EntityCoordinates? coordinates,
+        TransformComponent? xform = null)
     {
         coordinates = null;
 
-        if (!XformQuery.Resolve(uid, ref xform))
+        if (!XformQuery.Resolve(uid, ref xform, false))
             return false;
 
         if (!xform.ParentUid.IsValid())
             return false;
 
-        if (xform.MapUid is not { } map)
+        if (xform.MapUid is not { } map || TerminatingOrDeleted(map))
             return false;
 
-        var newParent = map;
         var oldPos = GetWorldPosition(xform);
-        if (_mapManager.TryFindGridAt(map, oldPos, out var gridUid, out _))
-            newParent = gridUid;
+        if (_mapManager.TryFindGridAt(map, oldPos, out var gridUid, out _) && !TerminatingOrDeleted(gridUid))
+        {
+            coordinates = gridUid == xform.ParentUid
+                ? new EntityCoordinates(gridUid, xform.LocalPosition)
+                : new EntityCoordinates(gridUid, Vector2.Transform(oldPos, GetInvWorldMatrix(gridUid)));
+        }
+        else
+        {
+            coordinates = new EntityCoordinates(map, oldPos);
+        }
 
-        coordinates = new(newParent, Vector2.Transform(oldPos, GetInvWorldMatrix(newParent)));
         return true;
     }
     #endregion