ui: redesign MA parameters UI (#956)

Closes: #860, #720

Editor/Inspector/Parameters/DefaultValueField.cs

@@ -1,5 +1,4 @@
 using System.Globalization;
-using nadena.dev.modular_avatar.core;
 using UnityEditor;
 using UnityEditor.UIElements;
 using UnityEngine;
@@ -15,6 +14,7 @@ namespace nadena.dev.modular_avatar.core.editor
 
         private readonly TextField _visibleField;
         private readonly FloatField _defaultValueField;
+        private readonly DropdownField _boolField;
         private readonly Toggle _hasExplicitDefaultSetField;
 
         public DefaultValueField()
@@ -22,6 +22,11 @@ namespace nadena.dev.modular_avatar.core.editor
             // Hidden binding elements
             _defaultValueField = new FloatField();
             _hasExplicitDefaultSetField = new Toggle();
+            _boolField = new DropdownField();
+
+            _boolField.choices.Add("");
+            _boolField.choices.Add("True");
+            _boolField.choices.Add("False");
 
             _defaultValueField.RegisterValueChangedCallback(
                 evt => UpdateVisibleField(evt.newValue, _hasExplicitDefaultSetField.value));
@@ -50,10 +55,22 @@ namespace nadena.dev.modular_avatar.core.editor
             _defaultValueField.SetEnabled(false);
             _hasExplicitDefaultSetField.style.width = 0;
             _hasExplicitDefaultSetField.SetEnabled(false);
+
+            _boolField.RegisterValueChangedCallback(evt =>
+            {
+                if (evt.newValue == "True")
+                    _defaultValueField.value = 1;
+                else
+                    _defaultValueField.value = 0;
+
+                _hasExplicitDefaultSetField.value = evt.newValue != "";
+            });
+            
             
             style.flexDirection = FlexDirection.Row;
             
             Add(_visibleField);
+            Add(_boolField);
             Add(_defaultValueField);
             Add(_hasExplicitDefaultSetField);
         }
@@ -73,6 +90,16 @@ namespace nadena.dev.modular_avatar.core.editor
             
             var str = hasExplicitValue ? value.ToString(CultureInfo.InvariantCulture) : "";
             _visibleField.SetValueWithoutNotify(str);
+
+            string boolStr;
+            if (!hasExplicitValue)
+                boolStr = "";
+            else if (value > 0.5)
+                boolStr = "True";
+            else
+                boolStr = "False";
+
+            _boolField.SetValueWithoutNotify(boolStr);
         }
     }
 }

Editor/Inspector/Parameters/ParameterConfigDrawer.cs

@@ -2,8 +2,8 @@
 
 using System;
 using UnityEditor;
-using UnityEditor.UIElements;
 using UnityEngine.UIElements;
+using Toggle = UnityEngine.UIElements.Toggle;
 
 namespace nadena.dev.modular_avatar.core.editor.Parameters
 {
@@ -20,105 +20,230 @@ namespace nadena.dev.modular_avatar.core.editor.Parameters
             Localization.UI.Localize(root);
             root.styleSheets.Add(uss);
 
-            var foldout = root.Q<Foldout>();
-            var foldoutLabel = foldout?.Q<Label>();
-            if (foldoutLabel != null)
-            {
-                foldoutLabel.bindingPath = "nameOrPrefix";
-            }
-            
-            var miniDisplay = root.Q<VisualElement>("MiniDisplay");
-            miniDisplay.RemoveFromHierarchy();
-            foldoutLabel.parent.Add(miniDisplay);
-            miniDisplay.styleSheets.Add(uss);
+            // Prototype UI
+            var proot = root.Q<VisualElement>("Root");
+            var type_field = proot.Q<DropdownField>("f-type");
 
-            var isPrefixProp = root.Q<PropertyField>("isPrefix");
-            bool isPrefix = false;
-            Action evaluateMiniDisplay = () =>
+            SetupPairedDropdownField(
+                proot,
+                type_field,
+                proot.Q<VisualElement>("f-sync-type"),
+                proot.Q<VisualElement>("f-is-prefix"),
+                ("Bool", "False", "params.syncmode.Bool"),
+                ("Float", "False", "params.syncmode.Float"),
+                ("Int", "False", "params.syncmode.Int"),
+                ("Not Synced", "False", "params.syncmode.NotSynced"),
+                (null, "True", "params.syncmode.PhysBonesPrefix")
+            );
+
+            var internalParamAccessor = proot.Q<Toggle>("f-internal-parameter");
+            internalParamAccessor.RegisterValueChangedCallback(evt =>
             {
-                miniDisplay.style.display = (isPrefix || foldout.value) ? DisplayStyle.None : DisplayStyle.Flex;
+                if (evt.newValue)
+                    proot.AddToClassList("st-internal-parameter");
+                else
+                    proot.RemoveFromClassList("st-internal-parameter");
+            });
+
+            var remapTo = proot.Q<TextField>("f-remap-to");
+            var defaultParam = proot.Q<Label>("f-default-param");
+            var name = proot.Q<TextField>("f-name");
+            var remapToInner = remapTo.Q<TextElement>();
+
+            Action updateDefaultParam = () =>
+            {
+                if (string.IsNullOrWhiteSpace(remapTo.value))
+                    defaultParam.text = name.value;
+                else
+                    defaultParam.text = "";
             };
-            
-            
-            foldout.RegisterValueChangedCallback(evt => evaluateMiniDisplay());
-            
-            isPrefixProp.RegisterValueChangeCallback(evt =>
-            {
-                var value = evt.changedProperty.boolValue;
-                if (value)
-                {
-                    root.AddToClassList("ParameterConfig__isPrefix_true");
-                    root.RemoveFromClassList("ParameterConfig__isPrefix_false");
-                }
-                else
-                {
-                    root.AddToClassList("ParameterConfig__isPrefix_false");
-                    root.RemoveFromClassList("ParameterConfig__isPrefix_true");
-                }
 
-                isPrefix = value;
-                evaluateMiniDisplay();
-            });
-            
-            var syncTypeProp = root.Q<PropertyField>("syncType");
-            // TODO: This callback is not actually invoked on initial bind...
-            syncTypeProp.RegisterValueChangeCallback(evt =>
-            {
-                var value = (ParameterSyncType) evt.changedProperty.enumValueIndex;
-                if (value == ParameterSyncType.NotSynced)
-                {
-                    root.AddToClassList("ParameterConfig__animatorOnly_true");
-                    root.RemoveFromClassList("ParameterConfig__animatorOnly_false");
-                }
-                else
-                {
-                    root.AddToClassList("ParameterConfig__animatorOnly_false");
+            name.RegisterValueChangedCallback(evt => { updateDefaultParam(); });
 
-                    root.RemoveFromClassList("ParameterConfig__animatorOnly_true");
-                }
-            });
+            remapTo.RegisterValueChangedCallback(evt => { updateDefaultParam(); });
 
-            /*
-            var overridePlaceholder = root.Q<Toggle>("overridePlaceholder");
-            overridePlaceholder.labelElement.AddToClassList("ndmf-tr");
-            overridePlaceholder.SetEnabled(false);
-            */
-            
-            var remapTo = root.Q<PropertyField>("remapTo");
-            var remapToPlaceholder = root.Q<TextField>("remapToPlaceholder");
-            remapToPlaceholder.labelElement.AddToClassList("ndmf-tr");
-            remapToPlaceholder.SetEnabled(false);
-            
-            Localization.UI.Localize(remapToPlaceholder.labelElement);
-            
-            root.Q<PropertyField>("internalParameter").RegisterValueChangeCallback(evt =>
-            {
-                remapTo.style.display = evt.changedProperty.boolValue ? DisplayStyle.None : DisplayStyle.Flex;
-                remapToPlaceholder.style.display = evt.changedProperty.boolValue ? DisplayStyle.Flex : DisplayStyle.None;
-            });
-            
-            
-            // This is a bit of a hack, but I'm not sure of another way to properly align property labels with a custom
-            // field, when we only want to manipulate a subset of fields on an object...
-            var defaultValueField = root.Q<VisualElement>("innerDefaultValueField"); // create ahead of time so it's bound...
-            
-            // Then move it into the property field once the property field has created its inner controls
-            var defaultValueProp = root.Q<PropertyField>("defaultValueProp");
-            defaultValueProp.RegisterCallback<GeometryChangedEvent>(evt =>
-            {
-                var floatField = defaultValueProp.Q<FloatField>();
-                var innerField = floatField?.Q<DefaultValueField>();
+            defaultParam.RemoveFromHierarchy();
+            remapToInner.Add(defaultParam);
 
-                if (floatField != null && innerField == null)
-                {
-                    defaultValueField.RemoveFromHierarchy();
-                    floatField.contentContainer.Add(defaultValueField);
-                }
-            });
+            updateDefaultParam();
 
             return root;
         }
 
