Search Unity

  1. Megacity Metro Demo now available. Download now.
    Dismiss Notice
  2. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

How best to make a timer utilizing StringBuilder?

Discussion in 'Scripting' started by QuinnWinters, May 8, 2018.

  1. QuinnWinters

    QuinnWinters

    Joined:
    Dec 31, 2013
    Posts:
    494
    I'd like to replace the way I'm updating my UI timer text with StringBuilder but I'm not quite sure how to go about it. As it currently is I'm replacing the entire string (technically two of them) every update which I know is a terrible way of going about it. I'm thinking perhaps I could split the current time float at the decimal and then separate out each individual number in the two halves and then separate out each individual number in the two UI strings and compare the time numbers to the string numbers based on their positions and if they don't match use StringBuilder.Replace. I don't know if that would end up being more inefficient than what I'm currently doing. How would I best go about this?

    This is my current timer code:
    Code (CSharp):
    1. public class Timer : MonoBehaviour {
    2.  
    3.     public Text currentTimeUI;    //Right justified
    4.     public Text currentTimeDecimalUI;    //Left justified
    5.     public float currentTime;
    6.     private float startTime;
    7.  
    8.     void Start () {
    9.  
    10.         startTime = Time.time;
    11.  
    12.     }
    13.  
    14.     void Update () {
    15.  
    16.         currentTime = Time.time - startTime;
    17.         float timeToSplit = Mathf.Round (currentTime * 1000f) / 1000f;
    18.         string timeString = timeToSplit.ToString ();
    19.         string [] split = timeString.Split (new char [] {'.'});
    20.         currentTimeUI.text = split [0];
    21.         try {    //Try because there may not be a decimal to split at, leaving no second array index
    22.             currentTimeDecimalUI.text = split [1];
    23.         }
    24.         catch {    //If there is no decimal default to triple 0's
    25.             currentTimeDecimalUI.text = ("000");
    26.         }
    27.  
    28.     }
    29.  
    30. }
     
  2. dgoyette

    dgoyette

    Joined:
    Jul 1, 2016
    Posts:
    4,193
    I'd would expect that the cost of verifying all of those changes, using string comparisons, would actually be more expensive than just updating. A couple of things:

    You should start by profiling this code, so you have actual evidence about what's slow, if anything at all. In general, you shouldn't try to optimize code unless you can confirm that the before-after results in an actual, measurable improvement. That means starting with a baseline. If you profile the code, and find it's taking some small fractions of a millisecond, it's probably not worth optimizing.

    If you can avoid performing string operations, you should. So, the split you're doing might end up being the more expensive

    I'd personally just have a single Text object. I don't expect there's any way that updating two Text objects would have different performance than just one.

    You could also consider updating this in a Coroutine instead of in Update, running every 1/10th of a second or so.
     
  3. GroZZleR

    GroZZleR

    Joined:
    Feb 1, 2015
    Posts:
    3,201
    Oh man that's ugly. That's actually 7 new string operations and 2 created arrays, plus that horrible try/catch block which you never want on a per-frame basis, more for "risky" operations like file loading. You definitely want to profile, like dgoyette has suggested, but I can tell you that code is creating a ton of garbage every single frame.

    StringBuilder is mostly used for building strings, as in adding data as you go such as processing some data or adding to an output log. Since the data you want is fixed, you should just use a regular string assignment to have as little overhead as possible.

    This should get you what you want with way less overhead:
    Code (csharp):
    1.  
    2. currentTime = Time.time - startTime;
    3.  
    4. int whole = (int)currentTime;
    5. float decimal = currentTime - whole;
    6.  
    7. currentTimeUI.text = whole;
    8. currentTimeDecimalUI = decimal.ToString("D3");
    9.  
    It's untested, wrote it right here, so let me know if there's any issues.
     
    40detectives and QuinnWinters like this.
  4. snacktime

    snacktime

    Joined:
    Apr 15, 2013
    Posts:
    3,356
    The most efficient way is basically what @GroZZleR has there but multiply decimal by the precision you want and cast it to an int. Then you can use a Dictionary<int, string> to cache the text.
     
    40detectives likes this.
  5. QuinnWinters

    QuinnWinters

    Joined:
    Dec 31, 2013
    Posts:
    494
    Thanks guys that helped a lot. That block of code has been a thorn in my project for months and I've been putting off optimizing it. I'm surprised I never thought of doing it Grozzler's way because it seems so obvious now. Here's what I ended up doing, using FastString instead of StringBuilder.

    Code (CSharp):
    1. FastString decimalString = new FastString (64);
    2. int decimalNum;
    3.  
    4. void Update () {
    5.  
    6.     currentTime = Time.time - startTime;
    7.     currentTime = Mathf.Round (currentTime * 1000f) / 1000f;
    8.     int wholeNum = (int) currentTime;
    9.     float remainder = (currentTime - wholeNum) * 1000;
    10.     decimalNum = (int) remainder;
    11.  
    12.     currentTimeUI.text = wholeNum.ToString ();
    13.     if (decimalNum != 0) currentTimeDecimalUI.text = FastString ();
    14.     else currentTimeDecimalUI.text = ("000");
    15.  
    16. }
    17.  
    18. private string FastString () {
    19.  
    20.     decimalString.Set (decimalNum.ToString ());
    21.     if (decimalNum < 10) decimalString.Append ("00");
    22.     else if (decimalNum < 100) decimalString.Append ("0");
    23.     return decimalString.ToString ();
    24.  
    25. }
     
    40detectives likes this.