Skip to content

Conversation

PaulDemeulenaere
Copy link
Contributor

@PaulDemeulenaere PaulDemeulenaere commented Nov 3, 2020

Purpose of this PR

We detected an unexpected behavior with Exact Fixed Time Step option on the VisualEffectGraph :
_repro_exact_time
See case : https://fogbugz.unity3d.com/f/cases/1289829/

The actual fix require a C++ change, which can't be deliver before 10.2, so, we are temporarily removing this option in VisualEffect inspector.

The inspector will also reset the VFXUpdateMode.ExactFixedTimeStep logging the sanitize has been done.


Testing status

Manual Only

Before
image

After
image


Comments to reviewers

This feature was exposed to user in 2020.1/2020.2, so :

@github-actions github-actions bot added the vfx label Nov 3, 2020
@PaulDemeulenaere PaulDemeulenaere marked this pull request as ready for review November 3, 2020 12:25
Actually, doesn't break editor test because Sanitize is only call on the first import (or opening the graph).
@PaulDemeulenaere
Copy link
Contributor Author

@julienf-unity As discussed on slack, use a sanitize instead of doing this in inspector at 9ea36db
image

Here an execution after loading the repro project ⬇️
_repro_exact_time_sanitize

Actually, I thought I had to update editor test but we can still test spawner behavior because the sanitize is only called on the first import (or opening the VisualEffectGraph window), added a explicit check of update mode after reimporting asset.

@PaulDemeulenaere
Copy link
Contributor Author

PaulDemeulenaere commented Nov 3, 2020

VFX_HDRP playmode :

Total tests: 96
Failed test count: 1
Successful tests count: 90
Not run tests count: 5

Expected result due to the unexpected message message: Converting invalid MinMaxAABB

@PaulDemeulenaere
Copy link
Contributor Author

🟢 (kind of), ready to be merged !

@julienf-unity julienf-unity merged commit fbffffb into master Nov 3, 2020
@julienf-unity julienf-unity deleted the vfx/fix/1289829-workaround-disabling-exact-fixed-time-step branch November 3, 2020 17:26
PaulDemeulenaere added a commit that referenced this pull request Nov 3, 2020
* Remove temporarily "Exact Fixed Time Step" option on VisualEffectAsset to avoid unexpected behavior

* Use santize instead of inspector behavior to remove ExactFixedTimeStep

Actually, doesn't break editor test because Sanitize is only call on the first import (or opening the graph).
julienf-unity pushed a commit that referenced this pull request Nov 4, 2020
* Remove temporarily "Exact Fixed Time Step" option on VisualEffectAsset to avoid unexpected behavior

* Use santize instead of inspector behavior to remove ExactFixedTimeStep

Actually, doesn't break editor test because Sanitize is only call on the first import (or opening the graph).
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.

2 participants