comments and cleanup

Content.Server/_NF/PublicTransit/Components/TransitShuttleComponent.cs

@@ -2,6 +2,11 @@
 
 namespace Content.Server._NF.PublicTransit.Components;
 
+/// <summary>
+/// Added to a grid to have it act as an automated public transit bus.
+/// Public Transit system will add this procedurally to any grid designated as a 'bus' through the CVAR
+/// Mappers may add it to their shuttle if they wish, but this is going to force it's use and function as a public transit bus
+/// </summary>
 [RegisterComponent, Access(typeof(PublicTransitSystem))]
 public sealed partial class TransitShuttleComponent : Component
 {

Content.Server/_NF/PublicTransit/PublicTransitSystem.cs

@@ -1,5 +1,3 @@
-
-using Content.Server.GameTicking.Events;
 using Content.Server._NF.PublicTransit.Components;
 using Content.Server.GameTicking;
 using Content.Server.Shuttles.Components;
@@ -17,7 +15,7 @@
 namespace Content.Server._NF.PublicTransit;
 
 /// <summary>
-/// If enabled spawns players on a separate arrivals station before they can transfer to the main station.
+/// If enabled, spawns a public trasnport grid as definied by cvar, to act as an automatic transit shuttle between designated grids
 /// </summary>
 public sealed class PublicTransitSystem : EntitySystem
 {
@@ -29,7 +27,7 @@ public sealed class PublicTransitSystem : EntitySystem
     [Dependency] private readonly ShuttleSystem _shuttles = default!;
 
     /// <summary>
-    /// If enabled then spawns players on an alternate map so they can take a shuttle to the station.
+    /// If enabled then spawns the bus and sets up the bus line.
     /// </summary>
     public bool Enabled { get; private set; }
     public float FlyTime = 50f;
@@ -42,12 +40,10 @@ public override void Initialize()
 
         SubscribeLocalEvent<StationTransitComponent, ComponentStartup>(OnStationStartup);
         SubscribeLocalEvent<StationTransitComponent, ComponentShutdown>(OnStationShutdown);
-
         SubscribeLocalEvent<TransitShuttleComponent, ComponentStartup>(OnShuttleStartup);
         SubscribeLocalEvent<TransitShuttleComponent, EntityUnpausedEvent>(OnShuttleUnpaused);
         SubscribeLocalEvent<TransitShuttleComponent, FTLTagEvent>(OnShuttleTag);
 
-        // Don't invoke immediately as it will get set in the natural course of things.
         Enabled = _cfgManager.GetCVar(NF14CVars.PublicTransit);
         FlyTime = _cfgManager.GetCVar(NF14CVars.PublicTransitFlyTime);
         Counter = 0;
@@ -56,50 +52,75 @@ public override void Initialize()
         _cfgManager.OnValueChanged(NF14CVars.PublicTransitFlyTime, SetFly);
     }
 
+    /// <summary>
+    /// Hardcoded snippit to intercept FTL events. It catches the transit shuttle and ensures its looking for the "DockTransit" priority dock.
+    /// </summary>
     private void OnShuttleTag(EntityUid uid, TransitShuttleComponent component, ref FTLTagEvent args)
     {
         if (args.Handled)
             return;
 
-        // Just saves mappers forgetting.
+        // Just saves mappers forgetting, or ensuring that a non-standard grid forced to be a bus will prioritize the "DockTransit" tagged docks
         args.Handled = true;
         args.Tag = "DockTransit";
     }
 
     public override void Shutdown()
     {
         base.Shutdown();
+        _cfgManager.UnsubValueChanged(NF14CVars.PublicTransitFlyTime, SetFly);
         _cfgManager.UnsubValueChanged(NF14CVars.PublicTransit, SetTransit);
     }
 
+    /// <summary>
+    /// Checks to make sure the grid is on the appropriate playfield, i.e., not in mapping space being worked on.
+    /// If so, adds the grid to the list of bus stops, but only if its not already there
+    /// </summary>
     private void OnStationStartup(EntityUid uid, StationTransitComponent component, ComponentStartup args)
     {
-        if (Transform(uid).MapID == _ticker.DefaultMap)
+        if (Transform(uid).MapID == _ticker.DefaultMap) //best solution i could find because of componentinit/mapinit race conditions
         {
-            if (!StationList.Contains(uid))
-                StationList.Add(uid);
-            if (Enabled)
+            if (!StationList.Contains(uid)) //if the grid isnt already in
+                StationList.Add(uid); //add it to the list
+            if (Enabled) //and just in case this has been added dynamically mid-round, lets do a setup check
                 SetupPublicTransit();
         }
     }
 
+    /// <summary>
+    /// When a bus stop gets deleted in-game, we need to remove it from the list of bus stops, or else we get FTL problems
+    /// </summary>
     private void OnStationShutdown(EntityUid uid, StationTransitComponent component, ComponentShutdown args)
     {
         if (StationList.Contains(uid))
             StationList.Remove(uid);
     }
 
+    /// <summary>
+    /// Again, this can and likely should be instructed to mappers to do, but just in case it was either forgotten or we are doing admemes,
+    /// we make sure that the bus is (mostly) griefer protected and that it cant be hijacked
+    /// </summary>
     private void OnShuttleStartup(EntityUid uid, TransitShuttleComponent component, ComponentStartup args)
     {
         EnsureComp<PreventPilotComponent>(uid);
         EnsureComp<ProtectedGridComponent>(uid);
     }
 
+    /// <summary>
+    /// ensuring that pausing the shuttle for any reason doesnt mess up our timing
+    /// </summary>
     private void OnShuttleUnpaused(EntityUid uid, TransitShuttleComponent component, ref EntityUnpausedEvent args)
     {
         component.NextTransfer += args.PausedTime;
     }
 
+    /// <summary>
+    /// Here is our bus stop list handler. Theres probably a better way...
+    /// First, sets our output to null just in case
+    /// then, makes sure that our counter/index isnt out of range (reaching the end of the list will force you back to the beginning, like a loop)
+    /// Then, it checks to make sure that there even is anything in the list
+    /// and if so, we return the next station, and then increment our counter for the next time its ran
+    /// </summary>
     private bool TryGetNextStation(out EntityUid? station)
     {
         station = null;
@@ -115,6 +136,16 @@ private bool TryGetNextStation(out EntityUid? station)
         return true;
     }
 
+    /// <summary>
+    /// We check the current time every tick, and if its not yet time, we just ignore.
+    /// If the timer is ready, we send the shuttle on an FTL journey to the destination it has saved
+    /// then we check our bus list, and if it returns true with the next station, we cache it on the component and reset the timer
+    /// if it returns false or gives a bad grid, we are just going to FTL back to where we are and try again until theres a proper destination
+    /// This could cause unintended behavior, if a destination is deleted while it's next in the cache, the shuttle is going to be stuck in FTL space
+    /// However the timer is going to force it to FTL to the next bus stop
+    /// If it happens that all bus stops are deleted and we never get a valid stop again, we are going to be stuck FTL'ing forever in ftl space
+    /// but at that point, theres nowhere to return to anyway
+    /// </summary>
     public override void Update(float frameTime)
     {
         base.Update(frameTime);
@@ -136,6 +167,13 @@ public override void Update(float frameTime)
         }
     }
 
+    /// <summary>
+    /// Here is handling a simple CVAR change to enable/disable the system
+    /// if the cvar is changed to enabled, we setup the transit system
+    /// if its changed to disabled, we delete any bus grids that exist
+    /// along with anyone/thing riding the bus
+    /// you've been warned
+    /// </summary>
     private void SetTransit(bool obj)
     {
         Enabled = obj;
@@ -155,35 +193,48 @@ private void SetTransit(bool obj)
         }
     }
 
+    /// <summary>
+    /// Simple cache reflection
+    /// </summary>
     private void SetFly(float obj)
     {
         FlyTime = obj;
     }
 
+    /// <summary>
+    /// Here is where we handle setting up the transit system, including sanity checks.
+    /// This is called multiple times, from a few different sources, to ensure that if the system is activated dynamically
+    /// it will still function as intended
+    /// </summary>
     private void SetupPublicTransit()
     {
+        // If a public bus alraedy exists, we simply return. No need to set up the system again.
         var query = EntityQueryEnumerator<TransitShuttleComponent>();
         while (query.MoveNext(out var euid, out _))
         {
             if (!Deleted(euid))
                 return;
         }
-        // Spawn arrivals on a dummy map then dock it to the source.
+
+        // Spawn the bus onto a dummy map
         var dummyMap = _mapManager.CreateMap();
         var busMap = _cfgManager.GetCVar(NF14CVars.PublicTransitBusMap);
         if (_loader.TryLoad(dummyMap, busMap, out var shuttleUids))
         {
             var shuttleComp = Comp<ShuttleComponent>(shuttleUids[0]);
+            // Here we are making sure that the shuttle has the TransitShuttle comp onto it, in case of dynamically changing the bus grid
             var transitComp = EnsureComp<TransitShuttleComponent>(shuttleUids[0]);
+
+            //We run our bus station function to try to get a valid station to FTL to. If for some reason, there are no bus stops, we will instead just delete the shuttle
             if (TryGetNextStation(out var station) && station is { Valid : true } destination)
             {
-                transitComp.NextStation = destination; //we set up a default in case theres only 1
-                // EnsureComp<StationEmpImmuneComponent>(uid); Enable in the case we want to ensure EMP immune grid
-
+                //we set up a default in case the second time we call it fails for some reason
+                transitComp.NextStation = destination;
                 _shuttles.FTLTravel(shuttleUids[0], shuttleComp, destination, hyperspaceTime: 5f, dock: true);
-                transitComp.NextTransfer = _timing.CurTime +
-                                           TimeSpan.FromSeconds(_cfgManager.GetCVar(NF14CVars.PublicTransitWaitTime));
+                transitComp.NextTransfer = _timing.CurTime + TimeSpan.FromSeconds(_cfgManager.GetCVar(NF14CVars.PublicTransitWaitTime));
 
+                //since the initial cached value of the next station is actually the one we are 'starting' from, we need to run the
+                //bus stop list code one more time so that our first trip isnt just Frontier - Frontier
                 if (TryGetNextStation(out var firstStop) && firstStop is { Valid : true } firstDestination)
                     transitComp.NextStation = firstDestination;
             }
@@ -196,7 +247,8 @@ private void SetupPublicTransit()
             }
         }
 
-        // Don't start the arrivals shuttle immediately docked so power has a time to stabilise?
+        // the FTL sequence takes a few seconds to warm up and send the grid, so we give the temp dummy map
+        // some buffer time before calling a self-delete
         var timer = AddComp<TimedDespawnComponent>(_mapManager.GetMapEntityId(dummyMap));
         timer.Lifetime = 15f;
     }