Fix dev crash when alt+clicking portals (#37540)

* Ghost portal dev crash

* ent<T>

* better comments

* refuse to reuse

* touchup comments

---------

Co-authored-by: iaada <iaada@users.noreply.github.com>

Content.Shared/Teleportation/Systems/SharedPortalSystem.cs

@@ -1,4 +1,4 @@
-using System.Linq;
+using System.Linq;
 using Content.Shared.Ghost;
 using Content.Shared.Movement.Pulling.Components;
 using Content.Shared.Movement.Pulling.Systems;
@@ -6,7 +6,6 @@ using Content.Shared.Popups;
 using Content.Shared.Projectiles;
 using Content.Shared.Teleportation.Components;
 using Content.Shared.Verbs;
-using Robust.Shared.Audio;
 using Robust.Shared.Audio.Systems;
 using Robust.Shared.Map;
 using Robust.Shared.Network;
@@ -19,8 +18,10 @@ using Robust.Shared.Utility;
 namespace Content.Shared.Teleportation.Systems;
 
 /// <summary>
-/// This handles teleporting entities through portals, and creating new linked portals.
+/// This handles teleporting entities from a portal to a linked portal, or to a random nearby destination.
+/// Uses <see cref="LinkedEntitySystem"/> to get linked portals.
 /// </summary>
+/// <seealso cref="PortalComponent"/>
 public abstract class SharedPortalSystem : EntitySystem
 {
     [Dependency] private readonly IRobustRandom _random = default!;
@@ -39,12 +40,13 @@ public abstract class SharedPortalSystem : EntitySystem
     /// <inheritdoc/>
     public override void Initialize()
     {
+        SubscribeLocalEvent<PortalComponent, GetVerbsEvent<AlternativeVerb>>(OnGetVerbs);
+
         SubscribeLocalEvent<PortalComponent, StartCollideEvent>(OnCollide);
         SubscribeLocalEvent<PortalComponent, EndCollideEvent>(OnEndCollide);
-        SubscribeLocalEvent<PortalComponent, GetVerbsEvent<AlternativeVerb>>(OnGetVerbs);
     }
 
-    private void OnGetVerbs(EntityUid uid, PortalComponent component, GetVerbsEvent<AlternativeVerb> args)
+    private void OnGetVerbs(Entity<PortalComponent> ent, ref GetVerbsEvent<AlternativeVerb> args)
     {
         // Traversal altverb for ghosts to use that bypasses normal functionality
         if (!args.CanAccess || !HasComp<GhostComponent>(args.User))
@@ -52,8 +54,9 @@ public abstract class SharedPortalSystem : EntitySystem
 
         // Don't use the verb with unlinked or with multi-output portals
         // (this is only intended to be useful for ghosts to see where a linked portal leads)
-        var disabled = !TryComp<LinkedEntityComponent>(uid, out var link) || link.LinkedEntities.Count != 1;
+        var disabled = !TryComp<LinkedEntityComponent>(ent, out var link) || link.LinkedEntities.Count != 1;
 
+        var subject = args.User;
         args.Verbs.Add(new AlternativeVerb
         {
             Priority = 11,
@@ -62,8 +65,13 @@ public abstract class SharedPortalSystem : EntitySystem
                 if (link == null || disabled)
                     return;
 
-                var ent = link.LinkedEntities.First();
+                // check prediction
-                TeleportEntity(uid, args.User, Transform(ent).Coordinates, ent, false);
+                if (_netMan.IsClient && !CanPredictTeleport((ent, link)))
+                    return;
+
+                var destination = link.LinkedEntities.First();
+
+                TeleportEntity(ent, subject, Transform(destination).Coordinates, destination, false);
             },
             Disabled = disabled,
             Text = Loc.GetString("portal-component-ghost-traverse"),
@@ -74,14 +82,7 @@ public abstract class SharedPortalSystem : EntitySystem
         });
     }
 
-    private bool ShouldCollide(string ourId, string otherId, Fixture our, Fixture other)
+    private void OnCollide(Entity<PortalComponent> ent, ref StartCollideEvent args)
-    {
-        // most non-hard fixtures shouldn't pass through portals, but projectiles are non-hard as well
-        // and they should still pass through
-        return ourId == PortalFixture && (other.Hard || otherId == ProjectileFixture);
-    }
-
-    private void OnCollide(EntityUid uid, PortalComponent component, ref StartCollideEvent args)
     {
         if (!ShouldCollide(args.OurFixtureId, args.OtherFixtureId, args.OurFixture, args.OtherFixture))
             return;
@@ -92,7 +93,7 @@ public abstract class SharedPortalSystem : EntitySystem
         if (Transform(subject).Anchored)
             return;
 
-        // break pulls before portal enter so we dont break shit
+        // break pulls before portal enter so we don't break shit
         if (TryComp<PullableComponent>(subject, out var pullable) && pullable.BeingPulled)
         {
             _pulling.TryStopPull(subject, pullable);
@@ -110,33 +111,27 @@ public abstract class SharedPortalSystem : EntitySystem
             return;
         }
 
-        if (TryComp<LinkedEntityComponent>(uid, out var link))
+        if (TryComp<LinkedEntityComponent>(ent, out var link))
         {
             if (link.LinkedEntities.Count == 0)
                 return;
 
-            // client can't predict outside of simple portal-to-portal interactions due to randomness involved
+            // check prediction
-            // --also can't predict if the target doesn't exist on the client / is outside of PVS
+            if (_netMan.IsClient && !CanPredictTeleport((ent, link)))
-            if (_netMan.IsClient)
+                return;
-            {
-                var first = link.LinkedEntities.First();
-                var exists = Exists(first);
-                if (link.LinkedEntities.Count != 1 || !exists || (exists && Transform(first).MapID == MapId.Nullspace))
-                    return;
-            }
 
             // pick a target and teleport there
             var target = _random.Pick(link.LinkedEntities);
 
             if (HasComp<PortalComponent>(target))
             {
-                // if target is a portal, signal that they shouldn't be immediately portaled back
+                // if target is a portal, signal that they shouldn't be immediately teleported back
                 var timeout = EnsureComp<PortalTimeoutComponent>(subject);
-                timeout.EnteredPortal = uid;
+                timeout.EnteredPortal = ent;
                 Dirty(subject, timeout);
             }
 
-            TeleportEntity(uid, subject, Transform(target).Coordinates, target);
+            TeleportEntity(ent, subject, Transform(target).Coordinates, target);
             return;
         }
 
@@ -144,49 +139,86 @@ public abstract class SharedPortalSystem : EntitySystem
             return;
 
         // no linked entity--teleport randomly
-        if (component.RandomTeleport)
+        if (ent.Comp.RandomTeleport)
-            TeleportRandomly(uid, subject, component);
+            TeleportRandomly(ent, subject);
     }
 
-    private void OnEndCollide(EntityUid uid, PortalComponent component, ref EndCollideEvent args)
+    private void OnEndCollide(Entity<PortalComponent> ent, ref EndCollideEvent args)
     {
         if (!ShouldCollide(args.OurFixtureId, args.OtherFixtureId, args.OurFixture, args.OtherFixture))
             return;
 
         var subject = args.OtherEntity;
 
-        // if they came from (not us), remove the timeout
+        // if they came from a different portal, remove the timeout
-        if (TryComp<PortalTimeoutComponent>(subject, out var timeout) && timeout.EnteredPortal != uid)
+        if (TryComp<PortalTimeoutComponent>(subject, out var timeout) && timeout.EnteredPortal != ent)
         {
             RemCompDeferred<PortalTimeoutComponent>(subject);
         }
     }
 
-    private void TeleportEntity(EntityUid portal, EntityUid subject, EntityCoordinates target, EntityUid? targetEntity = null, bool playSound = true,
+    /// <summary>
-        PortalComponent? portalComponent = null)
+    /// Checks if the colliding fixtures are the ones we want.
+    /// </summary>
+    /// <returns>
+    /// False if our fixture is not a portal fixture.
+    /// False if other fixture is not hard, but makes an exception for projectiles.
+    /// </returns>
+    private bool ShouldCollide(string ourId, string otherId, Fixture our, Fixture other)
     {
-        if (!Resolve(portal, ref portalComponent))
+        return ourId == PortalFixture && (other.Hard || otherId == ProjectileFixture);
-            return;
+    }
 
-        var ourCoords = Transform(portal).Coordinates;
+    /// <summary>
+    /// Checks if the client is able to predict the teleport.
+    /// Client can't predict outside 1-to-1 portal-to-portal interactions due to randomness involved.
+    /// </summary>
+    /// <returns>
+    /// False if the linked entity count isn't 1.
+    /// False if the linked entity doesn't exist on the client / is outside PVS.
+    /// </returns>
+    private bool CanPredictTeleport(Entity<LinkedEntityComponent> portal)
+    {
+        var first = portal.Comp.LinkedEntities.First();
+        var exists = Exists(first);
+
+        if (!exists ||
+            portal.Comp.LinkedEntities.Count != 1 || // 0 and >1 use RNG
+            exists && Transform(first).MapID == MapId.Nullspace) // The linked entity is most likely outside PVS
+            return false;
+
+        return true;
+    }
+
+    /// <summary>
+    /// Handles teleporting a subject from the portal entity to a coordinate.
+    /// Also deletes invalid portals.
+    /// </summary>
+    /// <param name="ent"> The portal being collided with. </param>
+    /// <param name="subject"> The entity getting teleported. </param>
+    /// <param name="target"> The location to teleport to. </param>
+    /// <param name="targetEntity"> The portal on the other side of the teleport. </param>
+    private void TeleportEntity(Entity<PortalComponent> ent, EntityUid subject, EntityCoordinates target, EntityUid? targetEntity = null, bool playSound = true)
+    {
+        var ourCoords = Transform(ent).Coordinates;
         var onSameMap = _transform.GetMapId(ourCoords) == _transform.GetMapId(target);
-        var distanceInvalid = portalComponent.MaxTeleportRadius != null
+        var distanceInvalid = ent.Comp.MaxTeleportRadius != null
                               && ourCoords.TryDistance(EntityManager, target, out var distance)
-                              && distance > portalComponent.MaxTeleportRadius;
+                              && distance > ent.Comp.MaxTeleportRadius;
 
-        if (!onSameMap && !portalComponent.CanTeleportToOtherMaps || distanceInvalid)
+        // Early out if this is an invalid configuration
+        if (!onSameMap && !ent.Comp.CanTeleportToOtherMaps || distanceInvalid)
         {
-            if (!_netMan.IsServer)
+            if (_netMan.IsClient)
                 return;
 
-            // Early out if this is an invalid configuration
             _popup.PopupCoordinates(Loc.GetString("portal-component-invalid-configuration-fizzle"),
                 ourCoords, Filter.Pvs(ourCoords, entityMan: EntityManager), true);
 
             _popup.PopupCoordinates(Loc.GetString("portal-component-invalid-configuration-fizzle"),
                 target, Filter.Pvs(target, entityMan: EntityManager), true);
 
-            QueueDel(portal);
+            QueueDel(ent);
 
             if (targetEntity != null)
                 QueueDel(targetEntity.Value);
@@ -194,8 +226,8 @@ public abstract class SharedPortalSystem : EntitySystem
             return;
         }
 
-        var arrivalSound = CompOrNull<PortalComponent>(targetEntity)?.ArrivalSound ?? portalComponent.ArrivalSound;
+        var arrivalSound = CompOrNull<PortalComponent>(targetEntity)?.ArrivalSound ?? ent.Comp.ArrivalSound;
-        var departureSound = portalComponent.DepartureSound;
+        var departureSound = ent.Comp.DepartureSound;
 
         // Some special cased stuff: projectiles should stop ignoring shooter when they enter a portal, to avoid
         // stacking 500 bullets in between 2 portals and instakilling people--you'll just hit yourself instead
@@ -205,36 +237,41 @@ public abstract class SharedPortalSystem : EntitySystem
             projectile.IgnoreShooter = false;
         }
 
-        LogTeleport(portal, subject, Transform(subject).Coordinates, target);
+        LogTeleport(ent, subject, Transform(subject).Coordinates, target);
 
         _transform.SetCoordinates(subject, target);
 
         if (!playSound)
             return;
 
-        _audio.PlayPredicted(departureSound, portal, subject);
+        _audio.PlayPredicted(departureSound, ent, subject);
         _audio.PlayPredicted(arrivalSound, subject, subject);
     }
 