+        private interface Accessor
+        {
+            Action<string> OnValueChanged { get; set; }
+            string Value { get; set; }
+        }
+
+        private class ToggleAccessor : Accessor
+        {
+            private readonly Toggle _toggle;
+
+            public ToggleAccessor(Toggle toggle)
+            {
+                _toggle = toggle;
+                _toggle.RegisterValueChangedCallback(evt => OnValueChanged?.Invoke(evt.newValue.ToString()));
+            }
+
+            public Action<string> OnValueChanged { get; set; }
+
+            public string Value
+            {
+                get => _toggle.value.ToString();
+                set => _toggle.value = value == "True";
+            }
+        }
+
+        private class DropdownAccessor : Accessor
+        {
+            private readonly DropdownField _dropdown;
+
+            public DropdownAccessor(DropdownField dropdown)
+            {
+                _dropdown = dropdown;
+                _dropdown.RegisterValueChangedCallback(evt => OnValueChanged?.Invoke(evt.newValue));
+            }
+
+            public Action<string> OnValueChanged { get; set; }
+
+            public string Value
+            {
+                get => _dropdown.value;
+                set => _dropdown.value = value;
+            }
+        }
+
+        private Accessor GetAccessor(VisualElement elem)
+        {
+            var toggle = elem.Q<Toggle>();
+            if (toggle != null) return new ToggleAccessor(toggle);
+
+            var dropdown = elem.Q<DropdownField>();
+            if (dropdown != null)
+            {
+                return new DropdownAccessor(dropdown);
+            }
+
+            throw new ArgumentException("Unsupported element type");
+        }
+
+        private void SetupPairedDropdownField(
+            VisualElement root,
+            DropdownField target,
+            VisualElement v_type,
+            VisualElement v_pbPrefix,
+            // p1, p2, localization key
+            params (string, string, string)[] choices
+        )
+        {
+            var p_type = GetAccessor(v_type);
+            var p_prefix = GetAccessor(v_pbPrefix);
+
+            v_type.style.display = DisplayStyle.None;
+            v_pbPrefix.style.display = DisplayStyle.None;
+
+            for (var i = 0; i < choices.Length; i++) target.choices.Add("" + i);
+
+            target.formatListItemCallback = s_n =>
+            {
+                if (int.TryParse(s_n, out var n) && n >= 0 && n < choices.Length)
+                {
+                    return Localization.S(choices[n].Item3);
+                }
+                else
+                {
+                    return "";
+                }
+            };
+            target.formatSelectedValueCallback = target.formatListItemCallback;
+
+            var inLoop = false;
+            string current_type_class = null;
+
+            target.RegisterValueChangedCallback(evt =>
+            {
+                if (inLoop) return;
+
+                if (int.TryParse(evt.newValue, out var n) && n >= 0 && n < choices.Length)
+                {
+                    p_type.Value = choices[n].Item1;
+                    p_prefix.Value = choices[n].Item2;
+                }
+                else
+                {
+                    p_type.Value = "";
+                    p_prefix.Value = "";
+                }
+            });
+
+            p_type.OnValueChanged = s =>
+            {
+                inLoop = true;
+                try
+                {
+                    if (!string.IsNullOrWhiteSpace(s))
+                    {
+                        var new_class = "st-ty-" + s.Replace(" ", "-");
+
+                        root.RemoveFromClassList(current_type_class);
+                        current_type_class = null;
+
+                        root.AddToClassList(new_class);
+                        current_type_class = new_class;
+                    }
+
+                    if (string.IsNullOrEmpty(s)) return;
+
+                    for (var i = 0; i < choices.Length; i++)
+                        if (choices[i].Item1 == s && (choices[i].Item2 == null || choices[i].Item2 == p_prefix.Value))
+                        {
+                            target.SetValueWithoutNotify("" + i);
+                            break;
+                        }
+                }
+                finally
+                {
+                    inLoop = false;
+                }
+            };
+
+            p_prefix.OnValueChanged = s =>
+            {
+                inLoop = true;
+                try
+                {
+                    if (string.IsNullOrEmpty(s)) return;
+
+                    if (bool.TryParse(s, out var b))
+                    {
+                        if (b) root.AddToClassList("st-pb-prefix");
+                        else root.RemoveFromClassList("st-pb-prefix");
+                    }
+
+                    for (var i = 0; i < choices.Length; i++)
+                        if ((choices[i].Item1 == null || choices[i].Item1 == p_type.Value) && choices[i].Item2 == s)
+                        {
+                            target.SetValueWithoutNotify("" + i);
+                            break;
+                        }
+                }
+                finally
+                {
+                    inLoop = false;
+                }
+            };
+
+            inLoop = true;
+            for (var i = 0; i < choices.Length; i++)
+                if (choices[i].Item1 == p_type.Value && choices[i].Item2 == p_prefix.Value)
+                {
+                    target.SetValueWithoutNotify("" + i);
+                    break;
+                }
+
+            inLoop = false;
+        }
     }
 }
 #endif

Editor/Inspector/Parameters/ParameterConfigDrawer.uxml

@@ -1,47 +1,43 @@
 <ui:UXML
-        xmlns:ui="UnityEngine.UIElements" 
-        xmlns:engine="UnityEditor.UIElements"
-        xmlns:ma="nadena.dev.modular_avatar.core.editor" 
+        xmlns:ui="UnityEngine.UIElements"
+        xmlns:ma="nadena.dev.modular_avatar.core.editor"
         editor-extension-mode="False"
 >
-    <ui:VisualElement name="MiniDisplay">
-        <ui:Label text="merge_parameter.ui.defaultValue" class="ndmf-tr"/>
-        <ma:DefaultValueField/>
-        <ui:Label text="merge_parameter.ui.saved" class="ndmf-tr"/>
-        <ui:Toggle binding-path="saved"/>
+    <ui:VisualElement name="Root">
+        <ui:VisualElement class="horizontal no-label">
+            <ui:TextField binding-path="nameOrPrefix" label="merge_parameter.ui.name" name="f-name" class="ndmf-tr"/>
+            <ui:DropdownField name="f-type"/>
+        </ui:VisualElement>
+
+        <ui:Toggle binding-path="isPrefix" name="f-is-prefix"/>
+        <ui:DropdownField binding-path="syncType" name="f-sync-type"/>
+
+        <ui:VisualElement class="horizontal small-label">
+            <ui:Toggle binding-path="internalParameter" name="f-internal-parameter"
+                       text="merge_parameter.ui.internalParameter" class="ndmf-tr no-left-margin"/>
+            <ui:VisualElement class="v-separator hide-with-internal-param">
+                <ui:VisualElement/>
+            </ui:VisualElement>
+            <ui:Label text="merge_parameter.ui.remapTo"
+                      class="ndmf-tr inner-label hide-with-internal-param no-left-margin"/>
+            <ui:TextField name="f-remap-to" binding-path="remapTo" class="hide-with-internal-param"/>
+            <ui:Label name="f-default-param" text="test test test"/>
+        </ui:VisualElement>
+
+        <ui:VisualElement class="horizontal small-label st-pb-prefix__hide">
+            <ui:VisualElement class="horizontal">
+                <ui:Label text="merge_parameter.ui.defaultValue" class="ndmf-tr no-left-margin"/>
+                <ma:DefaultValueField/>
+            </ui:VisualElement>
+            <ui:VisualElement class="v-separator">
+                <ui:VisualElement/>
+            </ui:VisualElement>
+            <ui:Toggle binding-path="saved" text="merge_parameter.ui.saved" class="ndmf-tr"/>
+            <ui:Toggle binding-path="localOnly" text="merge_parameter.ui.localOnly" class="ndmf-tr"/>
+            <ui:Toggle binding-path="m_overrideAnimatorDefaults" text="merge_parameter.ui.overrideAnimatorDefaults"
+                       class="ndmf-tr"/>
+        </ui:VisualElement>
+
     </ui:VisualElement>
-    
-    <ui:Foldout name="ParameterConfigRoot" text="(placeholder)" value="false">
-        <engine:PropertyField binding-path="nameOrPrefix" label="merge_parameter.ui.name" name="f-name" class="ndmf-tr ParameterConfig__isPrefix_falseOnly" />
-        <engine:PropertyField binding-path="nameOrPrefix" label="merge_parameter.ui.prefix" name="f-prefix" class="ndmf-tr ParameterConfig__isPrefix_trueOnly" />
-        <engine:PropertyField binding-path="remapTo" label="merge_parameter.ui.remapTo" name="remapTo" class="ndmf-tr" />
-        <ui:TextField label="merge_parameter.ui.remapTo" text="merge_parameter.ui.remapTo.automatic"
-                      name="remapToPlaceholder" enabled="false"
-                      class="ndmf-tr unity-base-field__aligned disabledPlaceholder"/>
-
-        <!-- this field is not visible until it's moved into the PropertyField below -->
-        <ma:DefaultValueField 
-                name="innerDefaultValueField"
-                class="unity-base-field__input unity-property-field__input"
-        /> 
-        
-        <engine:PropertyField binding-path="defaultValue" name="defaultValueProp" label="merge_parameter.ui.defaultValue" class="ndmf-tr ParameterConfig__isPrefix_falseOnly">
-            
-        </engine:PropertyField>
-        
-        <engine:PropertyField binding-path="saved" label="merge_parameter.ui.saved" class="ndmf-tr ParameterConfig__isPrefix_falseOnly" />"
-    
-        <engine:PropertyField binding-path="internalParameter" label="merge_parameter.ui.internalParameter" name="internalParameter" class="ndmf-tr" />
-        <engine:PropertyField binding-path="isPrefix" label="merge_parameter.ui.isPrefix" name="isPrefix" class="ndmf-tr" />
-
-        <engine:PropertyField binding-path="syncType" label="merge_parameter.ui.syncType"
-                              class="ParameterConfig__isPrefix_falseOnly ndmf-tr" name="syncType"/>
-        <engine:PropertyField binding-path="m_overrideAnimatorDefaults" name="overrideDefaults"
-                              label="merge_parameter.ui.overrideAnimatorDefaults"
-                              class="ParameterConfig__isPrefix_falseOnly ndmf-tr"/>
-        <!-- <ui:Toggle label="merge_parameter.ui.overrideAnimatorDefaults"  value="true" enabled="false" name="overridePlaceholder" class="ParameterConfig__isPrefix_falseOnly ParameterConfig__animatorOnly_trueOnly ndmf-tr" /> -->
-        
-        <engine:PropertyField binding-path="localOnly" label="merge_parameter.ui.localOnly" class="ParameterConfig__isPrefix_falseOnly ParameterConfig__animatorOnly_falseOnly ndmf-tr" />
-    </ui:Foldout>
 
 </ui:UXML>

Editor/Inspector/Parameters/Parameters.uss

@@ -1,116 +1,136 @@
 VisualElement {}
 
