Skip to content

Conversation

Nightmask3
Copy link
Contributor

@Nightmask3 Nightmask3 commented Sep 28, 2021


Purpose of this PR

Fixes bug: https://fogbugz.unity3d.com/f/cases/1348910/

Broken behavior:

WHZjAVBQaw.mp4

Fixed behavior:

rW6l5QO71q.mp4

Testing status

  • Verified bug behavior no longer repros
  • Tested selection and undo/redo interaction as those behaviors were entangled with this code

Notes to reviewers

  • Please do make sure to validate the selection stack and undo/redo behavior as there is some halo effect with these changes to that
    There is one known issue where trying to undo selection to an empty selection stack causes it to skip the empty selection step and go one selection step previous to that, but it is not consistent, and has been deemed okay to ship with, will be investigated as a separate bug.

@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)

Shader Graph
/.yamato%252Fall-shadergraph.yml%2523PR_ShaderGraph_trunk
Depending on your PR, you may also want
/.yamato%252Fall-shadergraph_builtin_foundation.yml%2523PR_ShaderGraph_BuiltIn_Foundation_trunk
/.yamato%252Fall-shadergraph_builtin_lighting.yml%2523PR_ShaderGraph_BuiltIn_Lighting_trunk

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

@jessebarker jessebarker left a comment

Choose a reason for hiding this comment

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

LGTM

@Nightmask3 Nightmask3 marked this pull request as ready for review September 28, 2021 18:09
@Nightmask3 Nightmask3 requested a review from a team as a code owner September 28, 2021 18:09
Copy link
Contributor

@bencloward bencloward left a comment

Choose a reason for hiding this comment

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

I tested the repro steps locally and was able to repro the error. Then I switched to this branch and followed the steps again, but this time the error did not occur. Nice work!

@Nightmask3 Nightmask3 merged commit f07c824 into master Sep 28, 2021
@Nightmask3 Nightmask3 deleted the sg/fix-category-add-input-undo-bug-1348910 branch September 28, 2021 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants