New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Include more supplementary app data from WebGL #94
Conversation
28ec4de
to
59a1ef8
Compare
// Reuse the reported version from Unity as the | ||
// app.version, unless something is already set | ||
if (/^unity$/i.test(strTabName) && strAttrName === 'version' && !Bugsnag.appVersion) { | ||
Bugsnag.appVersion = strAttrValue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is the right version. I think you want to set on boot the Application.version
-> https://docs.unity3d.com/ScriptReference/Application-version.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the version from here:
NativeBugsnag.AddToTab("Unity", "version", Application.version.ToString()); |
Which looks to me like what you linked to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be in register and should be an explicit call to Application.version. Are you able to call that from the js layer?
@@ -0,0 +1,54 @@ | |||
(function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What imports/runs this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It gets bundled in by emscripten – I wasn't sure if it would get concat'd or isolated, so thought I'd do the old school iife (with a semi-colon) just in case.
59a1ef8
to
2c6c6c3
Compare
…pp metadata Unity apps deployed to mobile devices as well as webgl would inconsistenly receive data about the time that the app was running and how long it spent in the foreground, which meant that the averages were thrown off in the dashboard. This commit implements the recording of the required information when the notifier is running in a browser.
The mobile notifiers auto-detect the app version (e.g. android from the app manifest). The version number is known in webgl and is added to the Unity metadata tab. This commit adds a LOC that only runs on webgl to pass the Application.version value through to the "native" JS layer.
2c6c6c3
to
2792a8e
Compare
Code changed. This review is now stale.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Manually tested in 5.6, 2017, and 2018 - all changes work as expected with no issue.
This PR makes a couple of improvements when running a Unity app on WebGL:
feat(webgl): Auto detect app version in WebGL
The mobile notifiers auto-detect the app version (e.g. android from the app manifest). The version number is known in webgl and is added to the Unity metadata tab. This commit adds a LOC that only runs on webgl to pass the
Application.version
value through to the "native" JS layer.feat(webgl): Add duration, inForeground and durationInForeground to app metadata
Unity apps deployed to mobile devices as well as webgl would inconsistenly receive data about the time that the app was running and how long it spent in the foreground, which meant that the averages were thrown off in the dashboard. This commit implements the recording of the required information when the notifier is running in a browser.
How to test
You can build and run the example project with a target of WebGL to verify:
appVersion
is now supplied by default (comment out this line that explicitly sets it) (look at the "app" tab for a new event in the dashboard)duration
,durationInForeground
andinForeground
are set and look correct (try switching browser tab and letting the app run for a while before sending an error, again find this in the "app" tab of an error)