Skip to content

Conversation

@bitsandfoxes
Copy link
Contributor

@bitsandfoxes bitsandfoxes commented Jul 10, 2025

Fixes #2224, #1827 🎉
Relies on the .NET SDK getsentry/sentry-dotnet#4357

Context

The SDK has to wait for EndOfFrame before capturing a screenshot as the behaviour is unreliable otherwise. See Unity docs.

Implementation

The ScreenshotProcessor uses the SentryMonoBehaviour to start a coroutine that waits for End of Frame to capture the screenshot. The screenshot gets sent as an envelop with a single attachment as envelope item.


public void CaptureScreenshotForEvent(SentryUnityOptions options, SentryId eventId)
{
StartCoroutine(CaptureScreenshot(options, eventId));
Copy link
Member

Choose a reason for hiding this comment

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

This method returns before the coroutine starts/ends, right? how do we know the screenshot will be taken before the event is created?

Copy link
Contributor Author

@bitsandfoxes bitsandfoxes Jul 18, 2025

Choose a reason for hiding this comment

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

The ScreenshotProcessor forwards the call to the SentryMonoBehaviour that starts the coroutine for the current EndOfFrame.

@bitsandfoxes bitsandfoxes force-pushed the fix/screenshot-capture branch from 276efc3 to 11548fe Compare July 18, 2025 14:25
@bitsandfoxes bitsandfoxes marked this pull request as ready for review July 18, 2025 15:17
@bruno-garcia
Copy link
Member

CI is unhappy

@bitsandfoxes bitsandfoxes mentioned this pull request Jul 21, 2025
bitsandfoxes and others added 4 commits July 24, 2025 10:59
Co-authored-by: seer-by-sentry[bot] <157164994+seer-by-sentry[bot]@users.noreply.github.com>
Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>
Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>
@bitsandfoxes
Copy link
Contributor Author

@sentry review

$BuildDir = $(GetNewProjectBuildPath)
$ApkFileName = "test.apk"
$ProcessName = "com.DefaultCompany.$(GetNewProjectName)"
$ProcessName = "io.sentry.unity.integrationtest"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With Unity 6 integration tests seem to fail to have their proper identifier set, causing the build to fail. We're setting this now explicitly in the Builder.cs and making use of the hardcoded name here.

Comment on lines +77 to +84
internal virtual byte[] CaptureScreenshot(SentryUnityOptions options)
=> SentryScreenshot.Capture(options);

internal virtual void CaptureAttachment(SentryId eventId, SentryAttachment attachment)
=> (Sentry.SentrySdk.CurrentHub as Hub)?.CaptureAttachment(eventId, attachment);

internal virtual YieldInstruction WaitForEndOfFrame()
=> new WaitForEndOfFrame();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm making these virtual so I can create a TestScreenshotEventProcessor that overwrites these for testing purposes.

Comment on lines -44 to -51

// This is a workaround for build issues with Unity 2022.3. and newer.
// https://discussions.unity.com/t/gradle-build-issues-for-android-api-sdk-35-in-unity-2022-3lts/1502187/10
#if UNITY_2022_3_OR_NEWER
Debug.Log("Builder: Setting Android target API level to 33");
PlayerSettings.Android.targetSdkVersion = AndroidSdkVersions.AndroidApiLevel33;
#endif

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Workaround that is no longer needed.

Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

LGTM

@bitsandfoxes bitsandfoxes merged commit 5012808 into main Jul 25, 2025
93 checks passed
@bitsandfoxes bitsandfoxes deleted the fix/screenshot-capture branch July 25, 2025 07:22
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.

Attached screenshots blank

4 participants