diff --git a/.agents/skills/uloop-launch/SKILL.md b/.agents/skills/uloop-launch/SKILL.md index 8d2f6096d5..a90f98f35b 100644 --- a/.agents/skills/uloop-launch/SKILL.md +++ b/.agents/skills/uloop-launch/SKILL.md @@ -21,8 +21,8 @@ uloop launch [project-path] [options] | Parameter | Type | Description | |-----------|------|-------------| | `project-path` | string | Optional. Use only when the target Unity project is not in the current directory. | -| `-r, --restart` | flag | Kill running Unity and restart | -| `-q, --quit` | flag | Kill an existing Unity process for the project without launching | +| `-r, --restart` | flag | Quit running Unity and restart | +| `-q, --quit` | flag | Quit an existing Unity process for the project without launching | | `--editor-version ` | string | Use this Unity Editor version instead of ProjectVersion.txt | | `-p, --platform

` | string | Build target (e.g., StandaloneOSX, Android, iOS) | | `--max-depth ` | number | Search depth when project-path is omitted (default: 3, -1 for unlimited) | @@ -42,6 +42,7 @@ The final JSON payload includes: - `Restarted`: whether this command stopped an existing process and launched a new one - `Quit`: whether this command stopped Unity without launching a new process - `PreviousProcessId`: process ID stopped by restart or quit, when available +- `StopMethod`: how restart or quit stopped that process, omitted when there was none — `Quit` when Unity exited on request, `ForceKill` when it had to be killed - `CurrentProcessId`: current Unity process ID, when available - `ProjectRoot`: resolved project root - `Message`: readiness summary @@ -56,6 +57,7 @@ running, or still does not respond after retries. ## Notes - If Unity is already running, focuses the existing window and verifies tool readiness +- `--restart` and `--quit` ask Unity to quit first: it saves its window layout and discards unsaved scene changes without a save dialog. Unity is force-killed instead, losing the layout, when it cannot take the request (frozen, or a package that predates it) or does not exit within 30 seconds - If process scan is blocked (e.g. sandboxed `ps`), plain launch falls back to IPC probing; `--restart` and `--quit` still fail because they need the process id - New Unity processes ignore project compiler errors during Editor startup by default when run by dispatcher 3.0.1-beta.8 or newer; run `uloop update` first if an older dispatcher is installed - `--editor-version` only affects new Unity processes; use it with `--restart` to replace an already-running Editor diff --git a/.claude/skills/uloop-launch/SKILL.md b/.claude/skills/uloop-launch/SKILL.md index 8d2f6096d5..a90f98f35b 100644 --- a/.claude/skills/uloop-launch/SKILL.md +++ b/.claude/skills/uloop-launch/SKILL.md @@ -21,8 +21,8 @@ uloop launch [project-path] [options] | Parameter | Type | Description | |-----------|------|-------------| | `project-path` | string | Optional. Use only when the target Unity project is not in the current directory. | -| `-r, --restart` | flag | Kill running Unity and restart | -| `-q, --quit` | flag | Kill an existing Unity process for the project without launching | +| `-r, --restart` | flag | Quit running Unity and restart | +| `-q, --quit` | flag | Quit an existing Unity process for the project without launching | | `--editor-version ` | string | Use this Unity Editor version instead of ProjectVersion.txt | | `-p, --platform

` | string | Build target (e.g., StandaloneOSX, Android, iOS) | | `--max-depth ` | number | Search depth when project-path is omitted (default: 3, -1 for unlimited) | @@ -42,6 +42,7 @@ The final JSON payload includes: - `Restarted`: whether this command stopped an existing process and launched a new one - `Quit`: whether this command stopped Unity without launching a new process - `PreviousProcessId`: process ID stopped by restart or quit, when available +- `StopMethod`: how restart or quit stopped that process, omitted when there was none — `Quit` when Unity exited on request, `ForceKill` when it had to be killed - `CurrentProcessId`: current Unity process ID, when available - `ProjectRoot`: resolved project root - `Message`: readiness summary @@ -56,6 +57,7 @@ running, or still does not respond after retries. ## Notes - If Unity is already running, focuses the existing window and verifies tool readiness +- `--restart` and `--quit` ask Unity to quit first: it saves its window layout and discards unsaved scene changes without a save dialog. Unity is force-killed instead, losing the layout, when it cannot take the request (frozen, or a package that predates it) or does not exit within 30 seconds - If process scan is blocked (e.g. sandboxed `ps`), plain launch falls back to IPC probing; `--restart` and `--quit` still fail because they need the process id - New Unity processes ignore project compiler errors during Editor startup by default when run by dispatcher 3.0.1-beta.8 or newer; run `uloop update` first if an older dispatcher is installed - `--editor-version` only affects new Unity processes; use it with `--restart` to replace an already-running Editor diff --git a/Assets/Tests/Editor/QuitEditorBridgeCommandTests.cs b/Assets/Tests/Editor/QuitEditorBridgeCommandTests.cs new file mode 100644 index 0000000000..b08a6272be --- /dev/null +++ b/Assets/Tests/Editor/QuitEditorBridgeCommandTests.cs @@ -0,0 +1,88 @@ +using System.Collections.Generic; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.Infrastructure; +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + ///

+ /// Tests the quit-editor bridge command: its routing, and when it exits the Editor. The handler itself is + /// not run here because it exits the Editor that runs the tests. + /// + [TestFixture] + public sealed class QuitEditorBridgeCommandTests + { + private const double ExitAt = 10.0; + + /// + /// Verifies quit-editor is routed as an internal bridge command, which uloop launch --restart and + /// --quit rely on before they fall back to force-killing the Editor. + /// + [Test] + public void InternalBridgeCommandRouter_QuitEditorCommand_IsInternal() + { + bool isInternal = InternalBridgeCommandRouter.IsInternalCommand( + UnityCliLoopConstants.COMMAND_NAME_QUIT_EDITOR); + + Assert.That(isInternal, Is.True); + } + + /// + /// Verifies the command name matches the one the CLI sends. + /// + [Test] + public void QuitEditorCommandName_MatchesTheCliRequest() + { + Assert.That(UnityCliLoopConstants.COMMAND_NAME_QUIT_EDITOR, Is.EqualTo("quit-editor")); + } + + /// + /// Verifies the Editor is not exited before the deadline, which leaves quit-editor time to answer. + /// + [Test] + public void DelayedEditorExit_BeforeTheDeadline_DoesNotExit() + { + List exitCodes = new(); + DelayedEditorExit delayedExit = new(ExitAt, exitCodes.Add); + + bool exitRequested = delayedExit.ExitIfDue(ExitAt - 0.1); + + Assert.That(exitRequested, Is.False); + Assert.That(exitCodes, Is.Empty); + } + + /// + /// Verifies the Editor exits with code 0 once the deadline is reached. + /// + [Test] + public void DelayedEditorExit_AtTheDeadline_ExitsWithCodeZero() + { + List exitCodes = new(); + DelayedEditorExit delayedExit = new(ExitAt, exitCodes.Add); + + bool exitRequested = delayedExit.ExitIfDue(ExitAt); + + Assert.That(exitRequested, Is.True); + Assert.That(exitCodes, Is.EqualTo(new[] { 0 })); + } + + /// + /// Verifies a check after the exit does not exit again, since the update callback can run once more + /// before it is removed. + /// + [Test] + public void DelayedEditorExit_AfterExiting_DoesNotExitAgain() + { + List exitCodes = new(); + DelayedEditorExit delayedExit = new(ExitAt, exitCodes.Add); + delayedExit.ExitIfDue(ExitAt); + + bool exitRequested = delayedExit.ExitIfDue(ExitAt + 1.0); + + Assert.That(exitRequested, Is.True); + Assert.That(exitCodes, Is.EqualTo(new[] { 0 })); + } + } +} diff --git a/Assets/Tests/Editor/QuitEditorBridgeCommandTests.cs.meta b/Assets/Tests/Editor/QuitEditorBridgeCommandTests.cs.meta new file mode 100644 index 0000000000..fbddeaa6aa --- /dev/null +++ b/Assets/Tests/Editor/QuitEditorBridgeCommandTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: cc55c7b28f4764b789b324eeb4afee80 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/CliOnlyTools~/Launch/Skill/SKILL.md b/Packages/src/Editor/CliOnlyTools~/Launch/Skill/SKILL.md index 8d2f6096d5..a90f98f35b 100644 --- a/Packages/src/Editor/CliOnlyTools~/Launch/Skill/SKILL.md +++ b/Packages/src/Editor/CliOnlyTools~/Launch/Skill/SKILL.md @@ -21,8 +21,8 @@ uloop launch [project-path] [options] | Parameter | Type | Description | |-----------|------|-------------| | `project-path` | string | Optional. Use only when the target Unity project is not in the current directory. | -| `-r, --restart` | flag | Kill running Unity and restart | -| `-q, --quit` | flag | Kill an existing Unity process for the project without launching | +| `-r, --restart` | flag | Quit running Unity and restart | +| `-q, --quit` | flag | Quit an existing Unity process for the project without launching | | `--editor-version ` | string | Use this Unity Editor version instead of ProjectVersion.txt | | `-p, --platform

