Skip to content

docs: correct seven doc-vs-implementation drifts (K1-K7) - #217

Open
bobbyg603 wants to merge 1 commit into
mainfrom
docs/fix-documentation-drift
Open

docs: correct seven doc-vs-implementation drifts (K1-K7)#217
bobbyg603 wants to merge 1 commit into
mainfrom
docs/fix-documentation-drift

Conversation

@bobbyg603

Copy link
Copy Markdown
Member

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.

# What I found What I changed
K1 Runtime/BugSplat.cs documented ShouldPostException as "1 crash every 10 seconds"; ShouldPostExceptionImpl.DefaultShouldPostExceptionImpl uses TimeSpan.FromSeconds(3). The README already said 3 seconds. Doc now reads "1 report every 3 seconds". Behaviour untouched — #170 owns the implementation change.
K2 On WebGL the constructor takes the #elif UNITY_WEBGL branch, which never calls UseDotNetHandler, so nativeCrashReportClient stays null and Post(FileInfo) logs "not implemented on this platform" and returns. "Migrating from 4.x" no longer claims all platforms; it names WebGL as the exception.
K3 ShouldPostException is not a field on BugSplatOptions (Runtime/Client/BugSplatOptions.cs); it is a runtime property on BugSplat backed by IClientSettingsRepository, and CreateFromOptions never sets it. Removed the row from the BugSplat Options table and added a note pointing at Preventing Repeated Reports.
K4 The metadata was wrong about the artifact. bugsplat-android-release.aar contains jni/arm64-v8a/, jni/armeabi-v7a/, and jni/x86_64/ (each with libbugsplat.so, libcrashpad_handler.so, libcurl.so), so CPU: ARMv7 mislabels a three-ABI archive. See the open question below. .aar.meta Android CPU: ARMv7AnyCPU. README now says ARM64 is the only configuration BugSplat tests, and states plainly that the .aar also ships armeabi-v7a and x86_64 binaries which are untested and unsupported.
K5 _attachNativeLogFileIos in Editor/IOS/ObjC/BugSplatBridge.mm is an empty stub with a comment explaining why (the delegate's attachmentForBugSplat: suppresses setValue:forAttribute: attributes). Android has no branch in AttachNativeLogFile at all. Only Windows (BugSplat_AddAttachment) and macOS (_attachNativeLogFileMac) do work. XML doc now says Windows and macOS only, and why iOS and Android are no-ops.
K6 No capability matrix existed in the README or the sample README. Added a Platform Support section before the per-platform sections: six capabilities across Windows, macOS, iOS, Android, Linux, and WebGL, plus footnotes for every qualified cell.
K7 Runtime/BugSplat.cs had // <summary> above Notes — the compiler treated it as an ordinary comment and dropped the summary. Fixed to ///. 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 #if branches in Runtime/BugSplat.cs and Editor/PostBuild.cs, not from the audit's table:

  • Managed C# exceptions — every constructor branch ends in UseDotNetHandler; WebGL substitutes WebGLReporter/WebGLExceptionClient. All six platforms: yes. WebGL's reporter logs "not implemented" for CaptureEditorLog/CapturePlayerLog/CaptureScreenshots, hence the footnote.
  • Native crashes — one branch each for UNITY_STANDALONE_WIN, UNITY_IOS, UNITY_STANDALONE_OSX, UNITY_ANDROID, all gated on the corresponding UseNativeCrashReportingFor… flag and all && !UNITY_EDITOR. Linux and WebGL fall through to UseDotNetHandler with no native reporter.
  • Hang / ANR — Windows: BugSplat_SetHangDetectionTimeout(0) at init, raised only when WindowsHangDetectionTimeoutMs > 0 in CreateFromOptions, so opt-in. iOS: bugsplat.enableHangDetection = YES in BugSplatBridge.mm, automatic. Android: com/bugsplat/android/AnrReporter.class is present in the .aar's classes.jar; API 30 requirement per the existing ANR section. macOS: BugSplatBridgeMac.mm never sets enableHangDetection → No.
  • Offline retry — Windows: BugSplat_PostAllCrashesAsync() runs at init. iOS/macOS/Android: the report is written at crash time and uploaded on the next start/initBugSplat. Managed posts are never persisted (DotNetStandardExceptionReporter posts once and reports the failure through the callback), which the footnote states explicitly.
  • FeedbackPostFeedback needs feedbackClient, assigned only in UseDotNetHandler; WebGL therefore logs "PostFeedback is not supported on this platform".
  • Symbol upload — from OnPostprocessBuild: iOS under #if UNITY_IOS (Xcode build phase, needs UploadDebugSymbolsForIos), Android under #elif UNITY_ANDROID (needs UploadDebugSymbolsForAndroid, 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 (needs UploadDebugSymbolsForMac, skipped for Xcode exports). No Linux or WebGL branch. The Android path uploads **/*.so only — no LineNumberMappings.json — which is why the trailing paragraph calls that out.

Verification

  • dotnet build of 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.
  • Rebuilt with 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.
  • Ran a github-slugger-equivalent anchor check over the README. Every link I added or touched resolves: #-android, #-ios, #-macos, #-windows, #background-thread-exceptions, #symbol-upload-credentials, #preventing-repeated-reports, and the new #-platform-support.

Open questions

  • K4 — is ARMv7a actually supported? Determinable: the .aar ships armeabi-v7a binaries, 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.
  • K4 — the CPU field on an .aar. Android native crash reporting demonstrably works today on the ARM64 builds the README tells users to make, while the importer said ARMv7, which suggests Unity ignores CPU for .aar plugins and the value is inert. AnyCPU is 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.
  • Two pre-existing broken README anchors, left alone as out of scope: [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

- 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>
Copilot AI lite review requested due to automatic review settings August 11, 2026 21:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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, fixed Notes XML doc comment, clarified AttachNativeLogFile platform behavior).
  • Added a platform capability matrix and corrected several README statements (WebGL minidump exception, Android ABI support/testing note, clarified ShouldPostException configuration).
  • Corrected Unity Android plugin importer metadata to avoid mislabeling the .aar as 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

K1–K7: Documentation drift batch

3 participants