Search Unity

Extensibility issues

Discussion in 'UIElements' started by SudoCat, Jun 25, 2019.

  1. SudoCat

    SudoCat

    Joined:
    Feb 19, 2013
    Posts:
    17
    I've been playing around with UIElements for the past few weeks. First off, as someone who started their career as a web developer, I really love what y'all have made here. Making new editor tools with this has been really enjoyable.

    Unfortunately, I've been running into a few issues when trying to extend functionality in various places. Lots of the API is internal, which causes some friction depending on what you're trying to do.

    There's been two particular cases I've encountered.


    1. Trying to create new fields. To save on rewriting a lot of similar code, it seemed like a good idea to try extending from BaseField<T>, as it handles much of the labels, binding, valueChanged, focus order and such automatically. This plan was scuppered, as theBaseField<T>.visualInput is an internal member.


    2. Trying to extend binding. Boy, this one was not an easy thing to navigate. Unfortunately, the docs are still pretty sparse on extending bindings, to the point where I'm starting to question whether they were even meant to be extended, or if I'm doing things I shouldn't be.

    - All the binding event classes are internal. This makes it impossible to create a custom alternative to say, PropertyField or Inspector. Both of these classes rely on using the binding events to bind the more complicated property types, such as arrays. I was hoping to implement some custom array editors that would auto update, but this has been proving rather nightmarish when using bindings. The only way to use the binding events is via reflection, which is pretty nasty.

    - All of the IBinding implementations are private, making it impossible to interact with the binding (without reflection). Admittedly, I haven't found an explicit use case for this just yet, but it would be nice to be able to access the SerializedObject/SerializedProperty directly at times.


    Any hopes of getting these public? Or am I simply going about this the wrong way?

    EDIT: Having a further look, also appears most things to do with ViewData are also internal (e.g.
    VisualElement.OnViewDataReady()). Again, this makes it pretty hard to make custom uses of ViewData.
     
    Last edited: Jun 25, 2019
  2. antoine-unity

    antoine-unity

    Unity Technologies

    Joined:
    Sep 10, 2015
    Posts:
    96
    Hello,

    1. It's true that this is not easily feasible. Would you mind sharing an idea for a custom field that we could try to replicate and essentially go through the same pain points as yourself ?

    2. We are very aware of the situation with bindings and we are planning to do something about it for the 2020 release cycle. Mainly we will open things up in order to support both SerializedObject/SerializedProperty and a different solution in the runtime for games/apps. This should make more endpoints public with the goal to decouple elements from a specific data binding backend.

    3. We were not very happy with the state of ViewData when the API was experimental. It might make its way back at some point but we don't consider it as critical as other topics in terms of extensibility.

    I will forward this to team who might more insights about some the specific questions you have.

    Thanks for the detailed feedback.
     
    SudoCat likes this.
  3. SudoCat

    SudoCat

    Joined:
    Feb 19, 2013
    Posts:
    17
    Thanks for getting back so promptly!

    1. Sure thing. Right now I'm trying to create a field similar to enum picker, but with dynamic string values.
    In HTML terms, a <Select> element. I haven't dug too deep just yet (as said, I stumbled at the first hurdle with BaseField), but I'm presuming that implementing it the HTML way (with Option child elements) was going to be impractical (I don't think you can add type constraints to children?).
    So my plan was just to provide a list of string values to start, and allow adding comma delimited values in the XML via an "options" attribute.

    Of course, most of that functionality could be handled with an enum picker for most cases, but the lists I'll be populating it with will need to be dynamic (in this particular instance, it's selecting from a list of Playable Characters).

    2. Glad to hear it's on the horizon! Sounds like it'll be a delight once you've ironed out the details. Very stoked to make in-game UI using the tools I know and love so well.

    3. That makes sense! It looked a little rough around the edges, so I'd mostly steered clear of it for now, although it'll undoubtedly prove useful one day!
     
  4. SudoCat

    SudoCat

    Joined:
    Feb 19, 2013
    Posts:
    17
    I've just about wrapped up a simple implementation for the simple text select.

    I got everything working and running alongside other fields nicely, but there's a lot of boilerplate code (in some cases, literally copy+pasted from BaseField).

    There were also some settings configured in the BaseField constructor that I'm unable to replicate.

    Code (csharp):
    1. isCompositeRoot = true;
    2. ...
    3. ...
    4. excludeFromFocusRing = true;
    5. delegatesFocus = true;
    delegatesFocus is public, but it depends on isCompositeRoot being true, which is internal. excludeFromFocusRing is also internal.

    I haven't quite finished reading all the areas of the source code these effect, so I'm not certain what these do. From what I can see, it appears my custom field element does not have the same focus behaviour as the builtt-in ones (my label and control are receive independent focus, unlike those which extend BaseField).

    The focus behaviour on click is also not ideal.

    upload_2019-6-25_18-23-15.png

    vs

    upload_2019-6-25_18-26-59.png

    upload_2019-6-25_18-27-10.png

    Happy to share the code for this element if it will help!
     
  5. SudoCat

    SudoCat

    Joined:
    Feb 19, 2013
    Posts:
    17
    Later last night I did a bit more work on this. Most of the visual focus issues were resolved with some extra USS, but it still didn't quite match the behaviours of the built-in fields.

    Consequently I went back and tried to convert my existing SelectField class to inherit from BaseField<string>. After some faffing, this provided a much better end result, with pretty much identical behaviour to the existing one.

    The code to accomplish it wasn't quite ideal. As mentioned, with the visualInput property creates some issues, as you cannot reference it directly.

    To get around this, I used uQuery to find the element created by the BaseField class, and assigned it to a local variable in my SelectField class. This allowed me to work around the issue, but I'm not sure it's the tidiest solution.

    Here's my own implementation, not inheriting from BaseField<T>

    Code (csharp):
    1.  
    2. public class SelectField : VisualElement, IBindable, INotifyValueChanged<string>
    3. {
    4.     private readonly TextElement _textElement;
    5.  
    6.     private string[] _options;
    7.  
    8.     public IBinding binding { get; set; }
    9.     public string bindingPath { get; set; }
    10.  
    11.     private string _value;
    12.  
    13.     public string value
    14.     {
    15.         get => _value;
    16.         set
    17.         {
    18.             if (!EqualityComparer<string>.Default.Equals(_value, value))
    19.             {
    20.                 if (panel != null)
    21.                 {
    22.                     using (var evt = ChangeEvent<string>.GetPooled(_value, value))
    23.                     {
    24.                         evt.target = this;
    25.                         SetValueWithoutNotify(value);
    26.                         SendEvent(evt);
    27.                     }
    28.                 }
    29.                 else
    30.                 {
    31.                     SetValueWithoutNotify(value);
    32.                 }
    33.             }
    34.         }
    35.     }
    36.  
    37.     private VisualElement _visualInput;
    38.  
    39.     private VisualElement VisualInput
    40.     {
    41.         get => _visualInput;
    42.         set
    43.         {
    44.             if (_visualInput != null)
    45.             {
    46.                 if (_visualInput.parent == this)
    47.                     _visualInput.RemoveFromHierarchy();
    48.                 _visualInput = null;
    49.             }
    50.  
    51.             if (value != null)
    52.             {
    53.                 _visualInput = value;
    54.             }
    55.             else
    56.             {
    57.                 _visualInput = new VisualElement()
    58.                 {
    59.                     pickingMode = PickingMode.Ignore
    60.                 };
    61.             }
    62.             _visualInput.focusable = true;
    63.             _visualInput.AddToClassList(BaseField<string>.inputUssClassName);
    64.             Add(_visualInput);
    65.         }
    66.     }
    67.  
    68.     public Label LabelElement { get; }
    69.  
    70.     public string Label
    71.     {
    72.         get => LabelElement.text;
    73.         set
    74.         {
    75.             if (LabelElement.text == value)
    76.                 return;
    77.  
    78.             LabelElement.text = value;
    79.  
    80.             if (string.IsNullOrEmpty(LabelElement.text))
    81.             {
    82.                 AddToClassList(BaseField<string>.noLabelVariantUssClassName);
    83.                 LabelElement.RemoveFromHierarchy();
    84.             }
    85.             else if (!Contains(LabelElement))
    86.             {
    87.                 Insert(0, LabelElement);
    88.                 RemoveFromClassList(BaseField<string>.noLabelVariantUssClassName);
    89.             }
    90.         }
    91.     }
    92.  
    93.     public SelectField(string[] options, string label = null)
    94.     {
    95.         focusable = true;
    96.         tabIndex = 0;
    97.  
    98.         AddToClassList(BaseField<string>.ussClassName);
    99.         AddToClassList(EnumField.ussClassName);
    100.  
    101.         // Label
    102.         LabelElement = new Label
    103.         {
    104.             focusable = true,
    105.             tabIndex = -1
    106.         };
    107.         LabelElement.AddToClassList(BaseField<string>.labelUssClassName);
    108.         if (label != null)
    109.         {
    110.             Label = label;
    111.         }
    112.         else
    113.         {
    114.             AddToClassList(BaseField<string>.noLabelVariantUssClassName);
    115.         }
    116.  
    117.         // Control
    118.         VisualInput = new VisualElement
    119.         {
    120.             pickingMode = PickingMode.Ignore
    121.         };
    122.  
    123.         VisualInput.AddToClassList(EnumField.inputUssClassName);
    124.  
    125.         _textElement = new TextElement
    126.         {
    127.             pickingMode = PickingMode.Ignore
    128.         };
    129.  
    130.         VisualInput.Add(_textElement);
    131.  
    132.         _options = options;
    133.         SetValueWithoutNotify(_options[0]);
    134.     }
    135.  
    136.     public void SetValueWithoutNotify(string newValue)
    137.     {
    138.         _value = newValue;
    139.         _textElement.text = newValue;
    140.     }
    141.  
    142.     public void SetOptions(string[] options, int selected = 0)
    143.     {
    144.         _options = options;
    145.         value = _options[selected];
    146.     }
    147.  
    148.     protected override void ExecuteDefaultActionAtTarget(EventBase evt)
    149.     {
    150.         base.ExecuteDefaultActionAtTarget(evt);
    151.  
    152.         if (evt == null)
    153.             return;
    154.  
    155.         var showMenu = false;
    156.         if (evt is KeyDownEvent kde)
    157.         {
    158.             if (kde.keyCode == KeyCode.Space ||
    159.                 kde.keyCode == KeyCode.KeypadEnter ||
    160.                 kde.keyCode == KeyCode.Return)
    161.             {
    162.                 showMenu = true;
    163.             }
    164.         }
    165.         else if ((evt as MouseDownEvent)?.button == (int)MouseButton.LeftMouse)
    166.         {
    167.             var mde = (MouseDownEvent)evt;
    168.             if (VisualInput.ContainsPoint(VisualInput.WorldToLocal(mde.mousePosition)))
    169.             {
    170.                 showMenu = true;
    171.             }
    172.         }
    173.  
    174.         if (showMenu)
    175.         {
    176.             ShowMenu();
    177.             evt.StopPropagation();
    178.         }
    179.     }
    180.  
    181.     private void ShowMenu()
    182.     {
    183.         if (_options == null)
    184.             return;
    185.  
    186.         var menu = new GenericMenu();
    187.  
    188.         int selectedIndex = Array.IndexOf(_options, value);
    189.  
    190.         for (int i = 0; i < _options.Length; ++i)
    191.         {
    192.             bool isSelected = selectedIndex == i;
    193.             menu.AddItem(new GUIContent(_options[i]), isSelected, ChangeValueFromMenu, _options[i]);
    194.         }
    195.  
    196.         var menuPosition = new Vector2(VisualInput.layout.xMin, VisualInput.layout.height);
    197.         menuPosition = this.LocalToWorld(menuPosition);
    198.         var menuRect = new Rect(menuPosition, Vector2.zero);
    199.         menu.DropDown(menuRect);
    200.     }
    201.  
    202.     private void ChangeValueFromMenu(object menuItem)
    203.     {
    204.         value = menuItem as string;
    205.     }
    206. }
    207.  
    And here's the new and improved version, inheriting from BaseField<string>

    Code (csharp):
    1.  
    2.  
    3. public class SelectField : BaseField<string>
    4. {
    5.     private readonly TextElement _textElement;
    6.  
    7.     private string[] _options;
    8.  
    9.     private readonly VisualElement _visualInput;
    10.  
    11.     public SelectField(string[] options, string label = null) : base(label, null)
    12.     {
    13.         AddToClassList(EnumField.ussClassName);
    14.  
    15.         labelElement.AddToClassList(EnumField.labelUssClassName);
    16.         labelElement.focusable = false;
    17.  
    18.         _visualInput = this.Q<VisualElement>(string.Empty, inputUssClassName);
    19.  
    20.         _visualInput.AddToClassList(EnumField.inputUssClassName);
    21.  
    22.         _textElement = new TextElement
    23.         {
    24.             pickingMode = PickingMode.Ignore
    25.         };
    26.  
    27.         _visualInput.Add(_textElement);
    28.  
    29.         _options = options;
    30.         SetValueWithoutNotify(_options[0]);
    31.     }
    32.  
    33.     public sealed override void SetValueWithoutNotify(string newValue)
    34.     {
    35.         base.SetValueWithoutNotify(newValue);
    36.         _textElement.text = newValue;
    37.     }
    38.  
    39.     public void SetOptions(string[] options, int selected = 0)
    40.     {
    41.         _options = options;
    42.         value = _options[selected];
    43.     }
    44.  
    45.     protected override void ExecuteDefaultActionAtTarget(EventBase evt)
    46.     {
    47.         base.ExecuteDefaultActionAtTarget(evt);
    48.  
    49.         if (evt == null)
    50.             return;
    51.  
    52.         var showMenu = false;
    53.         if (evt is KeyDownEvent kde)
    54.         {
    55.             if (kde.keyCode == KeyCode.Space ||
    56.                 kde.keyCode == KeyCode.KeypadEnter ||
    57.                 kde.keyCode == KeyCode.Return)
    58.             {
    59.                 showMenu = true;
    60.             }
    61.         }
    62.         else if ((evt as MouseDownEvent)?.button == (int)MouseButton.LeftMouse)
    63.         {
    64.             var mde = (MouseDownEvent)evt;
    65.             if (_visualInput.ContainsPoint(_visualInput.WorldToLocal(mde.mousePosition)))
    66.             {
    67.                 showMenu = true;
    68.             }
    69.         }
    70.  
    71.         if (showMenu)
    72.         {
    73.             ShowMenu();
    74.             evt.StopPropagation();
    75.         }
    76.     }
    77.  
    78.     private void ShowMenu()
    79.     {
    80.         if (_options == null)
    81.             return;
    82.  
    83.         var menu = new GenericMenu();
    84.  
    85.         int selectedIndex = Array.IndexOf(_options, value);
    86.  
    87.         for (int i = 0; i < _options.Length; ++i)
    88.         {
    89.             bool isSelected = selectedIndex == i;
    90.             menu.AddItem(new GUIContent(_options[i]), isSelected, ChangeValueFromMenu, _options[i]);
    91.         }
    92.  
    93.         var menuPosition = new Vector2(_visualInput.layout.xMin, _visualInput.layout.height);
    94.         menuPosition = this.LocalToWorld(menuPosition);
    95.         var menuRect = new Rect(menuPosition, Vector2.zero);
    96.         menu.DropDown(menuRect);
    97.     }
    98.  
    99.     private void ChangeValueFromMenu(object menuItem)
    100.     {
    101.         value = menuItem as string;
    102.     }
    103. }
    104.  
    105.  
     
  6. antoine-unity

    antoine-unity

    Unity Technologies

    Joined:
    Sep 10, 2015
    Posts:
    96
    Hello,

    Thanks for the follow up. It seems like the PopupField<T> might implement some of the base functionality you need. It could require less work in terms of inheriting behaviour (although a non generic implementation will make it possible to use it in UXML).

    In 2019.2+ see Window > UI > UIElements Samples window and find the PopupField example in "Choice Fields".
     
    SudoCat likes this.
  7. SudoCat

    SudoCat

    Joined:
    Feb 19, 2013
    Posts:
    17
    Ah-ha! Thank you. I had been using the docs UXML elements reference page, so was unaware of that option.
     
  8. SudoCat

    SudoCat

    Joined:
    Feb 19, 2013
    Posts:
    17
    Running into some fun new extensibility limitations today!

    I'm attempting to create some new custom events, but unfortunately it seems that the propagation property and related enum have been marked internal.

    Would someone be able to explain the rationale behind this choice? Considering this property is accessed across ~20 of the built-in events, I can't see why we're not allowed to set this on our own events.

    While on the subject of events, could anyone explain the sporadically inconsistent API for adding events? For 90% of use cases, we can rely on RegisterCallback, which is lovely to use, and works great, especially with passing extra callback args. Then in seemingly arbitrary edge cases, we have to use awkward alternative solutions (Looking at you, Button.clickable.clicked!).

    This just seems... pretty bad, honestly. I think the countless questions I've read asking "how to add click action to button" really proves the point that this is an unintuitive API. I understand it's unlikely to change now, but it'd be nice to understand why it's so convoluted.
     
  9. patrickf

    patrickf

    Unity Technologies

    Joined:
    Oct 24, 2016
    Posts:
    43
    Hi!
    We are well aware of the awkwardness of the Button. I will not try to explain why it is like it is, but please know that we will try to improve it in upcoming releases.

    As for the propagation property, it is internal because we did not polish the use case where people would create their own event types. I added a task to our backlog for this.
     
    SudoCat likes this.
  10. SudoCat

    SudoCat

    Joined:
    Feb 19, 2013
    Posts:
    17
    Haha okay, I did manage to dig up a thread in here somewhere that dived into some of the implementation reasons, alas I've misplaced it now.

    Darn, that's a bugger. Well I've ended up being a filthy bastard and setting it through reflection for now.

    I do really love the new UI Elements system, but it is a darn shame that it still feels quite rough around the edges. I look forward to seeing it grow over the coming years!
     
  11. DGordon

    DGordon

    Joined:
    Dec 8, 2013
    Posts:
    397
    Just flagging that you guys should assume people will want to make custom editors for just about anything we can think of. The more control you give to us, the better. If you want to err, err on the side of giving us _too much_ control. I'd rather not have to resort to pouring through docs trying to find how to use reflection to do something that really ... should have just been made accessible in the first place.

    A good majority of what I do ends up being editor scripting to improve the workflow for whatever I can find. Its very frustrating when I'm locked out of something because Unity says so. Please give us the freedom to extend / manipulate the editor as much as we see fit, unless its required to make Unity actually work.

    Just my two cents. Thanks!
     
    SudoCat likes this.
  12. patrickf

    patrickf

    Unity Technologies

    Joined:
    Oct 24, 2016
    Posts:
    43
    I understand the frustration of finding out that something you need is an internal API. The reason we make things internal by default is that public functionality is somewhat set in stone: we cannot modify its behavior or its the function signature without breaking user code, whereas changing an internal function to public is really harmless.