Skip to content

Commit 0b109cd

Browse files
Sebastian Muehrclaude
andcommitted
fix: address PR review feedback
- Verify download completion after wait: check GetDownloadState and return ErrorResponse on failure/timeout instead of always reporting success - Handle string parameter type in GetDownloadOperation for Unity version compatibility - Throttle async wait polling to every ~0.5s instead of every frame Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent cb8b2fb commit 0b109cd

File tree

1 file changed

+28
-48
lines changed

1 file changed

+28
-48
lines changed

MCPForUnity/Editor/Tools/ManageAssetStore.cs

Lines changed: 28 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -469,10 +469,18 @@ private static async Task<object> DownloadAsync(ToolParams p)
469469

470470
await WaitForDownloadAsync(productId, TimeSpan.FromMinutes(5));
471471

472+
// Verify download actually completed
473+
var (_, _, completed, downloadError) = GetDownloadState(productId);
472474
string packagePath = GetCachedPackagePath(productId);
473475

474476
DestroyHiddenWindow();
475477

478+
if (!string.IsNullOrEmpty(downloadError))
479+
return new ErrorResponse($"Download failed for product {productId}: {downloadError}");
480+
481+
if (!completed && string.IsNullOrEmpty(packagePath))
482+
return new ErrorResponse($"Download timed out for product {productId}. The package may still be downloading — try again later.");
483+
476484
return new SuccessResponse(
477485
$"Download completed for product {productId}.",
478486
new { product_id = productId, package_path = packagePath }
@@ -808,8 +816,8 @@ private static (string phase, float progress, bool completed, string error) GetD
808816
if (managerInstance != null && _getDownloadOpMethod != null)
809817
{
810818
var paramType = _getDownloadOpMethod.GetParameters().FirstOrDefault()?.ParameterType;
811-
var op = _getDownloadOpMethod.Invoke(managerInstance,
812-
new object[] { paramType == typeof(long) ? (object)productId : productId });
819+
object arg = paramType == typeof(string) ? (object)productId.ToString() : productId;
820+
var op = _getDownloadOpMethod.Invoke(managerInstance, new[] { arg });
813821

814822
if (op != null)
815823
{
@@ -975,43 +983,25 @@ private static object GetPackageManagerRoot()
975983

976984
private static Task WaitForOperationAsync(object listOp, TimeSpan timeout)
977985
{
986+
if (listOp == null || _listOperationType == null)
987+
return Task.CompletedTask;
988+
989+
var prop = _listOperationType.GetProperties(
990+
BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)
991+
.FirstOrDefault(p => p.Name == "isInProgress");
992+
if (prop == null)
993+
return Task.CompletedTask;
994+
978995
var tcs = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
979996
var start = DateTime.UtcNow;
997+
int frameCount = 0;
980998

981999
void Tick()
9821000
{
983-
if (tcs.Task.IsCompleted)
984-
{
985-
EditorApplication.update -= Tick;
986-
return;
987-
}
1001+
if (tcs.Task.IsCompleted) { EditorApplication.update -= Tick; return; }
1002+
if (frameCount++ % 30 != 0) return;
9881003

989-
if ((DateTime.UtcNow - start) > timeout)
990-
{
991-
EditorApplication.update -= Tick;
992-
tcs.TrySetResult(true); // Timeout — return what we have
993-
return;
994-
}
995-
996-
try
997-
{
998-
bool isInProgress = false;
999-
if (listOp != null && _listOperationType != null)
1000-
{
1001-
var prop = _listOperationType.GetProperties(
1002-
BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)
1003-
.FirstOrDefault(p => p.Name == "isInProgress");
1004-
if (prop != null)
1005-
isInProgress = (bool)(prop.GetValue(listOp) ?? false);
1006-
}
1007-
1008-
if (!isInProgress)
1009-
{
1010-
EditorApplication.update -= Tick;
1011-
tcs.TrySetResult(true);
1012-
}
1013-
}
1014-
catch
1004+
if ((DateTime.UtcNow - start) > timeout || !(bool)(prop.GetValue(listOp) ?? false))
10151005
{
10161006
EditorApplication.update -= Tick;
10171007
tcs.TrySetResult(true);
@@ -1026,14 +1016,12 @@ private static Task WaitForDownloadAsync(long productId, TimeSpan timeout)
10261016
{
10271017
var tcs = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
10281018
var start = DateTime.UtcNow;
1019+
int frameCount = 0;
10291020

10301021
void Tick()
10311022
{
1032-
if (tcs.Task.IsCompleted)
1033-
{
1034-
EditorApplication.update -= Tick;
1035-
return;
1036-
}
1023+
if (tcs.Task.IsCompleted) { EditorApplication.update -= Tick; return; }
1024+
if (frameCount++ % 30 != 0) return;
10371025

10381026
if ((DateTime.UtcNow - start) > timeout)
10391027
{
@@ -1042,16 +1030,8 @@ void Tick()
10421030
return;
10431031
}
10441032

1045-
try
1046-
{
1047-
var (_, _, completed, error) = GetDownloadState(productId);
1048-
if (completed || !string.IsNullOrEmpty(error))
1049-
{
1050-
EditorApplication.update -= Tick;
1051-
tcs.TrySetResult(true);
1052-
}
1053-
}
1054-
catch
1033+
var (_, _, completed, error) = GetDownloadState(productId);
1034+
if (completed || !string.IsNullOrEmpty(error))
10551035
{
10561036
EditorApplication.update -= Tick;
10571037
tcs.TrySetResult(true);

0 commit comments

Comments
 (0)