Prevent SecretRule From Picking Invalid Presets (#27456)

Content.IntegrationTests/Tests/GameRules/NukeOpsTest.cs

@@ -118,8 +118,8 @@ public async Task TryStopNukeOpsFromConstantlyFailing()
 
         // The game rule exists, and all the stations/shuttles/maps are properly initialized
         var rule = entMan.AllComponents<NukeopsRuleComponent>().Single().Component;
-        var mapRule = entMan.AllComponents<LoadMapRuleComponent>().Single().Component;
-        foreach (var grid in mapRule.MapGrids)
+        var gridsRule = entMan.AllComponents<RuleGridsComponent>().Single().Component;
+        foreach (var grid in gridsRule.MapGrids)
         {
             Assert.That(entMan.EntityExists(grid));
             Assert.That(entMan.HasComponent<MapGridComponent>(grid));
@@ -139,7 +139,7 @@ public async Task TryStopNukeOpsFromConstantlyFailing()
         Assert.That(entMan.EntityExists(nukieShuttlEnt));
 
         EntityUid? nukieStationEnt = null;
-        foreach (var grid in mapRule.MapGrids)
+        foreach (var grid in gridsRule.MapGrids)
         {
             if (entMan.HasComponent<StationMemberComponent>(grid))
             {
@@ -155,10 +155,8 @@ public async Task TryStopNukeOpsFromConstantlyFailing()
         Assert.That(entMan.EntityExists(nukieStation.Station));
         Assert.That(nukieStation.Station, Is.Not.EqualTo(rule.TargetStation));
 
-        Assert.That(server.MapMan.MapExists(mapRule.Map));
-        var nukieMap = mapSys.GetMap(mapRule.Map!.Value);
-        Assert.That(server.MapMan.MapExists(rule.NukiePlanet));
-        var nukieMap = mapSys.GetMap(rule.NukiePlanet!.Value);
+        Assert.That(server.MapMan.MapExists(gridsRule.Map));
+        var nukieMap = mapSys.GetMap(gridsRule.Map!.Value);
 
         var targetStation = entMan.GetComponent<StationDataComponent>(rule.TargetStation!.Value);
         var targetGrid = targetStation.Grids.First();

Content.Server/GameTicking/GameTicker.GameRule.cs

@@ -9,6 +9,7 @@
 using Robust.Shared.Console;
 using Robust.Shared.Map;
 using Robust.Shared.Prototypes;
+using Robust.Shared.Localization;
 
 namespace Content.Server.GameTicking;
 
@@ -71,6 +72,16 @@ public EntityUid AddGameRule(string ruleId)
         var ruleEntity = Spawn(ruleId, MapCoordinates.Nullspace);
         _sawmill.Info($"Added game rule {ToPrettyString(ruleEntity)}");
         _adminLogger.Add(LogType.EventStarted, $"Added game rule {ToPrettyString(ruleEntity)}");
+        var str = Loc.GetString("station-event-system-run-event", ("eventName", ToPrettyString(ruleEntity)));
+#if DEBUG
+        _chatManager.SendAdminAlert(str);
+#else
+        if (RunLevel == GameRunLevel.InRound) // avoids telling admins the round type before it starts so that can be handled elsewhere.
+        {
+            _chatManager.SendAdminAlert(str);
+        }
+#endif
+        Log.Info(str);
 
         var ev = new GameRuleAddedEvent(ruleEntity, ruleId);
         RaiseLocalEvent(ruleEntity, ref ev, true);
@@ -324,6 +335,13 @@ private void AddGameRuleCommand(IConsoleShell shell, string argstr, string[] arg
 
         foreach (var rule in args)
         {
+            if (!_prototypeManager.HasIndex(rule))
+            {
+                shell.WriteError($"Invalid game rule {rule} was skipped.");
+
+                continue;
+            }
+
             if (shell.Player != null)
             {
                 _adminLogger.Add(LogType.EventStarted, $"{shell.Player} tried to add game rule [{rule}] via command");
@@ -420,4 +438,4 @@ private string GetGameRulesListMessage(bool forChatWindow)
     }
 
     #endregion
-}
+}

Content.Server/GameTicking/GameTicker.RoundFlow.cs

@@ -186,6 +186,9 @@ public int ReadyPlayerCount()
                 if (!_playerManager.TryGetSessionById(userId, out _))
                     continue;
 
+                if (_banManager.GetRoleBans(userId) == null)
+                    continue;
+
                 total++;
             }
 

Content.Server/GameTicking/Rules/Components/LoadMapRuleComponent.cs

@@ -1,34 +1,34 @@
+using Content.Server.GameTicking.Rules;
 using Content.Server.Maps;
 using Content.Shared.GridPreloader.Prototypes;
-using Content.Shared.Storage;
-using Content.Shared.Whitelist;
-using Robust.Shared.Map;
 using Robust.Shared.Prototypes;
 using Robust.Shared.Utility;
 
 namespace Content.Server.GameTicking.Rules.Components;
 
 /// <summary>
 /// This is used for a game rule that loads a map when activated.
+/// Works with <see cref="RuleGridsComponent"/>.
 /// </summary>
-[RegisterComponent]
+[RegisterComponent, Access(typeof(LoadMapRuleSystem))]
 public sealed partial class LoadMapRuleComponent : Component
 {
-    [DataField]
-    public MapId? Map;
-
+    /// <summary>
+    /// A <see cref="GameMapPrototype"/> to load on a new map.
+    /// </summary>
     [DataField]
     public ProtoId<GameMapPrototype>? GameMap;
 
+    /// <summary>
+    /// A map path to load on a new map.
+    /// </summary>
     [DataField]
     public ResPath? MapPath;
 
+    /// <summary>
+    /// A <see cref="PreloadedGridPrototype"/> to move to a new map.
+    /// If there are no instances left nothing is done.
+    /// </summary>
     [DataField]
     public ProtoId<PreloadedGridPrototype>? PreloadedGrid;
-
-    [DataField]
-    public List<EntityUid> MapGrids = new();
-
-    [DataField]
-    public EntityWhitelist? SpawnerWhitelist;
 }

Content.Server/GameTicking/Rules/Components/RuleGridsComponent.cs

@@ -0,0 +1,30 @@
+using Content.Server.GameTicking.Rules;
+using Content.Shared.Whitelist;
+using Robust.Shared.Map;
+
+/// <summary>
+/// Stores grids created by another gamerule component.
+/// With <c>AntagSelection</c>, spawners on these grids can be used for its antags.
+/// </summary>
+[RegisterComponent, Access(typeof(RuleGridsSystem))]
+public sealed partial class RuleGridsComponent : Component
+{
+    /// <summary>
+    /// The map that was loaded.
+    /// </summary>
+    [DataField]
+    public MapId? Map;
+
+    /// <summary>
+    /// The grid entities that have been loaded.
+    /// </summary>
+    [DataField]
+    public List<EntityUid> MapGrids = new();
+
+    /// <summary>
+    /// Whitelist for a spawner to be considered for an antag.
+    /// All spawners must have <c>SpawnPointComponent</c> regardless to be found.
+    /// </summary>
+    [DataField]
+    public EntityWhitelist? SpawnerWhitelist;
+}

Content.Server/GameTicking/Rules/GameRuleSystem.Utility.cs

@@ -126,4 +126,8 @@ protected bool TryFindRandomTileOnStation(Entity<StationDataComponent> station,
         return found;
     }
 
+    protected void ForceEndSelf(EntityUid uid, GameRuleComponent? component = null)
+    {
+        GameTicker.EndGameRule(uid, component);
+    }
 }

Content.Server/GameTicking/Rules/GameRuleSystem.cs

@@ -41,11 +41,20 @@ private void OnStartAttempt(RoundStartAttemptEvent args)
             if (args.Players.Length >= minPlayers)
                 continue;
 
-            ChatManager.SendAdminAnnouncement(Loc.GetString("preset-not-enough-ready-players",
-                ("readyPlayersCount", args.Players.Length),
-                ("minimumPlayers", minPlayers),
-                ("presetName", ToPrettyString(uid))));
-            args.Cancel();
+            // if (gameRule.CancelPresetOnTooFewPlayers)
+            // {
+            //     ChatManager.SendAdminAnnouncement(Loc.GetString("preset-not-enough-ready-players",
+            //         ("readyPlayersCount", args.Players.Length),
+            //         ("minimumPlayers", minPlayers),
+            //         ("presetName", ToPrettyString(uid))));
+            //     args.Cancel();
+            // }
+            // else
+            // {
+            //     GameTicker.EndGameRule(uid, component);
+            // }
+
+            GameTicker.EndGameRule(uid, gameRule);
         }
     }
 
