Take advantage of Robust's logging cvars

- Finally add port test retries for up to 5s.
- Fix weirdness with logging to file vs reading std handles.
- Replace some "DD" references with "server"

src/Tgstation.Server.Host/Components/Engine/ByondInstallation.cs

@@ -29,6 +29,9 @@ sealed class ByondInstallation : EngineInstallationBase
 		/// <inheritdoc />
 		public override bool HasStandardOutput { get; }
 
+		/// <inheritdoc />
+		public override bool PreferFileLogging => false;
+
 		/// <inheritdoc />
 		public override Task InstallationTask { get; }
 

src/Tgstation.Server.Host/Components/Engine/EngineExecutableLock.cs

@@ -22,6 +22,9 @@ sealed class EngineExecutableLock : ReferenceCounter<IEngineInstallation>, IEngi
 		/// <inheritdoc />
 		public bool HasStandardOutput => Instance.HasStandardOutput;
 
+		/// <inheritdoc />
+		public bool PreferFileLogging => Instance.PreferFileLogging;
+
 		/// <inheritdoc />
 		public bool PromptsForNetworkAccess => Instance.PromptsForNetworkAccess;
 

src/Tgstation.Server.Host/Components/Engine/EngineInstallationBase.cs

@@ -24,6 +24,9 @@ abstract class EngineInstallationBase : IEngineInstallation
 		/// <inheritdoc />
 		public abstract bool HasStandardOutput { get; }
 
+		/// <inheritdoc />
+		public abstract bool PreferFileLogging { get; }
+
 		/// <inheritdoc />
 		public abstract bool PromptsForNetworkAccess { get; }
 

src/Tgstation.Server.Host/Components/Engine/IEngineInstallation.cs

@@ -36,6 +36,11 @@ public interface IEngineInstallation
 		/// </summary>
 		bool PromptsForNetworkAccess { get; }
 
+		/// <summary>
+		/// If <see cref="HasStandardOutput"/> is set, this indicates that the engine server has good file logging that should be preferred to ours.
+		/// </summary>
+		bool PreferFileLogging { get; }
+
 		/// <summary>
 		/// The <see cref="Task"/> that completes when the BYOND version finished installing.
 		/// </summary>

src/Tgstation.Server.Host/Components/Engine/OpenDreamInstallation.cs

@@ -28,6 +28,9 @@ sealed class OpenDreamInstallation : EngineInstallationBase
 		/// <inheritdoc />
 		public override bool HasStandardOutput => true;
 
+		/// <inheritdoc />
+		public override bool PreferFileLogging => true;
+
 		/// <inheritdoc />
 		public override Task InstallationTask { get; }
 
@@ -64,12 +67,10 @@ public override string FormatServerArguments(
 			ArgumentNullException.ThrowIfNull(parameters);
 			ArgumentNullException.ThrowIfNull(launchParameters);
 
-			if (logFilePath != null)
-				throw new NotSupportedException("OpenDream does not support logging to a file!");
-
 			var parametersString = EncodeParameters(parameters, launchParameters);
 
-			var arguments = $"--cvar net.port={launchParameters.Port.Value} --cvar opendream.topic_port=0 --cvar opendream.world_params=\"{parametersString}\" \"{dmbProvider.DmbName}\"";
+			var loggingEnabled = logFilePath != null;
+			var arguments = $"--cvar {(loggingEnabled ? $"log.path=\"{logFilePath}\"" : "log.enabled=false")} --cvar net.port={launchParameters.Port.Value} --cvar opendream.topic_port=0 --cvar opendream.world_params=\"{parametersString}\" \"{dmbProvider.DmbName}\"";
 			return arguments;
 		}
 

src/Tgstation.Server.Host/Components/Session/SessionControllerFactory.cs

@@ -130,12 +130,28 @@ sealed class SessionControllerFactory : ISessionControllerFactory
 		/// Check if a given <paramref name="port"/> can be bound to.
 		/// </summary>
 		/// <param name="port">The port number to test.</param>
