From c6261229805a94e29add8cde4b0122b80d1cd24f Mon Sep 17 00:00:00 2001 From: Makihiro <13536348+mackysoft@users.noreply.github.com> Date: Fri, 16 Oct 2026 01:14:28 +0900 Subject: [PATCH 1/2] fix(editor): notify inspector change checks after type selection Why Asynchronous type selection applied the serialized value outside the owning property GUI scope, so consumers such as TriInspector retained stale validation. What Changed Keep the pending change in Unity GUI control state so it survives drawer replacement, then report it through the owning property change check. Exercise real popup input for type changes, null selection and cancellation. Impact Inspector consumers receive ordinary field change notifications without a third-party dependency or a new public API. --- .../Editor/SubclassSelectorDrawer.cs | 18 ++ .../SubclassSelectorChangeCheckTests.cs | 209 ++++++++++++++++++ .../SubclassSelectorChangeCheckTests.cs.meta | 11 + 3 files changed, 238 insertions(+) create mode 100644 Assets/MackySoft/MackySoft.SerializeReferenceExtensions/Tests/Editor/SubclassSelectorChangeCheckTests.cs create mode 100644 Assets/MackySoft/MackySoft.SerializeReferenceExtensions/Tests/Editor/SubclassSelectorChangeCheckTests.cs.meta 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: From 76b43028d7f0becf4fd20eee6921ce0a181f0a63 Mon Sep 17 00:00:00 2001 From: Makihiro <13536348+mackysoft@users.noreply.github.com> Date: Fri, 16 Oct 2026 01:14:28 +0900 Subject: [PATCH 2/2] ci(tests): provide graphics for inspector interaction tests Why The new property interaction tests require an Editor GUI view, which cannot be initialized with -nographics. What Changed Run Linux EditMode tests under Xvfb with software OpenGL and document the corresponding local graphics requirement. Impact Both supported Unity versions exercise real popup input in the existing test matrix and remain covered by the final Verification check. --- .github/workflows/tests.yaml | 7 +++++-- AGENTS.md | 4 +++- 2 files changed, 8 insertions(+), 3 deletions(-) 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).