ListView binds more than it should

[Crouching-Tuna]

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:

Debug.Log("RemoveAt! " + i);
damageOut.RemoveAt(i);
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)

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
   

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
   

4. 6 items, scrolled to most bottom, then back to most top, clear console, then finally click 1st button (RemoveAt(0))
   

   

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 ListView does not grow to fill available height in custom editor , so i’m making mine with style.minHeight. In case that actually matters?

Cheers

[uMathieu]

Which version of the editor are you running?
Can you paste your List’View’s makeItem/bindItem delegates?

[Crouching-Tuna]

There’s some chain of calls, but basically this is from CreateInspectorGUI
Using 2019.4.0f1

public override void AppendVisualElement(VisualElement ret){ VisualElemen...

[Crouching-Tuna]

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

The debugger shows those fields are well, a lot more than it should

[Crouching-Tuna]

This bug is confirmed to happen in the example too
(Result from a single click)

To reproduce, from the E09_ListView example, i added this line inside the bindItem delegate:

(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:

Debug.Log("Click! " + (int)box.userData);

And then setting it in bindItem:

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

[uDamian]

@Crouching-Tuna

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?

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.

[mengkaima]

@Crouching-Tuna

This bug is confirmed to happen in the example too
(Result from a single click)

To reproduce, from the E09_ListView example, i added this line inside the bindItem delegate:

(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:

Debug.Log("Click! " + (int)box.userData);

And then setting it in bindItem:

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

Wow that’s just brilliant! Thank you so much for this solution