Skip to content

Commit 6356aad

Browse files
PaulDemeulenaereGitHub Enterprise
PaulDemeulenaere
authored and
GitHub Enterprise
committed
Fix Custom Spawn serialization (#132)
* Allow Set Spawn Count & Set Spawn Time TODO : List Spawn count in available variant * Fix reference lost in m_SerializableType No idea of implication of this :-/ * Squashed commit of the following: (retrieve fix from @tristan) commit c24981d7dad15100eb40a92a6a9370e9ba800acd Author: Paul Demeulenaere <[email protected]> Date: Fri Oct 9 17:27:57 2020 +0200 Update message from @vlad suggestion Fix issue https://github.cds.internal.unity3d.com/unity/vfx-graphics/pull/131#issuecomment-30757 commit a87ba18 Merge: 13b501e 296ffd3 Author: Paul Demeulenaere <[email protected]> Date: Fri Oct 9 09:25:02 2020 +0200 Merge branch 'vfx/staging' into vfx/fix/error-at-creation # Conflicts: # com.unity.visualeffectgraph/CHANGELOG.md commit 13b501e Author: Paul Demeulenaere <[email protected]> Date: Fri Oct 9 09:24:27 2020 +0200 *Add warning for CollisionDepthBuffer Update & Fix Changelog.md commit 4bfb1e7 Author: Tristan Genevet <[email protected]> Date: Thu Oct 8 18:10:42 2020 +0200 Fix for displaying error from the creation of a node. * Add error feedback on failing custom spawner reference * Revert "Allow Set Spawn Count & Set Spawn Time" This reverts commit 0a6c75c. # Conflicts: # com.unity.visualeffectgraph/Editor/Expressions/VFXAttributeExpression.cs * Remove unexpected change * *Revert change in changelog * *WIP* add ResolveCustomCallbackInstance Some refactor mark as "TODOPAUL" * Clean implementation : customBehavior & comment * *Add test to cover sanitize before modify references * Change namespace for builtin custom spawner & sanitize * Extend test to verify connexion * *Update changelog.md * Add specific error if customBehavior returns null * Fix corner case when ScriptableObject reference has been lost * Work but not idea with copy/past + potential leak * Better implementation : use directly "MonoScript" * Fix spawnerTest * Rename "Can't found" => "Can't find"
1 parent cc36c16 commit 6356aad

File tree

13 files changed

+972
-52
lines changed

13 files changed

+972
-52
lines changed

‎TestProjects/VisualEffectGraph_HDRP/Assets/AllTests/Editor/Tests/VFXSpawnerCustomCallbackBuiltin.vfx_‎

Lines changed: 808 additions & 0 deletions
Large diffs are not rendered by default.

‎TestProjects/VisualEffectGraph_HDRP/Assets/AllTests/Editor/Tests/VFXSpawnerCustomCallbackBuiltin.vfx_.meta‎

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎TestProjects/VisualEffectGraph_HDRP/Assets/AllTests/Editor/Tests/VFXSpawnerTest.cs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,31 @@ private void CreateAssetAndComponent(float spawnCountValue, string playEventName
7373
camera.transform.LookAt(vfxComponent.transform);
7474
}
7575

76+
[UnityTest]
77+
public IEnumerator Sanitize_VFXSpawnerCustomCallback_Namespace()
78+
{
79+
string kSourceAsset = "Assets/AllTests/Editor/Tests/VFXSpawnerCustomCallbackBuiltin.vfx_";
80+
var graph = VFXTestCommon.CopyTemporaryGraph(kSourceAsset);
81+
82+
Assert.AreEqual(1, graph.children.OfType<VFXBasicSpawner>().Count());
83+
var basicSpawner = graph.children.OfType<VFXBasicSpawner>().FirstOrDefault();
84+
Assert.AreEqual(4, basicSpawner.GetNbChildren());
85+
Assert.IsNotNull(basicSpawner.children.FirstOrDefault(o => o.name == ObjectNames.NicifyVariableName("SpawnOverDistance")));
86+
Assert.IsNotNull(basicSpawner.children.FirstOrDefault(o => o.name == ObjectNames.NicifyVariableName("SetSpawnTime")));
87+
Assert.IsNotNull(basicSpawner.children.FirstOrDefault(o => o.name == ObjectNames.NicifyVariableName("LoopAndDelay")));
88+
Assert.IsNotNull(basicSpawner.children.FirstOrDefault(o => o.name == ObjectNames.NicifyVariableName("IncrementStripIndexOnStart")));
89+
90+
foreach (var sanitizeSpawn in basicSpawner.children)
91+
{
92+
Assert.IsFalse(sanitizeSpawn.inputSlots.Any(o => !o.HasLink()));
93+
Assert.IsNotNull(sanitizeSpawn.GetSettingValue("m_customScript"));
94+
Assert.IsNotNull((sanitizeSpawn as VFXSpawnerCustomWrapper).customBehavior);
95+
}
96+
97+
yield return null;
98+
}
99+
100+
76101
static string[] k_Create_Asset_And_Check_Event_ListCases = new[] { "OnPlay", "Test_Event" };
77102

78103
[UnityTest]

‎TestProjects/VisualEffectGraph_HDRP/Assets/AllTests/Editor/Tests/VFXTestCommon.cs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Collections.Generic;
99
using UnityEngine.UIElements;
1010
using UnityEditor.Experimental.GraphView;
11+
using System.IO;
1112

1213
namespace UnityEditor.VFX.Test
1314
{
@@ -16,6 +17,20 @@ class VFXTestCommon
1617
static readonly string tempBasePath = "Assets/TmpTests/";
1718
static readonly string tempFileFormat = tempBasePath + "vfx_{0}.vfx";
1819

20+
public static VFXGraph CopyTemporaryGraph(string path)
21+
{
22+
var guid = System.Guid.NewGuid().ToString();
23+
string tempFilePath = string.Format(tempFileFormat, guid);
24+
System.IO.Directory.CreateDirectory(tempBasePath);
25+
File.Copy(path, tempFilePath);
26+
27+
AssetDatabase.ImportAsset(tempFilePath);
28+
var asset = AssetDatabase.LoadAssetAtPath<VisualEffectAsset>(tempFilePath);
29+
VisualEffectResource resource = asset.GetResource();
30+
var graph = resource.GetOrCreateGraph();
31+
return graph;
32+
}
33+
1934
public static VFXGraph MakeTemporaryGraph()
2035
{
2136
var guid = System.Guid.NewGuid().ToString();

‎com.unity.visualeffectgraph/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
1010

1111
### Fixed
1212
- Forbid incorrect link between incompatible context [Case 1269756](https://issuetracker.unity3d.com/product/unity/issues/guid/1269756/)
13+
- Serialization issue with VFXSpawnerCallbacks
1314

1415
### Added
1516
- Added new setting to output nodes to exclude from TAA

‎com.unity.visualeffectgraph/Editor/Compiler/VFXGraphCompiledData.cs‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -561,21 +561,7 @@ private static VFXEditorTaskDesc[] BuildEditorTaksDescFromBlockSpawner(IEnumerab
561561

562562
Object processor = null;
563563
if (spawnerBlock.customBehavior != null)
564-
{
565-
var assets = AssetDatabase.FindAssets("t:TextAsset " + spawnerBlock.customBehavior.Name);
566-
if (assets.Length != 1)
567-
{
568-
// AssetDatabase.FindAssets will not search in package by default. Search in our package explicitly
569-
assets = AssetDatabase.FindAssets("t:TextAsset " + spawnerBlock.customBehavior.Name, new string[] { VisualEffectGraphPackageInfo.assetPackagePath });
570-
if (assets.Length != 1)
571-
{
572-
throw new InvalidOperationException("Unable to find the definition .cs file for " + spawnerBlock.customBehavior + " Make sure that the class name and file name match");
573-
}
574-
}
575-
576-
var assetPath = AssetDatabase.GUIDToAssetPath(assets[0]);
577-
processor = AssetDatabase.LoadAssetAtPath<TextAsset>(assetPath);
578-
}
564+
processor = spawnerBlock.customBehavior;
579565

580566
taskDescList.Add(new VFXEditorTaskDesc
581567
{

‎com.unity.visualeffectgraph/Editor/Core/VFXSerializer.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ public SerializableType(string typeText)
3838

3939
public virtual void OnBeforeSerialize()
4040
{
41-
m_SerializableType = m_Type != null ? m_Type.AssemblyQualifiedName : string.Empty;
41+
if (m_Type != null)
42+
m_SerializableType = m_Type.AssemblyQualifiedName;
4243
}
4344

4445
public virtual void OnAfterDeserialize()

‎com.unity.visualeffectgraph/Editor/Models/Blocks/Implementations/Spawn/VFXAbstractSpawner.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,6 @@ abstract class VFXAbstractSpawner : VFXBlock
99
public override VFXContextType compatibleContexts { get { return VFXContextType.Spawner; } }
1010
public override VFXDataType compatibleData { get { return VFXDataType.SpawnEvent; } }
1111
public abstract VFXTaskType spawnerType { get; }
12-
public virtual Type customBehavior { get { return null; } }
12+
public virtual MonoScript customBehavior { get { return null; } }
1313
}
1414
}

‎com.unity.visualeffectgraph/Editor/Models/Blocks/Implementations/Spawn/VFXSpawnerCustomWrapper.cs‎

Lines changed: 87 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,41 +23,115 @@ protected override sealed Dictionary<string, object[]> variants
2323
[VFXInfo(category = "Custom", variantProvider = typeof(CustomSpawnerVariant))]
2424
class VFXSpawnerCustomWrapper : VFXAbstractSpawner
2525
{
26-
[SerializeField, VFXSetting]
26+
[SerializeField, VFXSetting(VFXSettingAttribute.VisibleFlags.None)]
2727
protected SerializableType m_customType;
2828

29-
protected override IEnumerable<string> filteredOutSettings
29+
[SerializeField, VFXSetting(VFXSettingAttribute.VisibleFlags.None)]
30+
protected MonoScript m_customScript;
31+
32+
private void ResolveCustomCallbackInstance()
3033
{
31-
get
34+
//m_customScript always prevails on m_customType, we cannot modify m_customType twice.
35+
if (m_customScript != null && m_customScript.GetClass() != null)
36+
m_customType = m_customScript.GetClass();
37+
38+
//m_customScript is null in three cases
39+
// - Newly created VFXSpawnerCustomWrapper, m_customType changed by SetSettingValue.
40+
// - VFXSpawnerCallbacks has been suppressed, in that case, m_customType.text can display a message and m_customType == null.
41+
// - VFXSpawnerCallbacks has been restored, m_customType != null, rebuild the m_instance.
42+
if (m_customScript == null && m_customType != null)
43+
{
44+
var instance = CreateInstance(m_customType);
45+
m_customScript = MonoScript.FromScriptableObject(instance);
46+
}
47+
}
48+
49+
public override void Sanitize(int version)
50+
{
51+
if (m_customScript == null && !object.ReferenceEquals(m_customType, null))
3252
{
33-
yield return "m_customType";
53+
//Handle previous name based serialization, these class move to UnityEngine.VFX
54+
if (m_customType.text.StartsWith("UnityEditor.VFX.LoopAndDelay,"))
55+
m_customType = typeof(LoopAndDelay);
56+
else if (m_customType.text.StartsWith("UnityEditor.VFX.SetSpawnTime,"))
57+
m_customType = typeof(SetSpawnTime);
58+
else if (m_customType.text.StartsWith("UnityEditor.VFX.SpawnOverDistance,"))
59+
m_customType = typeof(SpawnOverDistance);
60+
else if (m_customType.text.StartsWith("IncrementStripIndexOnStart,"))
61+
m_customType = typeof(IncrementStripIndexOnStart);
62+
63+
ResolveCustomCallbackInstance();
3464
}
65+
base.Sanitize(version);
66+
}
67+
68+
protected internal override void Invalidate(VFXModel model, InvalidationCause cause)
69+
{
70+
base.Invalidate(model, cause);
71+
ResolveCustomCallbackInstance();
3572
}
3673

74+
public override void OnEnable()
75+
{
76+
base.OnEnable();
77+
ResolveCustomCallbackInstance();
78+
}
3779

3880
public override void GetImportDependentAssets(HashSet<int> dependencies)
3981
{
4082
base.GetImportDependentAssets(dependencies);
83+
if (customBehavior != null && customBehavior != null)
84+
dependencies.Add(customBehavior.GetInstanceID());
85+
}
86+
87+
protected override IEnumerable<VFXPropertyWithValue> inputProperties
88+
{
89+
get
90+
{
91+
if (m_customType == null)
92+
return Enumerable.Empty<VFXPropertyWithValue>();
93+
return PropertiesFromType(((Type)m_customType).GetRecursiveNestedType(GetInputPropertiesTypeName()));
94+
}
95+
}
96+
97+
protected override void GenerateErrors(VFXInvalidateErrorReporter manager)
98+
{
99+
//Type isn't reachable ... but we already stored a type, log an error.
100+
if (m_customType == null
101+
&& !object.ReferenceEquals(m_customType, null)
102+
&& !string.IsNullOrEmpty(m_customType.text))
103+
{
104+
manager.RegisterError("CustomSpawnerIDNotFound", VFXErrorType.Error, "Can't find : " + m_customType.text);
105+
}
41106

42-
if (m_customType != null)
107+
if (customBehavior == null && m_customType != null)
43108
{
44-
var function = ScriptableObject.CreateInstance(m_customType);
45-
var monoScript = MonoScript.FromScriptableObject(function);
46-
if (monoScript != null)
47-
dependencies.Add(monoScript.GetInstanceID());
109+
if (m_customScript != null && m_customScript.GetClass() != null)
110+
manager.RegisterError("CustomSpawnerIDNotVFXSpawnerCallbacks", VFXErrorType.Error, string.Format("{0} isn't a VFXSpawnerCallbacks", m_customScript.GetClass()));
111+
else
112+
manager.RegisterError("CustomSpawnerIDInvalid", VFXErrorType.Error, "Invalid ScriptableObject : " + (Type)m_customType);
48113
}
49114
}
50115

51-
protected override IEnumerable<VFXPropertyWithValue> inputProperties
116+
public override sealed string name
52117
{
53118
get
54119
{
55-
return customBehavior == null ? Enumerable.Empty<VFXPropertyWithValue>() : PropertiesFromType(customBehavior.GetRecursiveNestedType(GetInputPropertiesTypeName()));
120+
if (m_customType != null)
121+
return ObjectNames.NicifyVariableName(((Type)m_customType).Name);
122+
return "null";
56123
}
57124
}
58125

59-
public override sealed string name { get { return customBehavior == null ? "null" : ObjectNames.NicifyVariableName((customBehavior).Name); } }
60-
public override sealed Type customBehavior { get { return (Type)m_customType; } }
126+
public override sealed MonoScript customBehavior
127+
{
128+
get
129+
{
130+
if (m_customScript != null && typeof(VFXSpawnerCallbacks).IsAssignableFrom(m_customScript.GetClass()))
131+
return m_customScript;
132+
return null;
133+
}
134+
}
61135
public override sealed VFXTaskType spawnerType { get { return VFXTaskType.CustomCallbackSpawner; } }
62136
}
63137
}

‎com.unity.visualeffectgraph/Runtime/CustomSpawners/IncrementStripIndexOnStart.cs‎

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,31 +2,34 @@
22
using UnityEngine;
33
using UnityEngine.VFX;
44

5-
class IncrementStripIndexOnStart : VFXSpawnerCallbacks
5+
namespace UnityEngine.VFX
66
{
7-
public class InputProperties
7+
class IncrementStripIndexOnStart : VFXSpawnerCallbacks
88
{
9-
[Tooltip("Maximum Strip Count (Used to cycle indices)")]
10-
public uint StripMaxCount = 8;
11-
}
9+
public class InputProperties
10+
{
11+
[Tooltip("Maximum Strip Count (Used to cycle indices)")]
12+
public uint StripMaxCount = 8;
13+
}
1214

13-
static private readonly int stripMaxCountID = Shader.PropertyToID("StripMaxCount");
14-
static private readonly int stripIndexID = Shader.PropertyToID("stripIndex");
15+
static private readonly int stripMaxCountID = Shader.PropertyToID("StripMaxCount");
16+
static private readonly int stripIndexID = Shader.PropertyToID("stripIndex");
1517

16-
uint m_Index = 0;
18+
uint m_Index = 0;
1719

18-
public override void OnPlay(VFXSpawnerState state, VFXExpressionValues vfxValues, VisualEffect vfxComponent)
19-
{
20-
m_Index = (m_Index + 1) % Math.Max(1, vfxValues.GetUInt(stripMaxCountID));
21-
state.vfxEventAttribute.SetUint(stripIndexID, m_Index);
22-
}
20+
public override void OnPlay(VFXSpawnerState state, VFXExpressionValues vfxValues, VisualEffect vfxComponent)
21+
{
22+
m_Index = (m_Index + 1) % Math.Max(1, vfxValues.GetUInt(stripMaxCountID));
23+
state.vfxEventAttribute.SetUint(stripIndexID, m_Index);
24+
}
2325

24-
public override void OnStop(VFXSpawnerState state, VFXExpressionValues vfxValues, VisualEffect vfxComponent)
25-
{
26-
m_Index = 0;
27-
}
26+
public override void OnStop(VFXSpawnerState state, VFXExpressionValues vfxValues, VisualEffect vfxComponent)
27+
{
28+
m_Index = 0;
29+
}
2830

29-
public override void OnUpdate(VFXSpawnerState state, VFXExpressionValues vfxValues, VisualEffect vfxComponent)
30-
{
31+
public override void OnUpdate(VFXSpawnerState state, VFXExpressionValues vfxValues, VisualEffect vfxComponent)
32+
{
33+
}
3134
}
3235
}

0 commit comments

Comments
 (0)