Skip to content
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

Ref: Simplified scene breadcrumbs messages #197

Merged
merged 10 commits into from
May 27, 2021
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Unreleased

- Simplified scene breadcrumbs messages ([#197](https://github.com/getsentry/sentry-unity/pull/197))
- Check/create directory before saving ([#196](https://github.com/getsentry/sentry-unity/pull/196))
- Embedded link.xml in assembly ([#194](https://github.com/getsentry/sentry-unity/pull/194))
- Exclude SentryOptions.json from release package ([#195](https://github.com/getsentry/sentry-unity/pull/195))
Expand Down
30 changes: 17 additions & 13 deletions src/Sentry.Unity/Integrations/SceneManagerIntegration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public SceneManagerIntegration() : this(SceneManagerAdapter.Instance)

public void Register(IHub hub, SentryOptions options)
{
options.DiagnosticLogger?.Log(SentryLevel.Debug, "Registering BeforeSceneLoad integration.");
options.DiagnosticLogger?.Log(SentryLevel.Debug, "Registering SceneManager integration.");

_sceneManager.SceneLoaded += OnSceneManagerOnSceneLoaded;
_sceneManager.SceneUnloaded += SceneManagerOnSceneUnloaded;
Expand All @@ -32,15 +32,15 @@ void OnSceneManagerOnSceneLoaded(SceneAdapter scene, LoadSceneMode mode)

hub.AddBreadcrumb(
$"Scene '{scene.Name}' was loaded",
category: "scene.loaded",
category: "scene.loaded"
// TODO: What is worth paying the price of allocation in order to add here?
data: new Dictionary<string, string>
{
{"name", scene.Name},
// data: new Dictionary<string, string>
bitsandfoxes marked this conversation as resolved.
Show resolved Hide resolved
// {
// {"name", scene.Name},
// TODO: Should we benchmark before getting these? Are these and/or other unused fields useful?
// {"path", scene.path},
// {"isDirty", scene.isDirty.ToString()},
}
// }
// TODO: Is this useful? Does it happen that IsValid returns false at runtime?
// level: scene.IsValid()
// ? BreadcrumbLevel.Error
Expand All @@ -58,11 +58,8 @@ void SceneManagerOnSceneUnloaded(SceneAdapter scene)

hub.AddBreadcrumb(
$"Scene '{scene.Name}' was unloaded",
category: "scene.unloaded",
data: new Dictionary<string, string>
{
{"name", scene.Name},
});
category: "scene.unloaded"
);
}

void SceneManagerOnActiveSceneChanged(SceneAdapter fromScene, SceneAdapter toScene)
Expand All @@ -73,9 +70,16 @@ void SceneManagerOnActiveSceneChanged(SceneAdapter fromScene, SceneAdapter toSce
return;
}

var message = $"Changed active scene '{fromScene.Name}' to '{toScene.Name}'";
if (fromScene.Name == null)
{
message = $"Changed active scene to '{toScene.Name}'";
}
bitsandfoxes marked this conversation as resolved.
Show resolved Hide resolved

hub.AddBreadcrumb(
$"Changed active scene '{fromScene.Name}' to '{toScene.Name}'",
category: "scene.changed");
message,
category: "scene.changed"
);
}
}
}
Expand Down
2 changes: 0 additions & 2 deletions test/Sentry.Unity.Tests/SceneManagerIntegrationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ public void SceneLoaded_EnabledHub_CrumbAdded()

Assert.AreEqual($"Scene '{sceneName}' was loaded", actualCrumb.Message);
Assert.AreEqual("scene.loaded", actualCrumb.Category);
Assert.AreEqual(sceneName, actualCrumb.Data!["name"]);
}

[Test]
Expand Down Expand Up @@ -72,7 +71,6 @@ public void SceneUnloaded_EnabledHub_CrumbAdded()

Assert.AreEqual($"Scene '{sceneName}' was unloaded", actualCrumb.Message);
Assert.AreEqual("scene.unloaded", actualCrumb.Category);
Assert.AreEqual(sceneName, actualCrumb.Data!["name"]);
}

[Test]
Expand Down