@@ -135,4 +144,4 @@ public override void Update(float frameTime)
             ActiveTick(uid, comp1, comp2, frameTime);
         }
     }
-}
+}

Content.Server/GameTicking/Rules/LoadMapRuleSystem.cs

@@ -1,9 +1,6 @@
-using Content.Server.Antag;
 using Content.Server.GameTicking.Rules.Components;
-using Content.Server.Spawners.Components;
 using Content.Server.GridPreloader;
 using Content.Shared.GameTicking.Components;
-using Content.Shared.Whitelist;
 using Robust.Server.GameObjects;
 using Robust.Server.Maps;
 using Robust.Shared.Map;
@@ -14,97 +11,70 @@ namespace Content.Server.GameTicking.Rules;
 public sealed class LoadMapRuleSystem : GameRuleSystem<LoadMapRuleComponent>
 {
     [Dependency] private readonly IPrototypeManager _prototypeManager = default!;
-    [Dependency] private readonly IMapManager _mapManager = default!;
     [Dependency] private readonly MapSystem _map = default!;
     [Dependency] private readonly MapLoaderSystem _mapLoader = default!;
     [Dependency] private readonly MetaDataSystem _metaData = default!;
     [Dependency] private readonly TransformSystem _transform = default!;
     [Dependency] private readonly GridPreloaderSystem _gridPreloader = default!;
-    [Dependency] private readonly EntityWhitelistSystem _whitelistSystem = default!;
-
-    public override void Initialize()
-    {
-        base.Initialize();
-
-        SubscribeLocalEvent<LoadMapRuleComponent, AntagSelectLocationEvent>(OnSelectLocation);
-        SubscribeLocalEvent<GridSplitEvent>(OnGridSplit);
-    }
-
-    private void OnGridSplit(ref GridSplitEvent args)
-    {
-        var rule = QueryActiveRules();
-        while (rule.MoveNext(out _, out var mapComp, out _))
-        {
-            if (!mapComp.MapGrids.Contains(args.Grid))
-                continue;
-
-            mapComp.MapGrids.AddRange(args.NewGrids);
-            break;
-        }
-    }
 
     protected override void Added(EntityUid uid, LoadMapRuleComponent comp, GameRuleComponent rule, GameRuleAddedEvent args)
     {
-        if (comp.Map != null)
+        if (comp.PreloadedGrid != null && !_gridPreloader.PreloadingEnabled)
+        {
+            // Preloading will never work if it's disabled, duh
+            Log.Debug($"Immediately ending {ToPrettyString(uid):rule} as preloading grids is disabled by cvar.");
+            ForceEndSelf(uid, rule);
             return;
+        }
 
         // grid preloading needs map to init after moving it
-        var mapUid = comp.PreloadedGrid != null ? _map.CreateMap(out var mapId, false) : _map.CreateMap(out mapId);
-        _metaData.SetEntityName(mapUid, $"LoadMapRule destination for rule {ToPrettyString(uid)}");
-        comp.Map = mapId;
+        var mapUid = _map.CreateMap(out var mapId, runMapInit: comp.PreloadedGrid == null);
+
+        Log.Info($"Created map {mapId} for {ToPrettyString(uid):rule}");
 
+        IReadOnlyList<EntityUid> grids;
         if (comp.GameMap != null)
         {
             var gameMap = _prototypeManager.Index(comp.GameMap.Value);
-            comp.MapGrids.AddRange(GameTicker.LoadGameMap(gameMap, comp.Map.Value, new MapLoadOptions()));
+            grids = GameTicker.LoadGameMap(gameMap, mapId, new MapLoadOptions());
         }
-        else if (comp.MapPath != null)
+        else if (comp.MapPath is {} path)
         {
-            if (!_mapLoader.TryLoad(comp.Map.Value,
-                    comp.MapPath.Value.ToString(),
-                    out var roots,
-                    new MapLoadOptions { LoadMap = true }))
+            var options = new MapLoadOptions { LoadMap = true };
+            if (!_mapLoader.TryLoad(mapId, path.ToString(), out var roots, options))
             {
-                _mapManager.DeleteMap(mapId);
+                Log.Error($"Failed to load map from {path}!");
+                Del(mapUid);
+                ForceEndSelf(uid, rule);
                 return;
             }
 
-            comp.MapGrids.AddRange(roots);
+            grids = roots;
         }
-        else if (comp.PreloadedGrid != null)
+        else if (comp.PreloadedGrid is {} preloaded)
         {
             // TODO: If there are no preloaded grids left, any rule announcements will still go off!
-            if (!_gridPreloader.TryGetPreloadedGrid(comp.PreloadedGrid.Value, out var loadedShuttle))
+            if (!_gridPreloader.TryGetPreloadedGrid(preloaded, out var loadedShuttle))
             {
-                _mapManager.DeleteMap(mapId);
+                Log.Error($"Failed to get a preloaded grid with {preloaded}!");
+                Del(mapUid);
+                ForceEndSelf(uid, rule);
                 return;
             }
 
             _transform.SetParent(loadedShuttle.Value, mapUid);
-            comp.MapGrids.Add(loadedShuttle.Value);
-            _map.InitializeMap(mapId);
+            grids = new List<EntityUid>() { loadedShuttle.Value };
+            _map.InitializeMap(mapUid);
         }
         else
         {
             Log.Error($"No valid map prototype or map path associated with the rule {ToPrettyString(uid)}");
+            Del(mapUid);
+            ForceEndSelf(uid, rule);
+            return;
         }
-    }
-
-    private void OnSelectLocation(Entity<LoadMapRuleComponent> ent, ref AntagSelectLocationEvent args)
-    {
-        var query = EntityQueryEnumerator<SpawnPointComponent, TransformComponent>();
-        while (query.MoveNext(out var uid, out _, out var xform))
-        {
-            if (xform.MapID != ent.Comp.Map)
-                continue;
-
-            if (xform.GridUid == null || !ent.Comp.MapGrids.Contains(xform.GridUid.Value))
-                continue;
 
-            if (_whitelistSystem.IsWhitelistFail(ent.Comp.SpawnerWhitelist, uid))
-                continue;
-
-            args.Coordinates.Add(_transform.GetMapCoordinates(xform));
-        }
+        var ev = new RuleLoadedGridsEvent(mapId, grids);
+        RaiseLocalEvent(uid, ref ev);
     }
 }

