Conversation
Moved Unity SDK template files from 'templates/unity/Runtime' and 'templates/unity/Editor' to 'templates/unity/Assets/Runtime' and 'templates/unity/Assets/Editor' for better alignment with Unity project conventions. Updated getFiles() in Unity.php to reflect new paths and added support for copying plugin DLLs and project settings. Improved file upload logic in Client.cs.twig to handle streams and byte arrays more robustly, and removed Unity-specific logging from Exception.cs.twig. Minor fixes in Realtime.cs.twig and Role.cs.twig for namespace and async handling.
Introduces Unity2021 test support by adding a Unity2021Test.php, Unity test source files, and updating the GitHub Actions workflow to include Unity2021 in the test matrix and set the UNITY_LICENSE environment variable. This enables automated testing for the Unity SDK within the CI pipeline.
|
I am also attaching instructions on how to obtain a Unity license painlessly. |
|
The basic functionality tested in the test is guaranteed to work, but I'm not sure about the rest. I will continue to refine it. Also, a question: What to implement next for the client SDK? |
|
The test failed because the secret is not configured in appwrite/sdk-generator. The successful test is here: |
// Cookie support
client.SetCookie() / client.GetCookie()
// Real-time subscriptions
realtime.Subscribe(channels, callback)
// Connection testing
await client.Ping()The successful test is here: |
Remove null-forgiving operator (!) from optional array mappings and use null-safe casting to preserve null vs empty semantics in generated models.
Adds conditional import of the Enums namespace in the Model.cs.twig template only when the model definition contains enum properties. This prevents unnecessary imports and improves template clarity.
Introduces a ToEnumerable extension method to unify array and enumerable conversions in generated .NET code. Updates code generation logic to use ToEnumerable for array properties, simplifying and improving type safety. Also adds necessary using statement for Extensions in generated model files.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (11)
.github/workflows/tests.yml (1)
9-11: Guard Unity test execution when UNITY_LICENSE is unavailable.Forked PRs don't receive repository secrets, so Unity tests will fail when
UNITY_LICENSEis empty. This causes false negatives in CI.Apply a conditional guard to the "Run Tests" step:
- name: Run Tests + if: ${{ matrix.sdk != 'Unity2021' || env.UNITY_LICENSE != '' }} run: | composer test tests/${{ matrix.sdk }}Test.phptemplates/unity/README.md.twig (3)
39-39: Fix broken relative releases link.The relative link
/releasesbreaks when the README is viewed outside the repository (e.g., in the Unity Package Manager or on external sites).Apply this diff:
-1. Download the latest release from [GitHub](/releases) or zip +1. Download the latest release from [GitHub](https://github.com/{{ sdk.gitUserName|url_encode }}/{{ sdk.gitRepoName|url_encode }}/releases) or zip
120-126: Instantiate Realtime before use.The code references an undefined
realtimevariable.Apply this diff:
// Realtime example // You need to create a Realtime instance manually or attach dependently - realtime.Initialize(client); + var realtime = new Realtime(); + realtime.Initialize(client); var subscription = realtime.Subscribe(
133-133: Fix syntax error in code sample.The double dot
client..Async()is invalid C# syntax.Apply this diff:
- var result = await client..Async(); + var result = await client.Ping();templates/unity/Assets/Runtime/Realtime.cs.twig (5)
121-123: WebGL message dispatch guard may be inverted.The preprocessor guard prevents
DispatchMessageQueuefrom running on WebGL runtime builds. A past review indicated this is incorrect and events won't dispatch properly on WebGL.Apply this diff if the guard is indeed inverted:
- #if !UNITY_WEBGL || UNITY_EDITOR + #if UNITY_WEBGL && !UNITY_EDITOR _webSocket?.DispatchMessageQueue(); #endif
146-146: Race condition: concurrent Subscribe calls can trigger multiple socket creations.Calling
CreateSocket().Forget()without awaiting allows concurrentSubscribecalls to bypass the_creatingSocketflag check, which occurs after the async boundary.Consider making
Subscribeasync and awaiting the socket creation:- public RealtimeSubscription Subscribe(string[] channels, Action<RealtimeResponseEvent<Dictionary<string, object>>> callback) + public async UniTask<RealtimeSubscription> Subscribe(string[] channels, Action<RealtimeResponseEvent<Dictionary<string, object>>> callback) { // ... existing code ... - CreateSocket().Forget(); + await CreateSocket(); return subscription; }Update all call sites to await the new async signature.
289-304: Add null guard for _client before accessing Config.
SendFallbackAuthenticationdereferences_client.Configwithout checking if_clientis null, risking aNullReferenceException.Apply this diff:
private void SendFallbackAuthentication() { + if (_client == null) + { + Debug.LogError("[Realtime] Cannot send fallback authentication: client is null"); + return; + } + var session = _client.Config.GetValueOrDefault("session");
311-329: Marshal user callbacks to Unity main thread.On non-WebGL platforms,
NativeWebSocketinvokesOnMessageon a background thread. Invoking user callbacks directly from this thread can break Unity APIs (which require main thread) and cause data races.Wrap callback invocations to post to the main thread:
var subscriptionsCopy = _subscriptions.Values.ToArray(); + UniTask.Post(() => + { foreach (var subscription in subscriptionsCopy) { if (subscription.Channels.Any(subChannel => eventData.Channels.Contains(subChannel))) { subscription.OnMessage?.Invoke(eventData); } } + });
460-462: Avoid async void in OnDestroy—Unity won't wait for completion.Using async operations in
OnDestroyis problematic because Unity doesn't wait for them to complete before destroying the object, leading to incomplete cleanup.Replace with synchronous teardown:
- private void OnDestroy() + private void OnDestroy() { - Disconnect().Forget(); + _reconnect = false; + StopHeartbeat(); + _cancellationTokenSource?.Cancel(); + + if (_webSocket != null && _webSocket.State != WebSocketState.Closed) + { + Debug.LogWarning("[Realtime] WebSocket connection not properly closed before destruction"); + _webSocket.Close(); + } }templates/unity/Assets/Runtime/Core/CookieContainer.cs.twig (2)
35-41: Refine domain matching to handle edge cases securely.The domain matching logic should validate domain structure and ensure subdomain matching is secure, particularly for domains starting with a dot.
Apply this diff:
public bool MatchesDomain(string requestDomain) { if (string.IsNullOrEmpty(Domain)) return true; var d = Domain.ToLowerInvariant(); var r = requestDomain.ToLowerInvariant(); - return r == d || r.EndsWith("." + d) || (d.StartsWith(".") && r.EndsWith(d)); + // Exact match or subdomain match + if (r == d) return true; + // For domain cookies (starting with .), check if request domain is a subdomain + if (d.StartsWith(".")) return r == d.Substring(1) || r.EndsWith(d); + // For non-dot domains, check if request is a subdomain + return r.EndsWith("." + d); }
135-136: Cookie parsing incorrectly splits on commas.Splitting Set-Cookie headers on commas breaks cookies with comma-containing values (e.g., date formats). HTTP Set-Cookie headers should be processed individually, not split.
Apply this diff:
public void ParseSetCookieHeader(string setCookieHeader, string domain) { if (string.IsNullOrWhiteSpace(setCookieHeader) || string.IsNullOrWhiteSpace(domain)) return; - foreach (var c in setCookieHeader.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries)) - ParseCookie(c.Trim(), domain); + // Set-Cookie headers should be processed as a single cookie + // Multiple cookies come as multiple Set-Cookie headers, not comma-separated + ParseCookie(setCookieHeader.Trim(), domain); }Ensure callers iterate actual Set-Cookie header entries when multiple cookies are present.
🧹 Nitpick comments (1)
example.php (1)
121-144: Wrap Unity generation in conditional to match other SDK patterns.Unlike other SDKs, the Unity generation block always executes regardless of the
$requestedSdkargument. This causes unnecessary generation when users request a specific SDK.Apply this diff to match the pattern used by other SDKs:
- - // Unity - $sdk = new SDK(new Unity(), new Swagger2($spec)); - - $sdk - ->setName('NAME') - ->setDescription('Repo description goes here') - ->setShortDescription('Repo short description goes here') - ->setURL('https://example.com') - ->setLogo('https://appwrite.io/v1/images/console.png') - ->setLicenseContent('test test test') - ->setWarning('**WORK IN PROGRESS - NOT READY FOR USAGE**') - ->setChangelog('**CHANGELOG**') - ->setVersion('0.0.1') - ->setGitUserName('repoowner') - ->setGitRepoName('reponame') - ->setTwitter('appwrite_io') - ->setDiscord('564160730845151244', 'https://appwrite.io/discord') - ->setDefaultHeaders([ - 'X-Appwrite-Response-Format' => '1.6.0', - ]) - ; - - $sdk->generate(__DIR__ . '/examples/unity'); + // Unity + if (!$requestedSdk || $requestedSdk === 'unity') { + $sdk = new SDK(new Unity(), new Swagger2($spec)); + configureSDK($sdk); + $sdk->generate(__DIR__ . '/examples/unity'); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/tests.yml(2 hunks)example.php(2 hunks)templates/unity/Assets/Runtime/Core/CookieContainer.cs.twig(1 hunks)templates/unity/Assets/Runtime/Realtime.cs.twig(1 hunks)templates/unity/README.md.twig(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
example.php (1)
src/SDK/Language/Unity.php (1)
Unity(5-409)
Replaces reflection-based service initialization in AppwriteManager with direct instantiation for better performance and AOT compatibility. Adds thread safety to CookieContainer with locking, improves cookie loading/saving, and clarifies platform-specific behavior. Enhances Realtime to support client/session updates and controlled reconnect logic. Minor fixes include exception type consistency and improved debug messages in OAuth and other templates. Adds Operator.cs to Unity SDK file list.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (10)
templates/unity/Assets/Runtime/AppwriteManager.cs.twig (2)
72-78: Start() init pattern looks good (no async-void).
247-269: Make Shutdown async and await realtime disconnect before destroying GameObject.Current flow can destroy the Realtime GameObject while
Disconnect()is still in-flight.- public async UniTask<bool> Reinitialize({{ spec.title | caseUcfirst }}Config newConfig = null, bool needRealtime = false) + public async UniTask<bool> Reinitialize({{ spec.title | caseUcfirst }}Config newConfig = null, bool needRealtime = false) { config = newConfig ?? config; - Shutdown(); + await Shutdown(); return await Initialize(needRealtime); } - private void Shutdown() + private async UniTask Shutdown() { if (!ReferenceEquals(_realtime, null)) { - _realtime.Disconnect().Forget(); + try { await _realtime.Disconnect(); } + catch (Exception ex) { Debug.LogError($"Error disconnecting realtime: {ex}"); } if (_realtime.gameObject != null) Destroy(_realtime.gameObject); } _realtime = null; _client = null; _isInitialized = false; _services.Clear(); OnClientDestroyed?.Invoke(); Debug.Log("{{ spec.title | caseUcfirst }} client shutdown"); }templates/unity/Assets/Runtime/Realtime.cs.twig (4)
147-155: WebGL DispatchMessageQueue guard is inverted (messages won’t dispatch on WebGL builds).
DispatchMessageQueue()needs to run on WebGL runtime builds; current preprocessor condition skips it.- #if !UNITY_WEBGL || UNITY_EDITOR - _webSocket?.DispatchMessageQueue(); - #endif + #if UNITY_WEBGL && !UNITY_EDITOR + _webSocket?.DispatchMessageQueue(); + #endif
276-310: Marshal message handling/user callbacks to Unity main thread (NativeWebSocket may invoke on background thread).Right now
HandleRealtimeEvent()invokessubscription.OnMessageinline. If this runs off-main-thread, it’s unsafe for Unity APIs and also races_subscriptions.A minimal approach is to
UniTask.Postthe dispatch:private void HandleRealtimeEvent(RealtimeResponseEvent<Dictionary<string, object>> eventData) { try { var subscriptionsCopy = _subscriptions.Values.ToArray(); - foreach (var subscription in subscriptionsCopy) - { - if (subscription.Channels.Any(subChannel => eventData.Channels.Contains(subChannel))) - { - subscription.OnMessage?.Invoke(eventData); - } - } + UniTask.Post(() => + { + foreach (var subscription in subscriptionsCopy) + { + if (subscription.Channels.Any(subChannel => eventData.Channels.Contains(subChannel))) + { + subscription.OnMessage?.Invoke(eventData); + } + } + }); } catch (Exception ex) { Debug.LogError($"[Realtime] HandleRealtimeEvent error: {ex.Message}"); OnError?.Invoke(ex); } }Also applies to: 345-363
323-338: Add defensive null-guards + don’t drop SendText failures.
SendFallbackAuthentication()assumes_clientand_client.Configexist, and it calls_webSocket.SendText(json)without awaiting/handling errors (depending on NativeWebSocket signature).private void SendFallbackAuthentication() { - var session = _client.Config.GetValueOrDefault("session"); + if (_client?.Config == null || _webSocket == null) + { + Debug.LogError("[Realtime] Cannot send fallback authentication: client/socket not ready"); + return; + } + + var session = _client.Config.GetValueOrDefault("session"); if (!string.IsNullOrEmpty(session)) { var authMessage = new RealtimeMessage<RealtimeAuthData> { Type = "authentication", Data = new RealtimeAuthData { Session = session } }; var json = JsonSerializer.Serialize(authMessage, Client.SerializerOptions); - _webSocket.SendText(json); + _ = _webSocket.SendText(json); // verify if this returns Task and decide whether to await/Forget } }NativeWebSocket (Unity) API: what are the return types/behavior of WebSocket.SendText(...) and WebSocket.Close(...), and on which thread does OnMessage fire for non-WebGL platforms?
510-513: Avoid fire-and-forget async cleanup in OnDestroy (Unity won’t wait).
Disconnect().Forget()can leave the socket open during teardown and can race with pending retries/heartbeat. Prefer a synchronous teardown (cancel tokens, stop heartbeat, best-effort close) or move async disconnect to an explicit shutdown path that callers can await.templates/unity/Assets/Runtime/Core/CookieContainer.cs.twig (2)
144-149: Don’t split Set-Cookie on commas (breaks Expires and multi-cookie handling).Treat a single header value as one cookie; callers should pass individual Set-Cookie header entries (or split on newline, not comma, if Unity concatenates).
public void ParseSetCookieHeader(string setCookieHeader, string domain) { if (string.IsNullOrWhiteSpace(setCookieHeader) || string.IsNullOrWhiteSpace(domain)) return; - foreach (var c in setCookieHeader.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries)) - ParseCookie(c.Trim(), domain); + ParseCookie(setCookieHeader.Trim(), domain); }
35-41: Harden domain matching (dot-domain semantics + subdomain safety).public bool MatchesDomain(string requestDomain) { if (string.IsNullOrEmpty(Domain)) return true; var d = Domain.ToLowerInvariant(); var r = requestDomain.ToLowerInvariant(); - return r == d || r.EndsWith("." + d) || (d.StartsWith(".") && r.EndsWith(d)); + if (r == d) return true; + if (d.StartsWith(".")) return r == d.Substring(1) || r.EndsWith(d); + return r.EndsWith("." + d); }templates/unity/Assets/Runtime/Core/Client.cs.twig (2)
460-469: Parse all Set-Cookie headers (Unity may return multiple / concatenated values).At minimum, use
GetResponseHeaders()and handle case-insensitive “Set-Cookie” keys; then pass each cookie string toCookieContainer.ParseSetCookieHeader()(after updating it to not comma-split).#if !(UNITY_WEBGL && !UNITY_EDITOR) - var setCookieHeader = request.GetResponseHeader("Set-Cookie"); - if (!string.IsNullOrEmpty(setCookieHeader)) - { - var uri = new Uri(request.url); - _cookieContainer.ParseSetCookieHeader(setCookieHeader, uri.Host); - } + var responseHeaders = request.GetResponseHeaders(); + if (responseHeaders != null) + { + var uri = new Uri(request.url); + foreach (var kvp in responseHeaders) + { + if (kvp.Key.Equals("Set-Cookie", StringComparison.OrdinalIgnoreCase) && + !string.IsNullOrWhiteSpace(kvp.Value)) + { + // If Unity concatenates, it's more likely newline-separated than comma-safe. + foreach (var line in kvp.Value.Split(new[] { '\n' }, StringSplitOptions.RemoveEmptyEntries)) + { + _cookieContainer.ParseSetCookieHeader(line.Trim(), uri.Host); + } + } + } + } #endif
646-709: ChunkedUpload corrupts the last chunk for stream/path sources (ignores bytesRead) + off-by-one for bytes source.For stream/path, you always send
buffer(full ChunkSize) even whenReadAsyncreturns fewer bytes; tail bytes are from previous chunk.while (offset < size) { + byte[] chunk; switch(input.SourceType) { case "path": case "stream": var stream = input.Data as Stream; if (stream == null) throw new InvalidOperationException("Stream data is null"); stream.Seek(offset, SeekOrigin.Begin); - await stream.ReadAsync(buffer, 0, ChunkSize); + var toRead = (int)Math.Min(ChunkSize, size - offset); + var bytesRead = await stream.ReadAsync(buffer, 0, toRead); + if (bytesRead <= 0) throw new EndOfStreamException("Unexpected end of stream during chunked upload"); + chunk = new byte[bytesRead]; + Buffer.BlockCopy(buffer, 0, chunk, 0, bytesRead); break; case "bytes": - buffer = ((byte[])input.Data) + chunk = ((byte[])input.Data) .Skip((int)offset) - .Take((int)Math.Min(size - offset, ChunkSize - 1)) + .Take((int)Math.Min(size - offset, ChunkSize)) .ToArray(); break; } var chunkHeaders = new Dictionary<string, string>(headers) { ["Content-Type"] = "multipart/form-data", - ["Content-Range"] = $"bytes {offset}-{Math.Min(offset + ChunkSize - 1, size - 1)}/{size}" + ["Content-Range"] = $"bytes {offset}-{Math.Min(offset + chunk.Length - 1, size - 1)}/{size}" }; ... - chunkParameters[paramName] = new InputFile + chunkParameters[paramName] = new InputFile { - Data = buffer, + Data = chunk, Filename = input.Filename, MimeType = input.MimeType, SourceType = "bytes" }; ... - offset += ChunkSize; + offset += chunk.Length;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
example.php(2 hunks)src/SDK/Language/Unity.php(1 hunks)templates/unity/Assets/Runtime/AppwriteManager.cs.twig(1 hunks)templates/unity/Assets/Runtime/Core/Client.cs.twig(1 hunks)templates/unity/Assets/Runtime/Core/CookieContainer.cs.twig(1 hunks)templates/unity/Assets/Runtime/Realtime.cs.twig(1 hunks)templates/unity/base/requests/oauth.twig(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- example.php
- templates/unity/base/requests/oauth.twig
- src/SDK/Language/Unity.php
Refactored AppwriteManager to use a service definitions dictionary for cleaner and more maintainable service initialization. Renamed AcceptAllCertificatesSignedWithASpecificKeyPublicKey to AcceptAllCertificatesHandler for clarity. Enhanced Realtime reconnect logic to properly handle cancellation tokens, ensuring pending retry operations are cancelled and disposed when disconnecting.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (9)
templates/unity/Assets/Runtime/Realtime.cs.twig (4)
147-155: WebGL message pump preprocessor guard is inverted.The
#if !UNITY_WEBGL || UNITY_EDITORcondition causesDispatchMessageQueueto run on non-WebGL platforms and in Editor, but NativeWebSocket requires manual dispatching only on WebGL builds. On native platforms, messages are already dispatched on background threads.The guard should be inverted to dispatch on WebGL runtime only:
- #if !UNITY_WEBGL || UNITY_EDITOR - _webSocket?.DispatchMessageQueue(); - #endif + #if UNITY_WEBGL && !UNITY_EDITOR + _webSocket?.DispatchMessageQueue(); + #endif
323-338: Add defensive null check for_client.While
_clientshould be initialized viaInitialize()before any WebSocket operations, a defensive guard would prevent NullReferenceException if the component lifecycle is disrupted.private void SendFallbackAuthentication() { + if (_client == null) return; + var session = _client.Config.GetValueOrDefault("session");
345-363: User callbacks may execute on background thread.On non-WebGL platforms, NativeWebSocket's
OnMessagefires on a background thread. Invoking user callbacks directly can break Unity API calls and cause race conditions.Consider marshaling to the main thread:
private void HandleRealtimeEvent(RealtimeResponseEvent<Dictionary<string, object>> eventData) { try { var subscriptionsCopy = _subscriptions.Values.ToArray(); + // Post to main thread to ensure Unity API safety + UniTask.Post(() => + { foreach (var subscription in subscriptionsCopy) { if (subscription.Channels.Any(subChannel => eventData.Channels.Contains(subChannel))) { subscription.OnMessage?.Invoke(eventData); } } + }); }
540-543: Fire-and-forget in OnDestroy may cause incomplete cleanup.Unity doesn't wait for async operations in
OnDestroy. TheDisconnect().Forget()call may not complete before the object is destroyed, potentially leaving WebSocket connections open.Consider synchronous cleanup:
private void OnDestroy() { - Disconnect().Forget(); + // Perform synchronous cleanup since Unity won't wait for async + _reconnect = false; + StopHeartbeat(); + _cancellationTokenSource?.Cancel(); + _cancellationTokenSource?.Dispose(); + + if (_webSocket != null && _webSocket.State != WebSocketState.Closed) + { + _webSocket.Close(); + } }templates/unity/Assets/Runtime/AppwriteManager.cs.twig (1)
244-259: Fire-and-forget disconnect may leave connections open.The
Disconnect().Forget()call may not complete before the GameObject is destroyed. Given thatShutdownis called fromOnDestroy, async won't help here.If
Realtime.OnDestroyperforms synchronous cleanup (as suggested), this becomes less of a concern. Alternatively, expose a synchronousDisconnectImmediate()method on Realtime:- _realtime.Disconnect().Forget(); + _realtime.DisconnectImmediate(); // Synchronous cleanuptemplates/unity/Assets/Runtime/Core/Client.cs.twig (4)
261-262: Content-Type header lookup is case-sensitive.
headers.TryGetValue("Content-Type", ...)won't match"content-type"keys. While internal headers use lowercase, external callers might pass different casing.- var isMultipart = headers.TryGetValue("Content-Type", out var contentType) && - "multipart/form-data".Equals(contentType, StringComparison.OrdinalIgnoreCase); + var isMultipart = headers.Any(h => + h.Key.Equals("Content-Type", StringComparison.OrdinalIgnoreCase) && + h.Value != null && + h.Value.StartsWith("multipart/form-data", StringComparison.OrdinalIgnoreCase));
461-469: May miss multiple Set-Cookie headers.
GetResponseHeader("Set-Cookie")typically returns only the first header. Consider usingGetResponseHeaders()to capture all cookies:#if !(UNITY_WEBGL && !UNITY_EDITOR) - var setCookieHeader = request.GetResponseHeader("Set-Cookie"); - if (!string.IsNullOrEmpty(setCookieHeader)) - { - var uri = new Uri(request.url); - _cookieContainer.ParseSetCookieHeader(setCookieHeader, uri.Host); - } + var responseHeaders = request.GetResponseHeaders(); + if (responseHeaders != null && responseHeaders.TryGetValue("Set-Cookie", out var setCookieHeader)) + { + var uri = new Uri(request.url); + _cookieContainer.ParseSetCookieHeader(setCookieHeader, uri.Host); + } #endif
658-664: Off-by-one error in chunk size calculation.
ChunkSize - 1causes each chunk to be one byte smaller than intended, potentially causing data corruption over multiple chunks.case "bytes": buffer = ((byte[])input.Data) .Skip((int)offset) - .Take((int)Math.Min(size - offset, ChunkSize - 1)) + .Take((int)Math.Min(size - offset, ChunkSize)) .ToArray(); break;
721-728: Certificate handler name is misleading.
AcceptAllCertificatesHandleraccepts all certificates unconditionally, but the previous class name implied specific key validation. The current name is more accurate. However, this is still security-sensitive and should only be used for development.Consider adding a warning:
// Custom certificate handler for self-signed certificates + // WARNING: This bypasses certificate validation entirely. Use only for development/testing. public class AcceptAllCertificatesHandler : CertificateHandler { protected override bool ValidateCertificate(byte[] certificateData) { - return true; // Accept all certificates + Debug.LogWarning("[Security] Certificate validation bypassed - do not use in production!"); + return true; } }
🧹 Nitpick comments (2)
templates/unity/Assets/Runtime/Realtime.cs.twig (1)
157-182: Subscribe is synchronous but socket creation is fire-and-forget.While the
_creatingSocketflag prevents concurrent socket creation, callers cannot know when the connection is established. Consider returning aUniTask<RealtimeSubscription>or providing anIsConnectingstate so callers can await readiness.If synchronous return is intentional for simplicity, this is acceptable. Otherwise, consider:
public async UniTask<RealtimeSubscription> Subscribe(string[] channels, Action<RealtimeResponseEvent<Dictionary<string, object>>> callback) { // ... setup code ... await CreateSocket(); return subscription; }templates/unity/Assets/Runtime/AppwriteManager.cs.twig (1)
72-78: Consider logging initialization failures from Start.Using
Initialize().Forget()is the correct UniTask pattern, but failures will be silently swallowed. Consider wrapping in an error handler:private void Start() { if (initializeOnStart) { - Initialize().Forget(); + InitializeWithLogging().Forget(); } } + +private async UniTaskVoid InitializeWithLogging() +{ + var success = await Initialize(); + if (!success) + { + Debug.LogError("{{ spec.title | caseUcfirst }}Manager failed to initialize on Start"); + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
templates/unity/Assets/Runtime/AppwriteManager.cs.twig(1 hunks)templates/unity/Assets/Runtime/Core/Client.cs.twig(1 hunks)templates/unity/Assets/Runtime/Realtime.cs.twig(1 hunks)
🔇 Additional comments (10)
templates/unity/Assets/Runtime/Realtime.cs.twig (3)
184-208: LGTM!The subscription cleanup logic correctly handles channel reference counting and appropriately closes the connection when no subscriptions remain.
210-258: LGTM!The socket creation logic properly guards against concurrent creation, handles URL changes, and includes appropriate error handling with retry.
423-461: LGTM!The retry logic now properly checks
_reconnectafter the delay and uses cancellation tokens to cancel pending retries when disconnecting, addressing the previous race condition concern.templates/unity/Assets/Runtime/AppwriteManager.cs.twig (3)
57-70: LGTM!The singleton pattern implementation correctly handles duplicate instances with appropriate warnings and cleanup.
80-126: LGTM!The initialization flow has proper validation, error handling, and success indication via return value.
128-156: LGTM!The factory-based service creation pattern avoids the IL2CPP reflection issues previously flagged with
Activator.CreateInstance.templates/unity/Assets/Runtime/Core/Client.cs.twig (4)
36-59: LGTM!The JSON serializer configuration with camelCase naming, null handling, and custom converters follows best practices.
61-87: LGTM!The constructor properly initializes all fields and loads persisted session data with appropriate error handling.
377-432: LGTM!The redirect method correctly disables auto-redirect and returns the Location header for OAuth flow handling.
443-535: LGTM!The
Call<T>method handles both JSON and binary responses appropriately with proper error extraction and optional type conversion.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/Unity2021Test.php(1 hunks)tests/languages/unity/Tests.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/languages/unity/Tests.cs
🔇 Additional comments (3)
tests/Unity2021Test.php (3)
1-13: LGTM! Configuration is consistent and well-structured.The Unity SDK metadata and language provider configuration are set up correctly.
14-18: Build steps look reasonable.The test asset preparation correctly copies Unity test files into the generated SDK structure.
30-45: LGTM! Comprehensive test coverage.The expected output array appropriately merges all response types from the base test class, ensuring comprehensive validation of Unity SDK functionality including HTTP operations, uploads/downloads, enums, exceptions, realtime, cookies, and helper utilities.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/languages/unity/Tests.cs`:
- Around line 84-116: The tests currently only call
foo.Get/foo.Post/foo.Put/foo.Patch/foo.Delete and bar.Get/... and use
Debug.Log(mock.Result) without verifying outcomes; update each invocation (e.g.,
calls to foo.Get, bar.Post, etc.) to include assertions that the returned Mock
is not null and that mock.Result equals the expected value for that call (use
Assert.IsNotNull(mock) and Assert.AreEqual(expectedValue, mock.Result) or
similar), and add any necessary expected values or helper variables for clarity
so each HTTP method test actually validates correctness.
♻️ Duplicate comments (5)
tests/languages/unity/Tests.cs (1)
127-133: Dispose file streams to avoid handle leaks.The
FileInfo.OpenRead()calls create streams that are never disposed. This was flagged in a previous review.tests/Unity2021Test.php (2)
20-20: Refactor the Unity test command and verify the editor image is still supportedThe inline docker one-liner is hard to maintain and masks license-activation failures. Consider moving this to a script and updating the Unity editor image to a currently supported LTS tag.
What is the current supported Unity LTS version and the recommended unityci/editor Ubuntu tag for headless CI?
24-25: Scope/document UNITY_TEST_MODE exclusionsUsing a global flag to exclude “problematic files” makes test behavior opaque. Prefer a scoped config or explicitly list the excluded files with rationale.
#!/bin/bash # Locate all UNITY_TEST_MODE usage to see what gets excluded and why rg -n "UNITY_TEST_MODE" -C2templates/unity/Assets/Runtime/Realtime.cs.twig (2)
585-588: Avoid async teardown inOnDestroyUnity doesn’t wait for async work in
OnDestroy, which can leave sockets open. Consider synchronous cleanup or move async shutdown toOnDisable/ an explicit teardown method.
147-154: WebGL message pump guard is inverted
DispatchMessageQueueshould run on WebGL builds (not in the Editor). The current guard skips WebGL, so realtime events won’t dispatch there.🐛 Proposed fix
- `#if` !UNITY_WEBGL || UNITY_EDITOR - _webSocket?.DispatchMessageQueue(); - `#endif` + `#if` UNITY_WEBGL && !UNITY_EDITOR + _webSocket?.DispatchMessageQueue(); + `#endif`
🧹 Nitpick comments (5)
tests/languages/unity/Tests.cs (4)
156-174: Error tests should assert expected exception details.The error handling tests catch
AppwriteExceptionbut only log the message. They should assert the expected error code or message to ensure the SDK correctly surfaces error information.💡 Suggested improvement
try { await general.Error400(); + Assert.Fail("Expected AppwriteException was not thrown"); } catch (AppwriteException e) { Debug.Log(e.Message); - Debug.Log(e.Response); + Assert.AreEqual(400, e.Code); + Assert.IsNotNull(e.Response); }
200-206: Cookie roundtrip should assert the retrieved value matches.The cookie tests set and get cookies but don't verify the retrieved cookie matches what was set. This is a critical path for OAuth session persistence mentioned in the PR objectives.
mock = await general.SetCookie(); Assert.AreEqual("success", mock.Result); mock = await general.GetCookie(); Assert.AreEqual("expected_cookie_value", mock.Result);
207-231: Query helper tests should assert generated output format.All query helper calls log output without verifying correctness. Consider adding at least a few representative assertions to catch regressions in query string generation:
Assert.AreEqual("{\"method\":\"equal\",\"attribute\":\"title\",\"values\":[\"Spiderman\",\"Dr. Strange\"]}", Query.Equal("title", new List<string> { "Spiderman", "Dr. Strange" })); Assert.AreEqual("{\"method\":\"limit\",\"values\":[50]}", Query.Limit(50));
24-26: Consider a more descriptive test name or splitting into focused tests.The test method
Test1covers many distinct features (HTTP methods, file uploads, realtime, cookies, query helpers, etc.). Consider:
- Renaming to something descriptive like
TestAppwriteClientIntegration- Splitting into separate test methods per feature area for easier failure diagnosis
This is a minor suggestion since integration tests often bundle operations, but it improves debuggability.
templates/dotnet/Package/Converters/ObjectToInferredTypesConverter.cs.twig (1)
45-51: Preserve numeric precision with a decimal fallback.
Falling back todoublecan lose precision for large integers or decimal values. If precision matters, preferTryGetDecimalbeforeGetDouble.♻️ Suggested tweak
case JsonValueKind.Number: if (element.TryGetInt64(out long l)) { return l; } + if (element.TryGetDecimal(out decimal d)) + { + return d; + } return element.GetDouble();Please verify
JsonElement.TryGetDecimalavailability/behavior in your target Unity/.NET runtime to avoid compatibility issues.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@templates/unity/Assets/Runtime/Core/Services/ServiceTemplate.cs.twig`:
- Around line 28-33: The template emits a closing </summary> without ever
outputting an opening <summary>, causing CS1570; update ServiceTemplate.cs.twig
so the entire summary block (the opening <summary>, the description lines that
use method.description | dotnetComment, and the closing </summary>) is emitted
only when method.description is present — i.e., move or add the opening
<summary> inside the {% if method.description %} block and ensure the
corresponding </summary> remains inside that same conditional to keep the XML
well-formed.
- Around line 13-16: The template currently imports System.Web under a
conditional that still targets player platforms; remove System.Web from the
using block in ServiceTemplate.cs.twig and update the code paths that call
HttpUtility.ParseQueryString (notably in WebAuthComponent.cs.twig and
oauth.twig) to use a platform-safe parser: either limit System.Web usage to `#if`
UNITY_EDITOR only, or replace HttpUtility.ParseQueryString calls with a .NET
Standard approach that parses Uri.Query into a NameValueCollection (or a small
custom parser that splits on '?'/'&' and decodes keys/values) so builds on
WebGL/iOS/Android succeed; update references to HttpUtility.ParseQueryString in
the mentioned templates to call the new parser or guard them with UNITY_EDITOR.
♻️ Duplicate comments (1)
templates/unity/Assets/Runtime/Core/Services/ServiceTemplate.cs.twig (1)
41-75: Return a value fromwebAuthmethods (currentlyUniTaskvoid)
webAuthis emitted asUniTask(void) and the unsupported-platform branch returnsUniTask.CompletedTask, which drops any auth URL/session value. Other SDKs return a string for webAuth. Consider returningUniTask<string>and a typed fallback.✅ Suggested template fix
- public {% if method.type == "webAuth" %}async {% endif ~%} UniTask{% if method.type == "webAuth" %}{% else %}<{{ utils.resultType(spec.title, method) }}>{% endif %} {{ method.name | caseUcfirst }}({{ utils.method_parameters(method.parameters, method.consumes) }}) + public {% if method.type == "webAuth" %}async {% endif ~%} UniTask{% if method.type == "webAuth" %}<string>{% else %}<{{ utils.resultType(spec.title, method) }}>{% endif %} {{ method.name | caseUcfirst }}({{ utils.method_parameters(method.parameters, method.consumes) }}) ... - public UniTask {{ method.name | caseUcfirst }}({{ utils.method_parameters(method.parameters, method.consumes) }}) + public UniTask<string> {{ method.name | caseUcfirst }}({{ utils.method_parameters(method.parameters, method.consumes) }}) { Debug.LogWarning("[{{ spec.title | caseUcfirst }}] OAuth2 authorization is not supported on this platform. Available only in Editor, WebGL, iOS or Android."); - return UniTask.CompletedTask; + return UniTask.FromResult<string>(null); }
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@templates/unity/Assets/Runtime/Core/Services/ServiceTemplate.cs.twig`:
- Around line 41-44: The webAuth methods currently use a void UniTask signature
causing SDK divergence; update the generated method signature logic so when
method.type == "webAuth" the return type is UniTask<string> (use
utils.resultType(spec.title, method) or explicitly string for webAuth) and
ensure the implementation in the generated method (the code that extracts the
auth secret/key from the callback) returns the auth URL/token (the value you
extract) instead of discarding it so callers receive the string result; adjust
any async/await and return paths in the method named via {{ method.name |
caseUcfirst }} accordingly.
In `@templates/unity/Assets/Runtime/Core/WebAuthComponent.cs.twig`:
- Around line 27-28: The debug log prints raw deep link/auth URLs which may
contain sensitive query parameters; create and call a helper like
RedactSensitiveQueryParams(string url) from WebAuthComponent (used where
Debug.Log currently logs the URL and where auth URL is logged at the other
occurrence) to produce a redactedUrl for logging while keeping the original url
passed to OnDeepLink.Invoke. The helper should parse the URI, identify common
sensitive keys (e.g., access_token, id_token, refresh_token, token, code,
secret, api_key, auth) and replace their values with a fixed mask (e.g.,
"[REDACTED]") or remove them, then return the reconstructed URL for Debug.Log;
update both Debug.Log calls to log the redactedUrl instead of the raw url.
♻️ Duplicate comments (3)
templates/unity/Assets/Runtime/Core/WebAuthComponent.cs.twig (2)
30-33: Static event subscription lacks editor cleanup for domain reloadsThe static constructor subscribes
HandleCallbacktoOnDeepLink, but in Editor domain reloads, this can lead to duplicate subscriptions or stale references. Consider adding editor-specific cleanup.♻️ Proposed fix for editor cleanup
static WebAuthComponent() { + OnDeepLink -= HandleCallback; // Prevent duplicate subscriptions on domain reload OnDeepLink += HandleCallback; } + +#if UNITY_EDITOR + [UnityEditor.InitializeOnLoadMethod] + private static void EditorCleanup() + { + UnityEditor.AssemblyReloadEvents.beforeAssemblyReload += () => + { + OnDeepLink -= HandleCallback; + }; + } +#endif
61-68: Consider adding authentication timeout to prevent indefinite hangsIf the user abandons the OAuth flow (closes browser, loses network), the
await tcs.Taskwill hang indefinitely. A timeout would allow the application to recover gracefully.♻️ Proposed timeout implementation
+ using var timeoutCts = new System.Threading.CancellationTokenSource(); + timeoutCts.CancelAfter(TimeSpan.FromMinutes(5)); + + timeoutCts.Token.Register(() => + { + if (PendingAuth.TryRemove(callbackScheme, out var timedOutTcs)) + { + timedOutTcs.TrySetException(new TimeoutException("OAuth authentication timed out after 5 minutes.")); + } + }); + try { return await tcs.Task; }templates/unity/base/requests/oauth.twig (1)
20-22: Validate callbackDomain to prevent cookie scope hijackingThe
callbackDomainis taken directly from the callback query parameter without validation. A malicious OAuth callback could specify an arbitrary domain, potentially affecting cookie scope in unintended ways.Restrict accepted domains to the endpoint host or its subdomains:
🔒 Proposed fix for domain validation
// Use domain from callback if available, otherwise fallback to endpoint host - var domain = !string.IsNullOrEmpty(callbackDomain) ? callbackDomain : new Uri(_client.Endpoint).Host; + var endpointHost = new Uri(_client.Endpoint).Host; + var domain = endpointHost; + if (!string.IsNullOrEmpty(callbackDomain)) + { + var normalizedCallback = callbackDomain.TrimStart('.'); + if (string.Equals(normalizedCallback, endpointHost, StringComparison.OrdinalIgnoreCase) || + normalizedCallback.EndsWith("." + endpointHost, StringComparison.OrdinalIgnoreCase)) + { + domain = normalizedCallback; + } + } var parsedDomain = domain.StartsWith(".") ? domain.Substring(1) : domain;
🧹 Nitpick comments (3)
templates/unity/base/requests/oauth.twig (1)
27-31: Skip cookie persistence in Editor since it's documented as non-functionalThe warning states cookies cannot be obtained in Editor, yet
ParseSetCookieHeaderis still called. This is misleading and may cause confusion during development.♻️ Proposed refactor to gate cookie write
- _client.CookieContainer.ParseSetCookieHeader(setCookieHeader, parsedDomain); - -#if UNITY_EDITOR +#if UNITY_EDITOR Debug.LogWarning("[{{ spec.title | caseUcfirst }}] OAuth authorization in Editor: you can open and authorize, but cookies cannot be obtained. The session will not be set."); +#else + _client.CookieContainer.ParseSetCookieHeader(setCookieHeader, parsedDomain); `#endif`templates/dotnet/Package/Extensions/Extensions.cs.twig (2)
16-24: Guard against unexpectedstring/JsonElementinputs inToEnumerable.
stringimplementsIEnumerable, so this will currently yield characters, which is usually surprising for list semantics. Also, anyJsonElementarrays will now throw. Please verify call sites and consider explicit handling.♻️ Suggested adjustment
public static IEnumerable<object> ToEnumerable(this object value) { return value switch { + string s => new object[] { s }, object[] array => array, IEnumerable<object> enumerable => enumerable, IEnumerable nonGeneric => nonGeneric.Cast<object>(), _ => throw new InvalidCastException($"Cannot convert {value?.GetType().Name ?? "null"} to IEnumerable<object>") }; }
52-69: Use URL-decoding that handles+and skip empty pairs.If the input is form-urlencoded (common for OAuth/query strings),
+should decode to space. Also, removing empty entries avoids inserting empty keys for trailing&.♻️ Suggested adjustment
- if (string.IsNullOrEmpty(query)) return dict; - if (query.StartsWith("?")) query = query.Substring(1); - var pairs = query.Split('&'); + if (string.IsNullOrEmpty(query)) return dict; + if (query.StartsWith("?")) query = query.Substring(1); + var pairs = query.Split('&', StringSplitOptions.RemoveEmptyEntries); foreach (var pair in pairs) { var kv = pair.Split(new char[] { '=' }, 2); if (kv.Length >= 1) { - var key = Uri.UnescapeDataString(kv[0]); - var value = kv.Length > 1 ? Uri.UnescapeDataString(kv[1]) : ""; + var key = WebUtility.UrlDecode(kv[0]); + var value = kv.Length > 1 ? WebUtility.UrlDecode(kv[1]) : ""; dict[key] = value; } }If you take this approach, add:
using System.Net;
| {% if method.type == "webAuth" %} | ||
| #if UNITY_EDITOR || UNITY_IOS || UNITY_ANDROID || UNITY_WEBGL | ||
| {% endif %} | ||
| public {% if method.type == "webAuth" %}async {% endif ~%} UniTask{% if method.type == "webAuth" %}{% else %}<{{ utils.resultType(spec.title, method) }}>{% endif %} {{ method.name | caseUcfirst }}({{ utils.method_parameters(method.parameters, method.consumes) }}) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check webAuth handling in DotNet template
echo "=== DotNet webAuth handling ==="
find templates/dotnet -name "*.twig" -o -name "*.cs" | head -5
rg -B 2 -A 5 'webAuth' templates/dotnet --type twig 2>/dev/null || echo "Searching DotNet templates for webAuth..."Repository: appwrite/sdk-generator
Length of output: 4095
🏁 Script executed:
#!/bin/bash
# Check all SDK templates for webAuth return types
echo "=== Search webAuth across SDK templates ==="
find templates -maxdepth 2 -type d | head -10
rg -l 'webAuth' templates --type twig | head -10Repository: appwrite/sdk-generator
Length of output: 776
🏁 Script executed:
#!/bin/bash
# Check the spec.json for webAuth method definitions
echo "=== WebAuth in spec.json ==="
if [ -f "tests/resources/spec.json" ]; then
rg -B 3 -A 10 '"type":\s*"webAuth"' tests/resources/spec.json | head -50
elif [ -f "spec.json" ]; then
rg -B 3 -A 10 '"type":\s*"webAuth"' spec.json | head -50
else
find . -name "spec.json" -type f 2>/dev/null | head -3
fiRepository: appwrite/sdk-generator
Length of output: 563
🏁 Script executed:
#!/bin/bash
# Check Dart and Python webAuth return types
echo "=== Dart webAuth handling ==="
rg -B 2 -A 5 'webAuth' templates/dart --type twig 2>/dev/null | head -50
echo -e "\n=== Python webAuth handling ==="
rg -B 2 -A 5 'webAuth' templates/python --type twig 2>/dev/null | head -50
echo -e "\n=== Web (TypeScript) webAuth handling ==="
rg -B 2 -A 5 'webAuth' templates/web/src/services --type twig 2>/dev/null | head -50Repository: appwrite/sdk-generator
Length of output: 7246
🏁 Script executed:
#!/bin/bash
# Check the actual oauth.twig implementations
echo "=== DotNet oauth.twig ==="
find templates/dotnet -name "oauth.twig" -exec cat {} \;
echo -e "\n=== Unity oauth.twig ==="
find templates/unity -name "oauth.twig" -exec cat {} \;Repository: appwrite/sdk-generator
Length of output: 2280
🏁 Script executed:
#!/bin/bash
# Verify the exact line 44 signature in Unity ServiceTemplate.cs.twig
echo "=== Unity ServiceTemplate.cs.twig around line 44 ==="
sed -n '40,50p' templates/unity/Assets/Runtime/Core/Services/ServiceTemplate.cs.twigRepository: appwrite/sdk-generator
Length of output: 731
WebAuth methods return void (UniTask) instead of UniTask<string>, inconsistent with other SDKs
DotNet returns Task<String>, Dart returns Future<String?>, Python returns str, and Web returns the auth URL as a string. Unity's webAuth methods emit UniTask (void return) instead, creating API divergence across SDKs.
The Unity oauth.twig already extracts authentication data (secret/key) from the callback but discards it. Update the signature to UniTask<string> and return the authentication URL or token to align with other SDK implementations.
🤖 Prompt for AI Agents
In `@templates/unity/Assets/Runtime/Core/Services/ServiceTemplate.cs.twig` around
lines 41 - 44, The webAuth methods currently use a void UniTask signature
causing SDK divergence; update the generated method signature logic so when
method.type == "webAuth" the return type is UniTask<string> (use
utils.resultType(spec.title, method) or explicitly string for webAuth) and
ensure the implementation in the generated method (the code that extracts the
auth secret/key from the callback) returns the auth URL/token (the value you
extract) instead of discarding it so callers receive the string result; adjust
any async/await and return paths in the method named via {{ method.name |
caseUcfirst }} accordingly.
There was a problem hiding this comment.
What should be returned? The OAuth2 process is configured automatically, it catches deep links itself, and then saves the result in cookies. In WebGL, it is not possible to use links for success errors, since this application is located at a single address.
There was a problem hiding this comment.
@Fellmonkey, thank you for the clarification on Unity's OAuth2 architecture. I understand the design now:
- Deep link handling is automatic via
WebAuthComponent - Session credentials are extracted from the callback and stored in cookies transparently
- The client is already configured with the session after the method completes
- WebGL's single-address constraint prevents traditional redirect-based flows
This makes sense for Unity's async/await model where the caller simply awaits completion and the session is automatically available in the Client instance afterward.
Question: Are there any developer scenarios where returning the session data (e.g., the secret/key or a success indicator) would be useful? For example:
- Inspecting which account was authenticated without an additional API call
- Logging/debugging the auth flow
- Cross-SDK consistency for developers familiar with other Appwrite SDKs
If the void return is intentional for Unity's "automatic session management" philosophy and there's no practical use case for a return value, I'm happy to close this comment. The exception-based error handling already covers failure cases, so the current design seems reasonable for Unity's architecture.
Let me know your thoughts!
There was a problem hiding this comment.
close this and remember it for the next commits
There was a problem hiding this comment.
@Fellmonkey, understood! I've recorded this for future reference. The Unity SDK's webAuth methods intentionally return UniTask (void) rather than a string/token because:
- Deep link handling and session management are automatic via
WebAuthComponent - Session credentials are transparently extracted and stored in cookies
- The
Clientinstance is already configured with the session after the method completes - This design accommodates WebGL's single-address constraint
This architectural difference from other SDKs is by design and won't be flagged in future reviews. Closing this thread.
✏️ Learnings added
Learnt from: Fellmonkey
Repo: appwrite/sdk-generator PR: 1105
File: templates/unity/Assets/Runtime/Core/Services/ServiceTemplate.cs.twig:41-44
Timestamp: 2026-01-19T19:12:41.055Z
Learning: Unity SDK webAuth methods intentionally return UniTask (void) instead of UniTask<string> or string like other SDKs (DotNet, Dart, Python, Web) because Unity implements automatic session management via WebAuthComponent with transparent deep link handling and cookie storage. The Client instance is already configured with the session after the method completes, and this design accommodates WebGL's single-address constraint.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
4 participants
What does this PR do?
This PR adds comprehensive Unity SDK support to the SDK generator by:
Unity Language Implementation: Introduces a new
Unitylanguage class (src/SDK/Language/Unity.php) that extends the baseLanguageclass, providing Unity-specific type mappings, keywords, and code generation logic for C# in Unity environment.Unity Template System: Adds a complete set of Unity-specific templates under
templates/unity/Assets/including:Automated Testing Integration: Introduces Unity2021 test support with:
Unity2021Test.phptest class that integrates with the existing test frameworkTests.cs,Tests.asmdef) for comprehensive SDK testingunityci/editor:ubuntu-2021.3.45f1-base-3.1.0CI/CD Integration: Updates the GitHub Actions workflow to:
UNITY_LICENSEenvironment variable for Unity Editor automationUnity Project Structure: Implements proper Unity package structure with:
Assets/Runtime/andAssets/Editor/Test Plan
The testing strategy includes:
Unit Testing: The
Unity2021Test.phpruns comprehensive tests covering:Docker-based CI Testing: Tests run in a controlled Unity environment using:
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
Yes
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.