Skip to content

Conversation

@RSlysz
Copy link
Contributor

@RSlysz RSlysz commented Aug 10, 2021

Purpose of this PR

Fix https://fogbugz.unity3d.com/f/cases/1355836/

Several PR have landed on the ProbeVolume. One of it added an hidding mechanisme. I am not fond of it as it may be more difficult now to add a new framesettings as we have some dynamically hidden. And this is also not compatible with the current UX workflow. But I understand the needs for this as it is experimental.

Then another FrameSettings was added with a dependency on the hidden one. But it was not hidden itself. So usual case, when the feature flag is disable, you have the dependant shown indented under somecing that have no relation with it (see below)
image

The fix here do:

  • Make also dependency hidden the same way (see below)
  • Add extra warning in the comment against the usage of hideUI
  • Add code documentation on the AmmendInfo to clarify each argument usage as they are more and more here
  • Grouped FrameSetting in code per group again. Initially, each group had an allotted page of bit to use but with time, devs haven't really used it this way. So only regroup them per group for now.

image or image


Testing status

Manually tested the hidding (see screenshot above)


Comments to reviewers

@FrancescoC-unity Have your PRs been backported ?

@github-actions
Copy link

Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed.
Link to Yamato: https://yamato.cds.internal.unity3d.com/jobs/902-Graphics
Search for your PR branch using the sidebar on the left, then add the following segment(s) to the end of the URL (you may need multiple tabs depending on how many packages you change)

HDRP
/.yamato%252Fall-hdrp.yml%2523PR_HDRP_2021.2

Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure.

Copy link
Contributor

@RemyUnity RemyUnity left a comment

Choose a reason for hiding this comment

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

Thank for the screenshots, I don't really see what we could test more.
LGTM 🟢

@JulienIgnace-Unity JulienIgnace-Unity merged commit 2320588 into hd/bugfix Aug 16, 2021
@JulienIgnace-Unity JulienIgnace-Unity deleted the hd/fix-framesettings-ordering branch August 16, 2021 13:52
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.

4 participants