String.Insert causes editor to crash

Unity Engine
,
1

I have a script which contains this foreach loop:

foreach (KeyCode code in System.Enum.GetValues(typeof(KeyCode)))
 if (Input.GetKeyDown(code))
{
    string key = code.ToString();

    switch (key)
    {
       case "Escape":
        key = placeholder.text;
            break;
        case "Mouse0":
            key = "Mouse Left";
            break;
        case "Mouse1":
            key = "Mouse Right";
            break;
        case "Mouse2":
            key = "Mouse Middle";
            break;
        default:
            if (key.Length == 1)
                key = key + " key"; //Single letter keys (ex. G => G key)
            else
                for (ushort n = 1; n < key.Length; n++)
                    if (Char.IsUpper(key[n]))
                        key = key.Insert(n - 1, " "); //insert space (ex. LeftShift => Left Shift) --- THE ERROR IS HERE
        break;
    }

    last = code;
    placeholder.text = key;

    change = false;
    background.color = Color.clear;
}

When the line where I’ve written “THE ERROR IS HERE” gets executed, the Unity Editor suddenly freezes, and I have to kill it from Windows Task Manager.

I’ve tried several things, and it turns out that the problem is not the insert function, but the assignement:

  • If I execute key.Insert(n - 1, " "); and I do not assign it, the Editor does not crash

  • If I assign key.Insert(n - 1, " "); to a new string, the Editor does not crash

  • If I assign key.Insert(n - 1, " "); to a new string and then I assign that string to key, the Editor does not crash

So , what could the error be? I’m new to Unity and 'm getting crazy…

2

My guess is because you are doing key.Length in your for statement and then modifying the length of “key”, you have essentially created an infinite loop as you keep increasing the length of key and you can’t ever exit the loop. Thus the freeze and crash. You might just want to grab the length of key up front and then use that value in your for loop.

5 Likes
3

Your loop runs until n < key.length is false. By inserting into key, you are making it longer. It’s possible that n < key.length is always true, because key.length is getting bigger as the loop runs.

4 Likes
4

Your break; statement APPEARS like it should be exiting your inner for() loop without issue.

But the indentation on that inner break; is also suspicious, almost as though the code isn’t compiling for some other reason, or failing to format. Formatting doesn’t matter but it can indicate a syntax issue.

It’s possible you want to completely exit all loops instead of just the one you’re in. Apart from the loop on the first line are there any more enclosing loops?

Easiest is just to attach the debugger and single-step the code and prove to yourself that the code flow is exiting your loops. I predict it is not but I cannot see at a glance why.

1 Like
5

My typical strategy when iterating over something I am modifying is to iterate over a copy instead while modifying the original. Usually that is for arrays/lists, but I think it would make sense here as well. It prevents any craziness to your iteration from the adding or removing.

1 Like
6

Thanks to everyone!
The issue was actually the fact that increasing the length of the string using Insert will cause an infinite loop…

Problem solved.

1 Like
7

1 Like
8

Yes, just go through your loop logically. When character at index n is an uppercase letter you will insert a space before that letter which will essentially place a space at the index n and it will move the upper case letter to n+1. That means the next iteration you will hit the same uppercase letter again and insert another space.

There are two solutions to this issue. The simplest one is just increasing n when you insert a space in order to skip the character you just processed:

for (ushort n = 1; n < key.Length; n++)
    if (char.IsUpper(key[n]))
        key = key.Insert(n++ - 1, " ");

Though a better solution is probably to check for a space before the uppercase letter:

for (ushort n = 1; n < key.Length; n++)
    if (key[n-1] != ' ' && char.IsUpper(key[n]))
        key = key.Insert(n - 1, " ");

This will only insert a space when there is no space before the upper case letter. So if there is already a space the uppercase letter is just ignored. Of course for this usecase it wouldn’t matter as KeyCode identifiers can’t contain spaces. Though as a general approach this is more robust.

9

the best way to debug these kinds of things, in general, and in my opinion, is to “unroll”.
track the individual variable states on a piece of paper, or in a text file, whatever suits you better.
do each iteration manually, get a feel for what’s going on, until you adopt heuristics and your mind is able to discern the nuances and patterns in your future algorithms of this kind. (i.e. don’t do donkey-carrot lol)

this is why I posted that picture above, this is what I see almost immediately (and why all of us liked that post). it happens. I freeze Unity on a weekly basis.

1 Like
10

@Bunny83 I like that second approach, that’s a true pattern recognition, almost a mini regex, and doesn’t require a dangling copy of anything.

I’d do it more verbosely though (insert not good, a new string instance for every change)

var sb = new StringBuilder();

// ...
else {
  sb.Clear();
  for(ushort n = 1; n < key.Length; n++) {
    if(char.IsLower(key[n-1]) && char.IsUpper(key[n])) sb.Append(" ");
    sb.Append(key[n]);
  }
  key = sb.ToString();
}