Big problems with how UI is being redrawn

[dramolxe]

Hey,

I hope someone can help out with this, because I am at my wits end.
I had a suspicion when I first began the course and got to the parts where we were building UI and started doing the Redraw() methods.
Now that I had to go back and actually revamp and design my UI, my fears came true.
To give some context, let’s take the Shop UI as an example. This is more to explain the process to people that might not understand at first on how this “dynamic” UI works:
This is the Redraw method in there (a part of it):

foreach (Transform child in listRoot)
                {
                    //Debug.Log($"Destroying {child.name}");
                    Destroy(child.gameObject);
                }

                foreach (ShopItem item in currentShop.GetFilteredItems())
                {
                    RowUI row = Instantiate(rowPrefab, listRoot);
                    row.Setup(currentShop, item);
                }

So the scenario is as follows, for a mild content populated “RPG” game:

• You arrive after some hours of gameplay to a town and decide to view the shops in the area;
• Since you were away exploring for a while, you ran short oh healing salves and you find an apothecary that has a total number of 100 small healing salves available;
  Now you will start to click the + button (Add) button as many times as you need to get to the number of total potions that you want to purchase.
  Now, our Shop UI has a reference to the currentShop, and we are actually subscribing to the shops onChange action. The Add() method (as well as the Remove() method) of the button adds the selected item to the current transaction. The adding of the item to the transaction is triggering the onChange action event from the Shop script.
  Since the ShopUI Redraw method is subscribed to the onChange action of the Shop, this means that each time that you click the + button, the action is triggered and the ShopUI’s Rerdaw method is called. What this means is that every shop row that contains the information of the items in the shop, they all get at first, destroyed, then for each item inside the shop, we instantiate a new row to store in the information of the items.
  Now imagine that the shop has 10 or 15 items available at once. Each time you click the + button, all those 15 game objects will get destroyed all at once, then all those 15 items will have to be instantiated all at once again in order to “refresh” and display an updated state of the items.
  This is a huge problem, because this will introduce small lag spikes (stutters) each time you click the + or - button, especially if you have a lot more items available in the shop.
  Also each time you would click any filter button, depending on how many items were available for that category, the Redraw process is triggered. So, if the Armor category had 9 items, then all the previous objects are deleted, then 9 new objects are being created.

The image above shows just that. I was clicking the + on the potions in order to get to the number I wanted to buy. As you can see, the blue vertical lines at the top represent the lag spikes (stutter) that the Redraw method is producing. Below you can see that the Instantiate.Produce is actually creating those 9 new objects that hold the item information. This happens for each spike you see. And in total, there is a 30+ms spike. That’s pretty noticeable, and it can get really really bad, the more items you have available.

Now another strange and weird behavior I saw is what actually happens once I finish adding the potions to the transaction, and then click the Buy button. There’s a whole new surprise there.

OK so, how to explain this… I really don’t know. But as you can see, the game had a literal 3 second lag spike in total. That is really really bad and really game breaking from my perspective. If you look at the highlighted areas, especially the Calls row, you will see that the Instantiate process (alongside other processes) had a LOT of calls being done. I don’t know why this is happening. Also, as an added WTH moment, if you look at the panel on the right side, you can see that there are a LOT of shop row game objects being instantiated. This is the WTH moment. It seems that for each item that I had in my transaction ( I think I had added something like 45+ potions), the Redraw method somehow got triggered for EACH of the item in my current transaction? I don’t know if I read that correctly, but that could only explain the 3 seconds lag spike that I had once I clicked the Buy button.

As a solution, I tried switching from Destroy / Instantiate to an Object Pooling system… And to my dismay that didn’t really help at all… Maybe it helps a bit because you avoid the extra garbage that the whole destroy / instantiate process is creating.

Object Pooling for those of you who are not familiar with, is when you create a bunch of objects that you might need, when the game starts. Think of it like some sort of factory. If I know that my shop will have a max number of 20 items displayed at once, I can create those 20 row prefabs at the start of the game, and then just set the as disabled. They will be there, and when I talk to a shopkeeper NPC, the Redraw method will simple pool the number needed of row prefabs from my Object Pooler. The process simple takes the number of row prefabs from the factory, sets their correct position under the listRoot, set the listRoot as a parent and then enables the game objects.

This sounded like a much better plan. But when I got to test the exact scenario, to my surprise this did not really help at all.
Here is how Enabling and Disabling objects works in Unity:
If you will want to have some decent UI / UX for your game, you will want it to look good. Good looking UI means that you will have to use a lot of child objects within a main object and those will all have a bunch of components on them. Components are things like scripts, layout elements, layout groups, Images, Toggles, Buttons, etc. (Talking from an UI pov.) When you have 1 shop row game object that has at least 6+ child game objects and each will have at least 3+ components on them, some a bit more, this will make the Disabling and Enabling process harder. Why, might you ask? Well that’s because each time you do a gameObject.SetActive(false / true) this means that Unity has to go in, and it doesn’t just Tick the enable box for the main game object only, no no, it has to do so for EACH of the available component in it, and child objects. So yes, even object pooling does not help in this problem.

