From 76d4565a8dfb84dd2753506ad513b1cf8d9e86d5 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Sun, 12 Jul 2026 15:25:54 +0900 Subject: [PATCH 1/5] feat: List valid tool enum values and suggest keyboard key names (#1720) Co-authored-by: Cursor --- .../skills/uloop-simulate-keyboard/SKILL.md | 2 +- .../skills/uloop-simulate-keyboard/SKILL.md | 2 +- ...CaseInsensitiveStringEnumConverterTests.cs | 54 ++++++++++++++ ...nsensitiveStringEnumConverterTests.cs.meta | 11 +++ .../Editor/KeyboardKeyNameSuggesterTests.cs | 32 +++++++++ .../KeyboardKeyNameSuggesterTests.cs.meta | 11 +++ .../KeyboardKeyNameSuggester.cs | 70 ++++++++++++++++++ .../KeyboardKeyNameSuggester.cs.meta | 11 +++ .../SimulateKeyboardUseCase.cs | 8 ++- .../SimulateKeyboard/Skill/SKILL.md | 2 +- .../CaseInsensitiveStringEnumConverter.cs | 71 +++++++++++++++++++ ...CaseInsensitiveStringEnumConverter.cs.meta | 11 +++ .../Editor/ToolContracts/UnityCliLoopTool.cs | 3 +- 13 files changed, 283 insertions(+), 5 deletions(-) create mode 100644 Assets/Tests/Editor/CaseInsensitiveStringEnumConverterTests.cs create mode 100644 Assets/Tests/Editor/CaseInsensitiveStringEnumConverterTests.cs.meta create mode 100644 Assets/Tests/Editor/KeyboardKeyNameSuggesterTests.cs create mode 100644 Assets/Tests/Editor/KeyboardKeyNameSuggesterTests.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardKeyNameSuggester.cs create mode 100644 Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardKeyNameSuggester.cs.meta create mode 100644 Packages/src/Editor/ToolContracts/CaseInsensitiveStringEnumConverter.cs create mode 100644 Packages/src/Editor/ToolContracts/CaseInsensitiveStringEnumConverter.cs.meta diff --git a/.agents/skills/uloop-simulate-keyboard/SKILL.md b/.agents/skills/uloop-simulate-keyboard/SKILL.md index d8ab1fbcf..f7a742d0e 100644 --- a/.agents/skills/uloop-simulate-keyboard/SKILL.md +++ b/.agents/skills/uloop-simulate-keyboard/SKILL.md @@ -28,7 +28,7 @@ uloop simulate-keyboard --action --key [options] | Parameter | Type | Default | Description | |-----------|------|---------|-------------| | `--action` | enum | `Press` | `Press`, `KeyDown`, `KeyUp` | -| `--key` | string | (required) | Key name matching Input System Key enum (e.g. `W`, `Space`, `LeftShift`, `A`, `Enter`). Case-insensitive. | +| `--key` | string | (required) | Key name matching Input System Key enum (e.g. `W`, `Space`, `LeftShift`, `A`, `Enter`). Case-insensitive. Digit keys use `Digit0`-`Digit9` or `Numpad0`-`Numpad9`, not bare `0`-`9`. | | `--duration` | number | `0` | Hold duration in seconds for Press action (0 = one-shot tap). Ignored by KeyDown/KeyUp. | ### Actions diff --git a/.claude/skills/uloop-simulate-keyboard/SKILL.md b/.claude/skills/uloop-simulate-keyboard/SKILL.md index d8ab1fbcf..f7a742d0e 100644 --- a/.claude/skills/uloop-simulate-keyboard/SKILL.md +++ b/.claude/skills/uloop-simulate-keyboard/SKILL.md @@ -28,7 +28,7 @@ uloop simulate-keyboard --action --key [options] | Parameter | Type | Default | Description | |-----------|------|---------|-------------| | `--action` | enum | `Press` | `Press`, `KeyDown`, `KeyUp` | -| `--key` | string | (required) | Key name matching Input System Key enum (e.g. `W`, `Space`, `LeftShift`, `A`, `Enter`). Case-insensitive. | +| `--key` | string | (required) | Key name matching Input System Key enum (e.g. `W`, `Space`, `LeftShift`, `A`, `Enter`). Case-insensitive. Digit keys use `Digit0`-`Digit9` or `Numpad0`-`Numpad9`, not bare `0`-`9`. | | `--duration` | number | `0` | Hold duration in seconds for Press action (0 = one-shot tap). Ignored by KeyDown/KeyUp. | ### Actions diff --git a/Assets/Tests/Editor/CaseInsensitiveStringEnumConverterTests.cs b/Assets/Tests/Editor/CaseInsensitiveStringEnumConverterTests.cs new file mode 100644 index 000000000..ee7617f08 --- /dev/null +++ b/Assets/Tests/Editor/CaseInsensitiveStringEnumConverterTests.cs @@ -0,0 +1,54 @@ +using io.github.hatayama.UnityCliLoop.FirstPartyTools; +using io.github.hatayama.UnityCliLoop.ToolContracts; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; +using NUnit.Framework; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + [TestFixture] + public class CaseInsensitiveStringEnumConverterTests + { + // Locks lowercase enum strings that already worked before the converter was added. + [Test] + public void Deserialize_PlayModeAction_AcceptsLowercaseAction() + { + JObject token = JObject.Parse("{\"action\":\"play\"}"); + + ControlPlayModeSchema schema = token.ToObject( + UnityCliLoopToolParameterSerializer.CamelCaseSerializer); + + Assert.That(schema.Action, Is.EqualTo(PlayModeAction.Play)); + } + + // Verifies invalid enum values surface the allowed action names in the error. + [Test] + public void Deserialize_PlayModeAction_InvalidValue_ListsValidValues() + { + JObject token = JObject.Parse("{\"action\":\"jump\"}"); + + JsonSerializationException exception = Assert.Throws(() => + token.ToObject(UnityCliLoopToolParameterSerializer.CamelCaseSerializer)); + + Assert.That(exception.Message, Does.Contain("Valid values:")); + Assert.That(exception.Message, Does.Contain("Play")); + Assert.That(exception.Message, Does.Contain("Stop")); + Assert.That(exception.Message, Does.Contain("Pause")); + Assert.That(exception.Message, Does.Contain("Step")); + } + + // Verifies integer enum tokens are rejected with an explicit string-only message. + [Test] + public void Deserialize_PlayModeAction_IntegerToken_RejectedWithExplicitMessage() + { + JObject token = JObject.Parse("{\"action\":1}"); + + JsonSerializationException exception = Assert.Throws(() => + token.ToObject(UnityCliLoopToolParameterSerializer.CamelCaseSerializer)); + + Assert.That(exception.Message, Does.Contain("Enum parameter values must be JSON strings")); + Assert.That(exception.Message, Does.Contain("Integer")); + Assert.That(exception.Message, Does.Contain("PlayModeAction")); + } + } +} diff --git a/Assets/Tests/Editor/CaseInsensitiveStringEnumConverterTests.cs.meta b/Assets/Tests/Editor/CaseInsensitiveStringEnumConverterTests.cs.meta new file mode 100644 index 000000000..faf7d091a --- /dev/null +++ b/Assets/Tests/Editor/CaseInsensitiveStringEnumConverterTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: ed26577fb4ae643a89c5e3c8e8b455ec +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/KeyboardKeyNameSuggesterTests.cs b/Assets/Tests/Editor/KeyboardKeyNameSuggesterTests.cs new file mode 100644 index 000000000..3e3fe00ac --- /dev/null +++ b/Assets/Tests/Editor/KeyboardKeyNameSuggesterTests.cs @@ -0,0 +1,32 @@ +#if ULOOP_HAS_INPUT_SYSTEM +using System.Collections.Generic; +using System.Linq; +using io.github.hatayama.UnityCliLoop.FirstPartyTools; +using NUnit.Framework; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + [TestFixture] + public class KeyboardKeyNameSuggesterTests + { + // Verifies bare digit keys suggest both Digit and Numpad enum names. + [Test] + public void Suggest_ForBareDigit_IncludesDigitAndNumpadNames() + { + IReadOnlyList suggestions = KeyboardKeyNameSuggester.Suggest("3"); + + Assert.That(suggestions, Does.Contain("Digit3")); + Assert.That(suggestions, Does.Contain("Numpad3")); + } + + // Verifies partial key names still return close enum matches. + [Test] + public void Suggest_ForPartialName_ReturnsPrefixMatches() + { + IReadOnlyList suggestions = KeyboardKeyNameSuggester.Suggest("LeftSh"); + + Assert.That(suggestions.Any(name => name == "LeftShift"), Is.True); + } + } +} +#endif diff --git a/Assets/Tests/Editor/KeyboardKeyNameSuggesterTests.cs.meta b/Assets/Tests/Editor/KeyboardKeyNameSuggesterTests.cs.meta new file mode 100644 index 000000000..5ee5d3386 --- /dev/null +++ b/Assets/Tests/Editor/KeyboardKeyNameSuggesterTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 1e787247d922846cb97905834c470a4f +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardKeyNameSuggester.cs b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardKeyNameSuggester.cs new file mode 100644 index 000000000..fdbfa27bf --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardKeyNameSuggester.cs @@ -0,0 +1,70 @@ +using System; +using System.Collections.Generic; +#if ULOOP_HAS_INPUT_SYSTEM +using UnityEngine.InputSystem; +#endif + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Suggests Input System Key enum names for unknown simulate-keyboard key inputs. + /// + internal static class KeyboardKeyNameSuggester + { + private const int MaxSuggestions = 6; + + public static IReadOnlyList Suggest(string invalidKeyName) + { + if (string.IsNullOrWhiteSpace(invalidKeyName)) + { + return Array.Empty(); + } + +#if !ULOOP_HAS_INPUT_SYSTEM + return Array.Empty(); +#else + string trimmed = invalidKeyName.Trim(); + List suggestions = new(); + + if (trimmed.Length == 1 && char.IsDigit(trimmed[0])) + { + string digit = trimmed; + AddUnique(suggestions, $"Digit{digit}"); + AddUnique(suggestions, $"Numpad{digit}"); + } + + string[] allNames = Enum.GetNames(typeof(Key)); + StringComparison comparison = StringComparison.OrdinalIgnoreCase; + for (int index = 0; index < allNames.Length; index++) + { + string name = allNames[index]; + if (name.Contains(trimmed, comparison)) + { + AddUnique(suggestions, name); + if (suggestions.Count >= MaxSuggestions) + { + break; + } + } + } + + return suggestions; +#endif + } + +#if ULOOP_HAS_INPUT_SYSTEM + private static void AddUnique(List suggestions, string candidate) + { + for (int index = 0; index < suggestions.Count; index++) + { + if (string.Equals(suggestions[index], candidate, StringComparison.OrdinalIgnoreCase)) + { + return; + } + } + + suggestions.Add(candidate); + } +#endif + } +} diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardKeyNameSuggester.cs.meta b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardKeyNameSuggester.cs.meta new file mode 100644 index 000000000..20117c593 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardKeyNameSuggester.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 9064de4241e224b38bdc0e3cbadc15f7 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs index 7f94d1884..94531d9db 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs @@ -1,5 +1,6 @@ #nullable enable using System; +using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; #if ULOOP_HAS_INPUT_SYSTEM @@ -70,10 +71,15 @@ public async Task ExecuteAsync( string normalizedKey = NormalizeKeyName(parameters.Key); if (!Enum.TryParse(normalizedKey, ignoreCase: true, out Key key) || key == Key.None) { + IReadOnlyList suggestions = KeyboardKeyNameSuggester.Suggest(parameters.Key); + string suggestionText = suggestions.Count == 0 + ? string.Empty + : $" Did you mean: {string.Join(", ", suggestions)}?"; return new SimulateKeyboardResponse { Success = false, - Message = $"Invalid key name: \"{parameters.Key}\". Use Input System Key enum names (e.g. \"W\", \"Space\", \"LeftShift\", \"A\", \"Enter\").", + Message = + $"Invalid key name: \"{parameters.Key}\". Use Input System Key enum names (e.g. \"W\", \"Space\", \"LeftShift\", \"A\", \"Enter\").{suggestionText}", Action = parameters.Action.ToString() }; } diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.md b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.md index d8ab1fbcf..f7a742d0e 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.md +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.md @@ -28,7 +28,7 @@ uloop simulate-keyboard --action --key [options] | Parameter | Type | Default | Description | |-----------|------|---------|-------------| | `--action` | enum | `Press` | `Press`, `KeyDown`, `KeyUp` | -| `--key` | string | (required) | Key name matching Input System Key enum (e.g. `W`, `Space`, `LeftShift`, `A`, `Enter`). Case-insensitive. | +| `--key` | string | (required) | Key name matching Input System Key enum (e.g. `W`, `Space`, `LeftShift`, `A`, `Enter`). Case-insensitive. Digit keys use `Digit0`-`Digit9` or `Numpad0`-`Numpad9`, not bare `0`-`9`. | | `--duration` | number | `0` | Hold duration in seconds for Press action (0 = one-shot tap). Ignored by KeyDown/KeyUp. | ### Actions diff --git a/Packages/src/Editor/ToolContracts/CaseInsensitiveStringEnumConverter.cs b/Packages/src/Editor/ToolContracts/CaseInsensitiveStringEnumConverter.cs new file mode 100644 index 000000000..9d5e97449 --- /dev/null +++ b/Packages/src/Editor/ToolContracts/CaseInsensitiveStringEnumConverter.cs @@ -0,0 +1,71 @@ +using System; +using Newtonsoft.Json; + +namespace io.github.hatayama.UnityCliLoop.ToolContracts +{ + /// + /// Parses tool enum parameters from JSON string tokens and lists valid enum names on failure. + /// Case-insensitive string matching already worked with the default Newtonsoft enum reader; + /// this converter exists so invalid values return an explicit Valid values list. + /// Integer tokens are rejected because the CLI only sends string enum values. + /// + internal sealed class CaseInsensitiveStringEnumConverter : JsonConverter + { + public override bool CanConvert(Type objectType) + { + Type enumType = Nullable.GetUnderlyingType(objectType) ?? objectType; + return enumType.IsEnum; + } + + public override object ReadJson( + JsonReader reader, + Type objectType, + object existingValue, + JsonSerializer serializer) + { + if (reader.TokenType == JsonToken.Null) + { + Type underlyingType = Nullable.GetUnderlyingType(objectType); + if (underlyingType != null) + { + return null; + } + + throw new JsonSerializationException( + $"Cannot convert null value to {objectType.Name}."); + } + + Type enumType = Nullable.GetUnderlyingType(objectType) ?? objectType; + + if (reader.TokenType != JsonToken.String) + { + throw new JsonSerializationException( + $"Enum parameter values must be JSON strings. Received {reader.TokenType} token " + + $"'{reader.Value}' for type '{enumType.Name}'."); + } + + string rawValue = reader.Value?.ToString() ?? string.Empty; + if (Enum.TryParse(enumType, rawValue, ignoreCase: true, out object parsed)) + { + return parsed; + } + + string[] validValues = Enum.GetNames(enumType); + string pathSuffix = string.IsNullOrEmpty(reader.Path) ? string.Empty : $" Path '{reader.Path}'."; + throw new JsonSerializationException( + $"Error converting value \"{rawValue}\" to type '{enumType.Name}'.{pathSuffix} " + + $"Valid values: {string.Join(", ", validValues)}."); + } + + public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer) + { + if (value == null) + { + writer.WriteNull(); + return; + } + + writer.WriteValue(value.ToString()); + } + } +} diff --git a/Packages/src/Editor/ToolContracts/CaseInsensitiveStringEnumConverter.cs.meta b/Packages/src/Editor/ToolContracts/CaseInsensitiveStringEnumConverter.cs.meta new file mode 100644 index 000000000..df21e4065 --- /dev/null +++ b/Packages/src/Editor/ToolContracts/CaseInsensitiveStringEnumConverter.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: fa84b16583af749adb82d8a60f3c4f58 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/ToolContracts/UnityCliLoopTool.cs b/Packages/src/Editor/ToolContracts/UnityCliLoopTool.cs index 831a7d6e2..ab76395d0 100644 --- a/Packages/src/Editor/ToolContracts/UnityCliLoopTool.cs +++ b/Packages/src/Editor/ToolContracts/UnityCliLoopTool.cs @@ -15,7 +15,8 @@ internal static class UnityCliLoopToolParameterSerializer // 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() + ContractResolver = new Newtonsoft.Json.Serialization.CamelCasePropertyNamesContractResolver(), + Converters = { new CaseInsensitiveStringEnumConverter() } }); } From ba0c3b545b96503d8b3b99feb2b7dfa7eb84f229 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Sun, 12 Jul 2026 15:43:41 +0900 Subject: [PATCH 2/5] feat: Add busy-stall focus rescue and richer server_busy / compile guidance (#1721) Co-authored-by: Cursor --- .../Editor/CompileResponseFactoryTests.cs | 92 +++++++++++++++++++ .../ToolExecutionEditorReadyPolicyTests.cs | 19 ++++ .../UnityCliLoopEditorStateGuard.cs | 4 +- .../UnityCliLoopToolBusyException.cs | 10 +- .../UnityCliLoopToolExecutionService.cs | 4 +- .../Domain/ToolExecutionEditorReadyPolicy.cs | 26 +++++- .../Compile/CompileResponse.cs | 5 + .../Compile/CompileResponseFactory.cs | 29 +++++- .../Api/JsonRpcResponseFactory.cs | 4 +- .../Infrastructure/Api/ServerBusyErrorData.cs | 12 ++- cli/common/errors/busy_editor_state.go | 66 +++++++++++++ cli/common/errors/busy_editor_state_test.go | 57 ++++++++++++ cli/common/errors/error_envelope.go | 11 ++- cli/common/errors/error_envelope_test.go | 22 +++++ .../dispatcher/error_envelope_test.go | 26 ++++++ cli/dispatcher/shared-inputs-stamp.json | 2 +- .../projectrunner/connection_retry.go | 16 ++++ .../projectrunner/connection_retry_test.go | 92 +++++++++++++++++++ cli/project-runner/shared-inputs-stamp.json | 2 +- 19 files changed, 481 insertions(+), 18 deletions(-) create mode 100644 cli/common/errors/busy_editor_state.go create mode 100644 cli/common/errors/busy_editor_state_test.go diff --git a/Assets/Tests/Editor/CompileResponseFactoryTests.cs b/Assets/Tests/Editor/CompileResponseFactoryTests.cs index a3ac89f1e..2b6b0f605 100644 --- a/Assets/Tests/Editor/CompileResponseFactoryTests.cs +++ b/Assets/Tests/Editor/CompileResponseFactoryTests.cs @@ -184,6 +184,98 @@ public void CreateResponse_WhenForceCompileHasPreservedFailure_MapsDetailedIssue Assert.That(response.Message, Does.Contain("externally")); } + [Test] + public void CreateResponse_WhenExternalSceneCannotBeResolved_AddsNextActions() + { + // Verifies unresolved external Scene changes include execute-dynamic-code reload guidance. + CompilerMessage error = new CompilerMessage + { + type = CompilerMessageType.Error, + message = "Compilation cannot resolve externally changed Scene files before compile.", + file = "Assets/Scenes/SampleScene.unity", + line = 0 + }; + CompileResult result = new CompileResult( + success: false, + errorCount: 1, + warningCount: 0, + completedAt: DateTime.Now, + messages: new[] { error }, + errors: new[] { error }, + warnings: Array.Empty(), + message: error.message); + + CompileResponse response = + CompileResponseFactory.CreateResponse(result, forceRecompile: false); + + Assert.That(response.NextActions, Is.Not.Null); + Assert.That(response.NextActions, Has.Length.EqualTo(2)); + Assert.That(response.NextActions[0], Does.Contain("execute-dynamic-code")); + Assert.That(response.NextActions[0], Does.Contain("EditorSceneManager.OpenScene")); + Assert.That(response.NextActions[0], Does.Contain("OpenSceneMode.Single discards unsaved")); + Assert.That(response.NextActions[1], Does.Contain("unsaved in-editor changes")); + Assert.That(response.NextActions[1], Does.Not.Contain("stop-on-external-scene-changes")); + } + + [Test] + public void CreateResponse_WhenExternalSceneCannotBeReloaded_AddsNextActions() + { + // Verifies reload-failure messages from ExternalSceneChangeResolver also surface reload guidance. + CompilerMessage error = new CompilerMessage + { + type = CompilerMessageType.Error, + message = + "Compilation cannot reload externally changed Scene files before compile. " + + "Scenes that could not be reloaded: Assets/Scenes/SampleScene.unity.", + file = "Assets/Scenes/SampleScene.unity", + line = 0 + }; + CompileResult result = new CompileResult( + success: false, + errorCount: 1, + warningCount: 0, + completedAt: DateTime.Now, + messages: new[] { error }, + errors: new[] { error }, + warnings: Array.Empty(), + message: error.message); + + CompileResponse response = + CompileResponseFactory.CreateResponse(result, forceRecompile: false); + + Assert.That(response.NextActions, Is.Not.Null); + Assert.That(response.NextActions, Has.Length.EqualTo(2)); + } + + [Test] + public void CreateResponse_WhenExternalSceneStopMessageUsesDifferentWording_DoesNotAddNextActions() + { + // Verifies the stopped variant ("changed externally") does not match the NextActions substring gate. + CompilerMessage error = new CompilerMessage + { + type = CompilerMessageType.Error, + message = + "Compilation stopped because open Scene files changed externally. " + + "External Scene changes: Assets/Scenes/SampleScene.unity.", + file = "Assets/Scenes/SampleScene.unity", + line = 0 + }; + CompileResult result = new CompileResult( + success: false, + errorCount: 1, + warningCount: 0, + completedAt: DateTime.Now, + messages: new[] { error }, + errors: new[] { error }, + warnings: Array.Empty(), + message: error.message); + + CompileResponse response = + CompileResponseFactory.CreateResponse(result, forceRecompile: false); + + Assert.That(response.NextActions, Is.Null); + } + [Test] public void CreateResponse_WhenUnityTestFrameworkSymbolIsMissing_AddsTestAsmdefHint() { diff --git a/Assets/Tests/Editor/ToolExecutionEditorReadyPolicyTests.cs b/Assets/Tests/Editor/ToolExecutionEditorReadyPolicyTests.cs index 3bc281544..e0ba75bde 100644 --- a/Assets/Tests/Editor/ToolExecutionEditorReadyPolicyTests.cs +++ b/Assets/Tests/Editor/ToolExecutionEditorReadyPolicyTests.cs @@ -47,6 +47,25 @@ public void Evaluate_WhenPlayModeControlRunsDuringEditorUpdate_ShouldReturnAsset Assert.That(decision.IsPaused, Is.True); } + [Test] + public void Evaluate_WhenDynamicCodeRunsDuringCompilationAndUpdate_ShouldReportBothEditorFlags() + { + // Verifies busy decisions copy live compile and import flags instead of inferring mutual exclusion. + ToolExecutionEditorState editorState = new ToolExecutionEditorState( + isCompiling: true, + isUpdating: true, + isPlaying: false, + isPaused: false); + + ToolExecutionEditorReadyDecision decision = ToolExecutionEditorReadyPolicy.Evaluate( + UnityCliLoopConstants.TOOL_NAME_EXECUTE_DYNAMIC_CODE, + editorState); + + Assert.That(decision.IsReady, Is.False); + Assert.That(decision.IsCompiling, Is.True); + Assert.That(decision.IsUpdating, Is.True); + } + [Test] public void Evaluate_WhenReadOnlyToolRunsDuringBusyEditorState_ShouldReturnReadyDecision() { diff --git a/Packages/src/Editor/Application/UnityCliLoopEditorStateGuard.cs b/Packages/src/Editor/Application/UnityCliLoopEditorStateGuard.cs index 6965f26fb..69eda41a5 100644 --- a/Packages/src/Editor/Application/UnityCliLoopEditorStateGuard.cs +++ b/Packages/src/Editor/Application/UnityCliLoopEditorStateGuard.cs @@ -29,7 +29,9 @@ public static void Validate(string toolName, IEditorRuntimeStatePort editorRunti decision.RunningOperationName, decision.RequestedToolName, decision.IsPlaying, - decision.IsPaused); + decision.IsPaused, + decision.IsCompiling, + decision.IsUpdating); } } } diff --git a/Packages/src/Editor/Application/UnityCliLoopToolBusyException.cs b/Packages/src/Editor/Application/UnityCliLoopToolBusyException.cs index 0a57a6721..14db031ab 100644 --- a/Packages/src/Editor/Application/UnityCliLoopToolBusyException.cs +++ b/Packages/src/Editor/Application/UnityCliLoopToolBusyException.cs @@ -11,13 +11,17 @@ public UnityCliLoopToolBusyException( string runningToolName, string requestedToolName, bool? isPlaying = null, - bool? isPaused = null) + bool? isPaused = null, + bool? isCompiling = null, + bool? isUpdating = null) : base(CreateMessage(runningToolName, requestedToolName)) { RunningToolName = runningToolName; RequestedToolName = requestedToolName; IsPlaying = isPlaying; IsPaused = isPaused; + IsCompiling = isCompiling; + IsUpdating = isUpdating; } public string RunningToolName { get; } @@ -28,6 +32,10 @@ public UnityCliLoopToolBusyException( public bool? IsPaused { get; } + public bool? IsCompiling { get; } + + public bool? IsUpdating { get; } + private static string CreateMessage(string runningToolName, string requestedToolName) { return $"Unity is busy running '{runningToolName}'. Retry '{requestedToolName}' after the running tool completes."; diff --git a/Packages/src/Editor/Application/UnityCliLoopToolExecutionService.cs b/Packages/src/Editor/Application/UnityCliLoopToolExecutionService.cs index 5a4db0cc8..5ef60d659 100644 --- a/Packages/src/Editor/Application/UnityCliLoopToolExecutionService.cs +++ b/Packages/src/Editor/Application/UnityCliLoopToolExecutionService.cs @@ -76,7 +76,9 @@ internal static UnityCliLoopToolBusyException CreateBusyException( runningToolName, requestedToolName, editorRuntimeStatePort.IsPlaying, - editorRuntimeStatePort.IsPaused); + editorRuntimeStatePort.IsPaused, + editorRuntimeStatePort.IsCompiling, + editorRuntimeStatePort.IsUpdating); } (bool HasValue, bool IsPlaying, bool IsPaused) playState = diff --git a/Packages/src/Editor/Domain/ToolExecutionEditorReadyPolicy.cs b/Packages/src/Editor/Domain/ToolExecutionEditorReadyPolicy.cs index c9cacf385..5637273fd 100644 --- a/Packages/src/Editor/Domain/ToolExecutionEditorReadyPolicy.cs +++ b/Packages/src/Editor/Domain/ToolExecutionEditorReadyPolicy.cs @@ -37,7 +37,9 @@ internal static ToolExecutionEditorReadyDecision Evaluate(string toolName, ToolE UnityCompileOperationName, toolName, editorState.IsPlaying, - editorState.IsPaused); + editorState.IsPaused, + editorState.IsCompiling, + editorState.IsUpdating); } if ((condition & GuardCondition.NotUpdating) != 0 && editorState.IsUpdating) @@ -46,7 +48,9 @@ internal static ToolExecutionEditorReadyDecision Evaluate(string toolName, ToolE UnityAssetDatabaseUpdateOperationName, toolName, editorState.IsPlaying, - editorState.IsPaused); + editorState.IsPaused, + editorState.IsCompiling, + editorState.IsUpdating); } return ToolExecutionEditorReadyDecision.Ready(toolName); @@ -98,13 +102,17 @@ internal readonly struct ToolExecutionEditorReadyDecision public readonly string RequestedToolName; public readonly bool IsPlaying; public readonly bool IsPaused; + public readonly bool IsCompiling; + public readonly bool IsUpdating; private ToolExecutionEditorReadyDecision( bool isReady, string runningOperationName, string requestedToolName, bool isPlaying, - bool isPaused) + bool isPaused, + bool isCompiling, + bool isUpdating) { Debug.Assert(!string.IsNullOrWhiteSpace(requestedToolName), "requestedToolName must not be null or whitespace"); Debug.Assert(isReady || !string.IsNullOrWhiteSpace(runningOperationName), "runningOperationName must not be null or whitespace for busy decisions"); @@ -114,6 +122,8 @@ private ToolExecutionEditorReadyDecision( RequestedToolName = requestedToolName; IsPlaying = isPlaying; IsPaused = isPaused; + IsCompiling = isCompiling; + IsUpdating = isUpdating; } public static ToolExecutionEditorReadyDecision Ready(string requestedToolName) @@ -123,6 +133,8 @@ public static ToolExecutionEditorReadyDecision Ready(string requestedToolName) string.Empty, requestedToolName, false, + false, + false, false); } @@ -130,14 +142,18 @@ public static ToolExecutionEditorReadyDecision Busy( string runningOperationName, string requestedToolName, bool isPlaying, - bool isPaused) + bool isPaused, + bool isCompiling, + bool isUpdating) { return new ToolExecutionEditorReadyDecision( false, runningOperationName, requestedToolName, isPlaying, - isPaused); + isPaused, + isCompiling, + isUpdating); } } } diff --git a/Packages/src/Editor/FirstPartyTools/Compile/CompileResponse.cs b/Packages/src/Editor/FirstPartyTools/Compile/CompileResponse.cs index 56391a7b6..386fe6c89 100644 --- a/Packages/src/Editor/FirstPartyTools/Compile/CompileResponse.cs +++ b/Packages/src/Editor/FirstPartyTools/Compile/CompileResponse.cs @@ -67,6 +67,11 @@ public class CompileResponse : UnityCliLoopToolResponse /// public string Message { get; set; } + /// + /// Optional recovery steps when compile cannot proceed automatically. + /// + public string[] NextActions { get; set; } + /// /// Unity project root path (from UnityEngine.Application.dataPath). /// Set only when WaitForDomainReload=true so the CLI can report which project produced the result. diff --git a/Packages/src/Editor/FirstPartyTools/Compile/CompileResponseFactory.cs b/Packages/src/Editor/FirstPartyTools/Compile/CompileResponseFactory.cs index 9cc8fca10..e8f66c774 100644 --- a/Packages/src/Editor/FirstPartyTools/Compile/CompileResponseFactory.cs +++ b/Packages/src/Editor/FirstPartyTools/Compile/CompileResponseFactory.cs @@ -15,6 +15,16 @@ internal static class CompileResponseFactory private const string MissingTestFrameworkReferenceHint = "Possible test asmdef issue: Unity test framework symbols are missing. Make sure com.unity.test-framework is installed and add optionalUnityReferences: [\"TestAssemblies\"] or enable testAssemblies on the test asmdef."; + private static readonly string[] ExternalSceneChangeNextActions = + { + "Reload each changed Scene with execute-dynamic-code, for example: " + + "`uloop execute-dynamic-code --code 'using UnityEditor.SceneManagement; using UnityEngine.SceneManagement; " + + "EditorSceneManager.OpenScene(\"\", OpenSceneMode.Single);'` " + + "using the Scene path from Errors[].File. OpenSceneMode.Single discards unsaved in-editor Scene changes.", + "If the Scene has unsaved in-editor changes that conflict with the external change, decide first: " + + "save the Scene to keep editor changes, or reload it to take the external content." + }; + internal static CompileResponse CreateResponse( CompileResult result, bool forceRecompile) @@ -37,13 +47,30 @@ internal static CompileResponse CreateResponse( message: result.Message ?? "Compilation status is unknown. Use get-logs to inspect the compiler output."); } - return new CompileResponse( + CompileResponse response = new CompileResponse( success: result.Success, errorCount: result.Errors?.Length ?? 0, warningCount: result.Warnings?.Length ?? 0, errors: ToIssues(result.Errors), warnings: ToIssues(result.Warnings), message: AddMissingTestFrameworkReferenceHint(result.Message, result.Errors)); + response.NextActions = CreateExternalSceneChangeNextActions(result.Message); + return response; + } + + private static string[] CreateExternalSceneChangeNextActions(string message) + { + if (string.IsNullOrWhiteSpace(message)) + { + return null; + } + + if (!message.Contains("externally changed Scene files", StringComparison.Ordinal)) + { + return null; + } + + return ExternalSceneChangeNextActions; } private static CompileResponse CreateForceCompileResult(CompileResult result) diff --git a/Packages/src/Editor/Infrastructure/Api/JsonRpcResponseFactory.cs b/Packages/src/Editor/Infrastructure/Api/JsonRpcResponseFactory.cs index b8c4edda0..d7133bb9e 100644 --- a/Packages/src/Editor/Infrastructure/Api/JsonRpcResponseFactory.cs +++ b/Packages/src/Editor/Infrastructure/Api/JsonRpcResponseFactory.cs @@ -79,7 +79,9 @@ internal static string CreateErrorResponse(object id, Exception ex) busyEx.IsPlaying, busyEx.IsPaused, exceptionResponse.Explanation ?? ex.Message, - EditorMainThreadLivenessTracker.SecondsSinceLastMainThreadTick()); + EditorMainThreadLivenessTracker.SecondsSinceLastMainThreadTick(), + busyEx.IsCompiling, + busyEx.IsUpdating); } else { diff --git a/Packages/src/Editor/Infrastructure/Api/ServerBusyErrorData.cs b/Packages/src/Editor/Infrastructure/Api/ServerBusyErrorData.cs index 93b9b2fea..4b2f9f8ee 100644 --- a/Packages/src/Editor/Infrastructure/Api/ServerBusyErrorData.cs +++ b/Packages/src/Editor/Infrastructure/Api/ServerBusyErrorData.cs @@ -19,6 +19,12 @@ public class ServerBusyErrorData : JsonRpcErrorData [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] public bool? isPaused { get; } + [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] + public bool? isCompiling { get; } + + [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] + public bool? isUpdating { get; } + // Lets a client distinguish "main thread still ticking, tool genuinely running long" from // a frozen/deadlocked Editor while BUSY, without needing native stack sampling. [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] @@ -30,13 +36,17 @@ public ServerBusyErrorData( bool? isPlaying, bool? isPaused, string message, - double? secondsSinceLastMainThreadTick = null) + double? secondsSinceLastMainThreadTick = null, + bool? isCompiling = null, + bool? isUpdating = null) : base(message) { this.runningToolName = runningToolName; this.requestedToolName = requestedToolName; this.isPlaying = isPlaying; this.isPaused = isPaused; + this.isCompiling = isCompiling; + this.isUpdating = isUpdating; this.secondsSinceLastMainThreadTick = secondsSinceLastMainThreadTick; } } diff --git a/cli/common/errors/busy_editor_state.go b/cli/common/errors/busy_editor_state.go new file mode 100644 index 000000000..1bbde241e --- /dev/null +++ b/cli/common/errors/busy_editor_state.go @@ -0,0 +1,66 @@ +package clierrors + +const unityServerBusyResponsivenessStallThresholdSeconds = 5.0 + +func unityServerBusyNextActions(data map[string]any) []string { + actions := []string{ + "Wait for the running Unity command to complete.", + "Retry the command after Unity reports it is no longer busy.", + } + + if rpcBoolData(data, "isCompiling") { + actions = append( + []string{"Unity is compiling scripts; wait for compilation to finish before retrying."}, + actions...) + } else if rpcBoolData(data, "isUpdating") { + actions = append( + []string{"Unity is importing assets; wait for the asset database update to finish before retrying."}, + actions...) + } + + if stallSeconds, ok := rpcFloatData(data, "secondsSinceLastMainThreadTick"); ok && + stallSeconds >= unityServerBusyResponsivenessStallThresholdSeconds { + actions = append( + actions, + "Run a light command such as `uloop get-logs --max-count 1` to check whether Unity is still responsive before treating this as a freeze.") + } + + return actions +} + +func unityServerBusyEditorActivitySummary(data map[string]any) map[string]any { + if data == nil { + return nil + } + + summary := map[string]any{} + copyOptionalBoolField(summary, data, "isCompiling") + copyOptionalBoolField(summary, data, "isUpdating") + copyOptionalBoolField(summary, data, "isPlaying") + copyOptionalBoolField(summary, data, "isPaused") + if stallSeconds, ok := rpcFloatData(data, "secondsSinceLastMainThreadTick"); ok { + summary["secondsSinceLastMainThreadTick"] = stallSeconds + } + if len(summary) == 0 { + return nil + } + return summary +} + +func rpcBoolData(data map[string]any, key string) bool { + value, ok := data[key].(bool) + return ok && value +} + +func rpcFloatData(data map[string]any, key string) (float64, bool) { + value, ok := data[key].(float64) + return value, ok +} + +func copyOptionalBoolField(destination map[string]any, source map[string]any, key string) { + value, ok := source[key].(bool) + if !ok || !value { + return + } + destination[key] = value +} diff --git a/cli/common/errors/busy_editor_state_test.go b/cli/common/errors/busy_editor_state_test.go new file mode 100644 index 000000000..2f4eb48a4 --- /dev/null +++ b/cli/common/errors/busy_editor_state_test.go @@ -0,0 +1,57 @@ +package clierrors + +import "testing" + +// Verifies compiling busy payloads surface script-compile guidance in NextActions. +func TestUnityServerBusyNextActions_WhenCompiling_IncludesCompileGuidance(t *testing.T) { + data := map[string]any{ + "isCompiling": true, + } + + actions := unityServerBusyNextActions(data) + if len(actions) < 3 { + t.Fatalf("expected compile-specific guidance, got %#v", actions) + } + if actions[0] != "Unity is compiling scripts; wait for compilation to finish before retrying." { + t.Fatalf("first action mismatch: %#v", actions) + } +} + +// Verifies stalled main-thread ticks add a lightweight responsiveness check action. +func TestUnityServerBusyNextActions_WhenMainThreadStalled_IncludesResponsivenessCheck(t *testing.T) { + data := map[string]any{ + "secondsSinceLastMainThreadTick": 12.0, + } + + actions := unityServerBusyNextActions(data) + found := false + for _, action := range actions { + if action == "Run a light command such as `uloop get-logs --max-count 1` to check whether Unity is still responsive before treating this as a freeze." { + found = true + break + } + } + if !found { + t.Fatalf("responsiveness action missing: %#v", actions) + } +} + +// Verifies editor activity summaries copy only populated busy-state fields. +func TestUnityServerBusyEditorActivitySummary_CopiesKnownFields(t *testing.T) { + data := map[string]any{ + "isCompiling": true, + "isUpdating": false, + "secondsSinceLastMainThreadTick": 1.5, + } + + summary := unityServerBusyEditorActivitySummary(data) + if summary["isCompiling"] != true { + t.Fatalf("isCompiling mismatch: %#v", summary) + } + if _, ok := summary["isUpdating"]; ok { + t.Fatalf("false bool fields should be omitted: %#v", summary) + } + if summary["secondsSinceLastMainThreadTick"] != 1.5 { + t.Fatalf("stall seconds mismatch: %#v", summary) + } +} diff --git a/cli/common/errors/error_envelope.go b/cli/common/errors/error_envelope.go index d35d6393f..acdbe8033 100644 --- a/cli/common/errors/error_envelope.go +++ b/cli/common/errors/error_envelope.go @@ -140,6 +140,10 @@ func unityServerBusyError( data map[string]any, context ErrorContext, ) CLIError { + if editorActivity := unityServerBusyEditorActivitySummary(data); editorActivity != nil { + details["EditorActivity"] = editorActivity + } + return CLIError{ ErrorCode: errorCodeUnityServerBusy, Phase: ErrorPhaseDispatch, @@ -148,11 +152,8 @@ func unityServerBusyError( SafeToRetry: true, ProjectRoot: context.ProjectRoot, Command: context.Command, - NextActions: []string{ - "Wait for the running Unity command to complete.", - "Retry the command after Unity reports it is no longer busy.", - }, - Details: details, + NextActions: unityServerBusyNextActions(data), + Details: details, } } diff --git a/cli/common/errors/error_envelope_test.go b/cli/common/errors/error_envelope_test.go index 58b1aca17..7860144d8 100644 --- a/cli/common/errors/error_envelope_test.go +++ b/cli/common/errors/error_envelope_test.go @@ -267,6 +267,28 @@ func TestClassifyServerBusyRPCError(t *testing.T) { } } +func TestClassifyServerBusyRPCError_WhenCompiling_IncludesEditorActivityAndGuidance(t *testing.T) { + // Verifies compiling busy payloads add editor activity details and compile-specific guidance. + err := &unityipc.RPCError{ + Code: -32603, + Message: "Unity is busy running 'unity-compile'.", + Data: json.RawMessage( + `{"type":"server_busy","runningToolName":"unity-compile","requestedToolName":"get-logs","isCompiling":true}`), + } + + cliErr := ClassifyError(err, ErrorContext{ProjectRoot: "/tmp/MyProject", Command: "get-logs"}) + editorActivity, ok := cliErr.Details["EditorActivity"].(map[string]any) + if !ok { + t.Fatalf("editor activity missing: %#v", cliErr.Details) + } + if editorActivity["isCompiling"] != true { + t.Fatalf("isCompiling mismatch: %#v", editorActivity) + } + if cliErr.NextActions[0] != "Unity is compiling scripts; wait for compilation to finish before retrying." { + t.Fatalf("next actions mismatch: %#v", cliErr.NextActions) + } +} + func TestWriteClassifiedServerBusyRPCErrorWritesErrorEnvelope(t *testing.T) { // Verifies server_busy output uses the same machine-readable error envelope as other failures. err := &unityipc.RPCError{ diff --git a/cli/dispatcher/internal/dispatcher/error_envelope_test.go b/cli/dispatcher/internal/dispatcher/error_envelope_test.go index a0b7ec499..c7518c316 100644 --- a/cli/dispatcher/internal/dispatcher/error_envelope_test.go +++ b/cli/dispatcher/internal/dispatcher/error_envelope_test.go @@ -1,6 +1,7 @@ package dispatcher import ( + "encoding/json" "errors" "strings" "testing" @@ -8,6 +9,7 @@ import ( clierrors "github.com/hatayama/unity-cli-loop/common/errors" "github.com/hatayama/unity-cli-loop/common/clicore" + "github.com/hatayama/unity-cli-loop/common/unityipc" ) func TestClassifyLaunchStartupTimeoutError(t *testing.T) { @@ -79,3 +81,27 @@ func TestClassifyInstallUnsupportedOS(t *testing.T) { t.Fatalf("next actions mismatch: %#v", cliErr.NextActions) } } + +func TestClassifyServerBusyRPCError_WhenCompiling_IncludesEditorActivity(t *testing.T) { + // Verifies dispatcher-side busy classification surfaces compile-specific editor activity guidance. + cliErr := clierrors.ClassifyError( + &unityipc.RPCError{ + Code: -32603, + Message: "Unity is busy running 'unity-compile'.", + Data: json.RawMessage( + `{"type":"server_busy","runningToolName":"unity-compile","requestedToolName":"get-logs","isCompiling":true}`), + }, + clierrors.ErrorContext{ProjectRoot: "/tmp/MyProject", Command: "get-logs"}, + ) + + editorActivity, ok := cliErr.Details["EditorActivity"].(map[string]any) + if !ok { + t.Fatalf("editor activity missing: %#v", cliErr.Details) + } + if editorActivity["isCompiling"] != true { + t.Fatalf("isCompiling mismatch: %#v", editorActivity) + } + if cliErr.NextActions[0] != "Unity is compiling scripts; wait for compilation to finish before retrying." { + t.Fatalf("next actions mismatch: %#v", cliErr.NextActions) + } +} diff --git a/cli/dispatcher/shared-inputs-stamp.json b/cli/dispatcher/shared-inputs-stamp.json index 1c4102845..2a128a874 100644 --- a/cli/dispatcher/shared-inputs-stamp.json +++ b/cli/dispatcher/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "8de73a7e2135cc3af0b4bc6cc32cd884a20338ec" + "sharedInputsHash": "dff7284c186f1c36decc25ab36242de2860feeac" } diff --git a/cli/project-runner/internal/projectrunner/connection_retry.go b/cli/project-runner/internal/projectrunner/connection_retry.go index d26679b90..649f1a851 100644 --- a/cli/project-runner/internal/projectrunner/connection_retry.go +++ b/cli/project-runner/internal/projectrunner/connection_retry.go @@ -20,6 +20,7 @@ const ( focusRestoreTimeout = 2 * time.Second serverConnectionRetryDefaultTimeout = 10 * time.Second serverConnectionRetryDefaultPoll = 1 * time.Second + defaultBusyFocusStallThreshold = 5 * time.Second ) type connectionRetryDeps struct { @@ -27,6 +28,7 @@ type connectionRetryDeps struct { focusUnityProcess func(context.Context, int) (unityprocess.RestoreFocusFunc, error) retryTimeout time.Duration retryPoll time.Duration + busyFocusStallThreshold time.Duration } func defaultConnectionRetryDeps() connectionRetryDeps { @@ -46,6 +48,7 @@ const ( focusReasonMainThreadStall connectionRetryFocusReason = "main_thread_stall" focusReasonHeartbeatSilenceTimeout connectionRetryFocusReason = "heartbeat_silence_timeout" focusReasonFinalResponseTimeout connectionRetryFocusReason = "final_response_timeout" + focusReasonBusyStall connectionRetryFocusReason = "busy_stall" ) // Why: domain reload on large projects can keep the IPC endpoint down well past the @@ -60,6 +63,13 @@ func unityAliveRetryWindow(deps connectionRetryDeps) time.Duration { return deps.retryTimeout * serverConnectionRetryUnityAliveFactor } +func busyFocusStallThresholdFor(deps connectionRetryDeps) time.Duration { + if deps.busyFocusStallThreshold > 0 { + return deps.busyFocusStallThreshold + } + return defaultBusyFocusStallThreshold +} + type connectionRetryFocusController struct { connection unityipc.Connection method string @@ -187,6 +197,7 @@ func sendWithTransientConnectionRetryWithDeps( var lastOutcome unityipc.UnitySendOutcome var lastErr error + busySequenceStartedAt := time.Time{} focusController := newConnectionRetryFocusController(connection, method, deps) defer func() { restoreContext, cancel := context.WithTimeout(context.Background(), focusRestoreTimeout) @@ -209,6 +220,11 @@ func sendWithTransientConnectionRetryWithDeps( if isUnityServerBusyRPCError(err) { // Busy means the request was never executed, so a bounded retry is safe and // usually absorbs back-to-back tool calls without bothering the caller. + if busySequenceStartedAt.IsZero() { + busySequenceStartedAt = time.Now() + } else if time.Since(busySequenceStartedAt) >= busyFocusStallThresholdFor(deps) { + focusController.tryFocus(ctx, focusReasonBusyStall, err) + } lastOutcome = outcome lastErr = err if finished, finalOutcome, finalErr := finishBusyRetry( diff --git a/cli/project-runner/internal/projectrunner/connection_retry_test.go b/cli/project-runner/internal/projectrunner/connection_retry_test.go index 7ad00b609..21dfe142d 100644 --- a/cli/project-runner/internal/projectrunner/connection_retry_test.go +++ b/cli/project-runner/internal/projectrunner/connection_retry_test.go @@ -20,6 +20,19 @@ import ( "github.com/hatayama/unity-cli-loop/common/unityipc" ) +// Verifies the default busy-stall focus threshold fires before the bounded busy retry window ends. +func TestDefaultBusyFocusStallThresholdFitsWithinBusyRetryWindow(t *testing.T) { + deps := defaultConnectionRetryDeps() + threshold := busyFocusStallThresholdFor(deps) + if threshold >= deps.retryTimeout { + t.Fatalf( + "busy focus stall threshold must stay below the busy retry window: threshold=%s window=%s", + threshold, + deps.retryTimeout, + ) + } +} + // Verifies transient IPC connection failures focus Unity once and restore focus before reporting server-not-responding. func TestSendWithTransientConnectionRetryReportsUnityServerNotResponding(t *testing.T) { deps := defaultConnectionRetryDeps() @@ -713,6 +726,85 @@ func TestSendWithTransientConnectionRetryReturnsBusyAfterRetryWindow(t *testing. } } +// TDD repro for B-7a: before busy_stall focus rescue, persistent server_busy never called +// focusUnityProcess (focusCallCount stayed 0). This assertion was Red on pre-fix +// connection_retry.go and turns Green after the busy stall threshold hook. +func TestSendWithTransientConnectionRetryFocusesOnceAfterPersistentBusy(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("TCP endpoint injection is only used by this non-Windows client test") + } + + deps := defaultConnectionRetryDeps() + deps.retryTimeout = 500 * time.Millisecond + deps.retryPoll = 5 * time.Millisecond + deps.busyFocusStallThreshold = 30 * time.Millisecond + focusCallCount := 0 + restoreCallCount := 0 + deps.findRunningUnityProcess = func(context.Context, string) (*clicore.UnityProcess, error) { + return &clicore.UnityProcess{Pid: 123}, nil + } + deps.focusUnityProcess = func(context.Context, int) (clicore.RestoreFocusFunc, error) { + focusCallCount++ + return func(context.Context) error { + restoreCallCount++ + return nil + }, nil + } + + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("failed to listen: %v", err) + } + defer func() { + _ = listener.Close() + }() + + busy := `{"jsonrpc":"2.0","id":1,"error":{"code":-32603,"message":"Unity is busy running 'compile'.","data":{"type":"server_busy","runningToolName":"compile","requestedToolName":"get-logs","message":"busy"}}}` + go func() { + for { + conn, acceptErr := listener.Accept() + if acceptErr != nil { + return + } + go func() { + defer func() { + _ = conn.Close() + }() + if _, readErr := unityipc.Read(bufio.NewReader(conn)); readErr != nil { + return + } + _ = unityipc.Write(conn, []byte(busy)) + }() + } + }() + + connection := unityipc.Connection{ + Endpoint: unityipc.Endpoint{ + Network: "tcp", + Address: listener.Addr().String(), + }, + ProjectRoot: t.TempDir(), + } + + _, err = sendWithTransientConnectionRetryWithDeps( + context.Background(), + connection, + "get-logs", + map[string]any{}, + nil, + 0, + deps) + if err == nil { + t.Fatal("expected busy error after retry window") + } + if focusCallCount != 1 { + t.Fatalf("expected one busy-stall focus attempt, got %d", focusCallCount) + } + if restoreCallCount != 1 { + t.Fatalf("expected focus restore after busy retry exit, got %d", restoreCallCount) + } +} + // Verifies that undispatched dial failures keep retrying past the base window while a // running Unity process is confirmed, so a domain reload longer than the base window // (e.g. on large projects) does not fail the command spuriously. diff --git a/cli/project-runner/shared-inputs-stamp.json b/cli/project-runner/shared-inputs-stamp.json index 3f5a3e007..cb9ee97da 100644 --- a/cli/project-runner/shared-inputs-stamp.json +++ b/cli/project-runner/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "9c44c24427c361475087a09bc9f0ed4271828ba3" + "sharedInputsHash": "be6d79a7389bbf54e3c5020913f91955c15ec3f4" } From ad42b99241e40478285eee8082a57efea72c5a29 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Sun, 12 Jul 2026 15:57:36 +0900 Subject: [PATCH 3/5] fix: Align execute-dynamic-code diagnostic context with user snippet lines (#1722) Co-authored-by: Cursor --- .../DynamicCodeDiagnosticColumnMapperTests.cs | 19 ++++ ...micCodeDiagnosticColumnMapperTests.cs.meta | 11 +++ ...ynamicCodeDiagnosticContextBuilderTests.cs | 70 +++++++++++++++ ...cCodeDiagnosticContextBuilderTests.cs.meta | 11 +++ ...ynamicCodeExecutionResponseFactoryTests.cs | 47 ++++++++++ .../DynamicCodeExecutionResponseFactory.cs | 86 +++++++++++-------- .../DynamicCodeDiagnosticColumnMapper.cs | 28 ++++++ .../DynamicCodeDiagnosticColumnMapper.cs.meta | 11 +++ .../DynamicCodeDiagnosticContextBuilder.cs | 52 +++++++++++ ...ynamicCodeDiagnosticContextBuilder.cs.meta | 11 +++ .../DynamicCodeUserSnippetLines.cs | 57 ++++++++++++ .../DynamicCodeUserSnippetLines.cs.meta | 11 +++ .../DynamicCompilation/PreUsingResolver.cs | 29 +------ .../WrappedDynamicCodeUserSnippetExtractor.cs | 61 +++++++++++++ ...pedDynamicCodeUserSnippetExtractor.cs.meta | 11 +++ .../DynamicCompilation/WrapperTemplate.cs | 3 +- .../ExecuteDynamicCodeUseCase.cs | 3 +- 17 files changed, 458 insertions(+), 63 deletions(-) create mode 100644 Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeDiagnosticColumnMapperTests.cs create mode 100644 Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeDiagnosticColumnMapperTests.cs.meta create mode 100644 Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeDiagnosticContextBuilderTests.cs create mode 100644 Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeDiagnosticContextBuilderTests.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeDiagnosticColumnMapper.cs create mode 100644 Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeDiagnosticColumnMapper.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeDiagnosticContextBuilder.cs create mode 100644 Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeDiagnosticContextBuilder.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeUserSnippetLines.cs create mode 100644 Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeUserSnippetLines.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/WrappedDynamicCodeUserSnippetExtractor.cs create mode 100644 Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/WrappedDynamicCodeUserSnippetExtractor.cs.meta diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeDiagnosticColumnMapperTests.cs b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeDiagnosticColumnMapperTests.cs new file mode 100644 index 000000000..c2b185da7 --- /dev/null +++ b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeDiagnosticColumnMapperTests.cs @@ -0,0 +1,19 @@ +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.DynamicCodeToolTests +{ + [TestFixture] + public sealed class DynamicCodeDiagnosticColumnMapperTests + { + [Test] + public void MapWrappedColumnToUserColumn_SubtractsWrapperIndentAndClampsToOne() + { + // Verifies wrapped physical columns are rebased to user-snippet columns. + Assert.That(DynamicCodeDiagnosticColumnMapper.MapWrappedColumnToUserColumn(20), Is.EqualTo(8)); + Assert.That(DynamicCodeDiagnosticColumnMapper.MapWrappedColumnToUserColumn(5), Is.EqualTo(1)); + Assert.That(DynamicCodeDiagnosticColumnMapper.MapWrappedColumnToUserColumn(0), Is.EqualTo(0)); + } + } +} diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeDiagnosticColumnMapperTests.cs.meta b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeDiagnosticColumnMapperTests.cs.meta new file mode 100644 index 000000000..1c9f1b7aa --- /dev/null +++ b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeDiagnosticColumnMapperTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: f57214938bfeb4c76acbd60106b41e7c +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeDiagnosticContextBuilderTests.cs b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeDiagnosticContextBuilderTests.cs new file mode 100644 index 000000000..882be45b6 --- /dev/null +++ b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeDiagnosticContextBuilderTests.cs @@ -0,0 +1,70 @@ +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.DynamicCodeToolTests +{ + [TestFixture] + public sealed class DynamicCodeDiagnosticContextBuilderTests + { + [Test] + public void BuildContext_WhenUserLineFiveHasSyntaxError_ShowsUserSnippetNotWrapperBoilerplate() + { + // Verifies #line-mapped user line 5 renders against the user snippet instead of wrapper usings. + string[] userSnippetLines = + { + "int a=1;", + "int b=2;", + "int c=3;", + "int d=4;", + "int e= ;", + "return a;" + }; + + string context = DynamicCodeDiagnosticContextBuilder.BuildContext(userSnippetLines, 5, 8); + + Assert.That(context, Does.Contain("L5:int e= ;")); + Assert.That(context, Does.Not.Contain("using System.Collections.Generic")); + Assert.That(context, Does.Not.Contain("L7:")); + } + + [Test] + public void BuildContext_WhenUserColumnProvided_PlacesCaretUnderErrorCharacter() + { + // Verifies the caret line uses the same 1-based column basis as the rendered user line. + string[] userSnippetLines = { "int e= ;" }; + string context = DynamicCodeDiagnosticContextBuilder.BuildContext(userSnippetLines, 1, 8); + string[] contextLines = context.Split('\n'); + + Assert.That(contextLines[1].IndexOf('^'), Is.EqualTo("L1:".Length + 7)); + } + + [Test] + public void TryExtract_WhenWrappedSourceContainsLineDirectives_ReturnsIndentedUserSnippet() + { + // Verifies wrapped sources expose only the user region between #line markers. + string wrappedSource = WrapperTemplate.Build( + System.Array.Empty(), + System.Array.Empty(), + "TestNs", + "TestClass", + "int a=1;\nint e= ;\nreturn a;"); + + bool extracted = WrappedDynamicCodeUserSnippetExtractor.TryExtract(wrappedSource, out string userSnippet); + + Assert.That(extracted, Is.True); + Assert.That(userSnippet, Does.Contain("int e= ;")); + Assert.That(userSnippet, Does.Not.Contain("using UnityEditor;")); + } + + [Test] + public void Split_WhenOriginalSnippetHasTrailingNewline_DropsTrailingEmptyLine() + { + // Verifies trailing newline does not create an extra empty context line. + string[] lines = DynamicCodeUserSnippetLines.Split("int a=1;\nreturn a;\n"); + + Assert.That(lines, Has.Length.EqualTo(2)); + Assert.That(lines[0], Is.EqualTo("int a=1;")); + } + } +} diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeDiagnosticContextBuilderTests.cs.meta b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeDiagnosticContextBuilderTests.cs.meta new file mode 100644 index 000000000..0e6768abe --- /dev/null +++ b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeDiagnosticContextBuilderTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: f66a60822655c4ae9945c33d36779c15 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeExecutionResponseFactoryTests.cs b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeExecutionResponseFactoryTests.cs index 4b7979ea3..fc920c456 100644 --- a/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeExecutionResponseFactoryTests.cs +++ b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeExecutionResponseFactoryTests.cs @@ -87,6 +87,53 @@ public void ConvertExecutionResultToResponse_WhenCompilationFails_MapsStructured Assert.That(response.Logs, Contains.Item(response.DiagnosticsSummary)); } + /// + /// Verifies wrapped UpdatedCode diagnostics render context from the user snippet region. + /// + [Test] + public void ConvertExecutionResultToResponse_WhenWrappedSourceFails_UsesUserSnippetContext() + { + DynamicCodeExecutionResponseFactory factory = new(); + string userCode = "int a=1;\nint b=2;\nint c=3;\nint d=4;\nint e= ;\nreturn a;"; + string wrappedSource = WrapperTemplate.Build( + Array.Empty(), + Array.Empty(), + "TestNs", + "TestClass", + userCode); + ExecutionResult result = new() + { + Success = false, + ErrorMessage = "Compilation error occurred", + UpdatedCode = wrappedSource, + CompilationErrors = new List + { + new CompilationError + { + ErrorCode = "CS1525", + Message = "Invalid expression term ';'", + Line = 5, + Column = 20 + } + } + }; + + ExecuteDynamicCodeResponse response = factory.ConvertExecutionResultToResponse(result, userCode); + + Assert.That(response.Diagnostics[0].Column, Is.EqualTo(8)); + Assert.That(response.Diagnostics[0].PointerColumn, Is.EqualTo(8)); + Assert.That(response.Diagnostics[0].Context, Does.Contain("L5:int e= ;")); + Assert.That(response.Diagnostics[0].Context, Does.Contain("int b=2;")); + Assert.That(response.Diagnostics[0].Context, Does.Not.Contain("__uloop_literal")); + Assert.That(response.Diagnostics[0].Context, Does.Not.Contain("using System.Collections.Generic")); + Assert.That(response.Diagnostics[0].Context, Does.Not.Contain("L7:")); + string[] contextLines = response.Diagnostics[0].Context + .Split(new[] { '\n' }, System.StringSplitOptions.RemoveEmptyEntries); + int caretLineIndex = System.Array.FindIndex(contextLines, line => line.Contains('^')); + Assert.That(caretLineIndex, Is.GreaterThanOrEqualTo(0)); + Assert.That(contextLines[caretLineIndex].IndexOf('^'), Is.EqualTo("L5:".Length + 7)); + } + /// /// Verifies known compilation failures preserve friendly explanations, examples, and solutions. /// diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeExecutionResponseFactory.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeExecutionResponseFactory.cs index 74f796f67..4f34b76f2 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeExecutionResponseFactory.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeExecutionResponseFactory.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Text; using io.github.hatayama.UnityCliLoop.ToolContracts; @@ -41,7 +40,9 @@ internal static ExecuteDynamicCodeResponse CreateCancelledResponse() }; } - internal ExecuteDynamicCodeResponse ConvertExecutionResultToResponse(ExecutionResult result) + internal ExecuteDynamicCodeResponse ConvertExecutionResultToResponse( + ExecutionResult result, + string originalUserSnippet = null) { ExecuteDynamicCodeResponse response = new() { Success = result.Success, @@ -54,7 +55,7 @@ internal ExecuteDynamicCodeResponse ConvertExecutionResultToResponse(ExecutionRe if (!result.Success) { - ApplyFailureResponseDetails(response, result); + ApplyFailureResponseDetails(response, result, originalUserSnippet); } if (result.Exception != null) @@ -72,19 +73,21 @@ internal ExecuteDynamicCodeResponse ConvertExecutionResultToResponse(ExecutionRe private void ApplyFailureResponseDetails( ExecuteDynamicCodeResponse response, - ExecutionResult result) + ExecutionResult result, + string originalUserSnippet) { DynamicCodeFriendlyError friendlyError = _friendlyErrorConverter.Convert(result); response.ErrorMessage = friendlyError.FriendlyMessage; response.Logs = result.Logs != null ? new List(result.Logs) : new List(); AddFriendlyFailureDetails(response.Logs, friendlyError); - ApplyCompilationDiagnostics(response, result); + ApplyCompilationDiagnostics(response, result, originalUserSnippet); response.UpdatedCode = result.UpdatedCode ?? response.UpdatedCode; } private static void ApplyCompilationDiagnostics( ExecuteDynamicCodeResponse response, - ExecutionResult result) + ExecutionResult result, + string originalUserSnippet) { if (result.CompilationErrors?.Any() != true) { @@ -94,6 +97,7 @@ private static void ApplyCompilationDiagnostics( response.Diagnostics = BuildDiagnostics( result.CompilationErrors, result.UpdatedCode, + originalUserSnippet, result.AmbiguousTypeCandidates); response.CompilationErrors = response.Diagnostics; response.DiagnosticsSummary = CreateDiagnosticsSummary(response.Diagnostics); @@ -168,27 +172,45 @@ private static void AddLogIfNotEmpty(List logs, string prefix, string va private static List BuildDiagnostics( List errors, string updatedCode, + string originalUserSnippet, Dictionary> ambiguousCandidates = null) { List list = new(); - string[] lines = string.IsNullOrEmpty(updatedCode) - ? Array.Empty() + bool hasUserSnippetRegion = WrappedDynamicCodeUserSnippetExtractor.TryExtract(updatedCode, out _); + string[] originalUserLines = DynamicCodeUserSnippetLines.Split(originalUserSnippet); + string[] fallbackLines = string.IsNullOrEmpty(updatedCode) + ? System.Array.Empty() : updatedCode.Split(new[] { '\n' }, StringSplitOptions.None); foreach (CompilationError error in errors) { (string hint, List suggestions) = GetHintAndSuggestions(error, ambiguousCandidates); - string context = ExtractContext(lines, error.Line, error.Column); + bool useOriginalUserContext = hasUserSnippetRegion + && DynamicCodeDiagnosticContextBuilder.IsUserSnippetLineInRange(originalUserLines, error.Line); + string[] contextLines = useOriginalUserContext ? originalUserLines : fallbackLines; + // why: compiler column is measured on hoisted wrapped source while Context shows original + // user text; literals hoisted on the same line before the error site can shift column a few + // positions even after indent subtraction. + int contextColumn = useOriginalUserContext + ? DynamicCodeDiagnosticColumnMapper.MapWrappedColumnToUserColumn(error.Column) + : error.Column; + int reportedColumn = useOriginalUserContext ? contextColumn : error.Column; + string context = ExtractContext(contextLines, error.Line, contextColumn); + if (hasUserSnippetRegion && !useOriginalUserContext && error.Line > 0) + { + hint = AppendWrapperOriginHint(hint); + } + list.Add(new CompilationErrorDto { Line = error.Line, - Column = error.Column, + Column = reportedColumn, Message = error.Message, ErrorCode = error.ErrorCode, Hint = hint, Suggestions = suggestions, Context = context, - PointerColumn = error.Column + PointerColumn = reportedColumn }); } @@ -204,6 +226,23 @@ private static List BuildDiagnostics( .ToList(); } + private static string AppendWrapperOriginHint(string hint) + { + const string wrapperOriginHint = + "This diagnostic line does not map to the user snippet; it likely refers to generated wrapper code."; + if (string.IsNullOrEmpty(hint)) + { + return wrapperOriginHint; + } + + if (hint.Contains(wrapperOriginHint, StringComparison.Ordinal)) + { + return hint; + } + + return hint + " " + wrapperOriginHint; + } + private static (string hint, List suggestions) GetHintAndSuggestions( CompilationError error, Dictionary> ambiguousCandidates = null) @@ -270,30 +309,7 @@ private static string ExtractContext( int lineNumber1Based, int column1Based) { - if (lines == null || lines.Length == 0 || lineNumber1Based <= 0 || lineNumber1Based > lines.Length) - { - return string.Empty; - } - - int start = Math.Max(1, lineNumber1Based - 3); - int end = Math.Min(lines.Length, lineNumber1Based + 3); - StringBuilder sb = new(); - for (int i = start; i <= end; i++) - { - string line = lines[i - 1].TrimEnd('\r'); - string linePrefix = $"L{i}:"; - sb.AppendLine(linePrefix + line); - if (i == lineNumber1Based) - { - int caretPos = Math.Max(1, column1Based); - sb.AppendLine( - new string(' ', linePrefix.Length) - + new string(' ', Math.Max(0, caretPos - 1)) - + "^"); - } - } - - return sb.ToString(); + return DynamicCodeDiagnosticContextBuilder.BuildContext(lines, lineNumber1Based, column1Based); } } } diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeDiagnosticColumnMapper.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeDiagnosticColumnMapper.cs new file mode 100644 index 000000000..530d739fe --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeDiagnosticColumnMapper.cs @@ -0,0 +1,28 @@ +using System.Diagnostics; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Maps wrapped-source compiler columns to user-snippet columns. + /// + internal static class DynamicCodeDiagnosticColumnMapper + { + internal static int MapWrappedColumnToUserColumn(int wrappedColumn1Based) + { + Debug.Assert(wrappedColumn1Based >= 0, "wrappedColumn1Based must not be negative"); + + if (wrappedColumn1Based <= 0) + { + return wrappedColumn1Based; + } + + int userColumn = wrappedColumn1Based - WrapperTemplate.UserBodyIndentSpaces; + if (userColumn < 1) + { + return 1; + } + + return userColumn; + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeDiagnosticColumnMapper.cs.meta b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeDiagnosticColumnMapper.cs.meta new file mode 100644 index 000000000..7adcadb33 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeDiagnosticColumnMapper.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: d51798d2dce7a4e1da2f7301ee0d39ec +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeDiagnosticContextBuilder.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeDiagnosticContextBuilder.cs new file mode 100644 index 000000000..788acff8a --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeDiagnosticContextBuilder.cs @@ -0,0 +1,52 @@ +using System; +using System.Diagnostics; +using System.Text; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Builds user-snippet diagnostic context from #line-mapped compiler locations. + /// + internal static class DynamicCodeDiagnosticContextBuilder + { + internal static string BuildContext( + string[] userSnippetLines, + int userLineNumber1Based, + int column1Based) + { + if (userSnippetLines == null + || userSnippetLines.Length == 0 + || userLineNumber1Based <= 0 + || userLineNumber1Based > userSnippetLines.Length) + { + return string.Empty; + } + + int start = Math.Max(1, userLineNumber1Based - 3); + int end = Math.Min(userSnippetLines.Length, userLineNumber1Based + 3); + StringBuilder builder = new(); + for (int lineIndex = start; lineIndex <= end; lineIndex++) + { + string line = userSnippetLines[lineIndex - 1]; + string linePrefix = $"L{lineIndex}:"; + builder.AppendLine(linePrefix + line); + if (lineIndex == userLineNumber1Based) + { + int caretPos = Math.Max(1, column1Based); + builder.AppendLine( + new string(' ', linePrefix.Length) + + new string(' ', Math.Max(0, caretPos - 1)) + + "^"); + } + } + + return builder.ToString(); + } + + internal static bool IsUserSnippetLineInRange(string[] userSnippetLines, int userLineNumber1Based) + { + Debug.Assert(userSnippetLines != null, "userSnippetLines must not be null"); + return userLineNumber1Based > 0 && userLineNumber1Based <= userSnippetLines.Length; + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeDiagnosticContextBuilder.cs.meta b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeDiagnosticContextBuilder.cs.meta new file mode 100644 index 000000000..04b827dbc --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeDiagnosticContextBuilder.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 7cd1229b5caab4c1985b0b720a2f01ff +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeUserSnippetLines.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeUserSnippetLines.cs new file mode 100644 index 000000000..034d90ccb --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeUserSnippetLines.cs @@ -0,0 +1,57 @@ +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Splits user-authored execute-dynamic-code snippets into diagnostic context lines. + /// + internal static class DynamicCodeUserSnippetLines + { + internal static string[] Split(string userSnippet) + { + if (string.IsNullOrEmpty(userSnippet)) + { + return System.Array.Empty(); + } + + string[] rawLines = userSnippet.Split('\n'); + string[] lines = new string[rawLines.Length]; + for (int index = 0; index < rawLines.Length; index++) + { + lines[index] = rawLines[index].TrimEnd('\r'); + } + + return TrimTrailingEmptyLines(lines); + } + + internal static string[] TrimTrailingEmptyLines(string[] lines) + { + if (lines == null || lines.Length == 0) + { + return System.Array.Empty(); + } + + int lastNonEmptyIndex = lines.Length - 1; + while (lastNonEmptyIndex >= 0 && lines[lastNonEmptyIndex].Length == 0) + { + lastNonEmptyIndex--; + } + + if (lastNonEmptyIndex < 0) + { + return System.Array.Empty(); + } + + if (lastNonEmptyIndex == lines.Length - 1) + { + return lines; + } + + string[] trimmedLines = new string[lastNonEmptyIndex + 1]; + for (int index = 0; index <= lastNonEmptyIndex; index++) + { + trimmedLines[index] = lines[index]; + } + + return trimmedLines; + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeUserSnippetLines.cs.meta b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeUserSnippetLines.cs.meta new file mode 100644 index 000000000..8dab239c4 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeUserSnippetLines.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: ae576e690b9ea4dac93dda5012854038 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/PreUsingResolver.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/PreUsingResolver.cs index a8fd1b30b..517802fd9 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/PreUsingResolver.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/PreUsingResolver.cs @@ -339,37 +339,14 @@ private static bool ShouldResolveQualifiedTypeAssembly(IReadOnlyList qua return qualifiedChainParts != null && qualifiedChainParts.Count >= 2; } - // WrapperTemplate marks user code with #line directives; - // scanning only this region avoids false positives from boilerplate identifiers private static string ExtractUserCodeSection(string wrappedSource) { - string startMarker = WrapperTemplate.UserCodeStartMarker; - string endMarker = WrapperTemplate.UserCodeEndMarker; - - int startIdx = wrappedSource.IndexOf(startMarker, System.StringComparison.Ordinal); - // Wrapped source from WrapperTemplate should always contain the marker - Debug.Assert(startIdx >= 0, "UserCodeStartMarker not found in wrappedSource"); - if (startIdx < 0) - { - return wrappedSource; - } - - int codeStart = wrappedSource.IndexOf('\n', startIdx); - Debug.Assert(codeStart >= 0, "newline after UserCodeStartMarker not found"); - if (codeStart < 0) - { - return wrappedSource; - } - codeStart++; - - int endIdx = wrappedSource.IndexOf(endMarker, codeStart, System.StringComparison.Ordinal); - Debug.Assert(endIdx >= 0, "UserCodeEndMarker not found in wrappedSource"); - if (endIdx < 0) + if (WrappedDynamicCodeUserSnippetExtractor.TryExtract(wrappedSource, out string userSnippet)) { - return wrappedSource.Substring(codeStart); + return userSnippet; } - return wrappedSource.Substring(codeStart, endIdx - codeStart); + return wrappedSource; } private static int SkipToEndOfLine(string s, int pos) diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/WrappedDynamicCodeUserSnippetExtractor.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/WrappedDynamicCodeUserSnippetExtractor.cs new file mode 100644 index 000000000..becbe8b0e --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/WrappedDynamicCodeUserSnippetExtractor.cs @@ -0,0 +1,61 @@ +using System.Diagnostics; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Extracts the user-authored snippet region from wrapped execute-dynamic-code sources. + /// + internal static class WrappedDynamicCodeUserSnippetExtractor + { + internal static bool TryExtract(string wrappedSource, out string userSnippet) + { + userSnippet = string.Empty; + if (string.IsNullOrEmpty(wrappedSource)) + { + return false; + } + + int startIndex = wrappedSource.IndexOf(WrapperTemplate.UserCodeStartMarker, System.StringComparison.Ordinal); + if (startIndex < 0) + { + return false; + } + + int codeStart = wrappedSource.IndexOf('\n', startIndex); + if (codeStart < 0) + { + return false; + } + codeStart++; + + int endIndex = wrappedSource.IndexOf(WrapperTemplate.UserCodeEndMarker, codeStart, System.StringComparison.Ordinal); + if (endIndex < 0) + { + userSnippet = wrappedSource.Substring(codeStart); + return true; + } + + userSnippet = wrappedSource.Substring(codeStart, endIndex - codeStart); + return true; + } + + internal static string[] SplitNormalizedLines(string userSnippet) + { + Debug.Assert(userSnippet != null, "userSnippet must not be null"); + + if (userSnippet.Length == 0) + { + return System.Array.Empty(); + } + + string[] rawLines = userSnippet.Split('\n'); + string[] normalizedLines = new string[rawLines.Length]; + for (int index = 0; index < rawLines.Length; index++) + { + normalizedLines[index] = rawLines[index].TrimStart().TrimEnd('\r'); + } + + return DynamicCodeUserSnippetLines.TrimTrailingEmptyLines(normalizedLines); + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/WrappedDynamicCodeUserSnippetExtractor.cs.meta b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/WrappedDynamicCodeUserSnippetExtractor.cs.meta new file mode 100644 index 000000000..d4005a178 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/WrappedDynamicCodeUserSnippetExtractor.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 017d4bb6a591a42e581747c5cb34488d +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/WrapperTemplate.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/WrapperTemplate.cs index c1454bc9e..92d2e6c3f 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/WrapperTemplate.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/WrapperTemplate.cs @@ -13,6 +13,7 @@ internal static class WrapperTemplate { internal const string UserCodeStartMarker = "#line 1 \"user-snippet.cs\""; internal const string UserCodeEndMarker = "#line default"; + internal const int UserBodyIndentSpaces = 12; private static readonly DefaultUsingAlias[] DefaultUsingAliases = { @@ -71,7 +72,7 @@ public static string Build( foreach (string line in body.Split('\n')) { - sb.AppendLine($" {line.TrimEnd('\r')}"); + sb.AppendLine($"{new string(' ', UserBodyIndentSpaces)}{line.TrimEnd('\r')}"); } sb.AppendLine(UserCodeEndMarker); diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeUseCase.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeUseCase.cs index 33dbd62e4..1e0e8ae2b 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeUseCase.cs @@ -75,7 +75,8 @@ await WarmForegroundExecutionPathIfNeededAsync(parameters, cancellationToken) return cancelledResponse; } - ExecuteDynamicCodeResponse response = _responseFactory.ConvertExecutionResultToResponse(finalResult); + ExecuteDynamicCodeResponse response = + _responseFactory.ConvertExecutionResultToResponse(finalResult, originalCode); response.EmitTimingsInJsonResponse = parameters.IncludeTimings; // Why: domain-reload timeouts can complete while Unity's synchronization context is stalled. bool domainReloadWaitRequired = From 1714d45e5da82b2fe46d9edafd395c5577e71317 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Sun, 12 Jul 2026 16:19:19 +0900 Subject: [PATCH 4/5] fix: Handle execute-dynamic-code transpiler constraints for static locals and byte casts (#1723) Co-authored-by: Cursor --- .../uloop-execute-dynamic-code/SKILL.md | 6 + .../uloop-execute-dynamic-code/SKILL.md | 6 + ...ynamicCodeExecutionResponseFactoryTests.cs | 32 +++ .../DynamicCodeLiteralHoisterTests.cs | 76 +++++++ ...namicCodeTranspilerConstraintHintsTests.cs | 58 +++++ ...CodeTranspilerConstraintHintsTests.cs.meta | 11 + .../DynamicCodeExecutionResponseFactory.cs | 12 ++ .../DynamicCodeLiteralHoister.cs | 85 +++++++- .../DynamicCodeTranspilerConstraintHints.cs | 49 +++++ ...namicCodeTranspilerConstraintHints.cs.meta | 11 + .../LiteralHoistScopeTracker.cs | 128 +++++++++++ .../LiteralHoistScopeTracker.cs.meta | 11 + .../StaticLocalFunctionHeaderScanner.cs | 202 ++++++++++++++++++ .../StaticLocalFunctionHeaderScanner.cs.meta | 11 + .../ExecuteDynamicCode/Skill/SKILL.md | 6 + 15 files changed, 701 insertions(+), 3 deletions(-) create mode 100644 Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeTranspilerConstraintHintsTests.cs create mode 100644 Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeTranspilerConstraintHintsTests.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeTranspilerConstraintHints.cs create mode 100644 Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeTranspilerConstraintHints.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/LiteralHoistScopeTracker.cs create mode 100644 Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/LiteralHoistScopeTracker.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/StaticLocalFunctionHeaderScanner.cs create mode 100644 Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/StaticLocalFunctionHeaderScanner.cs.meta diff --git a/.agents/skills/uloop-execute-dynamic-code/SKILL.md b/.agents/skills/uloop-execute-dynamic-code/SKILL.md index d5d4edc3d..42208832b 100644 --- a/.agents/skills/uloop-execute-dynamic-code/SKILL.md +++ b/.agents/skills/uloop-execute-dynamic-code/SKILL.md @@ -32,6 +32,12 @@ return x; Prefer terminal commands for file operations and keep snippets focused on Unity Editor state that existing uloop tools cannot inspect or change. +## Known transpiler constraints + +- Literals inside recognized static local function bodies are kept inline automatically. Unsupported header shapes (generic `where` clauses, tuple return types, statement lambdas inside expression bodies) may still hoist literals and surface CS8421; remove `static` or rewrite the helper. +- Static lambdas (`static x => ...`) cannot reference hoisted literals and surface CS8820; remove `static` from the lambda or use a non-static local function. +- Integer literals are hoisted as `int` values. APIs that require `byte` components (for example `new Color32(255, 0, 0, 255)`) need explicit casts such as `(byte)255` even when plain Unity scripts accept uncast numeric literals. + ## Shell Quoting - zsh/bash: single-quote the whole snippet so C# double quotes pass through unchanged: `--code 'return "hi";'`. For a single quote inside the snippet, close and reopen the shell string with `'\''`. diff --git a/.claude/skills/uloop-execute-dynamic-code/SKILL.md b/.claude/skills/uloop-execute-dynamic-code/SKILL.md index d5d4edc3d..42208832b 100644 --- a/.claude/skills/uloop-execute-dynamic-code/SKILL.md +++ b/.claude/skills/uloop-execute-dynamic-code/SKILL.md @@ -32,6 +32,12 @@ return x; Prefer terminal commands for file operations and keep snippets focused on Unity Editor state that existing uloop tools cannot inspect or change. +## Known transpiler constraints + +- Literals inside recognized static local function bodies are kept inline automatically. Unsupported header shapes (generic `where` clauses, tuple return types, statement lambdas inside expression bodies) may still hoist literals and surface CS8421; remove `static` or rewrite the helper. +- Static lambdas (`static x => ...`) cannot reference hoisted literals and surface CS8820; remove `static` from the lambda or use a non-static local function. +- Integer literals are hoisted as `int` values. APIs that require `byte` components (for example `new Color32(255, 0, 0, 255)`) need explicit casts such as `(byte)255` even when plain Unity scripts accept uncast numeric literals. + ## Shell Quoting - zsh/bash: single-quote the whole snippet so C# double quotes pass through unchanged: `--code 'return "hi";'`. For a single quote inside the snippet, close and reopen the shell string with `'\''`. diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeExecutionResponseFactoryTests.cs b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeExecutionResponseFactoryTests.cs index fc920c456..6c53eb7d5 100644 --- a/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeExecutionResponseFactoryTests.cs +++ b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeExecutionResponseFactoryTests.cs @@ -213,5 +213,37 @@ public void CancelledResult_WhenMapped_ReturnsNeutralCancellationResponse() Assert.That(response.CompilationErrors, Is.Empty); Assert.That(response.ErrorMessage, Is.EqualTo(UnityCliLoopConstants.ERROR_MESSAGE_EXECUTION_CANCELLED)); } + + /// + /// Verifies int-to-byte conversion failures include transpiler constraint guidance. + /// + [Test] + public void ConvertExecutionResultToResponse_WhenColor32ByteConversionFails_AddsTranspilerConstraintHint() + { + DynamicCodeExecutionResponseFactory factory = new(); + ExecutionResult result = new() + { + Success = false, + ErrorMessage = "Compilation error occurred", + CompilationErrors = new List + { + new CompilationError + { + ErrorCode = "CS1503", + Message = "CS1503: Argument 1: cannot convert from 'int' to 'byte'", + Line = 2, + Column = 20 + } + } + }; + + ExecuteDynamicCodeResponse response = factory.ConvertExecutionResultToResponse( + result, + "using UnityEngine;\nreturn new Color32(255, 0, 0, 255);"); + + Assert.That(response.Diagnostics[0].Hint, Does.Contain("Color32")); + Assert.That(response.Diagnostics[0].Suggestions, Contains.Item( + "Cast each component explicitly, for example: new Color32((byte)255, (byte)0, (byte)0, (byte)255).")); + } } } diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeLiteralHoisterTests.cs b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeLiteralHoisterTests.cs index 8fd222f37..6dcdaa10e 100644 --- a/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeLiteralHoisterTests.cs +++ b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeLiteralHoisterTests.cs @@ -21,5 +21,81 @@ public void Rewrite_WhenInterpolatedHoleContainsNestedStringLiteral_ShouldKeepIn Assert.That(result.Bindings, Is.Empty); Assert.That(result.DeclarationLines, Is.Empty); } + + /// + /// Verifies literals inside a nested static local function stay inline while outer literals still hoist. + /// + [Test] + public void Rewrite_WhenIntegerLiteralIsInsideStaticLocalFunction_ShouldKeepLiteralInline() + { + string source = @"int Compute(int value) +{ + static int Double(int x) => x * 2; + return Double(value); +} +return Compute(3);"; + + HoistedLiteralRewriteResult result = DynamicCodeLiteralHoister.Rewrite(source); + + Assert.That(result.RewrittenSource, Does.Contain("x * 2")); + Assert.That(result.RewrittenSource, Does.Contain("return Compute(__uloop_literal_0)")); + Assert.That(result.RewrittenSource, Does.Not.Contain("x * __uloop_literal")); + Assert.That(result.Bindings, Has.Count.EqualTo(1)); + Assert.That(result.Bindings[0].Value, Is.EqualTo(3)); + } + + /// + /// Verifies string literals inside a nested static local block body are not hoisted. + /// + [Test] + public void Rewrite_WhenStringLiteralIsInsideStaticLocalFunctionBlockBody_ShouldKeepLiteralInline() + { + string source = @"string Build() +{ + static string Label() { return ""ok""; } + return Label(); +} +return Build();"; + + HoistedLiteralRewriteResult result = DynamicCodeLiteralHoister.Rewrite(source); + + Assert.That(result.RewrittenSource, Does.Contain("return \"ok\"")); + Assert.That(result.RewrittenSource, Does.Not.Contain("__uloop_literal_0")); + Assert.That(result.Bindings, Is.Empty); + } + + /// + /// Verifies top-level expression-bodied static local functions keep literals inline at brace depth zero. + /// + [Test] + public void Rewrite_WhenTopLevelExpressionBodiedStaticLocalFunction_ShouldKeepLiteralInline() + { + string source = @"static int Double(int x) => x * 2; +return Double(3);"; + + HoistedLiteralRewriteResult result = DynamicCodeLiteralHoister.Rewrite(source); + + Assert.That(result.RewrittenSource, Does.Contain("x * 2")); + Assert.That(result.RewrittenSource, Does.Not.Contain("x * __uloop_literal")); + Assert.That(result.RewrittenSource, Does.Contain("return Double(__uloop_literal_0)")); + Assert.That(result.Bindings, Has.Count.EqualTo(1)); + Assert.That(result.Bindings[0].Value, Is.EqualTo(3)); + } + + /// + /// Verifies top-level block-bodied static local functions keep literals inline at brace depth zero. + /// + [Test] + public void Rewrite_WhenTopLevelBlockBodiedStaticLocalFunction_ShouldKeepLiteralInline() + { + string source = @"static string Label() { return ""ok""; } +return Label();"; + + HoistedLiteralRewriteResult result = DynamicCodeLiteralHoister.Rewrite(source); + + Assert.That(result.RewrittenSource, Does.Contain("return \"ok\"")); + Assert.That(result.RewrittenSource, Does.Not.Contain("__uloop_literal_0")); + Assert.That(result.Bindings, Is.Empty); + } } } diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeTranspilerConstraintHintsTests.cs b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeTranspilerConstraintHintsTests.cs new file mode 100644 index 000000000..51b457454 --- /dev/null +++ b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeTranspilerConstraintHintsTests.cs @@ -0,0 +1,58 @@ +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.DynamicCodeToolTests +{ + /// + /// Test fixture that verifies transpiler constraint hint generation. + /// + [TestFixture] + public class DynamicCodeTranspilerConstraintHintsTests + { + /// + /// Verifies CS8421 hoisted-literal failures suggest removing the static modifier. + /// + [Test] + public void TryBuildHint_WhenCs8421ReferencesHoistedLiteral_ShouldReturnStaticLocalFunctionGuidance() + { + (bool matched, string hint, string suggestion) = DynamicCodeTranspilerConstraintHints.TryBuildHint( + "CS8421", + "CS8421: A static local function cannot contain a reference to '__uloop_literal_0'."); + + Assert.That(matched, Is.True); + Assert.That(hint, Does.Contain("recognized static local function bodies")); + Assert.That(suggestion, Is.EqualTo("Remove the `static` modifier from the local function.")); + } + + /// + /// Verifies CS8820 static lambda failures suggest removing the static modifier. + /// + [Test] + public void TryBuildHint_WhenCs8820ReferencesHoistedLiteral_ShouldReturnStaticLambdaGuidance() + { + (bool matched, string hint, string suggestion) = DynamicCodeTranspilerConstraintHints.TryBuildHint( + "CS8820", + "CS8820: A static anonymous function cannot contain a reference to '__uloop_literal_0'."); + + Assert.That(matched, Is.True); + Assert.That(hint, Does.Contain("Static lambdas")); + Assert.That(suggestion, Is.EqualTo("Remove the `static` modifier from the lambda.")); + } + + /// + /// Verifies int-to-byte conversion failures include explicit cast guidance for Color32. + /// + [Test] + public void TryBuildHint_WhenCs1503CannotConvertIntToByte_ShouldReturnExplicitCastGuidance() + { + (bool matched, string hint, string suggestion) = DynamicCodeTranspilerConstraintHints.TryBuildHint( + "CS1503", + "CS1503: Argument 1: cannot convert from 'int' to 'byte'"); + + Assert.That(matched, Is.True); + Assert.That(hint, Does.Contain("Color32")); + Assert.That(suggestion, Does.Contain("(byte)255")); + } + } +} diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeTranspilerConstraintHintsTests.cs.meta b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeTranspilerConstraintHintsTests.cs.meta new file mode 100644 index 000000000..3b16043ea --- /dev/null +++ b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeTranspilerConstraintHintsTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 793fe6f168d864edb9afa78f79729471 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeExecutionResponseFactory.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeExecutionResponseFactory.cs index 4f34b76f2..e14047780 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeExecutionResponseFactory.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeExecutionResponseFactory.cs @@ -300,6 +300,18 @@ private static (string hint, List suggestions) GetHintAndSuggestions( return (hint, suggestions); default: + (bool matched, string constraintHint, string constraintSuggestion) = DynamicCodeTranspilerConstraintHints.TryBuildHint( + error.ErrorCode, + error.Message); + if (matched) + { + hint = constraintHint; + if (!string.IsNullOrEmpty(constraintSuggestion)) + { + suggestions.Add(constraintSuggestion); + } + } + return (hint, suggestions); } } diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeLiteralHoister.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeLiteralHoister.cs index d6d8b9181..cb5ff4942 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeLiteralHoister.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeLiteralHoister.cs @@ -10,7 +10,7 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools /// internal static class DynamicCodeLiteralHoister { - private const string LiteralParameterPrefix = "__uloop_literal_"; + internal const string LiteralParameterPrefix = "__uloop_literal_"; private static readonly Dictionary RegularStringEscapes = new() { { '\'', '\'' }, @@ -30,6 +30,7 @@ public static HoistedLiteralRewriteResult Rewrite(string source) { StringBuilder rewrittenSource = new(source.Length); List bindings = new(); + LiteralHoistScopeTracker scopeTracker = new(); int index = 0; while (index < source.Length) @@ -59,12 +60,49 @@ public static HoistedLiteralRewriteResult Rewrite(string source) continue; } - if (TryHoistRegularStringLiteral(source, rewrittenSource, bindings, ref index)) + if (scopeTracker.TryConsumeStaticLocalFunctionHeader(source, index, rewrittenSource, ref index)) { continue; } - if (TryHoistIntegerLiteral(source, rewrittenSource, bindings, ref index)) + if (source[index] == '{') + { + scopeTracker.OnOpenBrace(); + rewrittenSource.Append('{'); + index++; + continue; + } + + if (source[index] == '}') + { + scopeTracker.OnCloseBrace(); + rewrittenSource.Append('}'); + index++; + continue; + } + + if (source[index] == ';') + { + scopeTracker.OnSemicolon(); + rewrittenSource.Append(';'); + index++; + continue; + } + + if (scopeTracker.ShouldSuppressLiteralHoisting) + { + if (TryCopyRegularStringLiteral(source, rewrittenSource, ref index)) + { + continue; + } + } + else if (TryHoistRegularStringLiteral(source, rewrittenSource, bindings, ref index)) + { + continue; + } + + if (!scopeTracker.ShouldSuppressLiteralHoisting + && TryHoistIntegerLiteral(source, rewrittenSource, bindings, ref index)) { continue; } @@ -551,6 +589,47 @@ private static bool TryCopyCharLiteral( return false; } + private static bool TryCopyRegularStringLiteral( + string source, + StringBuilder rewrittenSource, + ref int index) + { + if (source[index] != '"') + { + return false; + } + + if (index > 0 && (source[index - 1] == '@' || source[index - 1] == '$')) + { + return false; + } + + int start = index; + index++; + + while (index < source.Length) + { + char current = source[index]; + if (current == '\\') + { + AdvanceEscapedLiteralSequence(source, ref index); + continue; + } + + if (current == '"') + { + index++; + rewrittenSource.Append(source, start, index - start); + return true; + } + + index++; + } + + index = start; + return false; + } + private static bool TryHoistRegularStringLiteral( string source, StringBuilder rewrittenSource, diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeTranspilerConstraintHints.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeTranspilerConstraintHints.cs new file mode 100644 index 000000000..c6ee4033e --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeTranspilerConstraintHints.cs @@ -0,0 +1,49 @@ +using System; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Builds compilation hints for execute-dynamic-code transpiler constraints. + /// + internal static class DynamicCodeTranspilerConstraintHints + { + internal static (bool Matched, string Hint, string Suggestion) TryBuildHint( + string errorCode, + string message) + { + if (string.Equals(errorCode, "CS8421", StringComparison.Ordinal) + && message.Contains(DynamicCodeLiteralHoister.LiteralParameterPrefix, StringComparison.Ordinal)) + { + return ( + true, + "Static local functions cannot reference hoisted literal parameters. " + + "Literals inside recognized static local function bodies are kept inline automatically; " + + "if this error persists, the header shape may be unsupported by the scanner.", + "Remove the `static` modifier from the local function."); + } + + if (string.Equals(errorCode, "CS8820", StringComparison.Ordinal) + && message.Contains(DynamicCodeLiteralHoister.LiteralParameterPrefix, StringComparison.Ordinal)) + { + return ( + true, + "Static lambdas cannot reference hoisted literal parameters. " + + "Remove the `static` modifier from the lambda, or use a non-static local function instead.", + "Remove the `static` modifier from the lambda."); + } + + if (string.Equals(errorCode, "CS1503", StringComparison.Ordinal) + && message.Contains("cannot convert from 'int' to 'byte'", StringComparison.Ordinal)) + { + return ( + true, + "Literal hoisting promotes integer literals to int values. " + + "Unity APIs such as Color32 expect byte components and do not accept implicit int-to-byte conversion " + + "after hoisting, even though plain numeric literals compile in normal Unity scripts.", + "Cast each component explicitly, for example: new Color32((byte)255, (byte)0, (byte)0, (byte)255)."); + } + + return (false, string.Empty, string.Empty); + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeTranspilerConstraintHints.cs.meta b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeTranspilerConstraintHints.cs.meta new file mode 100644 index 000000000..e053c0301 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeTranspilerConstraintHints.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: b998634941c2049ec90c456eda0e3820 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/LiteralHoistScopeTracker.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/LiteralHoistScopeTracker.cs new file mode 100644 index 000000000..12a2b08ba --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/LiteralHoistScopeTracker.cs @@ -0,0 +1,128 @@ +using System.Collections.Generic; +using System.Diagnostics; +using System.Text; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Tracks static local function bodies so literal hoisting does not inject outer-scope bindings. + /// + internal sealed class LiteralHoistScopeTracker + { + private readonly Stack _suppressedBodyBraceDepths = new(); + private int _braceDepth; + private bool _pendingBlockBody; + private bool _inExpressionBody; + + internal bool ShouldSuppressLiteralHoisting => + _inExpressionBody || _suppressedBodyBraceDepths.Count > 0; + + internal void OnOpenBrace() + { + _braceDepth++; + if (_pendingBlockBody) + { + _suppressedBodyBraceDepths.Push(_braceDepth); + _pendingBlockBody = false; + } + } + + internal void OnCloseBrace() + { + if (_suppressedBodyBraceDepths.Count > 0 + && _suppressedBodyBraceDepths.Peek() == _braceDepth) + { + _suppressedBodyBraceDepths.Pop(); + } + + _braceDepth--; + } + + internal void OnSemicolon() + { + if (_inExpressionBody) + { + _inExpressionBody = false; + } + } + + internal bool TryConsumeStaticLocalFunctionHeader( + string source, + int index, + StringBuilder rewrittenSource, + ref int nextIndex) + { + Debug.Assert(source != null, "source must not be null"); + Debug.Assert(rewrittenSource != null, "rewrittenSource must not be null"); + + if (!IsKeywordAt(source, index, "static")) + { + return false; + } + + int scanIndex = index + "static".Length; + if (!StaticLocalFunctionHeaderScanner.TrySkipHeader(source, scanIndex, out bool isExpressionBody, out int headerEndIndex)) + { + return false; + } + + rewrittenSource.Append(source, index, headerEndIndex - index); + if (isExpressionBody) + { + _inExpressionBody = true; + } + else + { + _pendingBlockBody = true; + } + + nextIndex = headerEndIndex; + return true; + } + + private static bool IsKeywordAt(string source, int index, string keyword) + { + if (index < 0 || index + keyword.Length > source.Length) + { + return false; + } + + if (!HasIdentifierBoundaryBefore(source, index)) + { + return false; + } + + for (int offset = 0; offset < keyword.Length; offset++) + { + if (source[index + offset] != keyword[offset]) + { + return false; + } + } + + return HasIdentifierBoundaryAfter(source, index + keyword.Length); + } + + private static bool HasIdentifierBoundaryBefore(string source, int index) + { + if (index <= 0) + { + return true; + } + + char previous = source[index - 1]; + return !char.IsLetterOrDigit(previous) && previous != '_'; + } + + private static bool HasIdentifierBoundaryAfter(string source, int index) + { + if (index >= source.Length) + { + return true; + } + + char next = source[index]; + return !char.IsLetterOrDigit(next) && next != '_'; + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/LiteralHoistScopeTracker.cs.meta b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/LiteralHoistScopeTracker.cs.meta new file mode 100644 index 000000000..4c938157a --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/LiteralHoistScopeTracker.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 8a799fe95102c424585ae3e7ca4fc768 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/StaticLocalFunctionHeaderScanner.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/StaticLocalFunctionHeaderScanner.cs new file mode 100644 index 000000000..1ccc87fae --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/StaticLocalFunctionHeaderScanner.cs @@ -0,0 +1,202 @@ +using System.Diagnostics; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Detects static local function headers so literal hoisting can skip their bodies. + /// + /// + /// Unsupported header shapes leave literals hoistable and may still surface CS8421: + /// generic constraints with where clauses, tuple return types such as static (int, int) F(), + /// and expression bodies that contain statement lambdas (the first semicolon ends suppression early). + /// + internal static class StaticLocalFunctionHeaderScanner + { + internal static bool TrySkipHeader( + string source, + int startIndex, + out bool isExpressionBody, + out int headerEndIndex) + { + Debug.Assert(source != null, "source must not be null"); + + isExpressionBody = false; + headerEndIndex = startIndex; + + int index = SkipWhitespace(source, startIndex); + if (!TrySkipReturnTypeAndName(source, ref index)) + { + return false; + } + + index = SkipWhitespace(source, index); + if (index >= source.Length || source[index] != '(') + { + return false; + } + + if (!TrySkipParameterList(source, ref index)) + { + return false; + } + + index = SkipWhitespace(source, index); + if (index + 1 < source.Length && source[index] == '=' && source[index + 1] == '>') + { + isExpressionBody = true; + headerEndIndex = index + 2; + return true; + } + + if (index < source.Length && source[index] == '{') + { + isExpressionBody = false; + headerEndIndex = index; + return true; + } + + return false; + } + + private static bool TrySkipReturnTypeAndName(string source, ref int index) + { + bool sawName = false; + + while (index < source.Length) + { + index = SkipWhitespace(source, index); + if (index >= source.Length) + { + return false; + } + + if (source[index] == '(') + { + return sawName; + } + + if (!TrySkipIdentifier(source, ref index)) + { + return false; + } + + sawName = true; + index = SkipWhitespace(source, index); + if (index < source.Length && source[index] == '<') + { + if (!TrySkipGenericArguments(source, ref index)) + { + return false; + } + } + + if (index < source.Length && source[index] == '.') + { + index++; + } + } + + return false; + } + + private static bool TrySkipParameterList(string source, ref int index) + { + if (index >= source.Length || source[index] != '(') + { + return false; + } + + int depth = 0; + while (index < source.Length) + { + char current = source[index]; + if (current == '(') + { + depth++; + } + else if (current == ')') + { + depth--; + if (depth == 0) + { + index++; + return true; + } + } + + index++; + } + + return false; + } + + private static bool TrySkipGenericArguments(string source, ref int index) + { + if (index >= source.Length || source[index] != '<') + { + return false; + } + + int depth = 0; + while (index < source.Length) + { + char current = source[index]; + if (current == '<') + { + depth++; + } + else if (current == '>') + { + depth--; + if (depth == 0) + { + index++; + return true; + } + } + + index++; + } + + return false; + } + + private static bool TrySkipIdentifier(string source, ref int index) + { + if (index >= source.Length) + { + return false; + } + + char first = source[index]; + if (!char.IsLetter(first) && first != '_') + { + return false; + } + + index++; + while (index < source.Length) + { + char current = source[index]; + if (!char.IsLetterOrDigit(current) && current != '_') + { + return true; + } + + index++; + } + + return true; + } + + private static int SkipWhitespace(string source, int index) + { + while (index < source.Length && char.IsWhiteSpace(source[index])) + { + index++; + } + + return index; + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/StaticLocalFunctionHeaderScanner.cs.meta b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/StaticLocalFunctionHeaderScanner.cs.meta new file mode 100644 index 000000000..c28ccff2f --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/StaticLocalFunctionHeaderScanner.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 81fb44ffbd1714d23bfc26a3afd08aab +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Skill/SKILL.md b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Skill/SKILL.md index d5d4edc3d..42208832b 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Skill/SKILL.md +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Skill/SKILL.md @@ -32,6 +32,12 @@ return x; Prefer terminal commands for file operations and keep snippets focused on Unity Editor state that existing uloop tools cannot inspect or change. +## Known transpiler constraints + +- Literals inside recognized static local function bodies are kept inline automatically. Unsupported header shapes (generic `where` clauses, tuple return types, statement lambdas inside expression bodies) may still hoist literals and surface CS8421; remove `static` or rewrite the helper. +- Static lambdas (`static x => ...`) cannot reference hoisted literals and surface CS8820; remove `static` from the lambda or use a non-static local function. +- Integer literals are hoisted as `int` values. APIs that require `byte` components (for example `new Color32(255, 0, 0, 255)`) need explicit casts such as `(byte)255` even when plain Unity scripts accept uncast numeric literals. + ## Shell Quoting - zsh/bash: single-quote the whole snippet so C# double quotes pass through unchanged: `--code 'return "hi";'`. For a single quote inside the snippet, close and reopen the shell string with `'\''`. From 86922193fa0b01f3a9a920b14da9c0cff5110615 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Sun, 12 Jul 2026 16:31:27 +0900 Subject: [PATCH 5/5] fix: Align execute-dynamic-code diagnostic lines after using extraction (#1724) Co-authored-by: Cursor --- ...ynamicCodeExecutionResponseFactoryTests.cs | 15 ++- .../DynamicCodeSourcePreparerTests.cs | 108 ++++++++++++++++++ .../DynamicCompilation/SourceShaper.cs | 51 ++++++++- 3 files changed, 169 insertions(+), 5 deletions(-) diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeExecutionResponseFactoryTests.cs b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeExecutionResponseFactoryTests.cs index 6c53eb7d5..497e91b41 100644 --- a/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeExecutionResponseFactoryTests.cs +++ b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeExecutionResponseFactoryTests.cs @@ -215,16 +215,22 @@ public void CancelledResult_WhenMapped_ReturnsNeutralCancellationResponse() } /// - /// Verifies int-to-byte conversion failures include transpiler constraint guidance. + /// Verifies int-to-byte conversion failures align diagnostics with extracted using lines. /// [Test] public void ConvertExecutionResultToResponse_WhenColor32ByteConversionFails_AddsTranspilerConstraintHint() { DynamicCodeExecutionResponseFactory factory = new(); + string userCode = "using UnityEngine;\nreturn new Color32(255, 0, 0, 255);"; + PreparedDynamicCode prepared = DynamicCodeSourcePreparer.Prepare( + userCode, + DynamicCodeConstants.DEFAULT_NAMESPACE, + DynamicCodeConstants.DEFAULT_CLASS_NAME); ExecutionResult result = new() { Success = false, ErrorMessage = "Compilation error occurred", + UpdatedCode = prepared.PreparedSource, CompilationErrors = new List { new CompilationError @@ -237,11 +243,12 @@ public void ConvertExecutionResultToResponse_WhenColor32ByteConversionFails_Adds } }; - ExecuteDynamicCodeResponse response = factory.ConvertExecutionResultToResponse( - result, - "using UnityEngine;\nreturn new Color32(255, 0, 0, 255);"); + ExecuteDynamicCodeResponse response = factory.ConvertExecutionResultToResponse(result, userCode); + Assert.That(response.Diagnostics[0].Line, Is.EqualTo(2)); Assert.That(response.Diagnostics[0].Hint, Does.Contain("Color32")); + Assert.That(response.Diagnostics[0].Context, Does.Contain("L2:return new Color32(255, 0, 0, 255);")); + Assert.That(response.Diagnostics[0].Context, Does.Not.Contain("L1:using UnityEngine;\n ^")); Assert.That(response.Diagnostics[0].Suggestions, Contains.Item( "Cast each component explicitly, for example: new Color32((byte)255, (byte)0, (byte)0, (byte)255).")); } diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeSourcePreparerTests.cs b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeSourcePreparerTests.cs index d8c131b6f..68dbc403c 100644 --- a/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeSourcePreparerTests.cs +++ b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeSourcePreparerTests.cs @@ -315,5 +315,113 @@ public void Prepare_WhenIntegerLiteralExceedsIntRange_ShouldLeaveLiteralInSource Assert.AreEqual(0, prepared.HoistedLiteralBindings.Count); StringAssert.Contains("return 3000000000;", prepared.PreparedSource); } + + /// + /// Verifies body line padding preserves original line numbers when a leading using is extracted. + /// + [Test] + public void Prepare_WhenLeadingUsingDirectiveIsExtracted_ShouldPreserveUserSnippetLineNumbers() + { + string source = "using UnityEngine;\nreturn null;"; + PreparedDynamicCode prepared = DynamicCodeSourcePreparer.PrepareWithoutLiteralHoisting( + source, + DynamicCodeConstants.DEFAULT_NAMESPACE, + DynamicCodeConstants.DEFAULT_CLASS_NAME); + + AssertUserSnippetLineAlignment(source, prepared.PreparedSource); + Assert.That(GetWrappedUserSnippetLines(prepared.PreparedSource)[1], Is.EqualTo("return null;")); + } + + /// + /// Verifies a blank line between an extracted using and the first statement keeps the statement on its original line. + /// + [Test] + public void Prepare_WhenBlankLineFollowsExtractedUsing_ShouldPreserveUserSnippetLineNumbers() + { + string source = "using UnityEngine;\n\nreturn null;"; + PreparedDynamicCode prepared = DynamicCodeSourcePreparer.PrepareWithoutLiteralHoisting( + source, + DynamicCodeConstants.DEFAULT_NAMESPACE, + DynamicCodeConstants.DEFAULT_CLASS_NAME); + + AssertUserSnippetLineAlignment(source, prepared.PreparedSource); + Assert.That(GetWrappedUserSnippetLines(prepared.PreparedSource)[2], Is.EqualTo("return null;")); + } + + /// + /// Verifies a skipped leading comment and extracted using both preserve later statement line numbers. + /// + [Test] + public void Prepare_WhenLeadingCommentPrecedesExtractedUsing_ShouldPreserveUserSnippetLineNumbers() + { + string source = "// setup\nusing UnityEngine;\nreturn null;"; + PreparedDynamicCode prepared = DynamicCodeSourcePreparer.PrepareWithoutLiteralHoisting( + source, + DynamicCodeConstants.DEFAULT_NAMESPACE, + DynamicCodeConstants.DEFAULT_CLASS_NAME); + + AssertUserSnippetLineAlignment(source, prepared.PreparedSource); + Assert.That(GetWrappedUserSnippetLines(prepared.PreparedSource)[2], Is.EqualTo("return null;")); + } + + /// + /// Verifies multiple extracted usings separated by a blank line preserve the first statement line number. + /// + [Test] + public void Prepare_WhenMultipleExtractedUsingsAreSeparatedByBlankLine_ShouldPreserveUserSnippetLineNumbers() + { + string source = "using System;\n\nusing UnityEngine;\nreturn null;"; + PreparedDynamicCode prepared = DynamicCodeSourcePreparer.PrepareWithoutLiteralHoisting( + source, + DynamicCodeConstants.DEFAULT_NAMESPACE, + DynamicCodeConstants.DEFAULT_CLASS_NAME); + + AssertUserSnippetLineAlignment(source, prepared.PreparedSource); + Assert.That(GetWrappedUserSnippetLines(prepared.PreparedSource)[3], Is.EqualTo("return null;")); + } + + /// + /// Verifies a blank line between top-level statements preserves the later statement line number. + /// + [Test] + public void Prepare_WhenBlankLineSeparatesTopLevelStatements_ShouldPreserveUserSnippetLineNumbers() + { + string source = "int a = 1;\n\nreturn null;"; + PreparedDynamicCode prepared = DynamicCodeSourcePreparer.PrepareWithoutLiteralHoisting( + source, + DynamicCodeConstants.DEFAULT_NAMESPACE, + DynamicCodeConstants.DEFAULT_CLASS_NAME); + + AssertUserSnippetLineAlignment(source, prepared.PreparedSource); + string[] wrappedLines = GetWrappedUserSnippetLines(prepared.PreparedSource); + Assert.That(wrappedLines[0], Is.EqualTo("int a = 1;")); + Assert.That(wrappedLines[2], Is.EqualTo("return null;")); + } + + private static string[] GetWrappedUserSnippetLines(string preparedSource) + { + WrappedDynamicCodeUserSnippetExtractor.TryExtract(preparedSource, out string snippet); + return WrappedDynamicCodeUserSnippetExtractor.SplitNormalizedLines(snippet); + } + + private static void AssertUserSnippetLineAlignment(string originalSource, string preparedSource) + { + string[] originalLines = DynamicCodeUserSnippetLines.Split(originalSource); + string[] wrappedLines = GetWrappedUserSnippetLines(preparedSource); + Assert.That(wrappedLines.Length, Is.EqualTo(originalLines.Length)); + + for (int lineIndex = 0; lineIndex < originalLines.Length; lineIndex++) + { + string originalLine = originalLines[lineIndex]; + string trimmedStart = originalLine.TrimStart(); + if (trimmedStart.StartsWith("using ", System.StringComparison.Ordinal) + || trimmedStart.StartsWith("global using ", System.StringComparison.Ordinal) + || trimmedStart.StartsWith("//", System.StringComparison.Ordinal) + || originalLine.Length == 0) + { + Assert.That(wrappedLines[lineIndex], Is.Empty, $"Line {lineIndex + 1} should stay empty in the #line region."); + } + } + } } } diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SourceShaper.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SourceShaper.cs index 60de0388e..e958654dc 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SourceShaper.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/SourceShaper.cs @@ -323,10 +323,58 @@ private static SourceTopLevelStep AnalyzeTopLevelStatement( result.HasTopLevelStatements = true; int nextBraceDepth = braceDepth; int stmtEnd = FindStatementEnd(source, pos, ref nextBraceDepth); - result.TopLevelBodyBuilder.AppendLine(source.Substring(pos, stmtEnd - pos + 1).TrimEnd()); + int originalLineNumber1Based = GetLineNumber1Based(source, pos); + PadTopLevelBodyBuilderToOriginalLine(result, originalLineNumber1Based); + string statementText = source.Substring(pos, stmtEnd - pos + 1).TrimEnd(); + result.TopLevelBodyBuilder.AppendLine(statementText); + result.NextBodyLineNumber1Based = originalLineNumber1Based + CountLinesInText(statementText); return new SourceTopLevelStep(stmtEnd + 1, nextBraceDepth); } + private static void PadTopLevelBodyBuilderToOriginalLine( + SourceShapeResult result, + int originalLineNumber1Based) + { + while (result.NextBodyLineNumber1Based < originalLineNumber1Based) + { + result.TopLevelBodyBuilder.AppendLine(); + result.NextBodyLineNumber1Based++; + } + } + + private static int GetLineNumber1Based(string source, int index) + { + int lineNumber = 1; + for (int position = 0; position < index && position < source.Length; position++) + { + if (source[position] == '\n') + { + lineNumber++; + } + } + + return lineNumber; + } + + private static int CountLinesInText(string text) + { + if (string.IsNullOrEmpty(text)) + { + return 0; + } + + int lineCount = 1; + for (int index = 0; index < text.Length; index++) + { + if (text[index] == '\n') + { + lineCount++; + } + } + + return lineCount; + } + private static SourceTopLevelStep SkipTopLevelBlock(string source, int pos, int braceDepth) { int nextBraceDepth = braceDepth; @@ -818,6 +866,7 @@ public SourceTopLevelStep(int position, int braceDepth) internal sealed class SourceShapeResult { public List UsingDirectives { get; } = new List(); + public int NextBodyLineNumber1Based { get; set; } = 1; public HashSet AliasedNames { get; } = new HashSet(System.StringComparer.Ordinal); public bool HasNamespaceDeclaration { get; set; } public bool HasTypeDeclaration { get; set; }