From 2c9fcd706acca84b9b068ec5570e4da3e1ecde91 Mon Sep 17 00:00:00 2001 From: Igor Stojkovic Date: Mon, 8 Jul 2024 13:02:37 +0200 Subject: [PATCH 1/2] Only execute Start/StopAssetEditing if something will change This fixes issue #660 where Unity in some situations deselects the edited object on any edit. --- .../Editor/NugetAssetPostprocessor.cs | 61 ++++++++++++++----- 1 file changed, 47 insertions(+), 14 deletions(-) diff --git a/src/NuGetForUnity/Editor/NugetAssetPostprocessor.cs b/src/NuGetForUnity/Editor/NugetAssetPostprocessor.cs index f89562b6..077b8922 100644 --- a/src/NuGetForUnity/Editor/NugetAssetPostprocessor.cs +++ b/src/NuGetForUnity/Editor/NugetAssetPostprocessor.cs @@ -97,15 +97,10 @@ internal static void OnPostprocessAllAssets( var absoluteRepositoryPath = GetNuGetRepositoryPath(); - AssetDatabase.StartAssetEditing(); - - try - { - LogResults(importedAssets.SelectMany(assetPath => HandleAsset(assetPath, absoluteRepositoryPath, true))); - } - finally + using (var delayedAssetEditor = new DelayedAssetEditor()) { - AssetDatabase.StopAssetEditing(); + // ReSharper disable once AccessToDisposedClosure + LogResults(importedAssets.SelectMany(assetPath => HandleAsset(assetPath, absoluteRepositoryPath, true, delayedAssetEditor))); } } @@ -113,14 +108,15 @@ internal static void OnPostprocessAllAssets( private static IEnumerable<(string AssetType, string AssetPath, ResultStatus Status)> HandleAsset( [NotNull] string projectRelativeAssetPath, [NotNull] string absoluteRepositoryPath, - bool reimport) + bool reimport, + DelayedAssetEditor delayedAssetEditor) { var assetFileName = Path.GetFileName(projectRelativeAssetPath); if (assetFileName.Equals(NugetConfigFile.FileName, StringComparison.OrdinalIgnoreCase) || assetFileName.Equals(PackagesConfigFile.FileName, StringComparison.OrdinalIgnoreCase)) { // Not sure why but for .config files we need to re-import always. I think this is because they are treated as native plug-ins. - var result = ModifyImportSettingsOfConfigurationFile(projectRelativeAssetPath, true); + var result = ModifyImportSettingsOfConfigurationFile(projectRelativeAssetPath, true, delayedAssetEditor); yield return ("ConfigurationFile", projectRelativeAssetPath, result); yield break; @@ -157,12 +153,14 @@ internal static void OnPostprocessAllAssets( var assetLablesToSet = new List(); if (configurationOfPackage != null) { + delayedAssetEditor.Start(); assetLablesToSet.AddRange(ModifyImportSettingsOfGeneralPlugin(configurationOfPackage, plugin)); yield return ("GeneralSetting", projectRelativeAssetPath, ResultStatus.Success); } if (assetPathComponents.Length > 1 && assetPathComponents[1].Equals(AnalyzersFolderName, StringComparison.OrdinalIgnoreCase)) { + delayedAssetEditor.Start(); assetLablesToSet.AddRange(ModifyImportSettingsOfRoslynAnalyzer(plugin)); yield return ("RoslynAnalyzer", projectRelativeAssetPath, ResultStatus.Success); } @@ -170,6 +168,7 @@ internal static void OnPostprocessAllAssets( UnityPreImportedLibraryResolver.GetAlreadyImportedEditorOnlyLibraries() .Contains(Path.GetFileNameWithoutExtension(assetPathComponents[assetPathComponents.Length - 1]))) { + delayedAssetEditor.Start(); assetLablesToSet.AddRange(ModifyImportSettingsOfPlayerOnly(plugin)); yield return ("PlayerOnly", projectRelativeAssetPath, ResultStatus.Success); } @@ -324,7 +323,11 @@ private static string[] ModifyImportSettingsOfPlayerOnly([NotNull] PluginImporte /// /// The path to the .config file. /// Whether or not to save and re-import the file. - private static ResultStatus ModifyImportSettingsOfConfigurationFile([NotNull] string analyzerAssetPath, bool reimport) + /// + private static ResultStatus ModifyImportSettingsOfConfigurationFile( + [NotNull] string analyzerAssetPath, + bool reimport, + DelayedAssetEditor delayedAssetEditor) { if (!GetPluginImporter(analyzerAssetPath, out var plugin)) { @@ -336,6 +339,7 @@ private static ResultStatus ModifyImportSettingsOfConfigurationFile([NotNull] st return ResultStatus.AlreadyProcessed; } + delayedAssetEditor.Start(); plugin.SetCompatibleWithPlatform(BuildTarget.WSAPlayer, false); AssetDatabase.SetLabels(plugin, new[] { ProcessedLabel }); @@ -435,14 +439,17 @@ private static bool AlreadyProcessed([NotNull] Object asset) private void OnPreprocessAsset() { var absoluteRepositoryPath = GetNuGetRepositoryPath(); - var results = HandleAsset(assetPath, absoluteRepositoryPath, false); - LogResults(results); + using (var delayedAssetEditor = new DelayedAssetEditor()) + { + var results = HandleAsset(assetPath, absoluteRepositoryPath, false, delayedAssetEditor); + LogResults(results); + } } [SuppressMessage( "StyleCop.CSharp.OrderingRules", "SA1201:Elements should appear in the correct order", - Justification = "We like private enums at the botom of the file.")] + Justification = "We like private enums at the bottom of the file.")] private enum ResultStatus { Success, @@ -451,5 +458,31 @@ private enum ResultStatus AlreadyProcessed, } + + [SuppressMessage( + "StyleCop.CSharp.OrderingRules", + "SA1201:Elements should appear in the correct order", + Justification = "We like private classes at the bottom of the file.")] + private class DelayedAssetEditor : IDisposable + { + private bool editingStarted; + + public void Start() + { + if (!editingStarted) + { + editingStarted = true; + AssetDatabase.StartAssetEditing(); + } + } + + public void Dispose() + { + if (editingStarted) + { + AssetDatabase.StopAssetEditing(); + } + } + } } } From 81a48844f0b8f6bb36d921191b8042dce772705f Mon Sep 17 00:00:00 2001 From: Igor Stojkovic Date: Mon, 15 Jul 2024 11:38:49 +0200 Subject: [PATCH 2/2] Fixed PreprocessAsset and resolved AccessToDisposedClosure warning --- .../Editor/NugetAssetPostprocessor.cs | 33 ++++++++++++------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/src/NuGetForUnity/Editor/NugetAssetPostprocessor.cs b/src/NuGetForUnity/Editor/NugetAssetPostprocessor.cs index 077b8922..17b9364b 100644 --- a/src/NuGetForUnity/Editor/NugetAssetPostprocessor.cs +++ b/src/NuGetForUnity/Editor/NugetAssetPostprocessor.cs @@ -97,10 +97,23 @@ internal static void OnPostprocessAllAssets( var absoluteRepositoryPath = GetNuGetRepositoryPath(); + LogResults(ProcessAssets(importedAssets, absoluteRepositoryPath)); + } + + [NotNull] + private static IEnumerable<(string AssetType, string AssetPath, ResultStatus Status)> ProcessAssets( + [NotNull]string[] importedAssets, + [NotNull] string absoluteRepositoryPath) + { using (var delayedAssetEditor = new DelayedAssetEditor()) { - // ReSharper disable once AccessToDisposedClosure - LogResults(importedAssets.SelectMany(assetPath => HandleAsset(assetPath, absoluteRepositoryPath, true, delayedAssetEditor))); + foreach (var assetPath in importedAssets) + { + foreach (var result in HandleAsset(assetPath, absoluteRepositoryPath, true, delayedAssetEditor)) + { + yield return result; + } + } } } @@ -109,7 +122,7 @@ internal static void OnPostprocessAllAssets( [NotNull] string projectRelativeAssetPath, [NotNull] string absoluteRepositoryPath, bool reimport, - DelayedAssetEditor delayedAssetEditor) + DelayedAssetEditor delayedAssetEditor = null) { var assetFileName = Path.GetFileName(projectRelativeAssetPath); if (assetFileName.Equals(NugetConfigFile.FileName, StringComparison.OrdinalIgnoreCase) || @@ -153,14 +166,14 @@ internal static void OnPostprocessAllAssets( var assetLablesToSet = new List(); if (configurationOfPackage != null) { - delayedAssetEditor.Start(); + delayedAssetEditor?.Start(); assetLablesToSet.AddRange(ModifyImportSettingsOfGeneralPlugin(configurationOfPackage, plugin)); yield return ("GeneralSetting", projectRelativeAssetPath, ResultStatus.Success); } if (assetPathComponents.Length > 1 && assetPathComponents[1].Equals(AnalyzersFolderName, StringComparison.OrdinalIgnoreCase)) { - delayedAssetEditor.Start(); + delayedAssetEditor?.Start(); assetLablesToSet.AddRange(ModifyImportSettingsOfRoslynAnalyzer(plugin)); yield return ("RoslynAnalyzer", projectRelativeAssetPath, ResultStatus.Success); } @@ -168,7 +181,7 @@ internal static void OnPostprocessAllAssets( UnityPreImportedLibraryResolver.GetAlreadyImportedEditorOnlyLibraries() .Contains(Path.GetFileNameWithoutExtension(assetPathComponents[assetPathComponents.Length - 1]))) { - delayedAssetEditor.Start(); + delayedAssetEditor?.Start(); assetLablesToSet.AddRange(ModifyImportSettingsOfPlayerOnly(plugin)); yield return ("PlayerOnly", projectRelativeAssetPath, ResultStatus.Success); } @@ -339,7 +352,7 @@ private static ResultStatus ModifyImportSettingsOfConfigurationFile( return ResultStatus.AlreadyProcessed; } - delayedAssetEditor.Start(); + delayedAssetEditor?.Start(); plugin.SetCompatibleWithPlatform(BuildTarget.WSAPlayer, false); AssetDatabase.SetLabels(plugin, new[] { ProcessedLabel }); @@ -439,11 +452,7 @@ private static bool AlreadyProcessed([NotNull] Object asset) private void OnPreprocessAsset() { var absoluteRepositoryPath = GetNuGetRepositoryPath(); - using (var delayedAssetEditor = new DelayedAssetEditor()) - { - var results = HandleAsset(assetPath, absoluteRepositoryPath, false, delayedAssetEditor); - LogResults(results); - } + LogResults(HandleAsset(assetPath, absoluteRepositoryPath, false)); } [SuppressMessage(