Content.Server/GameTicking/Rules/NukeopsRuleSystem.cs

@@ -266,10 +266,10 @@ private void OnMapInit(Entity<NukeOpsShuttleComponent> ent, ref MapInitEvent arg
     {
         var map = Transform(ent).MapID;
 
-        var rules = EntityQueryEnumerator<NukeopsRuleComponent, LoadMapRuleComponent>();
-        while (rules.MoveNext(out var uid, out _, out var mapRule))
+        var rules = EntityQueryEnumerator<NukeopsRuleComponent, RuleGridsComponent>();
+        while (rules.MoveNext(out var uid, out _, out var grids))
         {
-            if (map != mapRule.Map)
+            if (map != grids.Map)
                 continue;
             ent.Comp.AssociatedRule = uid;
             break;
@@ -330,7 +330,7 @@ private void OnWarDeclared(ref WarDeclaredEvent ev)
             if (nukeops.WarDeclaredTime != null)
                 continue;
 
-            if (TryComp<LoadMapRuleComponent>(uid, out var mapComp) && Transform(ev.DeclaratorEntity).MapID != mapComp.Map)
+            if (TryComp<RuleGridsComponent>(uid, out var grids) && Transform(ev.DeclaratorEntity).MapID != grids.Map)
                 continue;
 
             var newStatus = GetWarCondition(nukeops, ev.Status);
@@ -451,7 +451,7 @@ private void CheckRoundShouldEnd(Entity<NukeopsRuleComponent> ent)
 
         // Check that there are spawns available and that they can access the shuttle.
         var spawnsAvailable = EntityQuery<NukeOperativeSpawnerComponent>(true).Any();
-        if (spawnsAvailable && CompOrNull<LoadMapRuleComponent>(ent)?.Map == shuttleMapId)
+        if (spawnsAvailable && CompOrNull<RuleGridsComponent>(ent)?.Map == shuttleMapId)
             return; // Ghost spawns can still access the shuttle. Continue the round.
 
         // The shuttle is inaccessible to both living nuke operatives and yet to spawn nuke operatives,
@@ -484,7 +484,7 @@ private void OnAfterAntagEntSelected(Entity<NukeopsRuleComponent> ent, ref After
     /// Is this method the shitty glue holding together the last of my sanity? yes.
     /// Do i have a better solution? not presently.
     /// </remarks>
-    private EntityUid? GetOutpost(Entity<LoadMapRuleComponent?> ent)
+    private EntityUid? GetOutpost(Entity<RuleGridsComponent?> ent)
     {
         if (!Resolve(ent, ref ent.Comp, false))
             return null;