-    private void TeleportRandomly(EntityUid portal, EntityUid subject, PortalComponent? component = null)
+    /// <summary>
+    /// Finds a random coordinate within the portal's radius and teleports the subject there.
+    /// Attempts to not put the subject inside a static entity (e.g. wall).
+    /// </summary>
+    /// <param name="ent"> The portal being collided with. </param>
+    /// <param name="subject"> The entity getting teleported. </param>
+    private void TeleportRandomly(Entity<PortalComponent> ent, EntityUid subject)
     {
-        if (!Resolve(portal, ref component))
+        var xform = Transform(ent);
-            return;
-
-        var xform = Transform(portal);
         var coords = xform.Coordinates;
-        var newCoords = coords.Offset(_random.NextVector2(component.MaxRandomRadius));
+        var newCoords = coords.Offset(_random.NextVector2(ent.Comp.MaxRandomRadius));
         for (var i = 0; i < MaxRandomTeleportAttempts; i++)
         {
-            var randVector = _random.NextVector2(component.MaxRandomRadius);
+            var randVector = _random.NextVector2(ent.Comp.MaxRandomRadius);
             newCoords = coords.Offset(randVector);
             if (!_lookup.AnyEntitiesIntersecting(_transform.ToMapCoordinates(newCoords), LookupFlags.Static))
             {
+                // newCoords is not a wall
                 break;
             }
+            // after "MaxRandomTeleportAttempts" attempts, end up in the walls
         }
 
-        TeleportEntity(portal, subject, newCoords);
+        TeleportEntity(ent, subject, newCoords);
     }
 
     protected virtual void LogTeleport(EntityUid portal, EntityUid subject, EntityCoordinates source,