Skip to content
Open

Dev #159

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 39 additions & 35 deletions Editor/Resources/Builder/BuilderWindow.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
using Banter.SDKEditor;
using Unity.EditorCoroutines.Editor;
using System.Collections;
using UnityEngine.Networking;
using System.Net.Http;
using System.Text;
using UnityEditor.UIElements;
using System.Text.RegularExpressions;
using UnityEditor.SceneManagement;
Expand Down Expand Up @@ -49,6 +50,8 @@ public class KitObjectAndPath

public class BuilderWindow : EditorWindow
{
private static readonly HttpClient _httpClient = new HttpClient();

[SerializeField] private VisualTreeAsset _mainWindowVisualTree = default;
[SerializeField] private StyleSheet _mainWindowStyleSheet = default;

Expand Down Expand Up @@ -372,18 +375,12 @@ void ShowHideBuildButton()

public IEnumerator Texture(string url, Action<Texture2D> callback)
{
using (UnityWebRequest uwr = UnityWebRequestTexture.GetTexture(url))
{
yield return uwr.SendWebRequest();
if (uwr.result != UnityWebRequest.Result.Success)
{
throw new System.Exception(uwr.error);
}
else
{
callback(DownloadHandlerTexture.GetContent(uwr));
}
}
var task = _httpClient.GetByteArrayAsync(url);
while (!task.IsCompleted) yield return null;
if (task.IsFaulted) throw task.Exception.InnerException ?? task.Exception;
var tex = new Texture2D(1, 1);
tex.LoadImage(task.Result);
callback(tex);
}
Comment on lines 376 to 384
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Unchecked LoadImage return value.

Texture2D.LoadImage() returns a boolean indicating success. If the downloaded bytes are not a valid image format, this will fail silently and the callback will receive an invalid texture.

Suggested fix
     var tex = new Texture2D(1, 1);
-    tex.LoadImage(task.Result);
-    callback(tex);
+    if (!tex.LoadImage(task.Result))
+    {
+        UnityEngine.Object.DestroyImmediate(tex);
+        throw new Exception($"Failed to load image from URL: {url}");
+    }
+    callback(tex);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public IEnumerator Texture(string url, Action<Texture2D> callback)
{
using (UnityWebRequest uwr = UnityWebRequestTexture.GetTexture(url))
{
yield return uwr.SendWebRequest();
if (uwr.result != UnityWebRequest.Result.Success)
{
throw new System.Exception(uwr.error);
}
else
{
callback(DownloadHandlerTexture.GetContent(uwr));
}
}
var task = _httpClient.GetByteArrayAsync(url);
while (!task.IsCompleted) yield return null;
if (task.IsFaulted) throw task.Exception.InnerException ?? task.Exception;
var tex = new Texture2D(1, 1);
tex.LoadImage(task.Result);
callback(tex);
}
public IEnumerator Texture(string url, Action<Texture2D> callback)
{
var task = _httpClient.GetByteArrayAsync(url);
while (!task.IsCompleted) yield return null;
if (task.IsFaulted) throw task.Exception.InnerException ?? task.Exception;
var tex = new Texture2D(1, 1);
if (!tex.LoadImage(task.Result))
{
UnityEngine.Object.DestroyImmediate(tex);
throw new Exception($"Failed to load image from URL: {url}");
}
callback(tex);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Editor/Resources/Builder/BuilderWindow.cs` around lines 376 - 384, In the
Texture coroutine, check the boolean return value of Texture2D.LoadImage after
creating tex and loading task.Result; if LoadImage returns false,
dispose/destroy the invalid Texture2D, log or surface the error, and invoke the
provided callback with null (or throw a descriptive exception) instead of
passing an invalid texture; update the Texture method (the coroutine named
Texture and its callback usage) to handle the failed load path cleanly so
callers can detect and handle image decoding failures.


void SelectKit(int selectedIndex) {
Expand Down Expand Up @@ -1191,39 +1188,46 @@ private void SetupUI()
}
public IEnumerator Json<T>(string url, Action<T> callback)
{
UnityWebRequest uwr = UnityWebRequest.Get(url);
yield return uwr.SendWebRequest();
if (uwr.result != UnityWebRequest.Result.Success)
{
throw new System.Exception(uwr.error);
}
else
{
callback(JsonUtility.FromJson<T>(uwr.downloadHandler.text));
}
var task = _httpClient.GetAsync(url);
while (!task.IsCompleted) yield return null;
if (task.IsFaulted) throw task.Exception.InnerException ?? task.Exception;
var response = task.Result;
if (!response.IsSuccessStatusCode)
throw new System.Exception(response.StatusCode + ": " + response.ReasonPhrase);
var readTask = response.Content.ReadAsStringAsync();
while (!readTask.IsCompleted) yield return null;
if (readTask.IsFaulted) throw readTask.Exception.InnerException ?? readTask.Exception;
callback(JsonUtility.FromJson<T>(readTask.Result));
}

public IEnumerator Json<T>(string url, T postData, Action<string> callback, Dictionary<string, string> headers = null)
{
UnityWebRequest uwr = UnityWebRequest.Put(url, JsonUtility.ToJson(postData));
uwr.method = "POST";
var json = JsonUtility.ToJson(postData);
var request = new HttpRequestMessage(HttpMethod.Post, url)
{
Content = new StringContent(json, Encoding.UTF8, "application/json")
};
if (headers != null)
{
foreach (var header in headers)
{
uwr.SetRequestHeader(header.Key, header.Value);
if (!header.Key.Equals("Content-Type", StringComparison.OrdinalIgnoreCase))
request.Headers.TryAddWithoutValidation(header.Key, header.Value);
}
}
yield return uwr.SendWebRequest();

if (uwr.result != UnityWebRequest.Result.Success)
{
Debug.LogError(url + ":" + JsonUtility.ToJson(postData));
throw new System.Exception(uwr.error);
}
else
var task = _httpClient.SendAsync(request);
while (!task.IsCompleted) yield return null;
if (task.IsFaulted) throw task.Exception.InnerException ?? task.Exception;
var response = task.Result;
if (!response.IsSuccessStatusCode)
{
callback(uwr.downloadHandler.text);
Debug.LogError(url + ":" + json);
throw new System.Exception(response.StatusCode + ": " + response.ReasonPhrase);
}
var readTask = response.Content.ReadAsStringAsync();
while (!readTask.IsCompleted) yield return null;
if (readTask.IsFaulted) throw readTask.Exception.InnerException ?? readTask.Exception;
callback(readTask.Result);
}
private IEnumerator PopulateExistingKits(Action callback = null) {
if(sq.User == null) {
Expand Down
4 changes: 2 additions & 2 deletions Editor/Resources/Builder/BuilderWindow.uxml
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@
</ui:VisualElement>
<ui:VisualElement style="flex-grow: 1; flex-direction: row; justify-content: space-between; padding-right: 30px; padding-bottom: 30px;">
<ui:VisualElement style="flex-grow: 1;">
<ui:Label tabindex="-1" text="Create new Avatar or Upload to Existing" parse-escape-sequences="true" display-tooltip-when-elided="true" style="margin-left: 30px; margin-top: 10px; display: none;" />
<ui:DropdownField name="ddlAvatarId" style="margin-left: 30px; display: none;" />
<ui:Label tabindex="-1" text="Create new Avatar or Upload to Existing" parse-escape-sequences="true" display-tooltip-when-elided="true" style="margin-left: 30px; margin-top: 10px; display: flex;" />
<ui:DropdownField name="ddlAvatarId" style="margin-left: 30px; display:flex;" />
<ui:VisualElement style="flex-grow: 1; flex-direction: row;">
<ui:Toggle name="togAvatarIsPublic" style="height: 22px; margin-left: 30px;" />
<ui:Label tabindex="-1" text="Avatar is Public (other users can clone)" parse-escape-sequences="true" display-tooltip-when-elided="true" style="margin-left: 10px; margin-top: 5px;" />
Expand Down
41 changes: 41 additions & 0 deletions Editor/Resources/Builder/MenuUtils.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
using UnityEngine;
using UnityEditor;
using Banter.SDK;
using System.IO;
using UnityEditor.PackageManager;
using UnityEditor.PackageManager.Requests;

namespace Banter.SDKEditor
{
Expand All @@ -27,5 +30,43 @@ static void CreateBanterStarterUpper(MenuCommand menuCommand)
Undo.RegisterCreatedObjectUndo(go, "Create " + go.name);
Selection.activeObject = go;
}

#if !BANTER_EDITOR
[MenuItem("Banter/Uninstall SDK")]
static void UninstallBanter()
{
bool userResponse = EditorUtility.DisplayDialog(
"Uninstall Banter SDK",
"Are you sure? This will restart the unity editor.",
"Affirmative",
"Negative");

if (!userResponse) return;
RemoveRequest request = Client.Remove("com.sidequest.banter");
while(!request.IsCompleted)
{

}
Comment on lines +45 to +49
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

cat -n Editor/Resources/Builder/MenuUtils.cs | sed -n '40,60p'

Repository: SideQuestVR/BanterSDK

Length of output: 1033


🏁 Script executed:

# Search for RemoveRequest usage in the codebase to understand the pattern
rg "RemoveRequest" -A 5 -B 2

Repository: SideQuestVR/BanterSDK

Length of output: 1260


🌐 Web query:

UnityEditor.PackageManager.Client.Remove RemoveRequest async completion IsCompleted editor update

💡 Result:

Client.Remove(...) returns a RemoveRequest and runs asynchronously—there’s no callback/event on the request itself, so you detect completion by polling request.IsCompleted and then checking request.Status / request.Error. IsCompleted becomes true only when the UPM operation finishes. [1][2]

Typical Editor pattern (poll each editor tick): [3]

using UnityEditor;
using UnityEditor.PackageManager;
using UnityEditor.PackageManager.Requests;

static RemoveRequest _req;

static void RemovePkg(string name)
{
    _req = Client.Remove(name);
    EditorApplication.update += Tick;
}

static void Tick()
{
    if (!_req.IsCompleted) return;

    EditorApplication.update -= Tick;

    if (_req.Status == StatusCode.Success) UnityEngine.Debug.Log("Removed");
    else UnityEngine.Debug.LogError(_req.Error.message);
}

If you also need to know when the Editor has applied/compiled/imported the resulting package set (not just when the request completed), subscribe to UnityEditor.PackageManager.Events.registeredPackages (raised after the Editor applies the changed list). [4] Unity notes cases where the change is only effective after a “full editor tick”, so chaining package operations is often done sequentially via EditorApplication.update (or by waiting for registeredPackages). [5]

Sources: [1] Client.Remove doc, [2] Request.IsCompleted/Status/Error, [3] Unity manual example showing the EditorApplication.update + IsCompleted polling pattern, [4] Package Manager Events, [5] Unity Issue Tracker note about changes becoming effective after a full editor tick.


Replace the busy-wait loop with EditorApplication.update callback.

Client.Remove() completes asynchronously during editor updates. The tight loop on lines 46–49 blocks that update pump, freezing the editor and preventing the uninstall from ever completing. Use EditorApplication.update to poll IsCompleted instead.

Safer pattern
             RemoveRequest request = Client.Remove("com.sidequest.banter");
-            while(!request.IsCompleted)
-            {
-                
-            }
+            EditorApplication.update += WaitForRemove;
+            void WaitForRemove()
+            {
+                if (!request.IsCompleted) return;
+                EditorApplication.update -= WaitForRemove;
+                if (request.Status == StatusCode.Success)
+                {
+                    // continue cleanup here
+                }
+                else
+                {
+                    Debug.LogError($"Remove failed: {request.Error.message}");
+                }
+            }
+            return;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
RemoveRequest request = Client.Remove("com.sidequest.banter");
while(!request.IsCompleted)
{
}
RemoveRequest request = Client.Remove("com.sidequest.banter");
EditorApplication.update += WaitForRemove;
void WaitForRemove()
{
if (!request.IsCompleted) return;
EditorApplication.update -= WaitForRemove;
if (request.Status == StatusCode.Success)
{
// continue cleanup here
}
else
{
Debug.LogError($"Remove failed: {request.Error.message}");
}
}
return;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Editor/Resources/Builder/MenuUtils.cs` around lines 45 - 49, The busy-wait
loop that polls RemoveRequest.IsCompleted after calling
Client.Remove("com.sidequest.banter") blocks the editor update pump; instead
register a callback on EditorApplication.update to poll the RemoveRequest until
IsCompleted, then unregister the callback and continue processing. Locate the
call site using Client.Remove and the RemoveRequest variable name, replace the
while(!request.IsCompleted) { } spin with an EditorApplication.update delegate
that checks request.IsCompleted each tick, calls whatever follow-up logic is
needed when completed, and removes itself from EditorApplication.update to avoid
leaking the callback.

if(Directory.Exists("Packages/com.basis.bundlemanagement"))
{
Directory.Delete("Packages/com.basis.bundlemanagement", true);
}
if(Directory.Exists("Packages/com.basis.sdk"))
{
Directory.Delete("Packages/com.basis.sdk", true);
}
if(Directory.Exists("Packages/com.basis.odinserializer"))
{
Directory.Delete("Packages/com.basis.odinserializer", true);
}
if (Directory.Exists("Packages/com.sidequest.ora"))
{
Directory.Delete("Packages/com.sidequest.ora", true);
}
EditUtils.RemoveCompileDefine("BANTER_ORA", new BuildTargetGroup[] { BuildTargetGroup.Android, BuildTargetGroup.Standalone });
EditUtils.RemoveCompileDefine("BASIS_BUNDLE_MANAGEMENT", new BuildTargetGroup[] { BuildTargetGroup.Android, BuildTargetGroup.Standalone });
EditorApplication.OpenProject(Directory.GetCurrentDirectory());
Comment on lines +50 to +68
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "MenuUtils.cs" | head -20

Repository: SideQuestVR/BanterSDK

Length of output: 104


🏁 Script executed:

find . -type f -name "manifest.json" | head -10

Repository: SideQuestVR/BanterSDK

Length of output: 47


🏁 Script executed:

rg -l "ImportOraPackage\|ImportBasisPackages" --type cs

Repository: SideQuestVR/BanterSDK

Length of output: 47


🏁 Script executed:

cat -n "./Editor/Resources/Builder/MenuUtils.cs" | head -100

Repository: SideQuestVR/BanterSDK

Length of output: 3531


🏁 Script executed:

rg "manifest\.json|ImportOraPackage|ImportBasisPackages|InitialiseOnLoad" --type cs -i

Repository: SideQuestVR/BanterSDK

Length of output: 928


🏁 Script executed:

git ls-files | grep -i manifest

Repository: SideQuestVR/BanterSDK

Length of output: 47


🏁 Script executed:

cat -n "./Editor/SDKInitialiseOnLoad.cs"

Repository: SideQuestVR/BanterSDK

Length of output: 19671


Remove the manifest.json entries for the deleted packages.

InitialiseOnLoad.ImportOraPackage() and ImportBasisPackages() add com.sidequest.ora and the com.basis.* packages (com.basis.bundlemanagement, com.basis.sdk, com.basis.odinserializer) as file: dependencies to Packages/manifest.json. Deleting these directories in UninstallBanter() without removing those manifest entries leaves the reopened project with broken package references pointing to missing local packages.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Editor/Resources/Builder/MenuUtils.cs` around lines 50 - 68,
UninstallBanter() currently deletes the local package folders but leaves file:
entries in Packages/manifest.json, causing broken references; update
UninstallBanter() to load Packages/manifest.json, remove the dependencies for
"com.sidequest.ora" and the "com.basis.bundlemanagement", "com.basis.sdk",
"com.basis.odinserializer" keys (the same packages added by
InitialiseOnLoad.ImportOraPackage() and ImportBasisPackages()), write the
manifest back (preserving JSON formatting), then remove the directories and keep
the existing EditUtils.RemoveCompileDefine calls and
EditorApplication.OpenProject(Directory.GetCurrentDirectory()) so the project is
reopened with a cleaned manifest.

}
#endif
}
}
Loading