` | string | Build target (e.g., StandaloneOSX, Android, iOS) | | `--max-depth ` | number | Search depth when project-path is omitted (default: 3, -1 for unlimited) | @@ -42,6 +42,7 @@ The final JSON payload includes: - `Restarted`: whether this command stopped an existing process and launched a new one - `Quit`: whether this command stopped Unity without launching a new process - `PreviousProcessId`: process ID stopped by restart or quit, when available +- `StopMethod`: how restart or quit stopped that process, omitted when there was none — `Quit` when Unity exited on request, `ForceKill` when it had to be killed - `CurrentProcessId`: current Unity process ID, when available - `ProjectRoot`: resolved project root - `Message`: readiness summary @@ -56,6 +57,7 @@ running, or still does not respond after retries. ## Notes - If Unity is already running, focuses the existing window and verifies tool readiness +- `--restart` and `--quit` ask Unity to quit first: it saves its window layout and discards unsaved scene changes without a save dialog. Unity is force-killed instead, losing the layout, when it cannot take the request (frozen, or a package that predates it) or does not exit within 30 seconds - If process scan is blocked (e.g. sandboxed `ps`), plain launch falls back to IPC probing; `--restart` and `--quit` still fail because they need the process id - New Unity processes ignore project compiler errors during Editor startup by default when run by dispatcher 3.0.1-beta.8 or newer; run `uloop update` first if an older dispatcher is installed - `--editor-version` only affects new Unity processes; use it with `--restart` to replace an already-running Editor diff --git a/Packages/src/Editor/Infrastructure/Api/DelayedEditorExit.cs b/Packages/src/Editor/Infrastructure/Api/DelayedEditorExit.cs new file mode 100644 index 0000000000..3d06cfa572 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/Api/DelayedEditorExit.cs @@ -0,0 +1,42 @@ +using System; + +namespace io.github.hatayama.UnityCliLoop.Infrastructure +{ + ///

+ /// Exits the Editor once a deadline has passed, and only once, so that quit-editor can answer before + /// the process ends. + /// + internal sealed class DelayedEditorExit + { + private readonly double _exitAt; + private readonly Action _exit; + private bool _hasExited; + + internal DelayedEditorExit(double exitAt, Action exit) + { + _exitAt = exitAt; + _exit = exit; + } + + /// + /// Exits with code 0 once now has reached the deadline. Returns true once the exit has been + /// requested, so the caller can stop checking. + /// + internal bool ExitIfDue(double now) + { + if (_hasExited) + { + return true; + } + + if (now < _exitAt) + { + return false; + } + + _hasExited = true; + _exit(0); + return true; + } + } +} diff --git a/Packages/src/Editor/Infrastructure/Api/DelayedEditorExit.cs.meta b/Packages/src/Editor/Infrastructure/Api/DelayedEditorExit.cs.meta new file mode 100644 index 0000000000..d1e4104657 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/Api/DelayedEditorExit.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 7d93267c2c7af4b318efa74cea4ced4c +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Infrastructure/Api/InternalBridgeCommandRouter.cs b/Packages/src/Editor/Infrastructure/Api/InternalBridgeCommandRouter.cs index 42d178c6e3..1e6479d6cf 100644 --- a/Packages/src/Editor/Infrastructure/Api/InternalBridgeCommandRouter.cs +++ b/Packages/src/Editor/Infrastructure/Api/InternalBridgeCommandRouter.cs @@ -22,7 +22,8 @@ public static bool IsInternalCommand(string methodName) methodName == UnityCliLoopConstants.COMMAND_NAME_EXTEND_PAUSE_POINT_STATUS || methodName == UnityCliLoopConstants.COMMAND_NAME_SET_CODE_OPTIMIZATION_DEBUG || methodName == UnityCliLoopConstants.COMMAND_NAME_SET_CODE_OPTIMIZATION_DEBUG_STARTUP || - methodName == UnityCliLoopConstants.COMMAND_NAME_GET_TOOL_DETAILS; + methodName == UnityCliLoopConstants.COMMAND_NAME_GET_TOOL_DETAILS || + methodName == UnityCliLoopConstants.COMMAND_NAME_QUIT_EDITOR; } public static UnityCliLoopToolResponse Execute( @@ -83,6 +84,11 @@ public static UnityCliLoopToolResponse Execute( return SetCodeOptimizationDebugStartupBridgeCommand.Execute(); } + if (methodName == UnityCliLoopConstants.COMMAND_NAME_QUIT_EDITOR) + { + return QuitEditorBridgeCommand.Execute(); + } + throw new ArgumentException($"Unknown internal bridge command: {methodName}", nameof(methodName)); } } diff --git a/Packages/src/Editor/Infrastructure/Api/QuitEditorBridgeCommand.cs b/Packages/src/Editor/Infrastructure/Api/QuitEditorBridgeCommand.cs new file mode 100644 index 0000000000..a1882c983d --- /dev/null +++ b/Packages/src/Editor/Infrastructure/Api/QuitEditorBridgeCommand.cs @@ -0,0 +1,47 @@ +using UnityEditor; + +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.Infrastructure +{ + /// + /// Answers quit-editor; its Success is the only payload. + /// + public class QuitEditorResponse : UnityCliLoopToolResponse + { + } + + /// + /// Runs the quit-editor command behind uloop launch --restart and --quit: closes the Editor so that it + /// saves its window layout, which a force-kill loses. + /// + internal static class QuitEditorBridgeCommand + { + // Long enough for this answer to be written before the process ends. + private const double ExitDelaySeconds = 0.5; + + public static QuitEditorResponse Execute() + { + // Why Exit rather than the quit the Editor menu runs: that quit asks whether to save modified + // scenes and waits for an answer nobody gives, while Exit discards them without asking and still + // saves the window layout. + DelayedEditorExit delayedExit = new( + EditorApplication.timeSinceStartup + ExitDelaySeconds, + EditorApplication.Exit); + EditorApplication.CallbackFunction exitWhenDue = null; + exitWhenDue = () => + { + if (!delayedExit.ExitIfDue(EditorApplication.timeSinceStartup)) + { + return; + } + + EditorApplication.update -= exitWhenDue; + }; + // Why update rather than delayCall: an Editor in the background that is not repainting does not + // run delayCall, so the exit would never come. + EditorApplication.update += exitWhenDue; + return new QuitEditorResponse(); + } + } +} diff --git a/Packages/src/Editor/Infrastructure/Api/QuitEditorBridgeCommand.cs.meta b/Packages/src/Editor/Infrastructure/Api/QuitEditorBridgeCommand.cs.meta new file mode 100644 index 0000000000..ccef82e9dd --- /dev/null +++ b/Packages/src/Editor/Infrastructure/Api/QuitEditorBridgeCommand.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: a66f715073af64f58b6eae2ac16ccf08 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs b/Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs index a5db7b48d3..720d9ffc6a 100644 --- a/Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs +++ b/Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs @@ -82,6 +82,7 @@ public static UnityEditor.PackageManager.PackageInfo PackageInfo public const string COMMAND_NAME_GET_RUN_TESTS_STATUS = "get-run-tests-status"; public const string COMMAND_NAME_GET_EDITOR_STATUS = "get-editor-status"; public const string COMMAND_NAME_CANCEL_RUNNING_TOOL = "cancel-running-tool"; + public const string COMMAND_NAME_QUIT_EDITOR = "quit-editor"; public const string COMMAND_NAME_AWAIT_PAUSE_POINT = "await-pause-point"; public const string SETTINGS_TOOL_NAME_PAUSE_POINT = "pause-point"; public const string COMMAND_NAME_PAUSE_POINT_STATUS = "pause-point-status"; diff --git a/README.md b/README.md index 9060e89c28..dd293db168 100644 --- a/README.md +++ b/README.md @@ -286,7 +286,7 @@ uloop launch # Launch with build target (Android, iOS, StandaloneOSX, etc.) uloop launch -p Android -# Kill running Unity and restart +# Quit running Unity and restart uloop launch -r # Check .meta files, GUIDs, and conflict markers without Unity diff --git a/cli/dispatcher/internal/dispatcher/launch.go b/cli/dispatcher/internal/dispatcher/launch.go index 56a313a4e8..1add59ac65 100644 --- a/cli/dispatcher/internal/dispatcher/launch.go +++ b/cli/dispatcher/internal/dispatcher/launch.go @@ -118,17 +118,20 @@ func runLaunchWithDeps(ctx context.Context, options launchOptions, startPath str return code } + var previousStop launchStopMethod if runningProcess != nil { - if handled, code := handleExistingLaunchProcess(ctx, options, projectRoot, runningProcess, v2Project.IsV2, stdout, stderr, deps); handled { + stopMethod, handled, code := handleExistingLaunchProcess(ctx, options, projectRoot, runningProcess, v2Project.IsV2, stdout, stderr, deps) + if handled { return code } + previousStop = stopMethod } if options.quit { - return writeLaunchQuitResponse(stdout, stderr, projectRoot, nil, launchNoProcessMessage) + return writeLaunchQuitResponse(stdout, stderr, projectRoot, nil, "") } - return startUnityAndWaitForReadiness(ctx, options, projectRoot, runningProcess, v2Project.IsV2, previousV2ServerSessionID, stdout, stderr, deps) + return startUnityAndWaitForReadiness(ctx, options, projectRoot, runningProcess, previousStop, v2Project.IsV2, previousV2ServerSessionID, stdout, stderr, deps) } func writeLaunchProjectSearch(stdout io.Writer, options launchOptions, startPath string) { @@ -184,29 +187,26 @@ func handleExistingLaunchProcess( stdout io.Writer, stderr io.Writer, deps launchDeps, -) (bool, int) { +) (launchStopMethod, bool, int) { if !options.restart && !options.quit { if options.editorVersion != "" { clierrors.WriteClassifiedError(stderr, launchEditorVersionRequiresRestartError(options.editorVersion), clierrors.ErrorContext{ProjectRoot: projectRoot, Command: clicore.LaunchCommandName}) - return true, 1 + return "", true, 1 } if isV2 { - return true, waitForExistingV2LaunchReadiness(ctx, projectRoot, runningProcess.Pid, stdout, stderr, deps) + return "", true, waitForExistingV2LaunchReadiness(ctx, projectRoot, runningProcess.Pid, stdout, stderr, deps) } - return true, waitForExistingLaunchReadiness(ctx, projectRoot, runningProcess.Pid, stdout, stderr, deps) - } - if err := deps.killUnityProcess(runningProcess.Pid); err != nil { - clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ProjectRoot: projectRoot, Command: clicore.LaunchCommandName}) - return true, 1 + return "", true, waitForExistingLaunchReadiness(ctx, projectRoot, runningProcess.Pid, stdout, stderr, deps) } - if err := deps.waitForUnityProcessExit(ctx, projectRoot, runningProcess.Pid, launchProcessExitPoll, launchProcessExitTimeout); err != nil { + stopMethod, err := stopUnityProcess(ctx, projectRoot, runningProcess.Pid, isV2, deps) + if err != nil { clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ProjectRoot: projectRoot, Command: clicore.LaunchCommandName}) - return true, 1 + return "", true, 1 } if options.quit { - return true, writeLaunchQuitResponse(stdout, stderr, projectRoot, &runningProcess.Pid, launchStoppedMessage) + return stopMethod, true, writeLaunchQuitResponse(stdout, stderr, projectRoot, &runningProcess.Pid, stopMethod) } - return false, 0 + return stopMethod, false, 0 } func launchEditorVersionRequiresRestartError(editorVersion string) error { @@ -239,6 +239,7 @@ func startUnityAndWaitForReadiness( options launchOptions, projectRoot string, runningProcess *unityprocess.UnityProcess, + previousStop launchStopMethod, isV2 bool, previousV2ServerSessionID string, stdout io.Writer, @@ -289,7 +290,7 @@ func startUnityAndWaitForReadiness( return 1 } if isV2 { - return waitForV2ProjectOpened(ctx, projectRoot, runningProcess, currentPid, stdout, stderr, launchStartedAt, previousV2ServerSessionID, spinner, deps) + return waitForV2ProjectOpened(ctx, projectRoot, runningProcess, previousStop, currentPid, stdout, stderr, launchStartedAt, previousV2ServerSessionID, spinner, deps) } if err := deps.waitForUnityStartupMarker(ctx, unityLockfilePath(projectRoot), launchLockfilePoll, launchLockfileTimeout); err != nil { clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ProjectRoot: projectRoot, Command: clicore.LaunchCommandName}) @@ -306,7 +307,7 @@ func startUnityAndWaitForReadiness( if runningProcess != nil { previousPid = &runningProcess.Pid } - return writeLaunchedReadyResponse(stdout, stderr, projectRoot, previousPid, currentPid) + return writeLaunchedReadyResponse(stdout, stderr, projectRoot, previousPid, previousStop, currentPid) } func buildUnityLaunchArgs(projectRoot string, options launchOptions) []string { @@ -461,8 +462,8 @@ func printLaunchHelp(stdout io.Writer) { clicore.WriteLine(stdout, " uloop launch [options] [project-path]") clicore.WriteLine(stdout, "") clicore.WriteLine(stdout, "Options:") - clicore.WriteLine(stdout, " -r, --restart Kill an existing Unity process for the project before launching") - clicore.WriteLine(stdout, " -q, --quit Kill an existing Unity process for the project without launching") + clicore.WriteLine(stdout, " -r, --restart Quit an existing Unity process for the project before launching") + clicore.WriteLine(stdout, " -q, --quit Quit an existing Unity process for the project without launching") clicore.WriteLine(stdout, " -d, --delete-recovery Delete Assets/_Recovery before launch") clicore.WriteLine(stdout, " --editor-version ") clicore.WriteLine(stdout, " Use this Unity Editor version instead of ProjectVersion.txt") diff --git a/cli/dispatcher/internal/dispatcher/launch_deps.go b/cli/dispatcher/internal/dispatcher/launch_deps.go index 5faa84527e..b06219c1f0 100644 --- a/cli/dispatcher/internal/dispatcher/launch_deps.go +++ b/cli/dispatcher/internal/dispatcher/launch_deps.go @@ -14,6 +14,7 @@ type launchDeps struct { findRunningUnityProcess func(context.Context, string) (*unityprocess.UnityProcess, error) focusUnityProcess func(context.Context, int) error killUnityProcess func(int) error + requestUnityQuit func(context.Context, string) bool resolveUnityExecutablePath func(string) (string, error) waitForUnityProcessExit func(context.Context, string, int, time.Duration, time.Duration) error waitForUnityStartupMarker func(context.Context, string, time.Duration, time.Duration) error @@ -32,6 +33,7 @@ func defaultLaunchDeps() launchDeps { findRunningUnityProcess: unityprocess.FindRunningUnityProcess, focusUnityProcess: unityprocess.FocusUnityProcess, killUnityProcess: killUnityProcess, + requestUnityQuit: requestUnityQuit, resolveUnityExecutablePath: resolveUnityExecutablePath, waitForUnityProcessExit: waitForUnityProcessExit, waitForUnityStartupMarker: waitForUnityStartupMarkerOrTimeout, diff --git a/cli/dispatcher/internal/dispatcher/launch_ready.go b/cli/dispatcher/internal/dispatcher/launch_ready.go index 03d16e1239..f924e89a7d 100644 --- a/cli/dispatcher/internal/dispatcher/launch_ready.go +++ b/cli/dispatcher/internal/dispatcher/launch_ready.go @@ -15,7 +15,8 @@ const ( launchReadinessMessage = "Waiting for Unity CLI Loop server readiness..." launchReadyMessage = "Unity CLI Loop is ready." launchAlreadyRunningReadyMessage = "Unity is already running and ready." - launchStoppedMessage = "Unity process stopped." + launchQuitStoppedMessage = "Unity quit. It saved its window layout and discarded unsaved scene changes." + launchForceKillStoppedMessage = "Unity was force-killed. Its window layout was not saved, and unsaved scene changes were lost." launchNoProcessMessage = "No matching Unity process was found; it may have already exited." ) @@ -29,6 +30,7 @@ type launchReadyResponse struct { Restarted bool `json:"Restarted"` Quit bool `json:"Quit"` PreviousProcessId *int `json:"PreviousProcessId,omitempty"` + StopMethod string `json:"StopMethod,omitempty"` CurrentProcessId *int `json:"CurrentProcessId,omitempty"` ProjectRoot string `json:"ProjectRoot"` Message string `json:"Message"` @@ -88,6 +90,7 @@ func writeLaunchedReadyResponse( stderr io.Writer, projectRoot string, previousPid *int, + previousStop launchStopMethod, currentPid int, ) int { return writeLaunchResponse(stdout, stderr, launchReadyResponse{ @@ -98,6 +101,7 @@ func writeLaunchedReadyResponse( Launched: true, Restarted: previousPid != nil, PreviousProcessId: previousPid, + StopMethod: string(previousStop), CurrentProcessId: ¤tPid, ProjectRoot: projectRoot, Message: launchReadyMessage, @@ -109,6 +113,7 @@ func writeLaunchedV2ReadyResponse( stderr io.Writer, projectRoot string, previousPid *int, + previousStop launchStopMethod, currentPid int, ) int { return writeLaunchResponse(stdout, stderr, launchReadyResponse{ @@ -119,6 +124,7 @@ func writeLaunchedV2ReadyResponse( Launched: true, Restarted: previousPid != nil, PreviousProcessId: previousPid, + StopMethod: string(previousStop), CurrentProcessId: ¤tPid, ProjectRoot: projectRoot, Message: "Unity started and the V2 uloop server is ready. This project uses uloop V2; commands run through the V2 CLI.", @@ -130,17 +136,30 @@ func writeLaunchQuitResponse( stderr io.Writer, projectRoot string, previousPid *int, - message string, + stopMethod launchStopMethod, ) int { return writeLaunchResponse(stdout, stderr, launchReadyResponse{ Success: true, Quit: true, PreviousProcessId: previousPid, + StopMethod: string(stopMethod), ProjectRoot: projectRoot, - Message: message, + Message: launchQuitMessage(stopMethod), }) } +// launchQuitMessage words the --quit result for how the Editor stopped, or for no Editor to stop. +func launchQuitMessage(stopMethod launchStopMethod) string { + switch stopMethod { + case launchStopMethodQuit: + return launchQuitStoppedMessage + case launchStopMethodForceKill: + return launchForceKillStoppedMessage + default: + return launchNoProcessMessage + } +} + func writeLaunchResponse(stdout io.Writer, stderr io.Writer, response launchReadyResponse) int { payload, err := json.Marshal(response) if err != nil { diff --git a/cli/dispatcher/internal/dispatcher/launch_test.go b/cli/dispatcher/internal/dispatcher/launch_test.go index e1b80655ee..23b3df1736 100644 --- a/cli/dispatcher/internal/dispatcher/launch_test.go +++ b/cli/dispatcher/internal/dispatcher/launch_test.go @@ -523,7 +523,8 @@ func TestRunLaunchForExistingV2ProjectWaitsForServerReady(t *testing.T) { } } -// Verifies -r captures the pre-kill serverSessionId and passes it to the V2 readiness probe. +// Verifies -r captures the pre-kill serverSessionId and passes it to the V2 readiness probe, and reports +// the force-kill that stopped the previous Editor. func TestRunLaunchForV2RestartPassesPreviousServerSessionID(t *testing.T) { projectRoot := createLaunchTestProject(t) writeV2PackageManifest(t, projectRoot) @@ -573,6 +574,9 @@ func TestRunLaunchForV2RestartPassesPreviousServerSessionID(t *testing.T) { if !response.Success || !response.Ready || !response.ServerReady || !response.Restarted { t.Fatalf("V2 restart response = %#v", response) } + if response.StopMethod != string(launchStopMethodForceKill) { + t.Fatalf("V2 restart StopMethod = %q, want %q", response.StopMethod, launchStopMethodForceKill) + } } // Verifies a V2 server readiness timeout exits 1 and surfaces the classified timeout error. diff --git a/cli/dispatcher/internal/dispatcher/launch_unity_quit_request_unix_test.go b/cli/dispatcher/internal/dispatcher/launch_unity_quit_request_unix_test.go new file mode 100644 index 0000000000..cf049fbd6b --- /dev/null +++ b/cli/dispatcher/internal/dispatcher/launch_unity_quit_request_unix_test.go @@ -0,0 +1,141 @@ +//go:build !windows + +package dispatcher + +import ( + "bufio" + "context" + "encoding/json" + "net" + "testing" + "time" + + "github.com/hatayama/unity-cli-loop/common/unityipc" +) + +const quitRequestTestTimeout = 300 * time.Millisecond + +// quitRequestTestServer serves one connection: it reads the request, records its method, and then calls +// reply, which may answer, hold the connection open, or return to drop it. +func quitRequestTestServer(t *testing.T, reply func(net.Conn)) (unityipc.Connection, <-chan string) { + t.Helper() + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("failed to listen: %v", err) + } + t.Cleanup(func() { _ = listener.Close() }) + methods := make(chan string, 1) + go func() { + conn, err := listener.Accept() + if err != nil { + return + } + defer func() { _ = conn.Close() }() + payload, err := unityipc.Read(bufio.NewReader(conn)) + if err != nil { + return + } + var request struct { + Method string `json:"method"` + } + _ = json.Unmarshal(payload, &request) + methods <- request.Method + reply(conn) + }() + return unityipc.Connection{ + ProjectRoot: t.TempDir(), + Endpoint: unityipc.Endpoint{Network: "tcp", Address: listener.Addr().String()}, + }, methods +} + +// Verifies an Editor that answers the quit request is taken as quitting, and that the request names the +// quit-editor bridge command. +func TestSendUnityQuitRequestTakesAnAnswerAsAQuitInProgress(t *testing.T) { + connection, methods := quitRequestTestServer(t, func(conn net.Conn) { + _ = unityipc.Write(conn, []byte(`{"jsonrpc":"2.0","result":{"Success":true},"id":1}`)) + }) + + if !sendUnityQuitRequest(context.Background(), connection, quitRequestTestTimeout) { + t.Fatal("an answered quit request must be taken as a quit in progress") + } + if method := <-methods; method != quitEditorBridgeCommandName { + t.Fatalf("method = %q, want %q", method, quitEditorBridgeCommandName) + } +} + +// Verifies a connection that drops after the request reached the Editor is taken as a quit in progress, +// since an Editor that exits before its answer is written drops the connection the same way. +func TestSendUnityQuitRequestTakesADropAfterTheRequestArrivedAsAQuitInProgress(t *testing.T) { + connection, _ := quitRequestTestServer(t, func(net.Conn) {}) + + if !sendUnityQuitRequest(context.Background(), connection, quitRequestTestTimeout) { + t.Fatal("a connection dropped after the request arrived must be taken as a quit in progress") + } +} + +// Verifies an Editor that acknowledges the request but never runs it, as a frozen main thread does, is +// not taken as quitting once the request times out. +func TestSendUnityQuitRequestDoesNotTakeAnUnansweredRequestAsAQuit(t *testing.T) { + connection, _ := quitRequestTestServer(t, func(conn net.Conn) { + _ = unityipc.Write(conn, []byte(`{"jsonrpc":"2.0","uloop":{"phase":"accepted"},"id":1}`)) + time.Sleep(3 * quitRequestTestTimeout) + }) + + if sendUnityQuitRequest(context.Background(), connection, quitRequestTestTimeout) { + t.Fatal("a request the Editor never answered must not be taken as a quit in progress") + } +} + +// Verifies an Editor that rejects the request, as a package that predates quit-editor does, is not taken +// as quitting. +func TestSendUnityQuitRequestDoesNotTakeARejectedRequestAsAQuit(t *testing.T) { + connection, _ := quitRequestTestServer(t, func(conn net.Conn) { + _ = unityipc.Write(conn, []byte(`{"jsonrpc":"2.0","error":{"code":-32601,"message":"tool not found"},"id":1}`)) + }) + + if sendUnityQuitRequest(context.Background(), connection, quitRequestTestTimeout) { + t.Fatal("a rejected request must not be taken as a quit in progress") + } +} + +// Verifies a rejection whose message reads like a dropped connection is still taken as a rejection, since +// the Editor that sent it is up and will not exit. +func TestSendUnityQuitRequestDoesNotTakeARejectionThatMentionsEOFAsAQuit(t *testing.T) { + connection, _ := quitRequestTestServer(t, func(conn net.Conn) { + _ = unityipc.Write(conn, []byte(`{"jsonrpc":"2.0","error":{"code":-32603,"message":"unexpected EOF while reading settings"},"id":1}`)) + }) + + if sendUnityQuitRequest(context.Background(), connection, quitRequestTestTimeout) { + t.Fatal("a rejected request must not be taken as a quit in progress, whatever its message says") + } +} + +// Verifies an answer that carries no result is not taken as quitting: the quit handler always returns one, +// so the Editor that sent it did not run the request and will not exit. +func TestSendUnityQuitRequestDoesNotTakeAnAnswerWithoutAResultAsAQuit(t *testing.T) { + connection, _ := quitRequestTestServer(t, func(conn net.Conn) { + _ = unityipc.Write(conn, []byte(`{"jsonrpc":"2.0","id":1}`)) + }) + + if sendUnityQuitRequest(context.Background(), connection, quitRequestTestTimeout) { + t.Fatal("an answer without a result must not be taken as a quit in progress") + } +} + +// Verifies an Editor whose server is not listening is not taken as quitting. +func TestSendUnityQuitRequestDoesNotTakeAnUnreachableServerAsAQuit(t *testing.T) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("failed to listen: %v", err) + } + address := listener.Addr().String() + _ = listener.Close() + connection := unityipc.Connection{ + ProjectRoot: t.TempDir(), + Endpoint: unityipc.Endpoint{Network: "tcp", Address: address}, + } + + if sendUnityQuitRequest(context.Background(), connection, quitRequestTestTimeout) { + t.Fatal("a request that never reached a server must not be taken as a quit in progress") + } +} diff --git a/cli/dispatcher/internal/dispatcher/launch_unity_stop.go b/cli/dispatcher/internal/dispatcher/launch_unity_stop.go new file mode 100644 index 0000000000..53191c8e73 --- /dev/null +++ b/cli/dispatcher/internal/dispatcher/launch_unity_stop.go @@ -0,0 +1,91 @@ +package dispatcher + +import ( + "context" + "errors" + "time" + + clierrors "github.com/hatayama/unity-cli-loop/common/errors" + + "github.com/hatayama/unity-cli-loop/common/clicontract" + "github.com/hatayama/unity-cli-loop/common/project" + "github.com/hatayama/unity-cli-loop/common/unityipc" +) + +const ( + quitEditorBridgeCommandName = "quit-editor" + // Long enough for an Editor whose main thread is free to run the request, and short enough that a + // frozen Editor, which never runs it, is force-killed without a long wait. + launchQuitRequestTimeout = 5 * time.Second + // How long an Editor that took the quit request gets to save its window layout and exit before it is + // force-killed. + launchQuitExitTimeout = 30 * time.Second +) + +// launchStopMethod names how --restart or --quit stopped the running Editor. +type launchStopMethod string + +const ( + launchStopMethodQuit launchStopMethod = "Quit" + launchStopMethodForceKill launchStopMethod = "ForceKill" +) + +// stopUnityProcess ends the running Editor for --restart and --quit. It first asks the Editor to quit, +// which saves the window layout and discards unsaved scene changes without a dialog, and force-kills it +// when the Editor does not take the request or does not exit in time. +func stopUnityProcess(ctx context.Context, projectRoot string, pid int, isV2 bool, deps launchDeps) (launchStopMethod, error) { + // Why not for a V2 project: its server predates quit-editor, so asking would only delay the kill. + if !isV2 && deps.requestUnityQuit(ctx, projectRoot) { + err := deps.waitForUnityProcessExit(ctx, projectRoot, pid, launchProcessExitPoll, launchQuitExitTimeout) + if err == nil { + return launchStopMethodQuit, nil + } + var exitTimeout launchProcessExitTimeoutError + if !errors.As(err, &exitTimeout) { + return "", err + } + } + if err := deps.killUnityProcess(pid); err != nil { + return "", err + } + if err := deps.waitForUnityProcessExit(ctx, projectRoot, pid, launchProcessExitPoll, launchProcessExitTimeout); err != nil { + return "", err + } + return launchStopMethodForceKill, nil +} + +// requestUnityQuit asks the project's Editor to quit and reports whether it took the request. +func requestUnityQuit(ctx context.Context, projectRoot string) bool { + connection, err := project.ResolveConnection(projectRoot, projectRoot) + if err != nil { + return false + } + return sendUnityQuitRequest(ctx, connection, launchQuitRequestTimeout) +} + +// sendUnityQuitRequest sends quit-editor once. The Editor took the request when it answered, or when the +// connection dropped after the request reached it, which is how an Editor that exits before its answer +// is written looks. A rejection, an answer without a result, an unreachable server, and no answer in time +// all mean it did not. +func sendUnityQuitRequest(ctx context.Context, connection unityipc.Connection, timeout time.Duration) bool { + requestContext, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + outcome, err := unityipc.NewClient(connection, clicontract.ProjectRunnerVersion()). + SendWithProgressOutcome(requestContext, quitEditorBridgeCommandName, map[string]any{}, nil) + if err == nil { + return true + } + // Why rule out these answers first: the disconnect check also takes them as a dropped connection, but + // they come from an Editor that is up and did not run the request. The disconnect check matches a + // rejection whose message mentions EOF, and an answer without a result outright, while the quit handler + // always returns a result. + var rejection *unityipc.RPCError + if errors.As(err, &rejection) { + return false + } + var noResult *unityipc.NoResponseError + if errors.As(err, &noResult) { + return false + } + return outcome.RequestDispatched && requestContext.Err() == nil && clierrors.IsTransportDisconnectError(err) +} diff --git a/cli/dispatcher/internal/dispatcher/launch_unity_stop_test.go b/cli/dispatcher/internal/dispatcher/launch_unity_stop_test.go new file mode 100644 index 0000000000..164a98a723 --- /dev/null +++ b/cli/dispatcher/internal/dispatcher/launch_unity_stop_test.go @@ -0,0 +1,239 @@ +package dispatcher + +import ( + "bytes" + "context" + "errors" + "testing" + "time" + + "github.com/hatayama/unity-cli-loop/common/clicore" +) + +// unityStopRecorder records what stopUnityProcess asked of its dependencies. +type unityStopRecorder struct { + quitRequests int + killedPids []int + exitWaits []time.Duration +} + +// unityStopTestDeps returns launch dependencies whose quit request answers quitTaken and whose exit waits +// return the given results in order, recording every call in the returned recorder. +func unityStopTestDeps(quitTaken bool, exitWaitResults ...error) (launchDeps, *unityStopRecorder) { + recorder := &unityStopRecorder{} + deps := defaultLaunchDeps() + deps.requestUnityQuit = func(context.Context, string) bool { + recorder.quitRequests++ + return quitTaken + } + deps.killUnityProcess = func(pid int) error { + recorder.killedPids = append(recorder.killedPids, pid) + return nil + } + deps.waitForUnityProcessExit = func(_ context.Context, _ string, _ int, _ time.Duration, timeout time.Duration) error { + index := len(recorder.exitWaits) + recorder.exitWaits = append(recorder.exitWaits, timeout) + if index < len(exitWaitResults) { + return exitWaitResults[index] + } + return nil + } + return deps, recorder +} + +// Verifies an Editor that takes the quit request and exits is never killed, so it keeps the window +// layout it saved on the way out. +func TestStopUnityProcessLetsAnEditorThatQuitsExitOnItsOwn(t *testing.T) { + deps, recorder := unityStopTestDeps(true) + + method, err := stopUnityProcess(context.Background(), "/projects/sample", 222, false, deps) + if err != nil { + t.Fatalf("stopUnityProcess() error = %v", err) + } + if method != launchStopMethodQuit { + t.Fatalf("method = %q, want %q", method, launchStopMethodQuit) + } + if len(recorder.killedPids) != 0 { + t.Fatalf("killed %v, want no kill for an Editor that quit", recorder.killedPids) + } + if len(recorder.exitWaits) != 1 || recorder.exitWaits[0] != launchQuitExitTimeout { + t.Fatalf("exit waits = %v, want one wait of %s", recorder.exitWaits, launchQuitExitTimeout) + } +} + +// Verifies an Editor that took the quit request but is still running when the wait ends is force-killed, +// and that the kill is waited out before launch goes on. +func TestStopUnityProcessKillsAnEditorThatDoesNotExitAfterTakingTheQuitRequest(t *testing.T) { + deps, recorder := unityStopTestDeps(true, launchProcessExitTimeoutError{pid: 222, timeout: launchQuitExitTimeout}, nil) + + method, err := stopUnityProcess(context.Background(), "/projects/sample", 222, false, deps) + if err != nil { + t.Fatalf("stopUnityProcess() error = %v", err) + } + if method != launchStopMethodForceKill { + t.Fatalf("method = %q, want %q", method, launchStopMethodForceKill) + } + if len(recorder.killedPids) != 1 || recorder.killedPids[0] != 222 { + t.Fatalf("killed %v, want pid 222 killed once", recorder.killedPids) + } + if len(recorder.exitWaits) != 2 || recorder.exitWaits[1] != launchProcessExitTimeout { + t.Fatalf("exit waits = %v, want the quit wait and then the kill wait of %s", recorder.exitWaits, launchProcessExitTimeout) + } +} + +// Verifies an Editor that cannot take the quit request, such as a frozen one or one whose package predates +// the request, is force-killed at once instead of being waited on. +func TestStopUnityProcessKillsAnEditorThatDoesNotTakeTheQuitRequest(t *testing.T) { + deps, recorder := unityStopTestDeps(false) + + method, err := stopUnityProcess(context.Background(), "/projects/sample", 222, false, deps) + if err != nil { + t.Fatalf("stopUnityProcess() error = %v", err) + } + if method != launchStopMethodForceKill { + t.Fatalf("method = %q, want %q", method, launchStopMethodForceKill) + } + if recorder.quitRequests != 1 { + t.Fatalf("quit requests = %d, want 1", recorder.quitRequests) + } + if len(recorder.killedPids) != 1 { + t.Fatalf("killed %v, want one kill", recorder.killedPids) + } + if len(recorder.exitWaits) != 1 || recorder.exitWaits[0] != launchProcessExitTimeout { + t.Fatalf("exit waits = %v, want only the kill wait of %s", recorder.exitWaits, launchProcessExitTimeout) + } +} + +// Verifies a V2 project's Editor is force-killed without a quit request, because its server predates the +// request and asking would only delay the kill. +func TestStopUnityProcessKillsAV2EditorWithoutAskingItToQuit(t *testing.T) { + deps, recorder := unityStopTestDeps(true) + + method, err := stopUnityProcess(context.Background(), "/projects/sample", 222, true, deps) + if err != nil { + t.Fatalf("stopUnityProcess() error = %v", err) + } + if method != launchStopMethodForceKill { + t.Fatalf("method = %q, want %q", method, launchStopMethodForceKill) + } + if recorder.quitRequests != 0 { + t.Fatalf("quit requests = %d, want none for a V2 project", recorder.quitRequests) + } + if len(recorder.killedPids) != 1 { + t.Fatalf("killed %v, want one kill", recorder.killedPids) + } +} + +// Verifies a failure to watch the Editor exit, other than running out of time, is reported instead of +// being taken as an Editor to kill, since it says nothing about whether the Editor is still running. +func TestStopUnityProcessReportsAnExitWaitThatFailsForAnotherReason(t *testing.T) { + lookupFailure := errors.New("process listing failed") + deps, recorder := unityStopTestDeps(true, lookupFailure) + + _, err := stopUnityProcess(context.Background(), "/projects/sample", 222, false, deps) + + if !errors.Is(err, lookupFailure) { + t.Fatalf("err = %v, want the process listing failure", err) + } + if len(recorder.killedPids) != 0 { + t.Fatalf("killed %v, want no kill when the wait failed for another reason", recorder.killedPids) + } +} + +// Verifies a kill that fails is reported, so launch does not start a second Editor beside the first. +func TestStopUnityProcessReportsAFailedKill(t *testing.T) { + deps, _ := unityStopTestDeps(false) + killFailure := errors.New("kill denied") + deps.killUnityProcess = func(int) error { return killFailure } + + _, err := stopUnityProcess(context.Background(), "/projects/sample", 222, false, deps) + + if !errors.Is(err, killFailure) { + t.Fatalf("err = %v, want the kill failure", err) + } +} + +// Verifies --quit reports that Unity quit on its own, with a message saying the layout was kept and unsaved +// scene changes were dropped. +func TestRunLaunchQuitReportsAnEditorThatQuitOnItsOwn(t *testing.T) { + deps, recorder := unityStopTestDeps(true) + deps.findRunningUnityProcess = func(context.Context, string) (*clicore.UnityProcess, error) { + return &clicore.UnityProcess{Pid: 333}, nil + } + projectRoot := createLaunchTestProject(t) + var stdout bytes.Buffer + var stderr bytes.Buffer + + code := runLaunchWithDeps(context.Background(), launchOptions{projectPath: projectRoot, quit: true}, projectRoot, &stdout, &stderr, deps) + + if code != 0 { + t.Fatalf("exit code mismatch: %d stderr=%s", code, stderr.String()) + } + if len(recorder.killedPids) != 0 { + t.Fatalf("killed %v, want no kill", recorder.killedPids) + } + response := decodeLaunchResponseFromOutput(t, stdout.String()) + if !response.Quit || response.StopMethod != string(launchStopMethodQuit) { + t.Fatalf("response = %#v, want Quit with StopMethod %q", response, launchStopMethodQuit) + } + if response.Message != launchQuitStoppedMessage { + t.Fatalf("message = %q, want %q", response.Message, launchQuitStoppedMessage) + } +} + +// Verifies --quit reports a force-kill as one, with a message saying the layout was not saved. +func TestRunLaunchQuitReportsAForceKilledEditor(t *testing.T) { + deps, _ := unityStopTestDeps(false) + deps.findRunningUnityProcess = func(context.Context, string) (*clicore.UnityProcess, error) { + return &clicore.UnityProcess{Pid: 333}, nil + } + projectRoot := createLaunchTestProject(t) + var stdout bytes.Buffer + var stderr bytes.Buffer + + code := runLaunchWithDeps(context.Background(), launchOptions{projectPath: projectRoot, quit: true}, projectRoot, &stdout, &stderr, deps) + + if code != 0 { + t.Fatalf("exit code mismatch: %d stderr=%s", code, stderr.String()) + } + response := decodeLaunchResponseFromOutput(t, stdout.String()) + if response.StopMethod != string(launchStopMethodForceKill) { + t.Fatalf("StopMethod = %q, want %q", response.StopMethod, launchStopMethodForceKill) + } + if response.Message != launchForceKillStoppedMessage { + t.Fatalf("message = %q, want %q", response.Message, launchForceKillStoppedMessage) + } +} + +// Verifies --restart reports how the previous Editor stopped in the ready response of the new one. +func TestRunLaunchRestartReportsHowThePreviousEditorStopped(t *testing.T) { + deps, _ := unityStopTestDeps(true) + deps.findRunningUnityProcess = func(context.Context, string) (*clicore.UnityProcess, error) { + return &clicore.UnityProcess{Pid: 222}, nil + } + fakeUnityPath := fakeUnityExecutablePath(t) + deps.resolveUnityExecutablePath = func(string) (string, error) { return fakeUnityPath, nil } + deps.waitForUnityStartupMarker = func(context.Context, string, time.Duration, time.Duration) error { return nil } + deps.waitForToolReadiness = func(context.Context, string, time.Duration) error { return nil } + deps.focusUnityProcess = func(context.Context, int) error { return nil } + projectRoot := createLaunchTestProject(t) + var stdout bytes.Buffer + var stderr bytes.Buffer + + code := runLaunchWithDeps( + context.Background(), + launchOptions{projectPath: projectRoot, restart: true, editorVersion: "6000.0.0f1"}, + projectRoot, + &stdout, + &stderr, + deps, + ) + + if code != 0 { + t.Fatalf("exit code mismatch: %d stderr=%s", code, stderr.String()) + } + response := decodeLaunchResponseFromOutput(t, stdout.String()) + if !response.Restarted || response.StopMethod != string(launchStopMethodQuit) { + t.Fatalf("response = %#v, want Restarted with StopMethod %q", response, launchStopMethodQuit) + } +} diff --git a/cli/dispatcher/internal/dispatcher/launch_v2_lockfile.go b/cli/dispatcher/internal/dispatcher/launch_v2_lockfile.go index 70edf9b521..e39e760e68 100644 --- a/cli/dispatcher/internal/dispatcher/launch_v2_lockfile.go +++ b/cli/dispatcher/internal/dispatcher/launch_v2_lockfile.go @@ -78,6 +78,7 @@ func waitForV2ProjectOpened( ctx context.Context, projectRoot string, runningProcess *unityprocess.UnityProcess, + previousStop launchStopMethod, currentPid int, stdout io.Writer, stderr io.Writer, @@ -109,5 +110,5 @@ func waitForV2ProjectOpened( if runningProcess != nil { previousPid = &runningProcess.Pid } - return writeLaunchedV2ReadyResponse(stdout, stderr, projectRoot, previousPid, currentPid) + return writeLaunchedV2ReadyResponse(stdout, stderr, projectRoot, previousPid, previousStop, currentPid) } diff --git a/docs/windows-compatibility.md b/docs/windows-compatibility.md index 3343bc40f9..f38eef3ccc 100644 --- a/docs/windows-compatibility.md +++ b/docs/windows-compatibility.md @@ -133,6 +133,6 @@ Restart deletes the project's `Temp` directory once the old Editor has exited. O - Only `Temp/UnityLockfile` has to go. A leftover lockfile makes the next Editor refuse to open the project as already opened; everything else under `Temp` is a cache Unity recreates on startup. - The lockfile is deleted with a short retry (250 ms apart, up to 5 seconds) so a handle that is about to be released does not fail the restart. If it still cannot be deleted, restart stops with an error and does not launch Unity. - The rest of `Temp` is deleted on a best-effort basis. A failure prints one warning line to stderr and the launch continues. -- Windows kills the Editor with `taskkill /PID /T /F` so its helper processes go down with it, instead of leaving them holding files. If `taskkill` cannot be run, restart falls back to killing the Editor process alone. +- Restart first asks the Editor to quit, except in a V2 project, whose server predates the request; the kill below applies only when the Editor has to be force-killed instead. Windows then kills it with `taskkill /PID /T /F` so its helper processes go down with it, instead of leaving them holding files. If `taskkill` cannot be run, restart falls back to killing the Editor process alone. Unix does not need any of this: an open file can be unlinked there, so a helper process that outlives the Editor does not block the Temp cleanup.