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

add track for launch from dcl-editor #2476

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

popuz
Copy link
Collaborator

@popuz popuz commented Oct 17, 2024

What does this PR change?

Extended runtime field of analytics context with the support of launch from dcl-editor

How to test the changes?

  1. Launch the explorer
  2. Check happy path

Our Code Review Standards

https://github.com/decentraland/unity-renderer/blob/master/docs/code-review-standards.md

@popuz popuz self-assigned this Oct 17, 2024
Copy link

github-actions bot commented Oct 17, 2024

badge

Build failed! Check the logs to see what went wrong

Copy link
Member

@pravusjif pravusjif left a comment

Choose a reason for hiding this comment

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

LGTM.
This cannot be tested as the Creator's Hub Editor (AKA DCL Editor) cannot open a custom build.

We should tell the Creator's Hub devs to add the new app parameter when calling the E@ on Preview, to test this after we do a release with this PR merged

Copy link
Collaborator

Choose a reason for hiding this comment

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

LGTM, just left a minor question :)

private static string ChooseRuntime(IAppArgs appArgs)
{
if (Application.isEditor)
return "unity-editor";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shall we make all these literals const?

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.

3 participants