Here is a quick view of the part of the Redraw method that replaced the destroy / instantiate:

int rowCount = listRoot.childCount;
            for (int i = rowCount - 1; i > -1; i--)
            {
                if (listRoot.GetChild(i).gameObject.activeSelf) listRoot.GetChild(i).gameObject.SetActive(false);               
            }

            int itemIndex = currentShop.GetFilteredItems().ToList().Count;
            for (int i = itemIndex - 1; i > -1; i--)
            {
                var rowGo = shopPooler.SpawnFromPool("Shop1Row", listRoot.position, Quaternion.identity, listRoot, i);               
                RowUI row = rowGo.GetComponent<RowUI>();
                row.Setup(currentShop, currentShop.GetFilteredItems().ToList()[i]);
            }

So this part here simply disables all the shop items under the shops listRoot, then it grabs as many row prefabs as it needs from the pool, sets their correct location and then enables them.
Like I said, this did not help at all. I had the same exact spikes as before.

I think that the main issue is how we are doing all of this Redrawing.
I think that we need a more performant method where we do not just go in and delete / disable every object, and then instantiate / enable all of the items again.
We need a way to somehow tell the shop UI which row object that holds the item is being operated one ( either Adding, Removing and then Buying).
That way, if we click the + on one item, then that object that holds that items info should be marked somehow and that object would then be destroyed / disabled and then only 1 object needs to be instantiated / grabbed and enabled instead of all the items. This would significantly improve performance. Also, the part where you Buy the items you have in your transaction needs to be investigate because of that immense lag spike it produces. (Investigate why it instantiate for example 9 objects x 45 times, the number of items in the current transaction)

I tried myself to make this happen, but to no avail, and it’s driving me crazy to the point that it drains all my motivation and makes me want to just stop trying to build anything at all.
I have no other ideas, nor do I have the experience need to actually pull this through. I only got some months of experience when it comes to programming in general. I had 0 knowledge of programming prior to taking the course. But working on it a lot and experimenting has enabled me to make small things happen. But things that require actual experience go way over my head.

Sorry for this long post, but I wanted to try to explain the problem to a certain degree, so that people that are not so experienced like me can understand what happens.

I hope someone is able to help, as this would benefit everyone that is taking the course and is serious about actually developing and releasing a game with decent user experience.

Thank you.

[Brian_Trotter]

Hi Dramolxe, and thank you for this well researched question.

You’re quite right, the system could stand a bit of refactoring. Unfortunately, even a single click on a quantity button does require some refreshing as some buttons need to be enabled or disabled based on what is clicked. That doesn’t necessarily mean we need to throw the whole baby and the bathtub and the bathroom and possibly the house out with the bath water.

My usual advice is to use object pooling as much as possible, and I was surprised to see it didn’t cut the overhead much… This leaves what you’ve outlined as the 2nd option, which is more of a virtual refresh…

Unfortunately, I’m at work at the moment, and even with that, this particular refactor may take till the weekend, but I promise you I have a solution that should reduce UI refresh time, can be applied to the other Inventory based UI setups, and hopefully will make sense. You’re on the right track, but we’re going to let the pooling happen right in the container, adding when needed, deactivating when not needed, and re-activating when we have available slots. It’s just going to require a bit of behind the scenes code to manage these entries in a way that isn’t changing the parenting every click.

[dramolxe]

Thank you for the fat reply Brian. I do understand that it does require quite a bit of work behind the scenes to get it to work. But if we can manage to somehow make the refresh happen to only let’s say, the row object of the item at hand, and the buttons that need to be set to interactable or not interactable, that would help a lot.
If you want, I can post here my object pooling script. Maybe that can give you an idea on how I do it. Object pooling helped me in my other UI scripts where I had to refresh my UI, for example I have a dedicated Quest Journal, where I can see all my Ongoing and then I can switch to a Completed tab. Every time I click the quest journal button, I do the Redraw, so that all my active quests get populated in the list root. Then for the info area, I only need to pool a few objects. For my journal, Object pooling did help by reducing the lag spikes a lot.

