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

Updated project configuration checker window #6286

Conversation

Troy-Ferrell
Copy link
Contributor

@Troy-Ferrell Troy-Ferrell commented Oct 14, 2019

Overview

It has been a difficult balance for MRTK to control setting up Unity projects to help with configuration, performance, and expected functionality without annoying or frustrating users. We want this configuration to mostly be automated but also inform developers and give them control to select themselves.

This change revamps the configuration check dialog generally seen when adding MRTK to a new project. It creates it into a popup utility window that allows for a better user experience. Developers can now see and control more clearly what will be automated to their project. They can also ignore the popup for later (which will hide it for the rest of the current Unity session) or Ignore it entirely (which will set a user preference to never show the window).

image

Popup primarily shows on re-compiles, opened Unity projects, or changing build targets.

Developers can also access this popup via the MRTK > Utilities tab directly.

Note: This is not placed in the Tools package because it is more essential to the MRTK experience. Some users do not download the Tools package.

Created MRProjectConfigurator and MRProjectConfiguratorWindow
Moved MixedRealityEditorSettings file to new folder
Simplified MREditorSettings fucntionality to just track responsive to editor states (i.e play mode, etc)
Moved directory utilities out of MREditorSettings and into EditorProjectUtilities
Introduced capability checking in list

Changes

Verification

As a reviewer, it is possible to check out this change locally by using the following
commands (substituting {PR_ID} with the ID of this pull request):

git fetch origin pull/{PR_ID}/head:name_of_local_branch

git checkout name_of_local_branch

@Troy-Ferrell
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@Troy-Ferrell
Copy link
Contributor Author

@davidkline-ms, @keveleigh , or @Cameron-Micka , can you review this once any of you get a chance? It's about to hit 10 days online

@Troy-Ferrell
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

{
if (EditorLayerExtensions.SetupLayer(SpatialAwarenessDefaultLayer, "Spatial Awareness"))
{
Debug.LogWarning(string.Format($"Can't modify project layers. It's possible the format of the layers and tags data has changed in this version of Unity. Set layer {SpatialAwarenessDefaultLayer} to \"Spatial Awareness\" manually via Project Settings > Tags and Layers window."));

Choose a reason for hiding this comment

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

This message is misleading if the layer is already set to Spatial Awareness. Can we get a wording tweak or have it succeed if the value is already set?

Another option would be to differentiate "failure to set", "already set" and "set to something we didnt expect"

@david-c-kline
Copy link

Also seeing the first run not display the logo image

image

This is a fresh 2018.4.11f1 created project, importing a test version of the foundation package generated from @Troy-Ferrell's fork.

Copy link

@david-c-kline david-c-kline left a comment

Choose a reason for hiding this comment

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

Looks great. A couple of issues and it should be able to get merged.

Thanks!

@Troy-Ferrell
Copy link
Contributor Author

fork

Interesting, @keveleigh also saw something somewhat similar in Unity 2019.3.

I wonder if it's a timing issue in the way Unity loads texture...or at least the way we are doing it

@keveleigh
Copy link
Contributor

keveleigh commented Oct 25, 2019

Weird on the reported logo issue! I didn't see it when testing this PR, but I also didn't import from scratch. Interesting...

@david-c-kline
Copy link

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@david-c-kline david-c-kline dismissed their stale review October 28, 2019 17:58

merged in fix for spatial layer error... will re-test

@david-c-kline
Copy link

  • confirmed that the spatial layer no longer incorrectly appears.
  • First run (on package import) still shows no image
  • After the process completed, a package loaded and the dialog re-appeared (this was confusing as the settings are already applied)

@david-c-kline
Copy link

once merged, i will file separate issues based on what i encountered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants