Skip to content

[Cross-pipeline] HDRP Wizard window appears on opening a new URP project if both pipelines are present. #4898

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

Merged
merged 6 commits into from
Jun 17, 2021

Conversation

alex-vazquez
Copy link
Contributor


Purpose of this PR

https://fogbugz.unity3d.com/f/cases/1336296/

Testing status

  • Check that the wizard is not shown when opening the editor with URP as current pipeline
  • Check that the first time that the user changes to HDRP the wizard is shown
  • Check that the HDRP Wizard can not be opended if HDRP is not the current pipeline

image


Comments to reviewers

I closed the editor with visual studio and I was not able to show the Wizard because the setting HDUserSettings.wizardPopupAlreadyShownOnce was never set to false. Fixed that issue in this PR, adding a comment in code to explain the issue and why is that setting set there.

@alex-vazquez alex-vazquez self-assigned this Jun 14, 2021
@github-actions github-actions bot added the HDRP label Jun 14, 2021
@alex-vazquez alex-vazquez requested a review from RSlysz June 14, 2021 15:09
Copy link
Contributor

@RSlysz RSlysz left a comment

Choose a reason for hiding this comment

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

Right. Its starting to be a bit hard to follow as it is spread over 3 methods. Can you add a small comment on that? (HDProjectSettings.wizardIsStartPopup could only be true at the point I spoted above). Thanks

Copy link
Contributor

@jenniferd-unity jenniferd-unity left a comment

Choose a reason for hiding this comment

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

Just a quick note about our log, but otherwise I would prefer that we use HDRenderPipeline APIs instead of GetType to improve readability

@alex-vazquez alex-vazquez marked this pull request as ready for review June 16, 2021 07:39
Copy link
Contributor

@jenniferd-unity jenniferd-unity left a comment

Choose a reason for hiding this comment

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

Thanks for acting on suggestions too!

@alex-vazquez alex-vazquez requested a review from a team June 17, 2021 06:52
@alex-vazquez
Copy link
Contributor Author

@Unity-Technologies/gfx-qa-hdrp , could you please test

  • If URP is the active pipeline is not HDRP the wizard is not opened ( when starting the editor )
  • When chaning from URP to HDRP for the first time the wizard is opened
  • If HDRP is the active pipeline, the wizard is shown ( when starting the editor )

Copy link
Contributor

@TomasKiniulis TomasKiniulis left a comment

Choose a reason for hiding this comment

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

Checked listed requests on Graphics and Quality sections. Also checked that changing pipelines grays out or shows HDRP wizard section when assets are changed in Graphics or overriden in quality settings. Works as expected. Nice changes!

@alex-vazquez alex-vazquez merged commit 31b7b5d into master Jun 17, 2021
@alex-vazquez alex-vazquez deleted the x-pipeline/1336296-hdrp-wizard branch June 17, 2021 11:36
@TomasKiniulis
Copy link
Contributor

TomasKiniulis commented Jul 2, 2021

Looks like I've missed regression introduced by this PR(I've reported it here: https://fogbugz.unity3d.com/f/cases/1347461/). When completely empty project is created HDRP Wizard is not showing up after adding hdrp package, and the option is grayed out. Workaround is to create HDRP asset and assign it manually, then the wizard can be opened up manually.
image

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.

5 participants