As a quick rundown on what I tried for the Shop UI, here is the piece of code. I know it will be some horrendous code but, it’s what I was able to think of with my limited knowledge:

  int rowCount = content.childCount;
                for (int i = rowCount - 1; i > -1; i--)
                {
                    content.GetChild(i).gameObject.SetActive(false);
                    vendorPooler.ReturnToPool(content.GetChild(i).gameObject);
                    RowUI2 row = content.GetChild(i).GetComponent<RowUI2>();
                    if (row.IsChanged())
                    {
                        ShopItem2 item = row.GetShopItem();
                         row.SetIsChanged(false);
                         row.gameObject.SetActive(false);
                         row.Setup(currentShop, item);

                        int itemIndx = currentShop.GetFilteredItems().ToList().Count;
                        for (int j = itemIndx - 1; j > -1; j--)
                        {
                            if (item == currentShop.GetFilteredItems().ToList()[i])
                            {
                                row.Setup(currentShop.GetFilteredItems().ToList()[i], currentShop);
                            }
                        }
                        return;
                    }

                    if (content.GetChild(i).gameObject.activeSelf) content.GetChild(i).gameObject.SetActive(false);

                }

I thought that I can set the row object as changed when I clicked either the + or - button. Then, and I know I placed the logic in the wrong place, but just to test it, I laced it before I could disable all the game objects. So it would see if a child is a row and is changed, and if it is, then it would only disable / return to the pool and bring back a new one, just for that item only.
It somehow worked but not 100%. When I was clicking the + on the potion, I could see my total in transaction go up, but the quantity was not increasing in the shop UI. but they were being added to the transaction. And the spikes were reduced to like 4ms, which was great. But there was still an issue when I clicked Buy. The one I mentioned where apparently the refresh method would do the enabling of let’s say 9 shop items x 59 which would result in a total of 531 calls for the Instantiate / enable process. Yes, I remembered I had 59 potions in transaction, because I just did the quick 531 / 9 and got the total items that I had in transaction.

[Brian_Trotter]

Ok, I think we can uncomplicate this a bit from my original estimation…

First, we’re going to create a list to store the components. Up with the declarations for Shopper shopper; and Shop currentShop, add a List:

List<RowUI> existingRows = new List<RowUI>();

We’re going to take the snippet that clears the listRoot to Start. This will only need to be done once!

            foreach (FilterButtonUI button in GetComponentsInChildren<FilterButtonUI>())
            {
                button.SetShop(currentShop);
            }

Finally, in RefreshUI, we’re going to remove that piece of code we just added to Start(), we don’t need it every frame anymore…

This leaves us with the foreach loop against GetFilteredItems()

            int rowIdx = 0;
            foreach (var item in currentShop.GetFilteredItems())
            {
                RowUI row;
                if (rowIdx < existingRows.Count) //Activate an existing row 
                {
                    row = existingRows[rowIdx];
                    row.gameObject.SetActive(true);
                }
                else //not enough rows, add one to the heirarchy and cache it in existingRows
                {
                    row = Instantiate<RowUI>(rowPrefab, listRoot);
                    existingRows.Add(row);
                }
                rowIdx++;
                row.Setup(currentShop, item);
            }

This is an “in place” style of object pooling. A normal object pool involves releasing the object to a known pool, then retrieving that object in lieu of creating a new one. That’s what we’re doing here, only we’re not changing the parents. The first time this is run, we’ll always be creating new RowUIs, but once we have, then the pooling code will kick in.
rowIdx keeps track of how many rows we’ve used. The assumption is that every refresh, we’re going to need to setup the row again, but that doesn’t mean we need a new one. We cycle through the rows referring to the cached row until we run out of rows in the existingRows cache. If we still need more rows, we create new ones, and add them to the existingRows list. In either case, we continue to increase rowIdx, because we need to deactivate any existingRows we didn’t use in this refresh:

            for (int i = rowIdx; i < existingRows.Count; i++) //disable any rows we don't need.
            {
                existingRows[i].gameObject.SetActive(false);
            }

If we’ve used all the existing rows, whether or not we’ve created any, then rowIdx will be larger than the array, and nothing will be deactivated, but if we use less rows, then the remainder of the rows will be deactivated.

[Brian_Trotter]

This takes care of object caching, but as it turns out, this is NOT the reason for the slowdowns. The bottleneck on the slowdowns is coming on the confirmation of the transaction. The clue was in this little snipped in your last post:

@dramolxe

I just did the quick 531 / 9 and got the total items that I had in transaction.

This means that for some reason, when the transaction is being confirmed, we are calling onChange() multiple times, quite unneccessarily… Let’s explore why that is:

[Brian_Trotter]

