diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 073b7c0..842b6a8 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -73,13 +73,16 @@ jobs: restore-keys: Library-${{ runner.os }}-${{ matrix.unityVersion }}-EditMode- - name: Run EditMode tests + env: + LIBGL_ALWAYS_SOFTWARE: '1' run: | - unity --non-interactive test . \ + xvfb-run --auto-servernum --server-args="-screen 0 1280x1024x24" \ + unity --non-interactive test . \ --editor-version "${{ matrix.unityVersion }}" \ --mode EditMode \ --output Logs/TestResults/editmode.xml \ --timeout 900 \ - -- -nographics -logFile Logs/editmode.log + -- -force-glcore -logFile Logs/editmode.log - name: Return Unity license if: always() && env.UNITY_LICENSE_ACTIVATION_ATTEMPTED == 'true' diff --git a/AGENTS.md b/AGENTS.md index c5e59e8..8e4f024 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,9 +33,11 @@ Run tests through Unity CLI (which manages batch mode): ``` unity --non-interactive test . --editor-version 2021.3.45f2 \ --mode EditMode --output Logs/TestResults/editmode.xml --timeout 900 \ - -- -nographics -logFile Logs/editmode.log + -- -logFile Logs/editmode.log ``` Repeat with `--editor-version 2023.2.22f1` for the generic/variance feature gate. +GUI interaction tests require a graphics device; do not pass `-nographics`. On headless Linux, +run the command under `xvfb-run` with software OpenGL, as configured in the Tests workflow. ## Architecture guardrails - Runtime surface area should remain minimal (mainly attributes / data structures). diff --git a/Assets/MackySoft/MackySoft.SerializeReferenceExtensions/Editor/SubclassSelectorDrawer.cs b/Assets/MackySoft/MackySoft.SerializeReferenceExtensions/Editor/SubclassSelectorDrawer.cs index 1a64733..c91b6be 100644 --- a/Assets/MackySoft/MackySoft.SerializeReferenceExtensions/Editor/SubclassSelectorDrawer.cs +++ b/Assets/MackySoft/MackySoft.SerializeReferenceExtensions/Editor/SubclassSelectorDrawer.cs @@ -22,6 +22,11 @@ public TypePopupCache (AdvancedTypePopup typePopup, AdvancedDropdownState state) } } + private sealed class TypeChangeState + { + public bool Pending; + } + private const int MaxTypePopupLineCount = 13; private static readonly GUIContent NullDisplayName = new GUIContent(TypeMenuUtility.NullDisplayName); @@ -32,9 +37,12 @@ public TypePopupCache (AdvancedTypePopup typePopup, AdvancedDropdownState state) private readonly Dictionary typeNameCaches = new Dictionary(); private SerializedProperty targetProperty; + private TypeChangeState targetTypeChangeState; public override void OnGUI (Rect position, SerializedProperty property, GUIContent label) { + int controlID = GUIUtility.GetControlID(property.propertyPath.GetHashCode(), FocusType.Passive); + var typeChangeState = (TypeChangeState)GUIUtility.GetStateObject(typeof(TypeChangeState), controlID); bool isManagedReference = property.propertyType == SerializedPropertyType.ManagedReference; #if UNITY_2021_3_OR_NEWER // Prepare the label before BeginProperty captures it for rendering. @@ -50,6 +58,13 @@ public override void OnGUI (Rect position, SerializedProperty property, GUIConte EditorGUI.BeginProperty(position, label, property); + // A type change can recreate the drawer. Keep its pending notification with the owning GUI control. + if (typeChangeState.Pending) + { + typeChangeState.Pending = false; + GUI.changed = true; + } + if (isManagedReference) { // Render label first to avoid label overlap for lists @@ -65,6 +80,7 @@ public override void OnGUI (Rect position, SerializedProperty property, GUIConte { TypePopupCache popup = GetTypePopup(property); targetProperty = property; + targetTypeChangeState = typeChangeState; popup.TypePopup.Show(popupPosition); } @@ -182,6 +198,8 @@ private TypePopupCache GetTypePopup (SerializedProperty property) individualObject.ApplyModifiedProperties(); individualObject.Update(); } + + targetTypeChangeState.Pending = true; }; result = new TypePopupCache(popup, state); diff --git a/Assets/MackySoft/MackySoft.SerializeReferenceExtensions/Tests/Editor/SubclassSelectorChangeCheckTests.cs b/Assets/MackySoft/MackySoft.SerializeReferenceExtensions/Tests/Editor/SubclassSelectorChangeCheckTests.cs new file mode 100644 index 0000000..bb1a212 --- /dev/null +++ b/Assets/MackySoft/MackySoft.SerializeReferenceExtensions/Tests/Editor/SubclassSelectorChangeCheckTests.cs @@ -0,0 +1,209 @@ +using System; +using System.Collections; +using System.Linq; +using System.Reflection; +using NUnit.Framework; +using UnityEditor; +using UnityEditor.IMGUI.Controls; +using UnityEngine; +using UnityEngine.TestTools; + +namespace MackySoft.SerializeReferenceExtensions.Tests +{ + public sealed class SubclassSelectorChangeCheckTests + { + private ChangeCheckTarget target; + private ChangeCheckWindow window; + private EditorWindow popup; + + [UnitySetUp] + public IEnumerator SetUp () + { + target = ScriptableObject.CreateInstance(); + window = ScriptableObject.CreateInstance(); + window.Initialize(target); + window.ShowUtility(); + yield return null; + window.DrawNow(); + window.ClearChanges(); + } + + [TearDown] + public void TearDown () + { + if (popup != null) + { + popup.Close(); + } + if (window != null) + { + window.Close(); + } + if (target != null) + { + UnityEngine.Object.DestroyImmediate(target); + } + } + + [UnityTest] + public IEnumerator SelectingType_NotifiesTheOwningPropertyChangeCheck () + { + OpenPopup(1); + SelectItem("Beta"); + yield return null; + window.DrawNow(); + + Assert.That(window.GetValue(1), Is.TypeOf()); + Assert.That(window.Changed[1], Is.True, "The caller must observe the type selection through EndChangeCheck."); + Assert.That(window.GetValue(0), Is.TypeOf()); + Assert.That(window.Changed[0], Is.False, "Drawing another element must not consume or receive the selected element's change."); + } + + [UnityTest] + public IEnumerator SelectingNull_NotifiesTheOwningPropertyChangeCheck () + { + OpenPopup(1); + SelectItem(""); + yield return null; + window.DrawNow(); + + Assert.That(window.GetValue(1), Is.Null); + Assert.That(window.Changed[1], Is.True, "Clearing a reference must notify the caller through EndChangeCheck."); + Assert.That(window.GetValue(0), Is.TypeOf()); + Assert.That(window.Changed[0], Is.False); + } + + [UnityTest] + public IEnumerator CancellingThePopup_DoesNotNotifyPropertyChangeChecks () + { + OpenPopup(1); + popup.SendEvent(new Event { type = EventType.KeyDown, keyCode = KeyCode.Escape }); + yield return null; + window.DrawNow(); + + Assert.That(window.GetValue(0), Is.TypeOf()); + Assert.That(window.GetValue(1), Is.TypeOf()); + Assert.That(window.Changed, Is.All.False, "Opening and cancelling the selector does not edit either property."); + } + + private void OpenPopup (int index) + { + window.Focus(); + window.DrawNow(); + Rect field = window.FieldRects[index]; + window.SendEvent(new Event + { + type = EventType.MouseDown, + mousePosition = new Vector2(field.xMax - 30f, field.y + EditorGUIUtility.singleLineHeight * 0.5f), + button = 0 + }); + popup = Resources.FindObjectsOfTypeAll() + .SingleOrDefault(candidate => candidate.GetType().Name == "AdvancedDropdownWindow"); + Assert.That(popup, Is.Not.Null, "Clicking the property selector must open its type popup."); + } + + private void SelectItem (string name) + { + // Unity exposes popup input through SendEvent, but its visible rows and keyboard selection are internal. + // Read those solely to navigate the real control; do not invoke the drawer's selection callback. + const BindingFlags flags = BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic; + var tree = (AdvancedDropdownItem)popup.GetType().GetField("m_CurrentlyRenderedTree", flags).GetValue(popup); + object state = popup.GetType().GetField("m_State", flags).GetValue(popup); + MethodInfo getSelectedIndex = state.GetType().GetMethod("GetSelectedIndex", flags); + AdvancedDropdownItem[] rows = tree.children.ToArray(); + int targetIndex = Array.FindIndex(rows, row => row.name == name); + Assert.That(targetIndex, Is.GreaterThanOrEqualTo(0), "Popup item missing: " + name); + + for (int step = 0; step <= rows.Length; step++) + { + if ((int)getSelectedIndex.Invoke(state, new object[] { tree }) == targetIndex) + { + popup.SendEvent(new Event { type = EventType.KeyDown, keyCode = KeyCode.Return }); + return; + } + popup.SendEvent(new Event { type = EventType.KeyDown, keyCode = KeyCode.DownArrow }); + } + Assert.Fail("Keyboard navigation did not reach popup item: " + name); + } + + public interface IValue { } + + [Serializable, AddTypeMenu("Alpha")] + public sealed class Alpha : IValue { } + + [Serializable, AddTypeMenu("Beta")] + public sealed class Beta : IValue { } + + public sealed class ChangeCheckTarget : ScriptableObject + { + [SerializeReference, SubclassSelector] + private IValue[] values = { new Alpha(), new Alpha() }; + } + + public sealed class ChangeCheckWindow : EditorWindow + { + private SerializedObject serializedTarget; + + public bool[] Changed { get; } = new bool[2]; + public Rect[] FieldRects { get; } = new Rect[2]; + + public void Initialize (ChangeCheckTarget value) + { + serializedTarget = new SerializedObject(value); + position = new Rect(100f, 100f, 500f, 160f); + } + + public void DrawNow () + { + SendEvent(new Event { type = EventType.Layout }); + SendEvent(new Event { type = EventType.Repaint }); + } + + public void ClearChanges () + { + Array.Clear(Changed, 0, Changed.Length); + } + + public object GetValue (int index) + { + serializedTarget.Update(); + return serializedTarget.FindProperty("values").GetArrayElementAtIndex(index).managedReferenceValue; + } + + private void OnGUI () + { + if (serializedTarget == null) + { + return; + } + + serializedTarget.Update(); + SerializedProperty values = serializedTarget.FindProperty("values"); + float y = 12f; + for (int index = 0; index < values.arraySize; index++) + { + SerializedProperty property = values.GetArrayElementAtIndex(index); + var label = new GUIContent("Value " + index); + var rect = new Rect(12f, y, position.width - 24f, EditorGUI.GetPropertyHeight(property, label, true)); + FieldRects[index] = rect; + + // Each caller scope must see only edits made through its own property control. + EditorGUI.BeginChangeCheck(); + EditorGUI.PropertyField(rect, property, label, true); + Changed[index] |= EditorGUI.EndChangeCheck(); + y = rect.yMax + 8f; + } + serializedTarget.ApplyModifiedProperties(); + } + + private void OnDisable () + { + if (serializedTarget != null) + { + serializedTarget.Dispose(); + serializedTarget = null; + } + } + } + } +} diff --git a/Assets/MackySoft/MackySoft.SerializeReferenceExtensions/Tests/Editor/SubclassSelectorChangeCheckTests.cs.meta b/Assets/MackySoft/MackySoft.SerializeReferenceExtensions/Tests/Editor/SubclassSelectorChangeCheckTests.cs.meta new file mode 100644 index 0000000..26cf1dc --- /dev/null +++ b/Assets/MackySoft/MackySoft.SerializeReferenceExtensions/Tests/Editor/SubclassSelectorChangeCheckTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: a2e88798ed1ef4568adb6a4f999a9ec7 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: