docs: correct seven doc-vs-implementation drifts (K1-K7) - #217
Open
bobbyg603 wants to merge 1 commit into
Open
Conversation
- ShouldPostException's XML doc claimed a 10-second rate limit; ShouldPostExceptionImpl has always used 3 seconds, as the README says. #170 tracks changing the implementation. - Post(FileInfo minidump) no-ops on WebGL, which never constructs a nativeCrashReportClient, so the migration note drops "all platforms". - ShouldPostException is a runtime-only property, not a BugSplatOptions field, so it moves out of the options table. - The Android .aar ships arm64-v8a, armeabi-v7a, and x86_64 libraries, so the importer's `CPU: ARMv7` mislabels it; AnyCPU matches the artifact. ARM64 stays the only tested configuration. - AttachNativeLogFile is a no-op on iOS, whose bridge is a deliberate empty stub, as well as on Android, which has no bridge call at all. - Adds a platform capability matrix derived from the #if branches in BugSplat.cs and PostBuild.cs. - A two-slash `// <summary>` silently dropped the Notes doc comment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Aligns public documentation and metadata with the current 5.0.0+ implementation details discovered during the K1–K7 audit, reducing confusion for users across platforms and options configuration.
Changes:
- Updated XML documentation in
Runtime/BugSplat.cs(rate-limit wording, fixedNotesXML doc comment, clarifiedAttachNativeLogFileplatform behavior). - Added a platform capability matrix and corrected several README statements (WebGL minidump exception, Android ABI support/testing note, clarified
ShouldPostExceptionconfiguration). - Corrected Unity Android plugin importer metadata to avoid mislabeling the
.aaras ARMv7-only.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| Runtime/Plugins/Android/bugsplat-android-release.aar.meta | Updates Android plugin CPU setting to AnyCPU to match a multi-ABI .aar. |
| Runtime/BugSplat.cs | Fixes/clarifies XML docs for rate limiting, Notes, and native log attachment behavior. |
| README.md | Adds platform support matrix and corrects several doc-vs-implementation discrepancies (WebGL, options, Android ABI notes). |
Files not reviewed (1)
- Runtime/Plugins/Android/bugsplat-android-release.aar.meta: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
daveplunkett
approved these changes
Aug 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #166
Seven doc-vs-implementation drifts from the 5.0.0 audit. Each was verified against the current code before editing; six were fixed on the doc side, one (K4) on the metadata side.
Runtime/BugSplat.csdocumentedShouldPostExceptionas "1 crash every 10 seconds";ShouldPostExceptionImpl.DefaultShouldPostExceptionImplusesTimeSpan.FromSeconds(3). The README already said 3 seconds.#elif UNITY_WEBGLbranch, which never callsUseDotNetHandler, sonativeCrashReportClientstays null andPost(FileInfo)logs "not implemented on this platform" and returns.ShouldPostExceptionis not a field onBugSplatOptions(Runtime/Client/BugSplatOptions.cs); it is a runtime property onBugSplatbacked byIClientSettingsRepository, andCreateFromOptionsnever sets it.bugsplat-android-release.aarcontainsjni/arm64-v8a/,jni/armeabi-v7a/, andjni/x86_64/(each withlibbugsplat.so,libcrashpad_handler.so,libcurl.so), soCPU: ARMv7mislabels a three-ABI archive. See the open question below..aar.metaAndroidCPU: ARMv7→AnyCPU. README now says ARM64 is the only configuration BugSplat tests, and states plainly that the .aar also shipsarmeabi-v7aandx86_64binaries which are untested and unsupported._attachNativeLogFileIosinEditor/IOS/ObjC/BugSplatBridge.mmis an empty stub with a comment explaining why (the delegate'sattachmentForBugSplat:suppressessetValue:forAttribute:attributes). Android has no branch inAttachNativeLogFileat all. Only Windows (BugSplat_AddAttachment) and macOS (_attachNativeLogFileMac) do work.Runtime/BugSplat.cshad// <summary>aboveNotes— the compiler treated it as an ordinary comment and dropped the summary.///. Grepped the whole tree for^\s*//\s*<(summary|/summary|param|returns)outside///: this was the only occurrence.How I verified the K6 matrix
Every cell comes from the
#ifbranches inRuntime/BugSplat.csandEditor/PostBuild.cs, not from the audit's table:UseDotNetHandler; WebGL substitutesWebGLReporter/WebGLExceptionClient. All six platforms: yes. WebGL's reporter logs "not implemented" forCaptureEditorLog/CapturePlayerLog/CaptureScreenshots, hence the footnote.UNITY_STANDALONE_WIN,UNITY_IOS,UNITY_STANDALONE_OSX,UNITY_ANDROID, all gated on the correspondingUseNativeCrashReportingFor…flag and all&& !UNITY_EDITOR. Linux and WebGL fall through toUseDotNetHandlerwith no native reporter.BugSplat_SetHangDetectionTimeout(0)at init, raised only whenWindowsHangDetectionTimeoutMs > 0inCreateFromOptions, so opt-in. iOS:bugsplat.enableHangDetection = YESinBugSplatBridge.mm, automatic. Android:com/bugsplat/android/AnrReporter.classis present in the .aar'sclasses.jar; API 30 requirement per the existing ANR section. macOS:BugSplatBridgeMac.mmnever setsenableHangDetection→ No.BugSplat_PostAllCrashesAsync()runs at init. iOS/macOS/Android: the report is written at crash time and uploaded on the nextstart/initBugSplat. Managed posts are never persisted (DotNetStandardExceptionReporterposts once and reports the failure through the callback), which the footnote states explicitly.PostFeedbackneedsfeedbackClient, assigned only inUseDotNetHandler; WebGL therefore logs "PostFeedback is not supported on this platform".OnPostprocessBuild: iOS under#if UNITY_IOS(Xcode build phase, needsUploadDebugSymbolsForIos), Android under#elif UNITY_ANDROID(needsUploadDebugSymbolsForAndroid, skipped for Export Project or Debug Symbols = None), Windows under#elif UNITY_EDITOR_WIN(hence "from a Windows editor"; also needs Copy PDB files), macOS outside the chain (needsUploadDebugSymbolsForMac, skipped for Xcode exports). No Linux or WebGL branch. The Android path uploads**/*.soonly — noLineNumberMappings.json— which is why the trailing paragraph calls that out.Verification
dotnet buildof the Runtime + Tests tree against Unity 6000.5.6f1 reference assemblies: 0 errors, only the two pre-existing CS0649 warnings for fields assigned solely in native-only branches.GenerateDocumentationFile=true: no CS157x XML-doc warnings, and the generated XML now contains<member name="P:BugSplatUnity.BugSplat.Notes">with its summary — the K7 fix confirmed at the compiler level.#-android,#-ios,#-macos,#-windows,#background-thread-exceptions,#symbol-upload-credentials,#preventing-repeated-reports, and the new#-platform-support.Open questions
armeabi-v7abinaries, so the library is not ARM64-only, and the old parenthetical "(ARMV7a is not supported)" was at best imprecise. Not determinable from this repo: whether BugSplat supports 32-bit ARM Unity builds as a matter of policy or testing. I kept the existing support statement and made the factual half accurate rather than flipping the claim. If ARMv7a is in fact supported and tested, that sentence should change.CPUfield on an .aar. Android native crash reporting demonstrably works today on the ARM64 builds the README tells users to make, while the importer saidARMv7, which suggests Unity ignoresCPUfor.aarplugins and the value is inert.AnyCPUis correct either way — inert if ignored, correct if honoured — so this is a low-risk metadata fix, but it is worth a maintainer's eye since it is the only non-doc change in the PR.[usage](#usage)(the heading slugs to#-usage) and[upload windows minidumps](#windows)(slugs to#-windows). Both are one-character fixes in sections other agents are editing right now, so I did not touch them.🤖 Generated with Claude Code