We’ll start at the source of the problem: ConfirmTransaction:

 public void ConfirmTransaction()
        {
            Inventory shopperInventory = currentShopper.GetComponent<Inventory>();
            Purse shopperPurse = currentShopper.GetComponent<Purse>();
            if (shopperInventory == null || shopperPurse == null) return;

            // Transfer to or from the inventory
            foreach (ShopItem shopItem in GetAllItems())
            {
                InventoryItem item = shopItem.GetInventoryItem();
                int quantity = shopItem.GetQuantityInTransaction();
                float price = shopItem.GetPrice();
                for (int i = 0; i < quantity; i++)
                {
                    if (isBuyingMode)
                    {
                        BuyItem(shopperInventory, shopperPurse, item, price);
                    }
                    else
                    {
                        SellItem(shopperInventory, shopperPurse, item, price);
                    }
                }
            }
            // Removal from transaction
            // Debting or Crediting of funds

            if (onChange != null)
            {
                onChange();
            }
        }

So at first glance, we only see ONE onChange() event happening, not one for each item in the transaction. But we are doing something in the method that is happening once per individual item in the transaction, we’re either calling BuyItem() or SellItem(). Let’s take a look at BuyItem() next:

        private void BuyItem(Inventory shopperInventory, Purse shopperPurse, InventoryItem item, float price)
        {
            if (shopperPurse.GetBalance() < price) return;

            bool success = shopperInventory.AddToFirstEmptySlot(item, 1);
            if (success)
            {
                AddToTransaction(item, -1);
                if (!stockSold.ContainsKey(item))
                {
                    stockSold[item] = 0;
                }
                stockSold[item]++;
                shopperPurse.UpdateBalance(-price);
            }
        }

Ok, we’re not seeing any calls to OnChange here, and a quick look at SellItem doesn’t reveal much either… but if you follow the rabbit trail ONE more step further, you find the real culprit in this massive bogdown. (I never noticed this bog previously, by the way, even in my nearly ready to market game, because it never occurred to me to buy 99 potions!). Let’s take a look at AddToTransaction()

        public void AddToTransaction(InventoryItem item, int quantity) 
        {
            if (!transaction.ContainsKey(item))
            {
                transaction[item] = 0;
            }


            var availabilities = GetAvailabilities();
            int availability = availabilities[item];
            if (transaction[item] + quantity > availability)
            {
                transaction[item] = availability;
            }
            else
            {
                transaction[item] += quantity;
            }
            
            if (transaction[item] <= 0)
            {
                transaction.Remove(item);
            }

            if (onChange != null)
            {
                onChange();
            }
        }

and there it is, at the end of AddToTransaction, which our chain has us running once for each individual item in the transaction, there it is… onChange();
This means that onChange is indeed being called once for every item in the transaction (yep, that means 50 potions is 50 onChange events. With or without object pooling, this many event calls will grind the gears.

Now we still need AddToTransaction as part of this chain, and we also need AddToTransaction to add things to the transaction with the +/- buttons, and we need to call onChange() when that happens! While it’s possible to refactor all of this code and find a different approach, I have a much simpler method of avoiding all those extra draw calls… Let’s change the signature to AddToTransaction, to add an optional parameter:

public void AddToTransaction(InventoryItem item, int quantity, bool callOnChangeEvent = true)

By assigning a default value to the parameter, it means that any method that calls AddToTransaction() without the third parameter will have the default value of true. It also means that we have less refactoring to do, because we only need to change the calls where we don’t want onChange() to be called.
Next, at the end of the AddToTransaction() method, a quick change to the if statement:

if(callOnChangeEvent && onChange!=null)

Now, if callOnChangeEvent is false, then we won’t call the event.

Finally, a couple quick changes to BuyItem and SellItem’s call of AddToTransaction(). Just add false to the end of the AddToTransaction method calls

AddToTransaction(item, -1, false);

These changes should eliminate the bottlenecks when performing large transactions.

[dramolxe]

Hey Brian,

You are a life saver! This works like a charm! Now you can’t even notice the “stutter” when you click the add button, because it’s almost non existent.

And when purchasing a stack of items, there is only a very small stutter that’s not that noticeable now.
I still don’t understand what is causing it, because I checked the Profiler and there are some calls made to GameObject.Deactivate made, and it’s a bunch of Images that cause little to no ms

But this is A LOT better now. It’s actually enjoyable using a shop now.
One thing that I noticed is that, when I switch the shop, It doesn’t always show the items that I can sell to the shop. I will have to investigate this a little bit.

But thank you so much for your time. You managed to find a solution in such a short time that it amazes me!

I hope that people who run into this issue can find this post so that they can implement this solution!

[Brian_Trotter]

I tagged the post with Shops and Abilities, as well as my own brians_tips_and_tricks tag. If anybody is struggling with this in the future, I can point them right here.

[dramolxe]

That’s great, I was about to ask you how can we make sure people can find this easily. To be honest, I am using this method everywhere I am in need to populate UI things dynamically and it’s working really really well.
Thank you once again Brain! Your help has given me a second wind in developing my project, and now I can finally move onwards!