Search Unity

  1. Good news ✨ We have more Unite Now videos available for you to watch on-demand! Come check them out and ask our experts any questions!
    Dismiss Notice

Resolved ListView binds more than it should

Discussion in 'UI Toolkit' started by Crouching-Tuna, Jul 8, 2020.

  1. Crouching-Tuna

    Crouching-Tuna

    Joined:
    Apr 4, 2014
    Posts:
    56
    Halo

    My ListView items has a RemoveAt button, and outside the list there's a button to Add item. This part is pretty standard
    The problem is, pressing the RemoveAt button sometimes calls it multiple times.
    My button.click's anon function:
    Code (CSharp):
    1. Debug.Log("RemoveAt! " + i);
    2. damageOut.RemoveAt(i);
    3. damageList.Refresh();
    So in order to debug this whole thing, i Debug.Log in makeItem, in bindItem, and in the RemoveAt.

    A common case is, on the way to scroll to the bottom most element, the bind for those last elements can be called multiple times and when i click that one, indeed it does the multiple RemoveAt bug

    For all the tests, my scroll height fits around 2 items, and the minimum number of items this bug starts happening is 4

    1. 4 items, scroll to most bottom, cleared console, then try click at 3rd button (index 2). It did an extra RemoveAt(1) before actually doing the correct RemoveAt(2)

    upload_2020-7-8_23-47-25.png



    2. 5 items, scroll to most bottom then click 5th button (RemoveAt(4)). Again, this is the total console from a single click. It did RemoveAt(3) before RemoveAt(4), on which it errors out bcoz of the remove prior
    upload_2020-7-8_23-45-4.png

    3. 6 items. Didnt press any button. Just wanna show what happens when scrolling from most top, to all the way down, it binds 4 and 5 twice
    upload_2020-7-8_23-59-13.png

    4. 6 items, scrolled to most bottom, then back to most top, clear console, then finally click 1st button (RemoveAt(0))
    upload_2020-7-8_23-46-3.png

    Keep in mind that in the case of the 6 items, scrolling down all the way bottom, then top, bottom, top multiple times, seems to jumble up the indexing even more.

    So yeah, unless i'm doing something very wrong?
    Im also aware of the flex/height not working yet in inspector https://forum.unity.com/threads/lis...ill-available-height-in-custom-editor.841918/, so i'm making mine with style.minHeight. In case that actually matters?

    Cheers
     
  2. uMathieu

    uMathieu

    Unity Technologies

    Joined:
    Jun 6, 2017
    Posts:
    183
    Which version of the editor are you running?
    Can you paste your List'View's makeItem/bindItem delegates?
     
  3. Crouching-Tuna

    Crouching-Tuna

    Joined:
    Apr 4, 2014
    Posts:
    56
  4. Crouching-Tuna

    Crouching-Tuna

    Joined:
    Apr 4, 2014
    Posts:
    56
    Here's a gif. In there, the item is supposed to have 3 things (the 2 None's and 1 floatField). I make 4 of them
    Then just scroll down and up. The bind gets called a bit more than the rebuild, sometimes in the middle of the scroll u can see the bindItem (making elements) happens more times.

    It looks like having extra enum/float field, yes but they're actually overflowing from previous elements down
    upload_2020-7-10_3-21-51.png
    The debugger shows those fields are well, a lot more than it should
     

    Attached Files:

  5. Crouching-Tuna

    Crouching-Tuna

    Joined:
    Apr 4, 2014
    Posts:
    56
    This bug is confirmed to happen in the example too
    upload_2020-7-10_19-11-8.png (Result from a single click)

    To reproduce, from the E09_ListView example, i added this line inside the bindItem delegate:
    Code (CSharp):
    1. (e.ElementAt(1) as Button).clicked += () => Debug.Log("Clicked: " + i);
    Then just scroll down, and click a button of some element in the higher numbers

    edit: Now that i think of it, is the intended use of bindItem to simply bind (set sync values) to properties?

    The button.clicked uses a += delegate assignment (well, since u can't do assignment.. nor can i do -= method; += method; convention bcoz i need to pass the index and it's a parameterless Action..)

    This is probably the reason. And if so, how to properly do what i'm trying to do, which is adding a button and pass the index (which i can only do in bindItem, not makeItem) ?



    edit2: (Gee lots of figuring out happening!)

    So i managed to do this by setting the clicked delegate once in makeItem:

    Code (CSharp):
    1. Debug.Log("Click! " + (int)box.userData);
    And then setting it in bindItem:
    Code (CSharp):
    1. e.userData = i;
    Fair enough

    But i read somewhere (even from a Unity staff) that it's an acceptable practice to add elements in bindItem. This is my other problem (items keep adding) I guess i'll need to Clear() it before doing AppendVisualElement if i'm gonna do that in bindItem?


    So i guess problem is solved. Unless this is still not the official correct usage. Please let me know if there's a better approach
     
    Last edited: Jul 10, 2020
  6. uDamian

    uDamian

    Unity Technologies

    Joined:
    Dec 11, 2017
    Posts:
    945
    Yes, it's perfectly fine adding/removing elements in bindItem(), but you are also responsible for returning the state to some "default" state (probably to whatever your makeItem() created) _before_ adding/removing elements.

    One of the main performance benefits of ListView is that it can create all the visible elements/templates once and then only update their bound values (text or value or other attributes) as you scroll, without recreating the UI all the time. That's why it won't "reset" or call makeItem() again before a bindItem().

    But coming back to your original problem, yes, setting the callback in the makeItem() instead of the bindItem() was the ideal way to go about it. We'll try to make this more obvious in future samples.
     
    Crouching-Tuna likes this.
unityunity