Validate AgentIDs in ConfigValidator

Author: VirxEC

RLBotCS/Main.cs

@@ -10,7 +10,7 @@
 if (args.Length > 0 && args[0] == "--version")
 {
     Console.WriteLine(
-        "RLBotServer v5.beta.7.10\n"
+        "RLBotServer v5.beta.7.11\n"
             + $"Bridge {BridgeVersion.Version}\n"
             + "@ https://www.rlbot.org & https://github.com/RLBot/core"
     );

RLBotCS/ManagerTools/ConfigValidator.cs

@@ -18,7 +18,7 @@ public static class ConfigValidator
     /// If the config is invalid, the reasons are logged.
     /// </summary>
     /// <returns>Whether the given match is valid and can be started without issues.</returns>
-    public static bool Validate(MatchConfigurationT config)
+    public static bool Validate(MatchConfigurationT config, bool surpressWarnings = false)
     {
         bool valid = true;
         PsyonixLoadouts.Reset();
@@ -48,7 +48,7 @@ public static bool Validate(MatchConfigurationT config)
         config.PlayerConfigurations ??= new();
         config.ScriptConfigurations ??= new();
 
-        valid = ValidatePlayers(ctx, config.PlayerConfigurations) && valid;
+        valid = ValidatePlayers(ctx, config.PlayerConfigurations, surpressWarnings) && valid;
         valid = ValidateScripts(ctx, config.ScriptConfigurations) && valid;
 
         Logger.LogDebug(valid ? "Match config is valid." : "Match config is invalid!");
@@ -57,13 +57,17 @@ public static bool Validate(MatchConfigurationT config)
 
     private static bool ValidatePlayers(
         ConfigContextTracker ctx,
-        List<PlayerConfigurationT> players
+        List<PlayerConfigurationT> players,
+        bool surpressWarnings
     )
     {
         bool valid = true;
         int humanCount = 0;
         int humanIndex = -1;
 
+        // map of agentid -> rootdir,runcmd
+        Dictionary<string, (string rootDir, string runCmd)> agentIdTracker = new();
+
         for (int i = 0; i < players.Count; i++)
         {
             using var _ = ctx.Begin($"{Fields.CarsList}[{i}]");
@@ -99,6 +103,46 @@ List<PlayerConfigurationT> players
                     bot.Loadout.LoadoutPaint ??= new();
 
                     player.PlayerId = $"{bot.AgentId}/{player.Team}/{i}".GetHashCode();
+
+                    // Dont validate agent id for bots that will be manually started
+                    if (!surpressWarnings && !string.IsNullOrEmpty(bot.RunCommand))
+                    {
+                        // Reduce user confusion around how agent ids should be used
+                        // Same bot == same agent id, different bot == different agent id
+                        // This is not a hard requirement, so we just log a warning
+                        // We check for "same bot" by comparing RootDir and RunCommand
+                        if (agentIdTracker.TryGetValue(bot.AgentId, out var existing))
+                        {
+                            if (
+                                existing.rootDir != bot.RootDir
+                                || existing.runCmd != bot.RunCommand
+                            )
+                            {
+                                string errorStr;
+
+                                if (existing.rootDir != bot.RootDir)
+                                {
+                                    errorStr =
+                                        existing.runCmd != bot.RunCommand
+                                            ? "RootDirs and RunCommands"
+                                            : "RootDirs";
+                                }
+                                else
+                                {
+                                    errorStr = "RunCommands";
+                                }
+
+                                Logger.LogWarning(
+                                    $"Potential agent ID conflict: {bot.AgentId} is used by multiple bots with different {errorStr}.\n"
+                                        + "Agent configs using the same ID may get used interchangeably. Agents that behave differently should have unique IDs."
+                                );
+                            }
+                        }
+                        else
+                        {
+                            agentIdTracker[bot.AgentId] = (bot.RootDir, bot.RunCommand);
+                        }
+                    }
                     break;
                 case PsyonixBotT bot:
                     string skill = bot.BotSkill switch
@@ -172,6 +216,7 @@ List<ScriptConfigurationT> scripts
     )
     {
         bool valid = true;
+        HashSet<string> agentIds = new();
 
         for (int i = 0; i < scripts.Count; i++)
         {
@@ -192,6 +237,16 @@ List<ScriptConfigurationT> scripts
             script.RunCommand ??= "";
             script.RootDir ??= "";
             script.ScriptId = $"{script.AgentId}/{Team.Scripts}/{i}".GetHashCode();
+
+            // Scripts must have unique agent ids
+            if (!agentIds.Add(script.AgentId))
+            {
+                Logger.LogError(
+                    $"{ctx.ToStringWithEnd(Fields.AgentAgentId)} \"{script.AgentId}\" is already in use. "
+                        + "Each script must have a unique agent ID."
+                );
+                valid = false;
+            }
         }
 
         return valid;

RLBotCS/Server/ServerMessage/StartMatch.cs

@@ -9,7 +9,7 @@ readonly struct StartMatch(MatchConfigurationT MatchConfig) : IServerMessage
 {
     public ServerAction Execute(ServerContext context)
     {
-        Debug.Assert(ConfigValidator.Validate(MatchConfig));
+        Debug.Assert(ConfigValidator.Validate(MatchConfig, true));
 
         context.Bridge.TryWrite(new ClearRenders());