From c8c28fd7060de92e634657da92531e23cbab9cb0 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 14 Jul 2026 15:29:43 +0900 Subject: [PATCH 1/2] refactor: replace Presentation async void with Task and Forget() async void swallows exceptions and complicates UI lifecycle ownership. Convert Settings, migration wizard, and setup startup handlers to async Task, and bridge Action/delegate call sites with Forget() so failures stay logged. Co-authored-by: Cursor --- .../Setup/SetupWizardStartupFlow.cs | 4 +-- .../ThirdPartyToolMigrationWizardWindow.cs | 20 +++++------ .../UnityCliLoopSettingsWindow.cs | 36 +++++++++---------- 3 files changed, 30 insertions(+), 30 deletions(-) diff --git a/Packages/src/Editor/Presentation/Setup/SetupWizardStartupFlow.cs b/Packages/src/Editor/Presentation/Setup/SetupWizardStartupFlow.cs index d0cf8aaeaa..68c372e24b 100644 --- a/Packages/src/Editor/Presentation/Setup/SetupWizardStartupFlow.cs +++ b/Packages/src/Editor/Presentation/Setup/SetupWizardStartupFlow.cs @@ -154,7 +154,7 @@ internal static bool HasSkillUpdateForSetupWizard(IEnumerable RefreshUI().Forget(), + () => HandleMigrateThirdPartyTools().Forget(), HandleMigrationSkillTargetChanged, - HandleToggleMigrationSkill, + () => HandleToggleMigrationSkill().Forget(), Close); _resizer = new ThirdPartyToolMigrationWizardWindowResizer(this, _view.MainScrollView); _resizer.BindSizeUpdates(); @@ -425,10 +425,10 @@ private void ScheduleInitialRefresh(bool shouldStartInitialRefresh) return; } - rootVisualElement.schedule.Execute(RefreshUI).StartingIn(0); + rootVisualElement.schedule.Execute(() => RefreshUI().Forget()).StartingIn(0); } - private async void RefreshUI() + private async Task RefreshUI() { CancellationToken ct = BeginMigrationOperation(); ShowCheckingState(new ThirdPartyToolMigrationProgress(0, 0)); @@ -477,7 +477,7 @@ private async void RefreshUI() ShowMigrationTargetsState(preview.FilePaths); } - private async void HandleMigrateThirdPartyTools() + private async Task HandleMigrateThirdPartyTools() { if (!ConfirmMigrationApply( _pendingMigrationFilePaths.Length, @@ -546,7 +546,7 @@ private async void HandleMigrateThirdPartyTools() CompleteMigrationOperation(ct); if (shouldRefreshAfterInterruptedMigration) { - RefreshUI(); + await RefreshUI(); } } @@ -557,7 +557,7 @@ private async void HandleMigrateThirdPartyTools() if (ShouldRefreshAfterMigration(result)) { - RefreshUI(); + await RefreshUI(); return; } @@ -626,7 +626,7 @@ private void HandleMigrationSkillTargetChanged(SkillsTarget target) RefreshMigrationSkillState(); } - private async void HandleToggleMigrationSkill() + private async Task HandleToggleMigrationSkill() { CancellationToken ct = BeginMigrationSkillOperation(); string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); @@ -767,7 +767,7 @@ private void TryStartInitialRefresh() } ShowCheckingState(new ThirdPartyToolMigrationProgress(0, 0)); - rootVisualElement.schedule.Execute(RefreshUI).StartingIn(0); + rootVisualElement.schedule.Execute(() => RefreshUI().Forget()).StartingIn(0); } private void CompleteMigrationOperation(CancellationToken ct) diff --git a/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs b/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs index 0b4069b1c1..8265a86d0b 100644 --- a/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs +++ b/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs @@ -130,7 +130,7 @@ private void InitializeAll() InitializeEventHandler(); LoadSavedSettings(); RestoreSessionState(); - HandlePostCompileMode(); + HandlePostCompileMode().Forget(); } private void InitializeModel() @@ -164,9 +164,9 @@ private void InitializeView() private void SetupViewCallbacks() { - _view.OnRefreshCliVersion += HandleRefreshCliVersion; - _view.OnInstallCli += HandleInstallCli; - _view.OnInstallSkills += HandleInstallSkills; + _view.OnRefreshCliVersion += () => HandleRefreshCliVersion().Forget(); + _view.OnInstallCli += () => HandleInstallCli().Forget(); + _view.OnInstallSkills += () => HandleInstallSkills().Forget(); _view.OnRefreshSkillsState += HandleRefreshSkillsState; _view.OnSkillsTargetChanged += value => { @@ -201,7 +201,7 @@ private void RestoreSessionState() _model.LoadFromSessionState(); } - private async void HandlePostCompileMode() + private async Task HandlePostCompileMode() { _model.EnablePostCompileMode(); _sessionFlagsRepository.SetShowReconnectingUI(false); @@ -324,8 +324,8 @@ internal void RefreshAllSections( if (runExpensiveChecks) { - RefreshCliVersionInBackground(); - RefreshCliPathSetupInBackground(); + RefreshCliVersionInBackground().Forget(); + RefreshCliPathSetupInBackground().Forget(); if (refreshSkillInstallState) { RefreshSelectedTargetInstallStateInBackground(); @@ -340,7 +340,7 @@ internal void RefreshAllSections( } } - private async void RefreshCliVersionInBackground() + private async Task RefreshCliVersionInBackground() { if (_cliSetupApplicationService.IsCliCheckCompleted()) { @@ -348,12 +348,12 @@ private async void RefreshCliVersionInBackground() } await _cliSetupApplicationService.RefreshCliVersionAsync(CancellationToken.None); - RefreshCliPathSetupInBackground(); + RefreshCliPathSetupInBackground().Forget(); RefreshCliSetupSection(); RefreshSelectedTargetInstallStateInBackground(); } - private async void RefreshCliPathSetupInBackground() + private async Task RefreshCliPathSetupInBackground() { if (_isRefreshingCliPathSetup) { @@ -383,7 +383,7 @@ private async void RefreshCliPathSetupInBackground() } } - private async void HandleRefreshCliVersion() + private async Task HandleRefreshCliVersion() { if (_isRefreshingVersion) { @@ -398,7 +398,7 @@ private async void HandleRefreshCliVersion() Task forceRefresh = _cliSetupApplicationService.ForceRefreshCliVersionAsync(CancellationToken.None); Task minimumDelay = Task.Delay(500); await Task.WhenAll(forceRefresh, minimumDelay); - RefreshCliPathSetupInBackground(); + RefreshCliPathSetupInBackground().Forget(); } finally { @@ -544,10 +544,10 @@ private void HandleToolToggled(string toolName, bool enabled) _view?.UpdateSingleToolToggle(toolName, enabled); // Skill synchronization can touch many files, so defer it to keep UI input responsive. - EditorApplication.delayCall += () => ApplyToolToggleSideEffects(toolName, enabled); + EditorApplication.delayCall += () => ApplyToolToggleSideEffects(toolName, enabled).Forget(); } - private async void ApplyToolToggleSideEffects(string toolName, bool enabled) + private async Task ApplyToolToggleSideEffects(string toolName, bool enabled) { if (!enabled) { @@ -646,10 +646,10 @@ private void RefreshSelectedTargetInstallStateInBackground(bool allowDuringCliRe CancellationTokenSource cts = new(); _skillInstallStateRefreshCts = cts; - RefreshSelectedTargetInstallStateAsync(cts.Token); + RefreshSelectedTargetInstallStateAsync(cts.Token).Forget(); } - private async void RefreshSelectedTargetInstallStateAsync(CancellationToken ct) + private async Task RefreshSelectedTargetInstallStateAsync(CancellationToken ct) { string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); SkillInstallState installState = await Task.Run( @@ -707,7 +707,7 @@ private void CancelSkillInstallStateRefresh() _skillInstallStateRefreshCts = null; } - private async void HandleInstallCli() + private async Task HandleInstallCli() { CliSetupPrimaryAction clickedAction = GetCurrentCliPrimaryButtonAction(); @@ -944,7 +944,7 @@ private async Task HandleUninstallCli() } } - private async void HandleInstallSkills() + private async Task HandleInstallSkills() { if (!_cliSetupApplicationService.IsCliInstalled()) { From 27582561aed1f12e74794cce99b1b3c0dedcf9e7 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 14 Jul 2026 15:36:56 +0900 Subject: [PATCH 2/2] refactor: move Settings session, catalog, and CLI decisions into presenters Keep the EditorWindow as a thin orchestrator for one manual UI check. Move catalog cache/warmup into ToolSettingsPresenter, CLI primary-action mediation into CliSetupPresenter, and extract UnityEditor-independent dialog/PATH policies to Domain with focused tests. Co-authored-by: Cursor --- ...SettingsPresentationDecisionPolicyTests.cs | 58 ++++ ...ngsPresentationDecisionPolicyTests.cs.meta | 11 + ...nityCliLoopSettingsWindowCliActionTests.cs | 14 +- .../Editor/Domain/CliPathSetupCheckPolicy.cs | 18 ++ .../Domain/CliPathSetupCheckPolicy.cs.meta | 11 + .../Editor/Domain/SkillInstallDialogPolicy.cs | 30 ++ .../Domain/SkillInstallDialogPolicy.cs.meta | 11 + .../Presentation/Setup/SetupWizardWindow.cs | 13 +- .../UnityCliLoopSettingsCliSetupPresenter.cs | 85 +++++- ...ityCliLoopSettingsToolSettingsPresenter.cs | 139 +++++++++- .../UnityCliLoopSettingsWindow.cs | 262 ++---------------- 11 files changed, 389 insertions(+), 263 deletions(-) create mode 100644 Assets/Tests/Editor/SettingsPresentationDecisionPolicyTests.cs create mode 100644 Assets/Tests/Editor/SettingsPresentationDecisionPolicyTests.cs.meta create mode 100644 Packages/src/Editor/Domain/CliPathSetupCheckPolicy.cs create mode 100644 Packages/src/Editor/Domain/CliPathSetupCheckPolicy.cs.meta create mode 100644 Packages/src/Editor/Domain/SkillInstallDialogPolicy.cs create mode 100644 Packages/src/Editor/Domain/SkillInstallDialogPolicy.cs.meta diff --git a/Assets/Tests/Editor/SettingsPresentationDecisionPolicyTests.cs b/Assets/Tests/Editor/SettingsPresentationDecisionPolicyTests.cs new file mode 100644 index 0000000000..ba7a41fc5a --- /dev/null +++ b/Assets/Tests/Editor/SettingsPresentationDecisionPolicyTests.cs @@ -0,0 +1,58 @@ +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.Domain; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Verifies UnityEditor-independent Settings decision policies extracted for Presentation Move Method. + /// + public class SettingsPresentationDecisionPolicyTests + { + [TestCase(false, true, true)] + [TestCase(false, false, false)] + [TestCase(true, true, false)] + [TestCase(true, false, false)] + public void CliPathSetupCheckPolicy_ShouldCheck_UsesWindowsAndOwnership( + bool isWindowsEditor, + bool hasPackageOwnedCurrentUserInstall, + bool expected) + { + // Verifies PATH checks stay off for Windows and non package-owned installs. + bool result = CliPathSetupCheckPolicy.ShouldCheck( + isWindowsEditor, + hasPackageOwnedCurrentUserInstall); + + Assert.That(result, Is.EqualTo(expected)); + } + + [Test] + public void SkillInstallDialogPolicy_ShouldShowForInstallableTargets_RequiresAllEligible() + { + // Verifies wizard dialog policy requires every target to be first-install eligible. + SkillSetupTargetInfo[] targets = + { + CreateSkillTarget(SkillInstallState.Missing, false), + CreateSkillTarget(SkillInstallState.Outdated, false) + }; + + bool result = SkillInstallDialogPolicy.ShouldShowForInstallableTargets(targets); + + Assert.That(result, Is.False); + } + + private static SkillSetupTargetInfo CreateSkillTarget( + SkillInstallState installState, + bool hasDifferentLayoutSkills) + { + return new SkillSetupTargetInfo( + "Claude Code", + ".claude", + "--claude", + hasSkillsDirectory: true, + hasExistingSkills: true, + hasDifferentLayoutSkills, + installState); + } + } +} diff --git a/Assets/Tests/Editor/SettingsPresentationDecisionPolicyTests.cs.meta b/Assets/Tests/Editor/SettingsPresentationDecisionPolicyTests.cs.meta new file mode 100644 index 0000000000..6266ff9fd2 --- /dev/null +++ b/Assets/Tests/Editor/SettingsPresentationDecisionPolicyTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: fac85b7f95dfb4a3683922dfaea778f5 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs b/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs index d8b9b093c4..4f8cf14bc7 100644 --- a/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs +++ b/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs @@ -27,7 +27,7 @@ public void ShouldUninstallCliFromPrimaryButton_ReturnsExpectedAction( bool expected) { // Verifies that package-owned installs route to uninstall only when the dispatcher minimum is satisfied. - bool result = UnityCliLoopSettingsWindow.ShouldUninstallCliFromPrimaryButton( + bool result = UnityCliLoopSettingsCliSetupPresenter.ShouldUninstallCliFromPrimaryButton( cliVersion, cliIsDispatcher, canUninstallCli, @@ -45,7 +45,7 @@ public void ShouldRepairCliPathFromPrimaryButton_ReturnsExpectedAction( bool expected) { // Verifies that stale terminal PATH state routes to repair only when CLI replacement is not needed. - bool result = UnityCliLoopSettingsWindow.ShouldRepairCliPathFromPrimaryButton( + bool result = UnityCliLoopSettingsCliSetupPresenter.ShouldRepairCliPathFromPrimaryButton( needsCliPathSetup, needsUpdate); @@ -70,7 +70,7 @@ public void ResolveCliPrimaryButtonAction_ReturnsClickedPrimaryAction( { // Verifies that the Settings window chooses repair only when dispatcher replacement is unnecessary. CliSetupPrimaryAction result = - UnityCliLoopSettingsWindow.ResolveCliPrimaryButtonAction( + UnityCliLoopSettingsCliSetupPresenter.ResolveCliPrimaryButtonAction( needsCliPathSetup, cliVersion, cliIsDispatcher, @@ -94,7 +94,7 @@ public void ResolveExecutableCliPrimaryButtonAction_IgnoresUnsafeStaleActions( { // Verifies that a refreshed Settings state cannot turn a stale click into a destructive action. CliSetupPrimaryAction result = - UnityCliLoopSettingsWindow.ResolveExecutableCliPrimaryButtonAction( + UnityCliLoopSettingsCliSetupPresenter.ResolveExecutableCliPrimaryButtonAction( ParseAction(clickedAction), ParseAction(refreshedAction)); @@ -111,7 +111,7 @@ public void ShouldCheckCliPathSetupForPlatform_RequiresPackageOwnedCli( bool expected) { // Verifies that PATH repair only runs for POSIX package-owned current-user installs. - bool result = UnityCliLoopSettingsWindow.ShouldCheckCliPathSetupForPlatform( + bool result = UnityCliLoopSettingsCliSetupPresenter.ShouldCheckCliPathSetupForPlatform( platform, hasPackageOwnedCurrentUserInstall); @@ -131,7 +131,7 @@ public void IsCliUpdateNeeded_UsesDispatcherMinimumVersion( bool expected) { // Verifies that the settings UI updates non-dispatcher or older dispatcher installs. - bool result = UnityCliLoopSettingsWindow.IsCliUpdateNeeded( + bool result = UnityCliLoopSettingsCliSetupPresenter.IsCliUpdateNeeded( cliVersion, cliIsDispatcher, TestMinimumDispatcherVersion); @@ -150,7 +150,7 @@ public void ShouldShowSkillsInstalledDialog_ReturnsExpectedValue( // Verifies that Settings keeps the success dialog for first install only. SkillSetupTargetInfo targetInfo = CreateSkillTarget(installState, hasDifferentLayoutSkills); - bool result = UnityCliLoopSettingsWindow.ShouldShowSkillsInstalledDialog(targetInfo); + bool result = SkillInstallDialogPolicy.ShouldShowForSelectedTarget(targetInfo); Assert.That(result, Is.EqualTo(expected)); } diff --git a/Packages/src/Editor/Domain/CliPathSetupCheckPolicy.cs b/Packages/src/Editor/Domain/CliPathSetupCheckPolicy.cs new file mode 100644 index 0000000000..0de9a24b31 --- /dev/null +++ b/Packages/src/Editor/Domain/CliPathSetupCheckPolicy.cs @@ -0,0 +1,18 @@ +namespace io.github.hatayama.UnityCliLoop.Domain +{ + /// + /// Decides when Settings should run CLI PATH visibility checks. + /// + public static class CliPathSetupCheckPolicy + { + /// + /// PATH repair checks run only for non-Windows package-owned current-user installs. + /// + public static bool ShouldCheck( + bool isWindowsEditor, + bool hasPackageOwnedCurrentUserInstall) + { + return !isWindowsEditor && hasPackageOwnedCurrentUserInstall; + } + } +} diff --git a/Packages/src/Editor/Domain/CliPathSetupCheckPolicy.cs.meta b/Packages/src/Editor/Domain/CliPathSetupCheckPolicy.cs.meta new file mode 100644 index 0000000000..c421e6e9a1 --- /dev/null +++ b/Packages/src/Editor/Domain/CliPathSetupCheckPolicy.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: dbe22adc4dab44338bd089a25d15c8fa +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Domain/SkillInstallDialogPolicy.cs b/Packages/src/Editor/Domain/SkillInstallDialogPolicy.cs new file mode 100644 index 0000000000..2ad5853342 --- /dev/null +++ b/Packages/src/Editor/Domain/SkillInstallDialogPolicy.cs @@ -0,0 +1,30 @@ +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; + +namespace io.github.hatayama.UnityCliLoop.Domain +{ + /// + /// Decides when the skills-installed success dialog should appear after an install attempt. + /// + public static class SkillInstallDialogPolicy + { + /// + /// Returns true when Settings should show the success dialog for a single selected target. + /// + public static bool ShouldShowForSelectedTarget(SkillSetupTargetInfo targetInfo) + { + return targetInfo.InstallState != SkillInstallState.Outdated + && !targetInfo.HasDifferentLayoutSkills; + } + + /// + /// Returns true when Setup Wizard should show the success dialog for all installable targets. + /// + public static bool ShouldShowForInstallableTargets(IEnumerable targets) + { + Debug.Assert(targets != null, "targets must not be null"); + return targets.All(ShouldShowForSelectedTarget); + } + } +} diff --git a/Packages/src/Editor/Domain/SkillInstallDialogPolicy.cs.meta b/Packages/src/Editor/Domain/SkillInstallDialogPolicy.cs.meta new file mode 100644 index 0000000000..1b99a4478b --- /dev/null +++ b/Packages/src/Editor/Domain/SkillInstallDialogPolicy.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: cb88c5b20f2e34bab93527cb64cbcda7 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs b/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs index bbbaea3f7b..b3dd582718 100644 --- a/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs +++ b/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs @@ -541,10 +541,7 @@ private void CancelSkillInstallStateRefresh() internal static bool ShouldShowSkillsInstalledDialog( IEnumerable targets) { - Debug.Assert(targets != null, "targets must not be null"); - return targets.All(target => - target.InstallState != SkillInstallState.Outdated - && !target.HasDifferentLayoutSkills); + return SkillInstallDialogPolicy.ShouldShowForInstallableTargets(targets); } internal static bool ShouldUseFirstInstallSkillsUi(string lastSeenSetupWizardVersion) @@ -579,8 +576,9 @@ internal static bool ShouldCheckCliPathSetupForSetupWizard( RuntimePlatform platform, bool hasPackageOwnedCurrentUserInstall) { - return platform != RuntimePlatform.WindowsEditor - && hasPackageOwnedCurrentUserInstall; + return CliPathSetupCheckPolicy.ShouldCheck( + isWindowsEditor: platform == RuntimePlatform.WindowsEditor, + hasPackageOwnedCurrentUserInstall); } private static CliSetupCompatibilityState EvaluateCliSetupCompatibilityForSetupWizard( @@ -762,7 +760,8 @@ private async Task HandleInstallSkillsAsync(CancellationToken ct) : SetupWizardSkillsStepPresenter.FilterInstallableSkillTargets(targets); if (installableTargets.Count == 0) return; - bool shouldShowSkillsInstalledDialog = ShouldShowSkillsInstalledDialog(installableTargets); + bool shouldShowSkillsInstalledDialog = + SkillInstallDialogPolicy.ShouldShowForInstallableTargets(installableTargets); _isInstallingSkills = true; UpdateSkillsStep(true, targets); diff --git a/Packages/src/Editor/Presentation/UnityCliLoopSettingsCliSetupPresenter.cs b/Packages/src/Editor/Presentation/UnityCliLoopSettingsCliSetupPresenter.cs index ae666dc9da..f1a0057cfe 100644 --- a/Packages/src/Editor/Presentation/UnityCliLoopSettingsCliSetupPresenter.cs +++ b/Packages/src/Editor/Presentation/UnityCliLoopSettingsCliSetupPresenter.cs @@ -7,7 +7,7 @@ namespace io.github.hatayama.UnityCliLoop.Presentation { /// - /// Presents the CLI setup section in the Unity CLI Loop settings window. + /// Presents the CLI setup section and owns Settings primary-action mediation decisions. /// internal sealed class UnityCliLoopSettingsCliSetupPresenter { @@ -50,6 +50,89 @@ internal void Update( _view.UpdateCliSetup(cliData); } + internal CliSetupPrimaryAction ResolveCurrentPrimaryButtonAction(bool needsCliPathSetup) + { + string cliVersion = _cliSetupApplicationService.GetCachedCliVersion(); + bool cliIsDispatcher = _cliSetupApplicationService.GetCachedCliIsDispatcher(); + string cliExecutablePath = _cliSetupApplicationService.GetCachedCliExecutablePath(); + bool canUninstallCli = _cliSetupApplicationService.IsPackageOwnedCurrentUserInstallPath( + cliExecutablePath, + UnityEngine.Application.platform); + string requiredCliVersion = _cliSetupApplicationService.GetMinimumRequiredCliVersion(); + return ResolveCliPrimaryButtonAction( + needsCliPathSetup, + cliVersion, + cliIsDispatcher, + canUninstallCli, + requiredCliVersion); + } + + internal static bool ShouldUninstallCliFromPrimaryButton( + string cliVersion, + bool cliIsDispatcher, + bool canUninstallCli, + string requiredCliVersion) + { + bool isCliInstalled = cliVersion != null; + bool needsUpdate = IsCliUpdateNeeded(cliVersion, cliIsDispatcher, requiredCliVersion); + return CliSetupPrimaryActionPolicy.ShouldUninstallCli( + isCliInstalled, + needsUpdate, + canUninstallCli); + } + + internal static CliSetupPrimaryAction ResolveCliPrimaryButtonAction( + bool needsCliPathSetup, + string cliVersion, + bool cliIsDispatcher, + bool canUninstallCli, + string requiredCliVersion) + { + bool needsUpdate = IsCliUpdateNeeded(cliVersion, cliIsDispatcher, requiredCliVersion); + bool isCliInstalled = cliVersion != null; + return CliSetupPrimaryActionPolicy.ResolveSettingsPrimaryAction( + needsCliPathSetup, + needsUpdate, + isCliInstalled, + canUninstallCli); + } + + internal static CliSetupPrimaryAction ResolveExecutableCliPrimaryButtonAction( + CliSetupPrimaryAction clickedAction, + CliSetupPrimaryAction refreshedAction) + { + return CliSetupPrimaryActionPolicy.ResolveExecutableSettingsAction( + clickedAction, + refreshedAction); + } + + internal static bool ShouldRepairCliPathFromPrimaryButton( + bool needsCliPathSetup, + bool needsUpdate) + { + return CliSetupPrimaryActionPolicy.ShouldRepairCliPath(needsCliPathSetup, needsUpdate); + } + + internal static bool ShouldCheckCliPathSetupForPlatform( + RuntimePlatform platform, + bool hasPackageOwnedCurrentUserInstall) + { + return CliPathSetupCheckPolicy.ShouldCheck( + isWindowsEditor: platform == RuntimePlatform.WindowsEditor, + hasPackageOwnedCurrentUserInstall); + } + + internal static bool IsCliUpdateNeeded( + string cliVersion, + bool cliIsDispatcher, + string requiredCliVersion) + { + return CliSetupCompatibility.Evaluate( + cliVersion, + cliIsDispatcher, + requiredCliVersion).NeedsUpdate; + } + private CliSetupData CreateCliSetupData( bool needsCliPathSetup, bool isInstallingCli, diff --git a/Packages/src/Editor/Presentation/UnityCliLoopSettingsToolSettingsPresenter.cs b/Packages/src/Editor/Presentation/UnityCliLoopSettingsToolSettingsPresenter.cs index 477f89d5a3..929b81640b 100644 --- a/Packages/src/Editor/Presentation/UnityCliLoopSettingsToolSettingsPresenter.cs +++ b/Packages/src/Editor/Presentation/UnityCliLoopSettingsToolSettingsPresenter.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using UnityEditor; using UnityEngine; using io.github.hatayama.UnityCliLoop.Application; @@ -7,13 +8,24 @@ namespace io.github.hatayama.UnityCliLoop.Presentation { /// - /// Presents the Tool Settings section in the Unity CLI Loop settings window. + /// Presents the Tool Settings section and owns catalog cache / registry warmup. /// internal sealed class UnityCliLoopSettingsToolSettingsPresenter { + private const double RegistryWarmupInitialDelaySeconds = 0.05; + private const double RegistryWarmupMaxDelaySeconds = 0.8; + private const int RegistryWarmupMaxAttempts = 5; + private readonly UnityCliLoopSettingsWindowUI _view; private readonly ToolSettingsUseCase _toolSettingsUseCase; + private bool _isCatalogDirty = true; + private bool _isRegistryWarmupScheduled; + private double _registryWarmupDueTime; + private int _registryWarmupAttemptCount; + private bool _showToolSettings = true; + private bool _isViewReady; + internal UnityCliLoopSettingsToolSettingsPresenter( UnityCliLoopSettingsWindowUI view, ToolSettingsUseCase toolSettingsUseCase) @@ -26,25 +38,138 @@ internal UnityCliLoopSettingsToolSettingsPresenter( ?? throw new ArgumentNullException(nameof(toolSettingsUseCase)); } + internal void SetViewReady(bool isViewReady) + { + _isViewReady = isViewReady; + if (!isViewReady) + { + CancelRegistryWarmup(); + } + } + + internal void InvalidateCatalog() + { + _isCatalogDirty = true; + } + internal void UpdateHeader(bool showToolSettings) { + _showToolSettings = showToolSettings; ToolSettingsSectionData toolSettingsData = CreateToolSettingsHeaderData(showToolSettings); _view.UpdateToolSettings(toolSettingsData); } - internal ToolSettingsSectionData UpdateCatalog(bool showToolSettings) + internal void RefreshCatalogIfNeeded(bool showToolSettings) + { + _showToolSettings = showToolSettings; + if (!showToolSettings || !_isCatalogDirty || !_isViewReady) + { + return; + } + + RefreshCatalog(showToolSettings); + } + + internal void HandleShowToolSettingsChanged(bool show) + { + UpdateHeader(show); + if (!show) + { + _isCatalogDirty = true; + CancelRegistryWarmup(); + ResetRegistryWarmupAttemptCount(); + return; + } + + RefreshCatalogIfNeeded(show); + } + + internal void CancelRegistryWarmup() + { + if (!_isRegistryWarmupScheduled) + { + return; + } + + EditorApplication.update -= RunRegistryWarmupWhenDue; + _isRegistryWarmupScheduled = false; + } + + internal void ResetRegistryWarmupAttemptCount() + { + _registryWarmupAttemptCount = 0; + } + + private void RefreshCatalog(bool showToolSettings) { ToolSettingsSectionData toolSettingsData = CreateToolSettingsData(showToolSettings); _view.UpdateToolSettings(toolSettingsData); - return toolSettingsData; + + if (UnityCliLoopSettingsWindowRefreshPolicy.ShouldKeepToolSettingsCatalogDirty(toolSettingsData)) + { + if (ScheduleRegistryWarmup()) + { + _isCatalogDirty = true; + return; + } + + _isCatalogDirty = false; + return; + } + + CancelRegistryWarmup(); + ResetRegistryWarmupAttemptCount(); + _isCatalogDirty = false; + } + + private bool ScheduleRegistryWarmup() + { + if (UnityCliLoopSettingsWindowRefreshPolicy.ShouldStartToolSettingsRegistryWarmup( + _isRegistryWarmupScheduled, + _registryWarmupAttemptCount, + RegistryWarmupMaxAttempts)) + { + double delaySeconds = UnityCliLoopSettingsWindowRefreshPolicy.CalculateToolSettingsRegistryWarmupDelaySeconds( + RegistryWarmupInitialDelaySeconds, + RegistryWarmupMaxDelaySeconds, + _registryWarmupAttemptCount); + + _isRegistryWarmupScheduled = true; + _registryWarmupDueTime = EditorApplication.timeSinceStartup + delaySeconds; + _registryWarmupAttemptCount++; + EditorApplication.update += RunRegistryWarmupWhenDue; + return true; + } + + return _isRegistryWarmupScheduled; + } + + private void RunRegistryWarmupWhenDue() + { + if (EditorApplication.timeSinceStartup < _registryWarmupDueTime) + { + return; + } + + CancelRegistryWarmup(); + + if (!_isViewReady || !_showToolSettings) + { + ResetRegistryWarmupAttemptCount(); + return; + } + + _toolSettingsUseCase.WarmupRegistry(); + InvalidateCatalog(); + RefreshCatalogIfNeeded(_showToolSettings); } private static ToolSettingsSectionData CreateToolSettingsHeaderData(bool showToolSettings) { return new ToolSettingsSectionData( showToolSettings, - System.Array.Empty(), - System.Array.Empty(), + Array.Empty(), + Array.Empty(), true, false); } @@ -58,8 +183,8 @@ private ToolSettingsSectionData CreateToolSettingsData(bool showToolSettings) { return new ToolSettingsSectionData( showToolSettings, - System.Array.Empty(), - System.Array.Empty(), + Array.Empty(), + Array.Empty(), false, true); } diff --git a/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs b/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs index 8265a86d0b..0fec5b81c6 100644 --- a/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs +++ b/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs @@ -19,9 +19,6 @@ public class UnityCliLoopSettingsWindow : EditorWindow { private const bool ForceFlatSkillInstall = true; private const double DeferredInitialRefreshDelaySeconds = 0.05; - private const double ToolSettingsRegistryWarmupInitialDelaySeconds = 0.05; - private const double ToolSettingsRegistryWarmupMaxDelaySeconds = 0.8; - private const int ToolSettingsRegistryWarmupMaxAttempts = 5; private static IUnityCliLoopEditorSettingsPort RegisteredEditorSettingsPort; private static ISessionFlagsRepository RegisteredSessionFlagsRepository; @@ -49,13 +46,9 @@ public class UnityCliLoopSettingsWindow : EditorWindow private bool _isRefreshingVersion; private bool _isRefreshingCliPathSetup; private bool _needsCliPathSetup; - private bool _isToolSettingsCatalogDirty = true; private bool _isDeferredInitialRefreshScheduled; private bool _hasCompletedDeferredInitialRefresh; private double _deferredInitialRefreshDueTime; - private bool _isToolSettingsRegistryWarmupScheduled; - private double _toolSettingsRegistryWarmupDueTime; - private int _toolSettingsRegistryWarmupAttemptCount; private SkillInstallState _selectedTargetInstallState = SkillInstallState.Missing; private CancellationTokenSource _skillInstallStateRefreshCts; @@ -107,9 +100,10 @@ private void OnEnable() private void OnDestroy() { CancelDeferredInitialRefresh(); - CancelToolSettingsRegistryWarmup(); - ResetToolSettingsRegistryWarmupAttemptCount(); + _toolSettingsPresenter?.CancelRegistryWarmup(); + _toolSettingsPresenter?.ResetRegistryWarmupAttemptCount(); CancelSkillInstallStateRefresh(); + _toolSettingsPresenter?.SetViewReady(false); _view?.Dispose(); _view = null; _cliSetupPresenter = null; @@ -129,7 +123,7 @@ private void InitializeAll() InitializeModel(); InitializeEventHandler(); LoadSavedSettings(); - RestoreSessionState(); + _model.LoadFromSessionState(); HandlePostCompileMode().Forget(); } @@ -159,6 +153,7 @@ private void InitializeView() _toolSettingsPresenter = new UnityCliLoopSettingsToolSettingsPresenter( _view, _toolSettingsUseCase); + _toolSettingsPresenter.SetViewReady(true); SetupViewCallbacks(); } @@ -196,11 +191,6 @@ private void LoadSavedSettings() _installSkillsFlat = ForceFlatSkillInstall; } - private void RestoreSessionState() - { - _model.LoadFromSessionState(); - } - private async Task HandlePostCompileMode() { _model.EnablePostCompileMode(); @@ -226,11 +216,12 @@ private async Task HandlePostCompileMode() private void OnDisable() { CancelDeferredInitialRefresh(); - CancelToolSettingsRegistryWarmup(); - ResetToolSettingsRegistryWarmupAttemptCount(); + _toolSettingsPresenter?.CancelRegistryWarmup(); + _toolSettingsPresenter?.ResetRegistryWarmupAttemptCount(); CancelSkillInstallStateRefresh(); CleanupEventHandler(); - SaveSessionState(); + _model?.SaveToSessionState(); + _toolSettingsPresenter?.SetViewReady(false); _view?.Dispose(); _view = null; _cliSetupPresenter = null; @@ -242,11 +233,6 @@ private void CleanupEventHandler() _eventHandler?.Cleanup(); } - private void SaveSessionState() - { - _model.SaveToSessionState(); - } - private void ScheduleDeferredInitialRefresh() { if (!UnityCliLoopSettingsWindowRefreshPolicy.ShouldScheduleDeferredInitialRefresh( @@ -336,7 +322,7 @@ internal void RefreshAllSections( RefreshToolSettingsHeader(); if (runExpensiveChecks) { - RefreshToolSettingsCatalogIfNeeded(); + _toolSettingsPresenter?.RefreshCatalogIfNeeded(_model.UI.ShowToolSettings); } } @@ -409,7 +395,7 @@ private async Task HandleRefreshCliVersion() public void InvalidateToolSettingsCatalog() { - _isToolSettingsCatalogDirty = true; + _toolSettingsPresenter?.InvalidateCatalog(); } private void RefreshToolSettingsHeader() @@ -422,120 +408,10 @@ private void RefreshToolSettingsHeader() _toolSettingsPresenter.UpdateHeader(_model.UI.ShowToolSettings); } - private void RefreshToolSettingsCatalog() - { - if (_toolSettingsPresenter == null) - { - return; - } - - ToolSettingsSectionData toolSettingsData = - _toolSettingsPresenter.UpdateCatalog(_model.UI.ShowToolSettings); - - if (UnityCliLoopSettingsWindowRefreshPolicy.ShouldKeepToolSettingsCatalogDirty(toolSettingsData)) - { - if (ScheduleToolSettingsRegistryWarmup()) - { - _isToolSettingsCatalogDirty = true; - return; - } - - _isToolSettingsCatalogDirty = false; - return; - } - - CancelToolSettingsRegistryWarmup(); - ResetToolSettingsRegistryWarmupAttemptCount(); - _isToolSettingsCatalogDirty = false; - } - - private void RefreshToolSettingsCatalogIfNeeded() - { - if (!_model.UI.ShowToolSettings || !_isToolSettingsCatalogDirty) - { - return; - } - - if (_view == null) - { - return; - } - - RefreshToolSettingsCatalog(); - } - private void UpdateShowToolSettings(bool show) { _model.UpdateShowToolSettings(show); - RefreshToolSettingsHeader(); - - if (!show) - { - _isToolSettingsCatalogDirty = true; - CancelToolSettingsRegistryWarmup(); - ResetToolSettingsRegistryWarmupAttemptCount(); - return; - } - - RefreshToolSettingsCatalogIfNeeded(); - } - - private bool ScheduleToolSettingsRegistryWarmup() - { - if (UnityCliLoopSettingsWindowRefreshPolicy.ShouldStartToolSettingsRegistryWarmup( - _isToolSettingsRegistryWarmupScheduled, - _toolSettingsRegistryWarmupAttemptCount, - ToolSettingsRegistryWarmupMaxAttempts)) - { - double delaySeconds = UnityCliLoopSettingsWindowRefreshPolicy.CalculateToolSettingsRegistryWarmupDelaySeconds( - ToolSettingsRegistryWarmupInitialDelaySeconds, - ToolSettingsRegistryWarmupMaxDelaySeconds, - _toolSettingsRegistryWarmupAttemptCount); - - _isToolSettingsRegistryWarmupScheduled = true; - _toolSettingsRegistryWarmupDueTime = EditorApplication.timeSinceStartup + delaySeconds; - _toolSettingsRegistryWarmupAttemptCount++; - EditorApplication.update += RunToolSettingsRegistryWarmupWhenDue; - return true; - } - - return _isToolSettingsRegistryWarmupScheduled; - } - - private void RunToolSettingsRegistryWarmupWhenDue() - { - if (EditorApplication.timeSinceStartup < _toolSettingsRegistryWarmupDueTime) - { - return; - } - - CancelToolSettingsRegistryWarmup(); - - if (_view == null || !_model.UI.ShowToolSettings) - { - ResetToolSettingsRegistryWarmupAttemptCount(); - return; - } - - _toolSettingsUseCase.WarmupRegistry(); - InvalidateToolSettingsCatalog(); - RefreshToolSettingsCatalogIfNeeded(); - } - - private void CancelToolSettingsRegistryWarmup() - { - if (!_isToolSettingsRegistryWarmupScheduled) - { - return; - } - - EditorApplication.update -= RunToolSettingsRegistryWarmupWhenDue; - _isToolSettingsRegistryWarmupScheduled = false; - } - - private void ResetToolSettingsRegistryWarmupAttemptCount() - { - _toolSettingsRegistryWarmupAttemptCount = 0; + _toolSettingsPresenter?.HandleShowToolSettingsChanged(show); } private void HandleToolToggled(string toolName, bool enabled) @@ -597,18 +473,11 @@ private void RefreshCliSetupSection(bool includeSkillDirectoryChecks = true) private bool ShouldCheckCliPathSetup() { - return ShouldCheckCliPathSetupForPlatform( + return UnityCliLoopSettingsCliSetupPresenter.ShouldCheckCliPathSetupForPlatform( UnityEngine.Application.platform, _cliSetupApplicationService.HasPackageOwnedCurrentUserInstall(UnityEngine.Application.platform)); } - internal static bool ShouldCheckCliPathSetupForPlatform( - RuntimePlatform platform, - bool hasPackageOwnedCurrentUserInstall) - { - return platform != RuntimePlatform.WindowsEditor && hasPackageOwnedCurrentUserInstall; - } - private void RefreshSelectedTargetInstallStateFast() { if (!_cliSetupApplicationService.IsCliInstalled()) @@ -709,13 +578,14 @@ private void CancelSkillInstallStateRefresh() private async Task HandleInstallCli() { - CliSetupPrimaryAction clickedAction = GetCurrentCliPrimaryButtonAction(); + CliSetupPrimaryAction clickedAction = _cliSetupPresenter.ResolveCurrentPrimaryButtonAction(_needsCliPathSetup); await RefreshCliPrimaryActionStateAsync(CancellationToken.None); - CliSetupPrimaryAction refreshedAction = GetCurrentCliPrimaryButtonAction(); - CliSetupPrimaryAction executableAction = ResolveExecutableCliPrimaryButtonAction( - clickedAction, - refreshedAction); + CliSetupPrimaryAction refreshedAction = _cliSetupPresenter.ResolveCurrentPrimaryButtonAction(_needsCliPathSetup); + CliSetupPrimaryAction executableAction = + UnityCliLoopSettingsCliSetupPresenter.ResolveExecutableCliPrimaryButtonAction( + clickedAction, + refreshedAction); if (executableAction == CliSetupPrimaryAction.None) { return; @@ -771,69 +641,6 @@ await CliPathSetupPrompt.EnsureVisibleAndShowResultAsync( } } - private CliSetupPrimaryAction GetCurrentCliPrimaryButtonAction() - { - string cliVersion = _cliSetupApplicationService.GetCachedCliVersion(); - bool cliIsDispatcher = _cliSetupApplicationService.GetCachedCliIsDispatcher(); - string cliExecutablePath = _cliSetupApplicationService.GetCachedCliExecutablePath(); - bool canUninstallCli = _cliSetupApplicationService.IsPackageOwnedCurrentUserInstallPath( - cliExecutablePath, - UnityEngine.Application.platform); - string requiredCliVersion = _cliSetupApplicationService.GetMinimumRequiredCliVersion(); - return ResolveCliPrimaryButtonAction( - _needsCliPathSetup, - cliVersion, - cliIsDispatcher, - canUninstallCli, - requiredCliVersion); - } - - internal static bool ShouldUninstallCliFromPrimaryButton( - string cliVersion, - bool cliIsDispatcher, - bool canUninstallCli, - string requiredCliVersion) - { - bool isCliInstalled = cliVersion != null; - bool needsUpdate = IsCliUpdateNeeded(cliVersion, cliIsDispatcher, requiredCliVersion); - return CliSetupPrimaryActionPolicy.ShouldUninstallCli( - isCliInstalled, - needsUpdate, - canUninstallCli); - } - - internal static CliSetupPrimaryAction ResolveCliPrimaryButtonAction( - bool needsCliPathSetup, - string cliVersion, - bool cliIsDispatcher, - bool canUninstallCli, - string requiredCliVersion) - { - bool needsUpdate = IsCliUpdateNeeded(cliVersion, cliIsDispatcher, requiredCliVersion); - bool isCliInstalled = cliVersion != null; - return CliSetupPrimaryActionPolicy.ResolveSettingsPrimaryAction( - needsCliPathSetup, - needsUpdate, - isCliInstalled, - canUninstallCli); - } - - internal static CliSetupPrimaryAction ResolveExecutableCliPrimaryButtonAction( - CliSetupPrimaryAction clickedAction, - CliSetupPrimaryAction refreshedAction) - { - return CliSetupPrimaryActionPolicy.ResolveExecutableSettingsAction( - clickedAction, - refreshedAction); - } - - internal static bool ShouldRepairCliPathFromPrimaryButton( - bool needsCliPathSetup, - bool needsUpdate) - { - return CliSetupPrimaryActionPolicy.ShouldRepairCliPath(needsCliPathSetup, needsUpdate); - } - private async Task RefreshCliPrimaryActionStateAsync(CancellationToken ct) { _isRefreshingVersion = true; @@ -885,34 +692,6 @@ await CliPathSetupPrompt.EnsureVisibleAndShowResultAsync( } } - internal static bool IsCliUpdateNeeded( - string cliVersion, - bool cliIsDispatcher, - string requiredCliVersion) - { - return EvaluateCliSetupCompatibility( - cliVersion, - cliIsDispatcher, - requiredCliVersion).NeedsUpdate; - } - - internal static bool ShouldShowSkillsInstalledDialog(SkillSetupTargetInfo targetInfo) - { - return targetInfo.InstallState != SkillInstallState.Outdated - && !targetInfo.HasDifferentLayoutSkills; - } - - private static CliSetupCompatibilityState EvaluateCliSetupCompatibility( - string cliVersion, - bool cliIsDispatcher, - string requiredCliVersion) - { - return CliSetupCompatibility.Evaluate( - cliVersion, - cliIsDispatcher, - requiredCliVersion); - } - private async Task HandleUninstallCli() { if (!CliUninstallPrompt.ConfirmUninstall()) @@ -958,7 +737,8 @@ private async Task HandleInstallSkills() string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); SkillSetupTargetInfo selectedTargetInfo = GetSelectedTargetInfo(projectRoot, includeFreshnessCheck: true); - bool shouldShowSkillsInstalledDialog = ShouldShowSkillsInstalledDialog(selectedTargetInfo); + bool shouldShowSkillsInstalledDialog = + SkillInstallDialogPolicy.ShouldShowForSelectedTarget(selectedTargetInfo); CancelSkillInstallStateRefresh(); _isInstallingSkills = true; RefreshCliSetupSection();