diff --git a/Assets/Tests/Editor/LogFilteringServiceTests.cs b/Assets/Tests/Editor/LogFilteringServiceTests.cs new file mode 100644 index 0000000000..6169c43e44 --- /dev/null +++ b/Assets/Tests/Editor/LogFilteringServiceTests.cs @@ -0,0 +1,129 @@ +using System; +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Unit tests for LogFilteringService + /// Related classes: LogFilteringService, GetLogsUseCase, LogEntry + /// + [TestFixture] + public class LogFilteringServiceTests + { + private LogFilteringService _service; + + [SetUp] + public void SetUp() + { + _service = new LogFilteringService(); + } + + private static UnityCliLoopConsoleLogEntry CreateEntry(string message) + { + return new UnityCliLoopConsoleLogEntry(UnityCliLoopLogType.Log, message, $"stack of {message}"); + } + + /// + /// Verifies entries are returned newest-first (input order reversed) + /// + [Test] + public void FilterAndLimitLogs_ReturnsEntriesNewestFirst() + { + UnityCliLoopConsoleLogEntry[] entries = { CreateEntry("oldest"), CreateEntry("middle"), CreateEntry("newest") }; + + LogEntry[] result = _service.FilterAndLimitLogs(entries, 10, includeStackTrace: false); + + Assert.That(result.Length, Is.EqualTo(3)); + Assert.That(result[0].Message, Is.EqualTo("newest")); + Assert.That(result[1].Message, Is.EqualTo("middle")); + Assert.That(result[2].Message, Is.EqualTo("oldest")); + } + + /// + /// Verifies maxCount keeps only the newest entries when input exceeds the limit + /// + [Test] + public void FilterAndLimitLogs_LimitsToNewestEntries() + { + UnityCliLoopConsoleLogEntry[] entries = { CreateEntry("first"), CreateEntry("second"), CreateEntry("third"), CreateEntry("fourth") }; + + LogEntry[] result = _service.FilterAndLimitLogs(entries, 2, includeStackTrace: false); + + Assert.That(result.Length, Is.EqualTo(2)); + Assert.That(result[0].Message, Is.EqualTo("fourth")); + Assert.That(result[1].Message, Is.EqualTo("third")); + } + + /// + /// Verifies stack traces are included when includeStackTrace is true + /// + [Test] + public void FilterAndLimitLogs_IncludesStackTraceWhenRequested() + { + UnityCliLoopConsoleLogEntry[] entries = { CreateEntry("message") }; + + LogEntry[] result = _service.FilterAndLimitLogs(entries, 10, includeStackTrace: true); + + Assert.That(result[0].StackTrace, Is.EqualTo("stack of message")); + } + + /// + /// Verifies stack traces are null when includeStackTrace is false + /// + [Test] + public void FilterAndLimitLogs_OmitsStackTraceWhenNotRequested() + { + UnityCliLoopConsoleLogEntry[] entries = { CreateEntry("message") }; + + LogEntry[] result = _service.FilterAndLimitLogs(entries, 10, includeStackTrace: false); + + Assert.That(result[0].StackTrace, Is.Null); + } + + /// + /// Verifies maxCount of zero returns an empty array + /// + [Test] + public void FilterAndLimitLogs_WithZeroMaxCount_ReturnsEmptyArray() + { + UnityCliLoopConsoleLogEntry[] entries = { CreateEntry("message") }; + + LogEntry[] result = _service.FilterAndLimitLogs(entries, 0, includeStackTrace: false); + + Assert.That(result, Is.Empty); + } + + /// + /// Verifies empty input returns an empty array + /// + [Test] + public void FilterAndLimitLogs_WithEmptyInput_ReturnsEmptyArray() + { + LogEntry[] result = _service.FilterAndLimitLogs(Array.Empty(), 10, includeStackTrace: false); + + Assert.That(result, Is.Empty); + } + + /// + /// Verifies null entries are rejected with ArgumentNullException + /// + [Test] + public void FilterAndLimitLogs_WithNullEntries_ThrowsArgumentNullException() + { + Assert.Throws(() => _service.FilterAndLimitLogs(null, 10, includeStackTrace: false)); + } + + /// + /// Verifies negative maxCount is rejected with ArgumentOutOfRangeException + /// + [Test] + public void FilterAndLimitLogs_WithNegativeMaxCount_ThrowsArgumentOutOfRangeException() + { + UnityCliLoopConsoleLogEntry[] entries = { CreateEntry("message") }; + + Assert.Throws(() => _service.FilterAndLimitLogs(entries, -1, includeStackTrace: false)); + } + } +} diff --git a/Assets/Tests/Editor/LogFilteringServiceTests.cs.meta b/Assets/Tests/Editor/LogFilteringServiceTests.cs.meta new file mode 100644 index 0000000000..ecc54e189f --- /dev/null +++ b/Assets/Tests/Editor/LogFilteringServiceTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 060fc20e74a5842b79abd2cd406f4494 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Domain/CliConstants.cs b/Packages/src/Editor/Domain/CliConstants.cs index 79443441dd..f96383076b 100644 --- a/Packages/src/Editor/Domain/CliConstants.cs +++ b/Packages/src/Editor/Domain/CliConstants.cs @@ -6,7 +6,7 @@ namespace io.github.hatayama.UnityCliLoop.Domain public static class CliConstants { public const string EXECUTABLE_NAME = "uloop"; - public const string MINIMUM_REQUIRED_CLI_VERSION = "3.0.0-beta.30"; + public const string MINIMUM_REQUIRED_CLI_VERSION = "3.0.0-beta.31"; public const string MINIMUM_REQUIRED_CLI_RELEASE_TAG = CLI_RELEASE_TAG_PREFIX + MINIMUM_REQUIRED_CLI_VERSION; public const string VERSION_FLAG = "--version"; public const string SHORT_VERSION_FLAG = "-v"; diff --git a/Packages/src/Editor/FirstPartyTools/Common/Console/ConsoleLogRetriever.cs b/Packages/src/Editor/FirstPartyTools/Common/Console/ConsoleLogRetriever.cs index d6dc81985d..e8aa87ab8b 100644 --- a/Packages/src/Editor/FirstPartyTools/Common/Console/ConsoleLogRetriever.cs +++ b/Packages/src/Editor/FirstPartyTools/Common/Console/ConsoleLogRetriever.cs @@ -13,6 +13,13 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools public class ConsoleLogRetriever { private readonly Type _logEntriesType; + private readonly Type _logEntryType; + private readonly PropertyInfo _consoleFlagsProperty; + private readonly MethodInfo _getCountMethod; + private readonly MethodInfo _getEntryInternalMethod; + private readonly FieldInfo _messageField; + private readonly FieldInfo _modeField; + private readonly FieldInfo _callstackTextStartField; /// /// Initializes the retriever with necessary reflection types @@ -27,6 +34,30 @@ public ConsoleLogRetriever() { throw new InvalidOperationException("LogEntries type not found. Unity version compatibility issue."); } + + _logEntryType = editorAssembly.GetType("UnityEditor.LogEntry"); + if (_logEntryType == null) + { + throw new InvalidOperationException("LogEntry type not found. Unity version compatibility issue."); + } + + // GetLogEntryAt runs once per console entry, so member lookups are resolved once here + // instead of per entry; repeated GetMethod/GetField calls dominated get-logs latency. + _consoleFlagsProperty = _logEntriesType.GetProperty("consoleFlags", BindingFlags.Public | BindingFlags.Static); + _getCountMethod = _logEntriesType.GetMethod("GetCount", BindingFlags.Public | BindingFlags.Static); + _getEntryInternalMethod = _logEntriesType.GetMethod("GetEntryInternal", BindingFlags.Public | BindingFlags.Static); + _messageField = _logEntryType.GetField("message", BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance); + _modeField = _logEntryType.GetField("mode", BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance); + _callstackTextStartField = _logEntryType.GetField("callstackTextStartUTF8", BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance); + + // Missing members would otherwise surface as silently empty or zeroed log data, + // so an internal Unity API change must fail here instead. + if (_consoleFlagsProperty == null || _getCountMethod == null || _getEntryInternalMethod == null || + _messageField == null || _modeField == null || _callstackTextStartField == null) + { + throw new InvalidOperationException( + "Required LogEntries/LogEntry members not found. Unity version compatibility issue."); + } } /// @@ -73,11 +104,7 @@ public List GetAllLogs() private void RestoreOriginalMask(int originalUnityMask) { // Use consoleFlags property to restore the exact Unity mask - PropertyInfo consoleFlagsProperty = _logEntriesType.GetProperty("consoleFlags", BindingFlags.Public | BindingFlags.Static); - if (consoleFlagsProperty != null) - { - consoleFlagsProperty.SetValue(null, originalUnityMask); - } + _consoleFlagsProperty.SetValue(null, originalUnityMask); } /// @@ -146,15 +173,8 @@ public List GetLogsByType(LogType logType) public int GetCurrentMask() { // Use the consoleFlags property discovered in the investigation - PropertyInfo consoleFlagsProperty = _logEntriesType.GetProperty("consoleFlags", BindingFlags.Public | BindingFlags.Static); - if (consoleFlagsProperty != null) - { - object result = consoleFlagsProperty.GetValue(null); - return result != null ? (int)result : 0; - } - - // Fallback: try to get from ConsoleWindow if available - return GetMaskFromConsoleWindow(); + object result = _consoleFlagsProperty.GetValue(null); + return result != null ? (int)result : 0; } /// @@ -167,14 +187,7 @@ public void SetMask(int mask) int unityMask = ConvertToUnityMask(mask); // Use the consoleFlags property - PropertyInfo consoleFlagsProperty = _logEntriesType.GetProperty("consoleFlags", BindingFlags.Public | BindingFlags.Static); - if (consoleFlagsProperty != null) - { - consoleFlagsProperty.SetValue(null, unityMask); - return; - } - - Debug.LogWarning("Could not find consoleFlags property"); + _consoleFlagsProperty.SetValue(null, unityMask); } /// @@ -210,14 +223,8 @@ private int ConvertToUnityMask(int simpleMask) /// public int GetLogCount() { - MethodInfo getCount = _logEntriesType.GetMethod("GetCount", BindingFlags.Public | BindingFlags.Static); - if (getCount != null) - { - object result = getCount.Invoke(null, null); - return result != null ? (int)result : 0; - } - - return 0; + object result = _getCountMethod.Invoke(null, null); + return result != null ? (int)result : 0; } @@ -226,29 +233,12 @@ public int GetLogCount() /// private LogEntryDto GetLogEntryAt(int index) { - // Get LogEntry type from Unity's internal assembly - Assembly editorAssembly = Assembly.GetAssembly(typeof(EditorWindow)); - Type logEntryType = editorAssembly.GetType("UnityEditor.LogEntry"); - if (logEntryType == null) - { - Debug.LogError("LogEntry type not found"); - return null; - } - // Create LogEntry instance - object logEntryInstance = Activator.CreateInstance(logEntryType); - - // Use GetEntryInternal method discovered in investigation - MethodInfo getEntryInternal = _logEntriesType.GetMethod("GetEntryInternal", BindingFlags.Public | BindingFlags.Static); - if (getEntryInternal == null) - { - Debug.LogError("GetEntryInternal method not found"); - return null; - } + object logEntryInstance = Activator.CreateInstance(_logEntryType); // Call GetEntryInternal(int row, LogEntry outputEntry) object[] parameters = new object[] { index, logEntryInstance }; - bool success = (bool)getEntryInternal.Invoke(null, parameters); + bool success = (bool)_getEntryInternalMethod.Invoke(null, parameters); if (!success) { @@ -256,10 +246,10 @@ private LogEntryDto GetLogEntryAt(int index) return null; } - // Extract data from LogEntry instance using reflection - string fullMessage = GetFieldValue(logEntryInstance, "message")?.ToString() ?? ""; - int mode = (int)(GetFieldValue(logEntryInstance, "mode") ?? 0); - int callstackTextStart = (int)(GetFieldValue(logEntryInstance, "callstackTextStartUTF8") ?? 0); + // Extract data from LogEntry instance using cached field handles + string fullMessage = _messageField.GetValue(logEntryInstance)?.ToString() ?? ""; + int mode = (int)_modeField.GetValue(logEntryInstance); + int callstackTextStart = (int)_callstackTextStartField.GetValue(logEntryInstance); LogType logType = GetLogTypeFromMode(mode); @@ -270,16 +260,6 @@ private LogEntryDto GetLogEntryAt(int index) return new LogEntryDto(unityCliLoopLogType, message, stackTrace); } - /// - /// Helper method to get field value from object using reflection - /// - private object GetFieldValue(object obj, string fieldName) - { - Type type = obj.GetType(); - FieldInfo field = type.GetField(fieldName, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance); - return field?.GetValue(obj); - } - /// /// Converts a UTF-8 byte position to a character position in a string. /// Unity's LogEntry.callstackTextStartUTF8 provides byte offset, but @@ -384,14 +364,5 @@ private int GetMaskForLogType(LogType logType) }; } - /// - /// Fallback method to get mask from ConsoleWindow - /// - private int GetMaskFromConsoleWindow() - { - // Implementation would access ConsoleWindow's filter state - return 7; // Default to show all - } - } } diff --git a/Packages/src/Editor/FirstPartyTools/Common/Console/LogGetter.cs b/Packages/src/Editor/FirstPartyTools/Common/Console/LogGetter.cs index 0ead29a3e2..1b295c7b14 100644 --- a/Packages/src/Editor/FirstPartyTools/Common/Console/LogGetter.cs +++ b/Packages/src/Editor/FirstPartyTools/Common/Console/LogGetter.cs @@ -21,6 +21,14 @@ public static class LogGetter private static readonly Regex CompilerErrorPattern = new Regex(@":\s*error CS\d+\b", RegexOptions.Compiled); private static readonly Regex CompilerWarningPattern = new Regex(@":\s*warning CS\d+\b", RegexOptions.Compiled); + // AI clients typically poll get-logs with the same search pattern repeatedly, so the + // last user pattern is kept compiled instead of being re-built on every request. + private static Regex _cachedSearchRegex; + + // User-supplied patterns can backtrack pathologically; the match timeout keeps a bad + // pattern from stalling the Editor main thread while scanning every log entry. + private static readonly TimeSpan SearchRegexMatchTimeout = TimeSpan.FromSeconds(2); + static LogGetter() { LogRetriever = new ConsoleLogRetriever(); @@ -254,8 +262,8 @@ public static LogDisplayDto SearchConsoleLogs(string logType, string searchText, { if (useRegex) { - Regex regex = new(searchText); - allEntries = allEntries.FindAll(entry => + Regex regex = GetOrCreateSearchRegex(searchText); + allEntries = allEntries.FindAll(entry => { bool messageMatch = regex.IsMatch(entry.Message); bool stackTraceMatch = searchInStackTrace && !string.IsNullOrEmpty(entry.StackTrace) && regex.IsMatch(entry.StackTrace); @@ -277,6 +285,20 @@ public static LogDisplayDto SearchConsoleLogs(string logType, string searchText, return new LogDisplayDto(allEntries.ToArray(), allEntries.Count); } + /// + /// Returns a compiled regex for the user search pattern, reusing the previous + /// instance when the pattern is unchanged + /// + private static Regex GetOrCreateSearchRegex(string searchText) + { + if (_cachedSearchRegex == null || _cachedSearchRegex.ToString() != searchText) + { + _cachedSearchRegex = new Regex(searchText, RegexOptions.Compiled, SearchRegexMatchTimeout); + } + + return _cachedSearchRegex; + } + /// /// Gets the total number of console logs. /// diff --git a/Packages/src/Editor/FirstPartyTools/GetLogs/LogFilteringService.cs b/Packages/src/Editor/FirstPartyTools/GetLogs/LogFilteringService.cs index f8d45ce5bf..90e645b4b9 100644 --- a/Packages/src/Editor/FirstPartyTools/GetLogs/LogFilteringService.cs +++ b/Packages/src/Editor/FirstPartyTools/GetLogs/LogFilteringService.cs @@ -1,5 +1,4 @@ using System; -using System.Linq; namespace io.github.hatayama.UnityCliLoop.FirstPartyTools { @@ -22,17 +21,21 @@ public LogEntry[] FilterAndLimitLogs(UnityCliLoopConsoleLogEntry[] entries, int throw new ArgumentOutOfRangeException(nameof(maxCount), "maxCount must be zero or greater."); } - UnityCliLoopConsoleLogEntry[] limitedEntries = entries.Length > maxCount - ? entries.Skip(entries.Length - maxCount).ToArray() - : entries; - - limitedEntries = limitedEntries.Reverse().ToArray(); + // Take the newest maxCount entries and return them newest-first in a single + // pass; the previous LINQ chain copied the entries into three arrays per call. + int resultCount = Math.Min(entries.Length, maxCount); + LogEntry[] result = new LogEntry[resultCount]; + for (int i = 0; i < resultCount; i++) + { + UnityCliLoopConsoleLogEntry entry = entries[entries.Length - 1 - i]; + result[i] = new LogEntry( + type: entry.Type, + message: entry.Message, + stackTrace: includeStackTrace ? entry.StackTrace : null + ); + } - return limitedEntries.Select(entry => new LogEntry( - type: entry.Type, - message: entry.Message, - stackTrace: includeStackTrace ? entry.StackTrace : null - )).ToArray(); + return result; } } } diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/PlayModeTestExecuter.cs b/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/PlayModeTestExecuter.cs index 4f940a4a51..8bcfbfae72 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/PlayModeTestExecuter.cs +++ b/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/PlayModeTestExecuter.cs @@ -79,6 +79,10 @@ private static async Task ExecuteTestWithEventNotificati }; StartTestExecution(testMode, filter, callback); + // Without this registration the await below never completes on cancellation, + // keeping the TestRunnerApi callback subscription alive forever. + using CancellationTokenRegistration cancellationRegistration = + ct.Register(() => taskCompletionSource.TrySetCanceled(ct)); SerializableTestResult result = await taskCompletionSource.Task; ct.ThrowIfCancellationRequested(); return result; diff --git a/Packages/src/Editor/Infrastructure/Api/JsonRpcProcessor.cs b/Packages/src/Editor/Infrastructure/Api/JsonRpcProcessor.cs index f35d2458e3..8f92e095c7 100644 --- a/Packages/src/Editor/Infrastructure/Api/JsonRpcProcessor.cs +++ b/Packages/src/Editor/Infrastructure/Api/JsonRpcProcessor.cs @@ -33,6 +33,14 @@ public static class JsonRpcProcessor { private const string WaitForDomainReloadParamName = "WaitForDomainReload"; + // Shared by every response path; JsonConvert only reads the settings, so a single + // instance avoids allocating identical settings per response. + private static readonly JsonSerializerSettings ResponseSerializerSettings = new() + { + ReferenceLoopHandling = ReferenceLoopHandling.Ignore, + MaxDepth = UnityCliLoopServerConfig.DEFAULT_JSON_MAX_DEPTH + }; + internal delegate Task JsonRpcEarlyResponseWriter( string responseJson, bool cancelOnClientDisconnect, @@ -297,13 +305,7 @@ private static string CreateCliUpdateRequiredResponse(object id, string currentC $"{CliConstants.EXECUTABLE_NAME} update", $"{CliConstants.EXECUTABLE_NAME} update --to-version {requiredCliVersion}"))); - JsonSerializerSettings settings = new() - { - ReferenceLoopHandling = ReferenceLoopHandling.Ignore, - MaxDepth = UnityCliLoopServerConfig.DEFAULT_JSON_MAX_DEPTH - }; - - return JsonConvert.SerializeObject(errorResponse, Formatting.None, settings); + return JsonConvert.SerializeObject(errorResponse, Formatting.None, ResponseSerializerSettings); } internal static string CreateDispatchAcceptedResponse(object id, int heartbeatIntervalSeconds) @@ -333,13 +335,7 @@ internal static string CreateDispatchAcceptedResponse(object id, int heartbeatIn uloop = uloopMetadata }; - JsonSerializerSettings settings = new() - { - ReferenceLoopHandling = ReferenceLoopHandling.Ignore, - MaxDepth = UnityCliLoopServerConfig.DEFAULT_JSON_MAX_DEPTH - }; - - return JsonConvert.SerializeObject(response, Formatting.None, settings); + return JsonConvert.SerializeObject(response, Formatting.None, ResponseSerializerSettings); } internal static string CreateHeartbeatResponse(object id, double mainThreadStallSeconds) @@ -359,13 +355,7 @@ internal static string CreateHeartbeatResponse(object id, double mainThreadStall } }; - JsonSerializerSettings settings = new() - { - ReferenceLoopHandling = ReferenceLoopHandling.Ignore, - MaxDepth = UnityCliLoopServerConfig.DEFAULT_JSON_MAX_DEPTH - }; - - return JsonConvert.SerializeObject(response, Formatting.None, settings); + return JsonConvert.SerializeObject(response, Formatting.None, ResponseSerializerSettings); } private static void AppendTimingIfRequested(UnityCliLoopToolResponse result, string timing) @@ -396,12 +386,6 @@ private static void LogUnityCliLoopToolParameterValidationException(UnityCliLoop /// Command execution result private static string CreateSuccessResponse(object id, UnityCliLoopToolResponse result) { - JsonSerializerSettings settings = new() - { - ReferenceLoopHandling = ReferenceLoopHandling.Ignore, - MaxDepth = UnityCliLoopServerConfig.DEFAULT_JSON_MAX_DEPTH - }; - try { JsonRpcSuccessResponse response = new( @@ -409,7 +393,7 @@ private static string CreateSuccessResponse(object id, UnityCliLoopToolResponse id, result ); - return JsonConvert.SerializeObject(response, Formatting.None, settings); + return JsonConvert.SerializeObject(response, Formatting.None, ResponseSerializerSettings); } catch (Exception) { @@ -467,14 +451,8 @@ private static string CreateErrorResponse(object id, Exception ex) id, new JsonRpcError(UnityCliLoopServerConfig.INTERNAL_ERROR_CODE, errorMessage, errorData) ); - - JsonSerializerSettings settings = new() - { - ReferenceLoopHandling = ReferenceLoopHandling.Ignore, - MaxDepth = UnityCliLoopServerConfig.DEFAULT_JSON_MAX_DEPTH - }; - - return JsonConvert.SerializeObject(errorResponse, Formatting.None, settings); + + return JsonConvert.SerializeObject(errorResponse, Formatting.None, ResponseSerializerSettings); } /// diff --git a/Packages/src/Editor/Infrastructure/Settings/UnityCliLoopPackageRemovalSettingsResetter.cs b/Packages/src/Editor/Infrastructure/Settings/UnityCliLoopPackageRemovalSettingsResetter.cs index fb14098f6d..c78ff376cd 100644 --- a/Packages/src/Editor/Infrastructure/Settings/UnityCliLoopPackageRemovalSettingsResetter.cs +++ b/Packages/src/Editor/Infrastructure/Settings/UnityCliLoopPackageRemovalSettingsResetter.cs @@ -38,6 +38,9 @@ internal void RegisterForEditorStartup() return; } + // Unsubscribe first so a repeated registration cannot stack duplicate handlers, + // matching the guard pattern used by the other editor-lifetime subscriptions. + Events.registeringPackages -= HandleRegisteringPackages; Events.registeringPackages += HandleRegisteringPackages; } diff --git a/Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServer.cs b/Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServer.cs index dce0e7a60d..4955682846 100644 --- a/Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServer.cs +++ b/Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServer.cs @@ -739,7 +739,17 @@ private static async Task MonitorClientDisconnectAsync( return; } - await Task.Delay(ClientDisconnectMonitorPollMilliseconds); + try + { + await Task.Delay(ClientDisconnectMonitorPollMilliseconds, requestCancellationTokenSource.Token); + } + catch (OperationCanceledException) + { + // Cancellation is the normal stop signal from StopClientDisconnectMonitorAsync. + // Without the token the delay always ran to completion, adding one poll + // interval of tail latency to every request teardown and server shutdown. + return; + } } } diff --git a/Packages/src/Editor/ToolContracts/UnityCliLoopTool.cs b/Packages/src/Editor/ToolContracts/UnityCliLoopTool.cs index 2cc1c3a127..612753b9fd 100644 --- a/Packages/src/Editor/ToolContracts/UnityCliLoopTool.cs +++ b/Packages/src/Editor/ToolContracts/UnityCliLoopTool.cs @@ -5,6 +5,20 @@ namespace io.github.hatayama.UnityCliLoop.ToolContracts { + /// + /// Shared serializer for converting CLI request parameters into schema DTOs + /// + internal static class UnityCliLoopToolParameterSerializer + { + // A single shared instance lets the contract resolver reuse its per-type metadata + // cache across all tools and requests instead of rebuilding it per invocation. + // Both JsonSerializer and the resolver are thread-safe once configured. + internal static readonly JsonSerializer CamelCaseSerializer = JsonSerializer.Create(new JsonSerializerSettings + { + ContractResolver = new Newtonsoft.Json.Serialization.CamelCasePropertyNamesContractResolver() + }); + } + // Related classes: // - IUnityCliLoopTool: The interface that this class implements. // - UnityCliLoopToolRegistry: Registers and manages instances of tool implementations. @@ -20,11 +34,15 @@ public abstract class UnityCliLoopTool : IUnityCliLoopTool { public abstract string ToolName { get; } + // The schema is pure reflection output over an immutable type, so it is generated + // once per TSchema (static fields on a generic class are per closed generic type). + private static readonly ToolParameterSchema CachedParameterSchema = + UnityCliLoopToolParameterSchemaGenerator.FromDto(); + /// /// Automatically generates parameter schema from TSchema type /// - public virtual ToolParameterSchema ParameterSchema => - UnityCliLoopToolParameterSchemaGenerator.FromDto(); + public virtual ToolParameterSchema ParameterSchema => CachedParameterSchema; /// /// Execute tool with type-safe Schema parameters. @@ -60,21 +78,12 @@ private TSchema ConvertToSchema(JToken paramsToken) return new TSchema(); } - // Create JsonSerializerSettings with CamelCasePropertyNamesContractResolver - // This allows client side to use camelCase while C# uses PascalCase - JsonSerializerSettings settings = new() - { - ContractResolver = new Newtonsoft.Json.Serialization.CamelCasePropertyNamesContractResolver() - }; - - // Create JsonSerializer with custom settings - JsonSerializer serializer = JsonSerializer.Create(settings); - - // Try to deserialize from JToken with custom serializer + // Try to deserialize from JToken with the shared camelCase serializer. + // This allows client side to use camelCase while C# uses PascalCase. TSchema schema; try { - schema = paramsToken.ToObject(serializer); + schema = paramsToken.ToObject(UnityCliLoopToolParameterSerializer.CamelCaseSerializer); } catch (JsonSerializationException ex) { diff --git a/cli/contract.json b/cli/contract.json index 210e458fe1..9516c87585 100644 --- a/cli/contract.json +++ b/cli/contract.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "cliVersion": "3.0.0-beta.30" + "cliVersion": "3.0.0-beta.31" } diff --git a/cli/internal/cli/compile_wait.go b/cli/internal/cli/compile_wait.go index 34f5456e5a..9e3ee518d6 100644 --- a/cli/internal/cli/compile_wait.go +++ b/cli/internal/cli/compile_wait.go @@ -111,6 +111,8 @@ func waitForCompileCompletion(ctx context.Context, options compileCompletionOpti logCompileStatusPollStart(options, startedAt, deadline) + ticker := time.NewTicker(options.pollInterval) + defer ticker.Stop() for { now := time.Now() if !now.Before(deadline) { @@ -134,7 +136,7 @@ func waitForCompileCompletion(ctx context.Context, options compileCompletionOpti case <-ctx.Done(): logCompileWaitCancelled(options, startedAt, attempts, lastStatus, lastErr, ctx.Err()) return nil, false, ctx.Err() - case <-time.After(options.pollInterval): + case <-ticker.C: } } diff --git a/cli/internal/cli/completion.go b/cli/internal/cli/completion.go index 1786d8d8ca..e106d2d000 100644 --- a/cli/internal/cli/completion.go +++ b/cli/internal/cli/completion.go @@ -24,6 +24,8 @@ const ( pwshProfileSubpath = "Documents/PowerShell/Microsoft.PowerShell_profile.ps1" ) +var completionBlockPattern = regexp.MustCompile(`(?s)\n?# >>> uloop completion >>>.*?# <<< uloop completion <<<\n?`) + func tryHandleCompletionRequest(args []string, cache toolsCache, stdout io.Writer, stderr io.Writer) (bool, int) { if len(args) == 0 { return false, 0 @@ -410,8 +412,7 @@ func installCompletionScript(configPath string, shellName string, script string) } func removeExistingCompletionBlock(content string) string { - pattern := regexp.MustCompile(`(?s)\n?# >>> uloop completion >>>.*?# <<< uloop completion <<<\n?`) - return pattern.ReplaceAllString(content, "") + return completionBlockPattern.ReplaceAllString(content, "") } func getCompletionScript(shellName string) string { diff --git a/cli/internal/cli/connection_retry.go b/cli/internal/cli/connection_retry.go index a72fc37f00..e16232d0e1 100644 --- a/cli/internal/cli/connection_retry.go +++ b/cli/internal/cli/connection_retry.go @@ -98,6 +98,8 @@ func sendWithTransientConnectionRetryAndResponseTimeout( }() focusAttempted := false + retryTicker := time.NewTicker(serverConnectionRetryPoll) + defer retryTicker.Stop() for { client := unityipc.NewClient(connection, version) if responseTimeout > 0 { @@ -126,7 +128,7 @@ func sendWithTransientConnectionRetryAndResponseTimeout( return lastOutcome, ctx.Err() } return lastOutcome, lastErr - case <-time.After(serverConnectionRetryPoll): + case <-retryTicker.C: } continue } @@ -209,7 +211,7 @@ func sendWithTransientConnectionRetryAndResponseTimeout( endpoint: connection.Endpoint.Address, cause: lastErr, } - case <-time.After(serverConnectionRetryPoll): + case <-retryTicker.C: } } } diff --git a/cli/internal/cli/control_play_mode_wait.go b/cli/internal/cli/control_play_mode_wait.go index ddbcb4e50d..5fb0060d84 100644 --- a/cli/internal/cli/control_play_mode_wait.go +++ b/cli/internal/cli/control_play_mode_wait.go @@ -134,6 +134,8 @@ func waitForControlPlayModeState( lastResponse := controlPlayModeResponse{} var lastErr error hasResponse := false + ticker := time.NewTicker(controlPlayModeStatePoll) + defer ticker.Stop() for { response, err := requestControlPlayModeStatus(waitContext, connection) if err == nil { @@ -158,7 +160,7 @@ func waitForControlPlayModeState( return lastResponse, false, fmt.Errorf("timed out waiting for play mode state: %w", lastErr) } return lastResponse, false, fmt.Errorf("timed out waiting for play mode state") - case <-time.After(controlPlayModeStatePoll): + case <-ticker.C: } } } diff --git a/cli/internal/cli/pause_point_wait.go b/cli/internal/cli/pause_point_wait.go index e9ea6724b1..0c5b57159a 100644 --- a/cli/internal/cli/pause_point_wait.go +++ b/cli/internal/cli/pause_point_wait.go @@ -299,6 +299,8 @@ func waitForPausePoint( lastResponse := pausePointStatusResponse{Id: options.id} var lastErr error hasResponse := false + ticker := time.NewTicker(pausePointStatusPoll) + defer ticker.Stop() for { response, err := queryPausePointStatus(waitContext, connection, options.id) if err == nil { @@ -334,7 +336,7 @@ func waitForPausePoint( return lastResponse, "", fmt.Errorf("timed out waiting for pause point status: %w", lastErr) } return lastResponse, pausePointWaitStateTimeout, nil - case <-time.After(pausePointStatusPoll): + case <-ticker.C: } } } diff --git a/cli/internal/cli/tool_readiness.go b/cli/internal/cli/tool_readiness.go index 3f6f554585..491174fff8 100644 --- a/cli/internal/cli/tool_readiness.go +++ b/cli/internal/cli/tool_readiness.go @@ -28,6 +28,8 @@ func waitForToolReadiness(ctx context.Context, projectRoot string) error { defer cancel() var lastErr error + ticker := time.NewTicker(toolReadinessPoll) + defer ticker.Stop() for { if err := probeToolReadinessSequence(timeoutContext, projectRoot); err == nil { return nil @@ -38,7 +40,7 @@ func waitForToolReadiness(ctx context.Context, projectRoot string) error { select { case <-timeoutContext.Done(): return toolReadinessDoneError(ctx, projectRoot, lastErr) - case <-time.After(toolReadinessPoll): + case <-ticker.C: } } } @@ -62,8 +64,12 @@ func toolReadinessDoneError(ctx context.Context, projectRoot string, cause error } func probeToolReadinessSequence(ctx context.Context, projectRoot string) error { + // The tool catalog is read from disk; it can change between poll ticks (Unity writes + // it during server startup) but not within one probe sequence, so read it once here + // instead of once per probe. + executeDynamicCodeAvailable := isExecuteDynamicCodeAvailable(projectRoot) for probeIndex := 0; probeIndex < toolReadinessProbeCount; probeIndex++ { - if err := probeToolReadiness(ctx, projectRoot); err != nil { + if err := probeToolReadiness(ctx, projectRoot, executeDynamicCodeAvailable); err != nil { return err } } @@ -71,7 +77,7 @@ func probeToolReadinessSequence(ctx context.Context, projectRoot string) error { return nil } -func probeToolReadiness(ctx context.Context, projectRoot string) error { +func probeToolReadiness(ctx context.Context, projectRoot string, executeDynamicCodeAvailable bool) error { probeContext, cancel := context.WithTimeout(ctx, toolReadinessProbeTimeout) defer cancel() @@ -80,7 +86,7 @@ func probeToolReadiness(ctx context.Context, projectRoot string) error { return err } - if !isExecuteDynamicCodeAvailable(projectRoot) { + if !executeDynamicCodeAvailable { _, err := unityipc.NewClient(connection, version).Send(probeContext, "get-version", map[string]any{}) return err }