Skip to content

Commit 9341cef

Browse files
Address code review feedback from coderabbit, codex and cursor
- Add null-guard after OpenFolderPanel in case import window is destroyed during modal dialog - Track dismissed companions so users can close the companion window without it reappearing for the same import session - Add ClearStaleEntries to clean up activeCompanions dictionary - Add input validation to public ChangeAssetItemPath API - Throttle watcher to 0.25s intervals to reduce per-frame allocations - Reuse PackageImportType in ShowImportPackageMethodInfo getter Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 88f142c commit 9341cef

1 file changed

Lines changed: 37 additions & 3 deletions

File tree

Editor/Package2Folder.cs

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,7 @@ private static MethodInfo ShowImportPackageMethodInfo
112112
{
113113
if (showImportPackageMethodInfo == null)
114114
{
115-
var packageImport = typeof(MenuItem).Assembly.GetType("UnityEditor.PackageImport");
116-
showImportPackageMethodInfo = packageImport.GetMethod("ShowImportPackage");
115+
showImportPackageMethodInfo = PackageImportType.GetMethod("ShowImportPackage");
117116
}
118117

119118
return showImportPackageMethodInfo;
@@ -166,8 +165,13 @@ private static void SetupPackageImportWatcher()
166165
EditorApplication.update += WatchForPackageImportWindows;
167166
}
168167

168+
private static double nextWatchTime;
169+
169170
private static void WatchForPackageImportWindows()
170171
{
172+
if (EditorApplication.timeSinceStartup < nextWatchTime) return;
173+
nextWatchTime = EditorApplication.timeSinceStartup + 0.25;
174+
171175
var windows = Resources.FindObjectsOfTypeAll(PackageImportType);
172176
if (windows == null || windows.Length == 0) return;
173177

@@ -253,6 +257,9 @@ public static void ImportPackageToFolder(string packagePath, string selectedFold
253257

254258
public static void ChangeAssetItemPath(object assetItem, string selectedFolderPath)
255259
{
260+
if (string.IsNullOrEmpty(selectedFolderPath) || !selectedFolderPath.StartsWith("Assets"))
261+
throw new ArgumentException("selectedFolderPath must start with 'Assets'", "selectedFolderPath");
262+
256263
string destinationPath = (string)DestinationAssetPathFieldInfo.GetValue(assetItem);
257264
if (destinationPath.StartsWith("Packages/")) return;
258265

@@ -392,6 +399,7 @@ private static string GetSelectedFolderPath()
392399
internal class Package2FolderCompanion : EditorWindow
393400
{
394401
private static readonly Dictionary<int, Package2FolderCompanion> activeCompanions = new Dictionary<int, Package2FolderCompanion>();
402+
private static readonly HashSet<int> dismissedImportWindows = new HashSet<int>();
395403

396404
[SerializeField] private EditorWindow importWindow;
397405
[SerializeField] private string[] originalPaths;
@@ -401,6 +409,11 @@ internal static void ShowForImportWindow(EditorWindow importWindow)
401409
{
402410
var id = importWindow.GetInstanceID();
403411

412+
if (dismissedImportWindows.Contains(id))
413+
return;
414+
415+
ClearStaleEntries();
416+
404417
Package2FolderCompanion existing;
405418
if (activeCompanions.TryGetValue(id, out existing) && existing != null)
406419
return;
@@ -414,6 +427,21 @@ internal static void ShowForImportWindow(EditorWindow importWindow)
414427
activeCompanions[id] = companion;
415428
}
416429

430+
private static void ClearStaleEntries()
431+
{
432+
var staleKeys = new List<int>();
433+
foreach (var kvp in activeCompanions)
434+
{
435+
if (kvp.Value == null || kvp.Value.importWindow == null)
436+
staleKeys.Add(kvp.Key);
437+
}
438+
foreach (var key in staleKeys)
439+
{
440+
activeCompanions.Remove(key);
441+
dismissedImportWindows.Remove(key);
442+
}
443+
}
444+
417445
private void CacheOriginalPaths()
418446
{
419447
originalPaths = Package2Folder.GetImportItemPaths(importWindow);
@@ -463,6 +491,7 @@ private void SelectFolderAndModifyPaths()
463491
{
464492
var absolutePath = EditorUtility.OpenFolderPanel("Select target folder", "Assets", "");
465493
if (string.IsNullOrEmpty(absolutePath)) return;
494+
if (importWindow == null) return;
466495

467496
absolutePath = absolutePath.Replace('\\', '/');
468497
var dataPath = Application.dataPath.Replace('\\', '/');
@@ -491,7 +520,12 @@ private void SelectFolderAndModifyPaths()
491520
private void OnDestroy()
492521
{
493522
if (importWindow != null)
494-
activeCompanions.Remove(importWindow.GetInstanceID());
523+
{
524+
var id = importWindow.GetInstanceID();
525+
activeCompanions.Remove(id);
526+
// Import window still alive means user dismissed companion manually
527+
dismissedImportWindows.Add(id);
528+
}
495529
}
496530
}
497531
}

0 commit comments

Comments
 (0)