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.

Sign up for GitHub

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

Jump to bottom

Create VFX from "No asset" tab does not open it #6521

Merged
merged 2 commits into from
Jan 4, 2022
Merged

Create VFX from "No asset" tab does not open it #6521

merged 2 commits into from
Jan 4, 2022

Conversation

julienamsellem
Copy link
Contributor

@julienamsellem julienamsellem commented Dec 7, 2021 •
edited
Loading

Purpose of this PR

This PR #6385 introduced the following regression.

Steps to reproduce the issue:

  • Click on the Window -> Visual Effect -> Visual Effect Graph menu entry
  • Create a new VFX graph using the dedicated button
    => The newly created VFX is not opened in place of the "No Asset" tab

Testing status

Additional tests:

  • Multiple VFX can be opened
  • Deleting multiple VFX (that are opened) from project browser close opened VFX tabs and leave a single "No Asset" tab

Comment for reviewers

I noticed another bug related to "No Asset" tab which is also fixed in this PR:

  • Open two subgraph windows (operator or block)
  • Try to open the "No asset" window with the Window -> Visual Effect -> Visual Effect Graph menu entry
    => The "No Asset" window is not opened.

@julienamsellem
When new VFX is created from "No asset" tab open it in that tab
a2be78e 
This is a regression that was introduced with previous branch
@github-actions
Copy link

github-actions bot commented Dec 7, 2021

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://unity-ci.cds.internal.unity3d.com/project/902/
Search for your PR branch using the search bar at the top, then add the following segment(s) to the end of the URL (you may need multiple tabs depending on how many packages you change)

VFX
/jobDefinition/.yamato%252Fall-vfx.yml%2523PR_VFX_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.

@github-actions github-actions bot added the vfx label Dec 7, 2021
@julienamsellem julienamsellem requested a review from a team December 7, 2021 17:28
@VitaVFX VitaVFX requested review from VitaVFX and removed request for a team December 8, 2021 08:35
VitaVFX
VitaVFX approved these changes Dec 13, 2021
View reviewed changes
Copy link

@VitaVFX VitaVFX left a comment

Choose a reason for hiding this comment

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

Looks solid! Double checked changes with various reproduction steps:

  • Repro provided by dev
  • Replacing, creating a new one effect
  • Deleting from project browser and creating a new VFX
  • Deleting multiple effect from project browser when having multi windows. One no asset window is present and VFX is opened or replaced successfully.

@julienamsellem julienamsellem marked this pull request as ready for review December 13, 2021 09:44
PaulDemeulenaere
PaulDemeulenaere approved these changes Dec 16, 2021
View reviewed changes
Copy link
Contributor

@PaulDemeulenaere PaulDemeulenaere left a comment

Choose a reason for hiding this comment

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

LGTM, the bug seems too recent to require a changelog (and if there isn't fogbugz, it's fine)
I launch VFX_HDRP Edit mode by security.
Thanks for the fix 🟢

@julienamsellem julienamsellem merged commit ce68ff0 into master Jan 4, 2022
@julienamsellem julienamsellem deleted the vfx/fix/multiple-windows-various-fixes-followup branch January 4, 2022 09:03
julienf-unity pushed a commit that referenced this pull request Jan 13, 2022
@julienamsellem @julienf-unity
Create VFX from "No asset" tab does not open it (#6521)
f4c234f 
* When new VFX is created from "No asset" tab open it in that tab

This is a regression that was introduced with previous branch

* "No Asset" window could be mixed up with Subgraph window because the asset is null in both cases
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PaulDemeulenaere PaulDemeulenaere PaulDemeulenaere approved these changes

@VitaVFX VitaVFX VitaVFX approved these changes

Assignees
No one assigned
Labels
backported-2022.1 vfx
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@julienamsellem @PaulDemeulenaere @VitaVFX