Search Unity

  1. Unity 2019.2 is now released.
    Dismiss Notice

Why are there multiple Event metaphors in UIElements?

Discussion in 'UIElements' started by SonicBloomEric, Jun 2, 2019.

  1. SonicBloomEric

    SonicBloomEric

    Joined:
    Sep 11, 2014
    Posts:
    643
    I'm just beginning to play around with UIElements and I've been struggling with figuring out some basics. The biggest confusion has been how to hook into UI events of various types.

    Why does the Button element have a "clickable" action?

    Why do most other elements use "RegisterForEvent<EventType>" to receive callbacks?

    Is there a method to the madness? Why aren't these consistent?

    And why isn't the "clickable" event documented in the Events Reference? Why do I have to go to the Button class documentation to find out what "clickable" is?
     
    SudoCat likes this.
  2. AlexandreT-unity

    AlexandreT-unity

    Unity Technologies

    Joined:
    Feb 1, 2018
    Posts:
    33
    Hi SonicBloomEric,

    Clickable is not an event, but a manipulator that wraps lower-level events and allows to get higher-level click-related callbacks and other features, such as being able to enable/disable clicking.
     
  3. SonicBloomEric

    SonicBloomEric

    Joined:
    Sep 11, 2014
    Posts:
    643
    @AlexandreT-unity Neat. We're getting into semantics.

    At the time I wrote this post I had not yet come across the term "Manipulator". The "Responding to Events" section of the documentation doesn't mention Manipulators even once. In fact, the only place the term is even mentioned in relation to UIElements in the entire manual is in the "Built-in Controls" section. And there it's simply dropped with no definition or explanation: it is expected that you understand the term already. The only hint is that the explanation suggests that adding a manipulator somehow "adds callbacks that respond to certain events".

    This is... extremely confusing.

    Is there somewhere to describe what a manipulator is and why they exist? From what I've seen they simply add an extra layer of callback confusion to the system.

    Let me expand upon the confusion. You guys have designed UIElements to work use a "RegisterCallbacks" based metaphor for subscribing to event callbacks. This is clean and easy to understand. However, when it comes to looking for a "Click" event, a developer (e.g. me) isn't able to find one in the Events Reference. Imagine my surprise when looking over the Button class I notice a "clickable" that has a C# event called "clicked" and that unlike all [so far as I can tell] other events in UIElements I subscribe to these with a standard C# approach:

    Code (CSharp):
    1. someButton.clickable.clicked += OnClicked;
    But... why not simply allow me to do the following:

    Code (CSharp):
    1. someButton.RegisterCallback<ClickEvent>(OnClicked);
    What is going on here? What's the method to this madness? Why isn't this documented?
     
    SudoCat likes this.
  4. uDamian

    uDamian

    Unity Technologies

    Joined:
    Dec 11, 2017
    Posts:
    351
    In the case of the Button and Clickable, the Button needs to:
    1. run its callback on MouseUpEvent
    2. but not run if the corresponding MouseDownEvent did not occur on top of it first

    Therefore, it is not enough to register for a single event and it's why we need Manipulators. We've debated adding a dedicated ClickEvent before but it never materialized so for now we need to use the Clickable manipulator.

    For a demo-based explanation of Manipulators, try this (timestamped URL) video:
     
    MrMatthias likes this.
  5. SonicBloomEric

    SonicBloomEric

    Joined:
    Sep 11, 2014
    Posts:
    643
    Certainly. But those are implementation details. There is nothing stopping you from managing the state required to check for "MouseDown→MouseUp" and then, when detected, trigger a ClickEvent, right?

    So... I just poked around the Manipulator source code a bit and it looks a bit like an anti-pattern. From the source code:
    Code (CSharp):
    1. public static void AddManipulator(this VisualElement ele, IManipulator manipulator)
    2. {
    3.     if (manipulator != null)
    4.     {
    5.         manipulator.target = ele;
    6.     }
    7. }
    8.  
    9. public static void RemoveManipulator(this VisualElement ele, IManipulator manipulator)
    10. {
    11.     if (manipulator != null)
    12.     {
    13.         manipulator.target = null;
    14.     }
    15. }
    You are inverting the Add/Remove semantics here. "Adding" a manipulator to a VisualElement is actually adding the VisualElement as a target to the Manipulator. Why not simply use one of the following:

    Code (CSharp):
    1. // Given:
    2. VisualElement someElt = root.Q("dragger");
    3. MyDragManipulator dragControl;
    4.  
    5. // Option one:
    6. dragControl = new MyDragManipulator(someElt);
    7.  
    8. // Option two:
    9. dragControl = new MyDragManipulator();
    10. dragControl.target = someElt;
    11.  
    12. // Option etc:
    13. dragControl = new MyDragManipulator();
    14. dragControl.Manipulate(someElt);
    That's not only far more clear, it's also easier to understand the reference ownership. When you say "VisualElement.AddManipulator(manipulator)" you make it seem as though the VisualElement now has an explicit reference to the "manipulator" instance that it tracks in some Manipulator array. But that's not true.

    Honestly, the whole "Manipulator" paradigm feels a bit unnecessary here. Is there some deeper-seated reason that they are being sprinkled into the API? The only two places I've seen them look as though they would be better off without them:
    1. Button.Clickable - This should absolutely be a ClickEvent.
    2. VisualElement.Add/RemoveManipulator - These should go away and be replaced with some way to clearly set the Manipulator instance's "target".
    As a concept, "Manipulators" seem fine. They are a way to conceptually deal with state across multiple atomic UI "events". Neat. But these are effectively implementation details that appear to "poke out of" the core API in odd ways. I think the UIElements API might be improved by relegating Manipulators to their own self-contained subsection of the API that users can optionally use if they wish to define complex event interactions.

    I mean... even the ManipulatorActivationFilter system is little more than a compartmentalized "if-check" to see if certain buttons or modifiers were hit... Were I implementing my own "Manipulator", I'd probably skip that whole thing and write my own simple check against the event's modifiers and buttons...

    Given that you've gone out of your way to make UIElements seem as web-like as possible, you're likely to find more and more people checking for a ClickEvent as the click event is perhaps the most basic event used on the web...

    The timestamp didn't carry over. For those interested, click here.
     
    Last edited: Jun 5, 2019
  6. AlexandreT-unity

    AlexandreT-unity

    Unity Technologies

    Joined:
    Feb 1, 2018
    Posts:
    33
    Hi SonicBloomEric,

    I understand your concern regarding the "anti-pattern" and the confusion that it may create. I brought your post to the attention of the UIElements team. Manipulators need some love, and we may be revisiting this API in the future, but the shape it will take is yet to be defined. They may end up in the garbage truck or be revamped (e.g. adding multi-target support), so stay tuned.

    Regarding the sparse documentation, we are aware of the issue and this will improve as our documentation team iterates on the API doc and manuals.

    Thanks
     
    Ludiq, taylank, bjarkeck and 3 others like this.
  7. bjarkeck

    bjarkeck

    Joined:
    Oct 26, 2014
    Posts:
    251
    Great to hear they'll get revisited! I also wrote down some feedback regarding them before discovering your message, but I had a couple of other points which I feel also needs attention as well.

    First point is similar to what @SonicBloomEric also brought up: If you consider the following code, you would expect the manipulator to work for both elements, but in truth it will only work for ele2, since there's a hidden UnregisterCallbacksFromTarget() when you add it to the second element.

    Code (CSharp):
    1. ele1.AddManipulator(manipulator);
    2. ele2.AddManipulator(manipulator);
    Secondly, the AddManipulator and RemoveManipulator methods has a strange null check. Why not give a NullReferenceException? Especially since its behavior is not to do anything. If I somehow ended up in a situation where I added a null manipulator, I would want to know about it, otherwise it most likely would just be a bad bug.

    Code (CSharp):
    1. public static void AddManipulator(this VisualElement ele, IManipulator manipulator)
    2. {
    3.     if (manipulator != null)
    4.     {
    5.         manipulator.target = ele;
    6.     }
    7. }
    8. public static void RemoveManipulator(this VisualElement ele, IManipulator manipulator)
    9. {
    10.       if (manipulator != null)
    11.       {
    12.          manipulator.target = null;
    13.      }
    14. }
    And funny enough, that is almost exactly what happened here, becase RemoveManipulator doesn't actually work! RemoveManipulator sets manipulator.target to null with the intention of removing the manipulator, but setting manipulator.target to null doesn't actually invoke UnregisterCallbacksFromTarget(), because that has a null check!

    Code (CSharp):
    1. VisualElement m_Target;
    2. public VisualElement target
    3. {
    4.     get { return m_Target; }
    5.     set
    6.     {
    7.         if (target != null)
    8.         {
    9.             UnregisterCallbacksFromTarget();
    10.         }
    11.         m_Target = value;
    12.         if (target != null)
    13.         {
    14.             RegisterCallbacksOnTarget();
    15.         }
    16.     }
    17. }
    If we fix that, and get rid of the add and remove manipulator methods, then we're left with the code Sonic also suggested.
    Code (CSharp):
    1. // Register manipulator
    2. manipulator.target = ele1;
    3.  
    4. // Register another manipulator:
    5. manipulator.target = ele2;
    6.  
    7. // Unregister?
    8. manipulator.target = null;
    However that is not great API design, even the creators of UIElements got it wrong as we just saw.
     
    Last edited: Aug 29, 2019
  8. SonicBloomEric

    SonicBloomEric

    Joined:
    Sep 11, 2014
    Posts:
    643
    I guess that depends on if you want a one-to-one or a one-to-many relationship between Manipulators and their Targets.

    That said, I think the Manipulator "metaphor" is way overkill and should be removed or otherwise relegated to "support". All core events (e.g. click) should have a single, standard interface.
     
    Ludiq and bjarkeck like this.
  9. bjarkeck

    bjarkeck

    Joined:
    Oct 26, 2014
    Posts:
    251
    Yeah I agree, and that's true! My point was more, that I think it would be much more clear to call manipulator.Register() and manipulator.Unregister() than setting a value on a property and hope that it does what you think it does.