Skip to content

Commit a9b50df

Browse files
keveleighkeveleigh
committed
Ensure invalid types are handled throughout the flow
1 parent 1d8d42e commit a9b50df

6 files changed

Lines changed: 129 additions & 39 deletions

File tree

‎org.mixedrealitytoolkit.theming/Editor/BaseThemeBinderDrawer.cs‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,17 @@ public override void OnGUI(Rect position, SerializedProperty property, GUIConten
2727
cachedLabel = label.text.Replace("Element", "Binder");
2828
cachedLabels[label.text] = cachedLabel;
2929
}
30-
label.text = cachedLabel;
30+
label = new GUIContent(label) { text = cachedLabel };
3131
}
3232

3333
SerializedProperty themeDataSourceProperty = property.serializedObject.FindProperty("themeDataSource");
3434

3535
string[] names = null;
3636
SerializedProperty themeDefinitionItemName = null;
37+
bool hasDataSource = themeDataSourceProperty != null && themeDataSourceProperty.objectReferenceValue != null;
3738

3839
// Only pay the cost of parsing available items when the property is actively expanded
39-
if (property.isExpanded && property.managedReferenceValue != null && themeDataSourceProperty != null && themeDataSourceProperty.objectReferenceValue != null)
40+
if (property.isExpanded && property.managedReferenceValue != null && hasDataSource)
4041
{
4142
themeDefinitionItemName = property.FindPropertyRelative(ThemeDefinitionItemNameField);
4243

@@ -51,18 +52,16 @@ public override void OnGUI(Rect position, SerializedProperty property, GUIConten
5152

5253
// Draw the foldout and all child properties within the allocated rect.
5354
Rect propertyRect = position;
54-
if (names != null)
55+
if (hasDataSource)
5556
{
5657
// Reserve the last line for the Bound Theme Item popup when expanded.
5758
propertyRect.height -= property.isExpanded ? EditorGUIUtility.singleLineHeight + EditorGUIUtility.standardVerticalSpacing : 0;
5859
}
5960
EditorGUI.PropertyField(propertyRect, property, label, true);
6061

6162
// Draw the Bound Theme Item popup using the Rect API, within the allocated position.
62-
if (names != null && property.isExpanded)
63+
if (hasDataSource && property.isExpanded)
6364
{
64-
int selected = System.Array.IndexOf(names, themeDefinitionItemName.stringValue);
65-
6665
// Child fields are indented 15px from position.x. Unity's label/control
6766
// split is at position.x + labelWidth, so the label width is (labelWidth - 15f)
6867
// and the control starts at that same absolute split point.
@@ -73,20 +72,30 @@ public override void OnGUI(Rect position, SerializedProperty property, GUIConten
7372
Rect labelRect = new Rect(position.x + indentWidth, rowY, EditorGUIUtility.labelWidth - indentWidth, EditorGUIUtility.singleLineHeight);
7473
Rect controlRect = new Rect(splitX, rowY, rightEdge - splitX, EditorGUIUtility.singleLineHeight);
7574

76-
using (var check = new EditorGUI.ChangeCheckScope())
75+
if (names != null)
76+
{
77+
int selected = System.Array.IndexOf(names, themeDefinitionItemName.stringValue);
78+
using (var check = new EditorGUI.ChangeCheckScope())
79+
{
80+
EditorGUI.LabelField(labelRect, "Bound Theme Item");
81+
selected = EditorGUI.Popup(controlRect, selected, names);
82+
if (check.changed)
83+
{
84+
themeDefinitionItemName.stringValue = names[selected];
85+
}
86+
}
87+
}
88+
else
7789
{
78-
EditorGUI.LabelField(labelRect, "Bound Theme Item");
79-
selected = EditorGUI.Popup(controlRect, selected, names);
80-
if (check.changed)
90+
using (new EditorGUI.DisabledScope(true))
8191
{
82-
themeDefinitionItemName.stringValue = names[selected];
92+
EditorGUI.LabelField(labelRect, "Bound Theme Item");
93+
EditorGUI.Popup(controlRect, 0, new string[] { "(No matching items)" });
8394
}
8495
}
8596
}
8697

8798
EditorGUI.EndProperty();
88-
89-
property.serializedObject.ApplyModifiedProperties();
9099
}
91100

92101
public override float GetPropertyHeight(SerializedProperty property, GUIContent label)

‎org.mixedrealitytoolkit.theming/Editor/FontIconSetMapEditor.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ public void OnEnable()
3737

3838
public override void OnInspectorGUI()
3939
{
40+
serializedObject.Update();
41+
4042
if (Event.current.type == EventType.Layout)
4143
{
4244
if (pendingIconSet != null && pendingIconToRenameOld != null && pendingIconToRenameNew != null)

‎org.mixedrealitytoolkit.theming/Editor/ThemeDefinitionEditor.cs‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Copyright (c) Mixed Reality Toolkit Contributors
2+
// Licensed under the BSD 3-Clause
3+
4+
using UnityEditor;
5+
6+
namespace MixedReality.Toolkit.Theming.Editor
7+
{
8+
[CustomEditor(typeof(ThemeDefinition))]
9+
public class ThemeDefinitionEditor : UnityEditor.Editor
10+
{
11+
public override void OnInspectorGUI()
12+
{
13+
serializedObject.Update();
14+
15+
DrawDefaultInspector();
16+
17+
ThemeDefinition def = target as ThemeDefinition;
18+
if (def != null && def.ThemeDefinitionItems != null)
19+
{
20+
foreach (var item in def.ThemeDefinitionItems)
21+
{
22+
string displayLabel = string.IsNullOrWhiteSpace(item.Name) ? "(null)" : item.Name;
23+
System.Type type = item.DataType?.Type;
24+
25+
if (type == null)
26+
{
27+
EditorGUILayout.HelpBox($"Item '{displayLabel}' has no DataType selected, or the type could not be resolved.", MessageType.Warning);
28+
}
29+
else if (type.IsAbstract || type.IsInterface || type.IsGenericTypeDefinition)
30+
{
31+
EditorGUILayout.HelpBox($"Item '{displayLabel}' uses an invalid DataType ({type.Name}). It must be a concrete, non-generic class to be instantiated.", MessageType.Warning);
32+
}
33+
}
34+
}
35+
36+
serializedObject.ApplyModifiedProperties();
37+
}
38+
}
39+
}

‎org.mixedrealitytoolkit.theming/Editor/ThemeDefinitionEditor.cs.meta‎

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

‎org.mixedrealitytoolkit.theming/Editor/ThemeEditor.cs‎

Lines changed: 58 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ public class ThemeEditor : UnityEditor.Editor
1616

1717
private static bool itemsFoldout = false;
1818

19+
private static readonly System.Collections.Generic.HashSet<Type> failedTypes = new System.Collections.Generic.HashSet<Type>();
20+
1921
private string nameField;
2022
private string dataField;
2123
private string valueField;
@@ -35,6 +37,8 @@ protected void OnEnable()
3537
/// </summary>
3638
public override void OnInspectorGUI()
3739
{
40+
serializedObject.Update();
41+
3842
EditorGUILayout.PropertyField(themeDefinitionProp);
3943

4044
ThemeDefinition themeDefinition = themeDefinitionProp.objectReferenceValue as ThemeDefinition;
@@ -47,6 +51,12 @@ public override void OnInspectorGUI()
4751
if (Event.current.type == EventType.Layout)
4852
{
4953
ReconcileThemeItems(themeDefinition, themeItemsProp);
54+
55+
// Force the SerializedObject to immediately sync any newly instantiated
56+
// [SerializeReference] payloads so their child properties (like 'Value')
57+
// are fully discoverable during this exact same Layout pass.
58+
serializedObject.ApplyModifiedProperties();
59+
serializedObject.Update();
5060
}
5161

5262
using (new EditorGUI.IndentLevelScope())
@@ -57,18 +67,43 @@ public override void OnInspectorGUI()
5767
SerializedProperty dataProp = themeItemProp.FindPropertyRelative(dataField);
5868
SerializedProperty valueProp = dataProp?.FindPropertyRelative(valueField);
5969

70+
string themeItemName = themeItemProp.FindPropertyRelative(nameField).stringValue;
71+
string displayLabel = string.IsNullOrWhiteSpace(themeItemName) ? "(null)" : themeItemName;
72+
6073
if (valueProp != null)
6174
{
62-
// Draw just the Value field, labelled with the item name,
75+
// Draw just the Value field, labeled with the item name,
6376
// skipping the intermediate "Data" foldout entirely.
64-
string themeItemName = themeItemProp.FindPropertyRelative(nameField).stringValue;
65-
EditorGUILayout.PropertyField(valueProp, new GUIContent(themeItemName), true);
77+
EditorGUILayout.PropertyField(valueProp, new GUIContent(displayLabel), true);
6678
}
6779
else
6880
{
69-
// Fallback for any item whose Data doesn't follow the
70-
// BaseThemeItemData<T> shape (e.g. a null reference).
71-
EditorGUILayout.PropertyField(themeItemProp, true);
81+
if (dataProp?.managedReferenceValue == null)
82+
{
83+
if (i < themeDefinition.ThemeDefinitionItems.Length)
84+
{
85+
var definitionItem = themeDefinition.ThemeDefinitionItems[i];
86+
if (definitionItem.DataType?.Type == null)
87+
{
88+
EditorGUILayout.HelpBox($"'{displayLabel}' has no valid DataType selected in the ThemeDefinition.", MessageType.Warning);
89+
}
90+
else
91+
{
92+
EditorGUILayout.HelpBox($"Failed to initialize data for '{displayLabel}'. Ensure its DataType ({definitionItem.DataType.Type.Name}) is a concrete class with a default constructor.", MessageType.Warning);
93+
}
94+
}
95+
96+
using (new EditorGUI.DisabledScope(true))
97+
{
98+
EditorGUILayout.LabelField(displayLabel, "null");
99+
}
100+
}
101+
else
102+
{
103+
// Fallback for any item whose Data doesn't follow the
104+
// BaseThemeItemData<T> shape.
105+
EditorGUILayout.PropertyField(dataProp, new GUIContent(displayLabel), true);
106+
}
72107
}
73108
}
74109
}
@@ -109,34 +144,31 @@ private void ReconcileThemeItems(ThemeDefinition themeDefinition, SerializedProp
109144
{
110145
// Move the found item up to position i, preserving its saved values.
111146
themeItemsProp.MoveArrayElement(existingIndex, i);
147+
themeItem = themeItemsProp.GetArrayElementAtIndex(i);
112148
}
113149
else
114150
{
115151
// No existing item found — insert a fresh one with default values.
116-
117152
themeItemsProp.InsertArrayElementAtIndex(i);
118153
themeItem = themeItemsProp.GetArrayElementAtIndex(i);
154+
themeItem.managedReferenceValue = new Theme.ThemeItem(themeDefinitionItemName, null);
155+
}
156+
}
119157

120-
Type dataType = definitionItem.DataType?.Type;
121-
object dataInstance = null;
158+
// Unified instantiation and auto-healing for missing or mismatched data types
159+
Type expectedType = definitionItem.DataType?.Type;
160+
SerializedProperty dataProp = themeItem.FindPropertyRelative(dataField);
161+
Type actualType = dataProp?.managedReferenceValue?.GetType();
122162

123-
if (dataType != null)
124-
{
125-
try
126-
{
127-
dataInstance = Activator.CreateInstance(dataType);
128-
}
129-
catch (Exception e)
130-
{
131-
Debug.LogWarning($"Failed to instantiate data for ThemeItem '{themeDefinitionItemName}' (Type: {dataType.Name}). Falling back to null. Exception: {e.Message}");
132-
}
133-
}
134-
else
135-
{
136-
Debug.LogWarning($"Could not resolve DataType for ThemeItem '{themeDefinitionItemName}'. Falling back to null.");
137-
}
138-
139-
themeItem.managedReferenceValue = new Theme.ThemeItem(themeDefinitionItemName, dataInstance);
163+
if (expectedType != null && expectedType != actualType && !failedTypes.Contains(expectedType))
164+
{
165+
try
166+
{
167+
dataProp.managedReferenceValue = Activator.CreateInstance(expectedType);
168+
}
169+
catch
170+
{
171+
failedTypes.Add(expectedType);
140172
}
141173
}
142174
}

‎org.mixedrealitytoolkit.uxcore/Editor/Inspectors/FontIconSet/FontIconSelectorInspector.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,9 @@ public void DrawIconGrid(FontIconSelector fontIconSelector, float tileSize)
136136
GUILayout.Height(tileSize),
137137
GUILayout.Width(tileSize)))
138138
{
139+
// Flush any pending changes from other Inspector fields (e.g. lost focus) manually editing the icon name
140+
serializedObject.ApplyModifiedProperties();
141+
139142
if (fontIconSelector.TextMeshProComponent != null)
140143
{
141144
Undo.RecordObjects(new Object[] { fontIconSelector, fontIconSelector.TextMeshProComponent }, "Changed icon");
@@ -152,6 +155,9 @@ public void DrawIconGrid(FontIconSelector fontIconSelector, float tileSize)
152155
{
153156
PrefabUtility.RecordPrefabInstancePropertyModifications(fontIconSelector.TextMeshProComponent);
154157
}
158+
159+
// Resync the serialized object after manually editing the icon name
160+
serializedObject.Update();
155161
}
156162

157163
Rect textureRect = GUILayoutUtility.GetLastRect();

0 commit comments

Comments
 (0)