-		void PortBindTest(ushort port)
+		/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation.</param>
+		/// <returns>A <see cref="ValueTask"/> representing the running operation.</returns>
+		async ValueTask PortBindTest(ushort port, CancellationToken cancellationToken)
 		{
+			logger.LogTrace("Bind test: {port}", port);
 			try
 			{
-				logger.LogTrace("Bind test: {port}", port);
-				SocketExtensions.BindTest(platformIdentifier, port, false);
+				// GIVE ME THE FUCKING PORT BACK WINDOWS!!!!
+				const int MaxAttempts = 5;
+				for (var i = 0; i < MaxAttempts; ++i)
+					try
+					{
+						SocketExtensions.BindTest(platformIdentifier, port, false);
+						if (i > 0)
+							logger.LogDebug("Clearing the socket took {iterations} attempts :/", i + 1);
+
+						break;
+					}
+					catch (SocketException ex) when (platformIdentifier.IsWindows && ex.SocketErrorCode == SocketError.AddressAlreadyInUse && i < (MaxAttempts - 1))
+					{
+						await asyncDelayer.Delay(TimeSpan.FromSeconds(1), cancellationToken);
+					}
 			}
 			catch (SocketException ex) when (ex.SocketErrorCode == SocketError.AddressAlreadyInUse)
 			{
@@ -255,7 +271,7 @@ public async ValueTask<ISessionController> LaunchNew(
 				if (engineType == EngineType.Byond)
 					await CheckPagerIsNotRunning();
 
-				PortBindTest(launchParameters.Port.Value);
+				await PortBindTest(launchParameters.Port.Value, cancellationToken);
 
 				string outputFilePath = null;
 				var preserveLogFile = true;
@@ -269,13 +285,13 @@ public async ValueTask<ISessionController> LaunchNew(
 					outputFilePath = diagnosticsIOManager.ResolvePath(
 						diagnosticsIOManager.ConcatPath(
 							dateDirectory,
-							$"dd-utc-{now.ToString("yyyy-MM-dd-HH-mm-ss", CultureInfo.InvariantCulture)}{(apiValidate ? "-dmapi" : String.Empty)}.log"));
+							$"server-utc-{now.ToString("yyyy-MM-dd-HH-mm-ss", CultureInfo.InvariantCulture)}{(apiValidate ? "-dmapi" : String.Empty)}.log"));
 
-					logger.LogInformation("Logging DreamDaemon output to {path}...", outputFilePath);
+					logger.LogInformation("Logging server output to {path}...", outputFilePath);
 				}
 				else if (!hasStandardOutput)
 				{
-					outputFilePath = gameIOManager.ConcatPath(dmbProvider.Directory, $"{Guid.NewGuid()}.dd.log");
+					outputFilePath = gameIOManager.ConcatPath(dmbProvider.Directory, $"{Guid.NewGuid()}.server.log");
 					preserveLogFile = false;
 				}
 
@@ -478,7 +494,7 @@ async ValueTask<IProcess> CreateGameServerProcess(
 					{ DMApiConstants.ParamAccessIdentifier, accessIdentifier },
 				},
 				launchParameters,
-				!engineLock.HasStandardOutput
+				!engineLock.HasStandardOutput || engineLock.PreferFileLogging
 					? logFilePath
 					: null);
 
@@ -563,18 +579,20 @@ async ValueTask LogDDOutput(IProcess process, string outputFilePath, bool cliSup
 							}
 							catch (Exception ex)
 							{
-								logger.LogWarning(ex, "Failed to delete DreamDaemon log file {outputFilePath}!", outputFilePath);
+								// this is expected on OD at time of the support changes.
+								// I've open a change to fix it: https://github.com/space-wizards/RobustToolbox/pull/4501
+								logger.LogWarning(ex, "Failed to delete server log file {outputFilePath}!", outputFilePath);
 							}
 					}
 
 				logger.LogTrace(
-					"DreamDaemon Output:{newLine}{output}",
+					"Server Output:{newLine}{output}",
 					Environment.NewLine,
 					ddOutput);
 			}
 			catch (Exception ex)
 			{
-				logger.LogWarning(ex, "Error reading DreamDaemon output!");
+				logger.LogWarning(ex, "Error reading server output!");
 			}
 		}
 

tests/Tgstation.Server.Tests/Live/TestLiveServer.cs

@@ -72,7 +72,7 @@ public sealed class TestLiveServer
 						result.AddRange(System.Diagnostics.Process.GetProcessesByName("dd"));
 					break;
 				case EngineType.OpenDream:
-					result.AddRange(System.Diagnostics.Process.GetProcessesByName("OpenDreamServer"));
+					result.AddRange(System.Diagnostics.Process.GetProcessesByName("Robust.Server"));
 					break;
 				default:
 					Assert.Fail($"Unknown engine type: {engineType}");