Validate AgentIDs in ConfigValidator (#139)

* Validate AgentIDs in ConfigValidator

* Track AgentIDs across bots and scripts

* Update RLBotCS/ManagerTools/ConfigValidator.cs

Co-authored-by: Nicolaj 'Eastvillage' Ø Jensen <[email protected]>

* Ensure config is invalidated upon dup'd script aid

* Update version

---------

Co-authored-by: Nicolaj 'Eastvillage' Ø Jensen <[email protected]>

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.0.0-rc.1\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,16 +48,21 @@ public static bool Validate(MatchConfigurationT config)
         config.PlayerConfigurations ??= new();
         config.ScriptConfigurations ??= new();
 
-        valid = ValidatePlayers(ctx, config.PlayerConfigurations) && valid;
-        valid = ValidateScripts(ctx, config.ScriptConfigurations) && valid;
+        Dictionary<string, (string rootDir, string runCmd)> agentIdTracker = new();
+        valid =
+            ValidatePlayers(ctx, config.PlayerConfigurations, agentIdTracker, surpressWarnings)
+            && valid;
+        valid = ValidateScripts(ctx, config.ScriptConfigurations, agentIdTracker) && valid;
 
         Logger.LogDebug(valid ? "Match config is valid." : "Match config is invalid!");
         return valid;
     }
 
     private static bool ValidatePlayers(
         ConfigContextTracker ctx,
-        List<PlayerConfigurationT> players
+        List<PlayerConfigurationT> players,
+        Dictionary<string, (string rootDir, string runCmd)> agentIdTracker,
+        bool surpressWarnings
     )
     {
         bool valid = true;
@@ -99,6 +104,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
@@ -168,7 +213,8 @@ List<PlayerConfigurationT> players
 
     private static bool ValidateScripts(
         ConfigContextTracker ctx,
-        List<ScriptConfigurationT> scripts
+        List<ScriptConfigurationT> scripts,
+        Dictionary<string, (string rootDir, string runCmd)> agentIdTracker
     )
     {
         bool valid = true;
@@ -192,6 +238,19 @@ List<ScriptConfigurationT> scripts
             script.RunCommand ??= "";
             script.RootDir ??= "";
             script.ScriptId = $"{script.AgentId}/{Team.Scripts}/{i}".GetHashCode();
+
+            if (agentIdTracker.TryGetValue(script.AgentId, out var existing))
+            {
+                Logger.LogError(
+                    $"{ctx.ToStringWithEnd(Fields.AgentAgentId)} \"{script.AgentId}\" is already in use. "
+                        + "Each script must have a unique agent ID."
+                );
+                valid = false;
+            }
+            else
+            {
+                agentIdTracker[script.AgentId] = (script.RootDir, script.RunCommand);
+            }
         }
 
         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());