-.ParameterConfig__isPrefix_true .ParameterConfig__isPrefix_falseOnly {
-    display: none;
+
+/* I hate CSS precedence rules... */
+.horizontal .no-left-margin {
+    margin-left: 0 !important;
 }
 
-.ParameterConfig__isPrefix_false .ParameterConfig__isPrefix_trueOnly {
-    display: none;
+.horizontal .no-left-margin.unity-label {
+    margin-left: 0 !important;
 }
 
-.ParameterConfig__animatorOnly_true .ParameterConfig__animatorOnly_falseOnly {
-    display: none;
+.horizontal .no-left-margin Label.unity-label {
+    margin-left: 0 !important;
 }
 
-.ParameterConfig__animatorOnly_false .ParameterConfig__animatorOnly_trueOnly {
-    display: none;
+.horizontal > Label {
+    height: auto;
 }
 
-#defaultValueGroup {
-    display: flex;
-    flex-direction: row;
+.horizontal > * {
+    margin-top: 0;
+    margin-bottom: 0;
 }
 
-#defaultValueGroup > .unity-base-field__input {
-    flex-grow: 1;
-}
-#defaultValueGroup > .unity-base-field__input > * {
-    flex-grow: 1;
+.v-separator {
+    width: 1px;
+    height: 100%;
+    margin-left: 8px;
+    margin-right: 8px;
+    justify-content: center;
+    align-content: center;
+    flex-shrink: 0;
 }
 
-#defaultValueProp > FloatField > FloatInput {
-    display: none;
+.v-separator VisualElement {
+    width: 100%;
+    height: 80%;
+    background-color: rgba(0, 0, 0, 0.4);
 }
 
-#ParameterConfigRoot > DefaultValueField {
-    display: none;
-}
-
-#innerDefaultValueField {
-    flex-grow: 1;
-}
-
-DefaultValueField > TextField {
-    flex-grow: 1;
-    margin-left: 0;
-}
-
-#MiniDisplay {
-    flex-direction: row;
-    align-self: flex-end;
-}
-
-#MiniDisplay > * {
-    align-self: center;
-}
-
-#MiniDisplay > DefaultValueField {
-    max-width: 60px;
-    flex-grow: 0;
-    margin-right: 10px;
-}
-
-
-#MiniDisplay > DefaultValueField TextElement {
+.horizontal TextField {
     margin-left: 0px;
 }
 
-
-#MiniDisplay > Toggle {
-    margin-right: 5px;
+.horizontal TextField Label.unity-label {
+    margin-left: 0px !important;
 }
 
-#ParameterConfigRoot > Toggle .unity-toggle__text {
-    flex-grow: 1;
-}
-
-#UnregisteredParameters #unity-list-view__footer {
-    display: none;
-}
-
-#UnregisteredParameters Label {
+.horizontal Label {
+    padding-top: 0;
+    padding-bottom: 0;
     align-self: center;
 }
 
-.DetectedParameter {
+.horizontal {
     flex-direction: row;
-    margin-top: 2px;
-    margin-bottom: 2px;
+    align-content: center;
 }
 
-.DetectedParameter > Label {
+.horizontal > * {
+    height: 100%;
+}
+
+.no-label Label.unity-base-field__label {
+    display: none;
+}
+
+#Root .horizontal #f-rename-destination {
     flex-grow: 1;
 }
 
-.SourceButton {
-    flex-grow:0;
-    align-self: flex-end;
-    height: 24px;
-    width: 24px;
-    padding: 1px;
+.inner-label > Label {
+    margin-left: 6px;
+    display: none;
 }
 
-/* Vertically align the reorder handle with the foldout chevron */
-#Parameters #unity-list-view__reorderable-handle {
-    padding-top: 10px;
+.small-label Label.unity-label {
+    min-width: 0;
+    margin-left: 4px;
 }
 
-#ParameterConfigRoot .unity-foldout__input > #unity-checkmark {
-    margin-top: 4px;
-    align-self: flex-start;
+VisualElement.small-label > * {
+    flex-grow: 0;
 }
 
-#ParameterConfigRoot .unity-foldout__input > Label {
-    margin-top: 4px;
-    align-self: flex-start;
+VisualElement.small-label > PropertyField {
+    flex-direction: row;
+}
+
+#Root #f-name {
+    flex-grow: 1;
+}
+
+#Root DefaultValueField {
+    width: 60px;
+    flex-grow: 0;
+}
+
+.st-internal-parameter .hide-with-internal-param {
+    display: none;
+}
+
+DefaultValueField DropdownField {
+    display: none;
+}
+
+.st-ty-Bool DefaultValueField DropdownField {
+    display: flex;
+}
+
+.st-ty-Bool DefaultValueField TextField {
+    display: none;
+}
+
+.st-ty-NotSynced DefaultValueField {
+    display: none;
+}
+
+.st-pb-prefix .st-pb-prefix__hide {
+    display: none;
+}
+
+#f-remap-to {
+    flex-grow: 1;
+}
+
+/** Ghostly text for the renameTo text box **/
+Label#f-default-param {
+    position: absolute;
+    width: 100%;
+    height: 100%
+    margin: 0 0 0 0;
+    overflow: hidden;
+    color: rgba(255, 255, 255, 0.4) !important;
 }