Screenshot management #91
Merged
Conversation
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
rqbacktrace
reviewed
Jun 7, 2021
|
|
||
| #if UNITY_2019_1_OR_NEWER | ||
| RenderTexture screenTexture = RenderTexture.GetTemporary(Screen.width, Screen.height); | ||
| ScreenCapture.CaptureScreenshotIntoRenderTexture(screenTexture); |
There was a problem hiding this comment.
Is it worth doing this asynchronously? https://docs.unity3d.com/ScriptReference/ScreenCapture.CaptureScreenshotIntoRenderTexture.html
Especially if it is a non-fatal exception, maybe we don't want to block important game threads. I imagine the downsampling of the image will add some extra processing time as well. It might be worth profiling to see if we're actually saving time with this during runtime.
There was a problem hiding this comment.
The problem that I have with async in this solution is that we will need to write async callbacks to continue error submission. Overall our solution uses coroutines. Just after this database operation, we will yield a return from the Backtrace client to free the current frame.
There was a problem hiding this comment.
Do we save time here? It looks like we still capture the screenshot at full resolution and then take additional time to do downsampling. It looks like we maybe are adding time overall. Maybe we don't care but I'm only asking because of this user story:
"As a user, I would like to define attachments settings that allow the game to save resources/time when the game generates image attachment."
I understand we would save resources by saving downsampled images, but are we saving time?
There was a problem hiding this comment.
I guess we can save time because we are spending less time on the file write. But it might be worth doing a simple test to check if we are actually saving time overall including the downsampling algorithm. Otherwise I'm sure the functionality works and there's no issues there.
There was a problem hiding this comment.
Do we save time?
I don't think so:
(without image modifications)
(without image modification)
Do we save anything at all? Yes - transfer, attachment size, disk usage.
There was a problem hiding this comment.
Sounds good, if user enables this setting they will have smaller attachment sizes which can enable user to capture more screenshots and otherwise save on disk usage, as well as save transfer time at crash time. But user will not save during run time. Thank you for providing the numbers!
konraddysput
added a commit
that referenced
this pull request
Jun 21, 2021
* Divided commit into two branches for reviewers. Added attribute provider and Backtrace http client for testing purpose. * Adjusted labels * Start default event on the application startup + send empty string instead of null * Event aggregation logic for reviewers (#74) * Event aggregation logic for reviewers * Adjusted the most flexible cases * Code review suggestions * Default event support * backoff for session events when request failed * Retry only on 503 * Exponential backoff * Avoid casting to float, fixed Math.Pow * Added Rameez suggestions * Code review suggestions * Fixed wrong retry function argument * line ending fix * Fixed invalid property name after merge * Adjusted nullable JSON values * Create attribute provider dynamically * Enable attribute provider * Disable maximum number of events in store from config * Adjust session aggregation support to spec requirements and meeting suggestions * Fixed typo * git attributes for line endings * Adjust current flow to new requirements + editor changes * Allow for overriding default unique event * Added getter and setter for backtracesesion support * Adjust backtrace session object * Backtrace session API improvements * Safe native attribute ios * Safe native attributes * Adjusted native build * Use empty list of classifiers instead of empty string * Adjusted API * Adjusted API + Session=>Summed + BacktraceSession=>BacktraceMetrics * Adjusted API + Session=>Summed + BacktraceSession=>BacktraceMetrics * Renamed ms to sec * Event aggregation support (#79) * Divided commit into two branches for reviewers. Added attribute provider and Backtrace http client for testing purpose. * Adjusted labels * Start default event on the application startup + send empty string instead of null * Event aggregation logic for reviewers (#74) * Event aggregation logic for reviewers * Adjusted the most flexible cases * Code review suggestions * Default event support * backoff for session events when request failed * Retry only on 503 * Exponential backoff * Avoid casting to float, fixed Math.Pow * Added Rameez suggestions * Code review suggestions * Fixed wrong retry function argument * line ending fix * Fixed invalid property name after merge * Adjusted nullable JSON values * Create attribute provider dynamically * Enable attribute provider * Disable maximum number of events in store from config * Adjust session aggregation support to spec requirements and meeting suggestions * Fixed typo * git attributes for line endings * Adjust current flow to new requirements + editor changes * Allow for overriding default unique event * Added getter and setter for backtracesesion support * Adjust backtrace session object * Backtrace session API improvements * Safe native attribute ios * Safe native attributes * Adjusted native build * Use empty list of classifiers instead of empty string * Adjusted API * Adjusted API + Session=>Summed + BacktraceSession=>BacktraceMetrics * Adjusted API + Session=>Summed + BacktraceSession=>BacktraceMetrics * Renamed ms to sec * Fixed stucture size (#80) * Guess native library path when application context is not available (#81) * Guessing lib path based on the current game directory when the activity is not available * Code reformat * Renamed ms to sec * Handle different unity player java class name (#87) * Removed warnings generated by Unity2020 + Unity2021 (#85) * Fixed problem with Unity2021 - symbols upload warnings on the android upload symbols task * Removed warnings generated by Unity2020 * Fixed compatibility issues * Formatting * Metrics support imrovements (#88) * divided summed/unique events into different queues * Fully worked unit tests * adjusted url + json keys * Code review feedback * Breadcrumbs support (#83) * Breadcrumbs support: before database operations cleanup * Breadcrumbs improvements * Database adjustements * Adjusted database code * Removed try/catch block from function that handles exception and prevent from sending nullable records * Allow nullable record in SendData method * Squashed commit of the following: commit 8c2cec9 Author: kdysput <konrad.dysput@gmail.com> Date: Thu May 6 00:00:10 2021 +0200 Adjusted nullable JSON values commit 2ed2d7f Author: kdysput <konrad.dysput@gmail.com> Date: Wed May 5 23:55:47 2021 +0200 Fixed invalid property name after merge * Undo session aggregation changes * Create attribute provider dynamically * Undo maximum number of events * Undo attribute provider management * Fixed labels * Expose enableBreadcrumbs API * Fixed name * breadcrumbs tests * Breadcrumbs unit tests - breadcrumb file tests * Renamed enums * Log cleanup tests * Size consistency test * Add stack trace attribute only to error/exception logs * Adjusted native build * Safe native attributes * Safe native attribute ios * Fixed timestamp comparision * Fixed missing namespace + line endings * API update + removed two low memory warnings * breadcrumbs adjustements * Unit tests that validates navigation and logs events * Renamed AddBreadcrumb to Log * Send unique attachments (or include (#n) to attachment name) * Fixed condition + handling empty nullable files * Order beadcrumbs * breadcrumbs support - fixed ios side (delay breadcrumbs startup) * Removed database checks * added breadcrumbs.lastId + offline db support * removed early access flag * Fixed breadcrumb structure + unit tests * typos + new lines updates * Adjusted formating * Adjusted if statement * Removed extra line * COde cleanup - adjustemenets after CR * Fixed compilation issue * Don't dispose before checking network status * Use UTC timestamp * Removed universe ane token submission URL - allow user to override them (#89) * fixed line endings * Editor configuraiton improvements - foldout improvements * Apply default log file size * Label change * Final ios library * Use copy of attributes * Fixed reference shared between native client and managed client * applicaiton.session attribute * Limit number of events to 50 * Use breadcrumbs as double not long * Use only one variable to determinate maximum number of elements in queue * Setting maximum number of events per unique/summed events + breadcrumbId type change * Getter/Setter api interface improvements * Breadcrumbs improvements -> changed types + metrics expose maximum number of events + adjust api to new requiremenets * Unit tests for 502 status code + disabled auto send, long => uint api change, adjusted metrics initialization flow * Changed breadcurmb type: http => system for internet connection check * Read application version and name from cache to add support for background threads * Block WebGL * Calculate retry based on the time * version update * Update CHANGELOG.md * Update CHANGELOG.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update CHANGELOG.md Minor edits * TOC Update for Breadcrumbs and Crash Free Metrics * Ignore LogType.Error (#92) * Ignore errors * Adjusted comment * Screenshot management (#91) * Screenshot management * Changed default screenshot quality * Ratio typo * version bump: preview.2 update * Adjusted attachments + breadcrumbs support * Allow overriding default attributes * Add missing namespace * Application.session + api adjustements backtrace metrics * empty json object adjustement * Windows native dump event aggregation attributes * Correct unit test namespace * Convert device attributes to scoped attributes + include native attributes before native integration start * Breadcrumbs log level tooltip adjustement * Native upload handler tests * Version 3.5.0: Version update * Update README.md * Update README.md * Update CHANGELOG.md * Update README.md * Label change Co-authored-by: Vincent Lussenburg <vlussenburg@users.noreply.github.com> Co-authored-by: jasoncdavis0 <jdavis@backtrace.io>
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.
Why
As a user, I would like to define attachments settings that allow the game to save resources/time when the game generates image attachment.
As a power user, I would like to use database API to define screenshot settings.