Simple & quick search for bugs in Unity games (for C# developers)
Tags: csharp, gamedev, programming
Published: 2024-04-03 07:49:45

Each of us encounters game bugs, and some games have more of them than others\. Many games suffer from bugs caused by errors in code\. Today, we'll see how static analysis can help easily detect errors and enhance your project\.

![](https://import.viva64.com/docx/blog/1111_TowerDefenceDemo/image1.png)

The video game industry is growing fast\. Every year, game development gets more expensive, and so do the games too\. We have an article about it: "[Why should Unity game developers use static analysis?](https://pvs-studio.com/en/blog/posts/csharp/0945/)"\.

Once you have read it and seen how well the static analysis helps game devs, let's talk about the Best Warnings mechanism\. This is a good way to quickly get to grips with the static analyzer\. You may ask, "Why do we need that?"\.

Well, we answer it\. The analyzer report often may contain hundreds or even thousands of warnings on large projects\. Working with them can be a challenge for developers\. When you've already chosen a static analyzer, you can simply suppress irrelevant warnings and see only new ones\. However, what do you do if you're new to the tool? We've purposely created the Best Warnings mechanism for it\.

## How to get Best Warnings?

Let's take a look at TowerDefense\-GameFramework\-Demo\. It's an open\-source project, the source code is available on [GitHub](https://github.com/DrFlower/TowerDefense-GameFramework-Demo)\. Unfortunately, not so many Unity games are open\-source, but we need them to show you results\. But even on this small project, we still get to see the work of Best Warnings\.

Firstly, install the analyzer and enter the license\. If you haven't already done so, this [page](https://pvs-studio.com/en/pvs-studio/try-free/?utm_source=website&utm_medium=habr&utm_campaign=article&utm_content=1111) will guide you\.

Open your project in Unity Hub\. Next, in Unity, click **Assets \> Open C\# Project**\. You will see the IDE set in the Unity editor settings\. It's set here:

![](https://import.viva64.com/docx/blog/1111_TowerDefenceDemo/image2.png)

I'll be using the Visual Studio 2022 IDE that I use for work\. To analyze the project in the IDE version, use the **Extensions \> PVS\-Studio \> Check Solution** menu item\.

![](https://import.viva64.com/docx/blog/1111_TowerDefenceDemo/image3.png)

After the analysis is completed, click the **Best** button, and that's all\. Then you'll see a list of the 10 analyzer warnings:

![](https://import.viva64.com/docx/blog/1111_TowerDefenceDemo/image4.png)

## Let's break down warnings

As we already know, the Best Warnings section contains 10 warnings indicating the code where an error may occur\. We'll study all 10\. Spoiler: there will be real errors, just suspicious code, and, of course, false\-positive warnings\. Indeed, false positives are OK for the static analysis methodology\. The main thing is that the percent of such warnings should not be very high\. Diving in now\!

**Fragment 1**

```cpp
public void OnRecycle()
{
  transform.SetParent(initRoot, false);
  transform.localPosition = transform.localPosition;
  transform.eulerAngles = initRotation;
  transform.localScale = initScale;
  ....
}
```

The PVS\-Studio warning: [V3005](https://pvs-studio.com/en/docs/warnings/v3005/) The 'transform\.localPosition' variable is assigned to itself\. Item\.cs 150

As you can see from the code, the *transform\.localPosition* property is assigned to itself\. This is a quite common error that we often see in analyzed projects\.

**Fragment 2**

```cpp
public static int Read7BitEncodedInt32(this BinaryReader binaryReader)
{
  int value = 0;
  int shift = 0;
  byte b;
  do
  {
    if (shift >= 35)
    {
      throw new GameFrameworkException("7 bit encoded int is invalid.");
    }

    b = binaryReader.ReadByte();
    value |= (b & 0x7f) << shift;
    shift += 7;
  } while ((b & 0x80) != 0);

  return value;
}
```

The PVS\-Studio warning: [V3134](https://pvs-studio.com/en/docs/warnings/v3134/) Shift by \[32\.\.34\] bits is greater than the size of 'Int32' type of expression '\(b & 0x7f\)'\. BinaryExtension\.cs 37

Here, the analyzer is wrong\. It displays a message about the incorrect shift\. Unfortunately, the analyzer doesn't recognize that the *shift* increases by 7 each time\. Therefore, it can't have values such as 32, 33, and 34\. The code operates correctly\. However, it's worth noting that we can replace the *shift \>= 35* check with *shift \>= 32*\. Then other developers and the analyzer will understand the possible value ranges\.

If you don't know how data flow analysis works, or you're interested in the PVS\-Studio technologies, I recommend you read the [article](https://pvs-studio.com/en/blog/posts/0908/)\.

**Fragment 3**

```cpp
public void RemoveData(Data data)
{
  if (data == null)
  {
    throw new GameFrameworkException(Utility.Text.Format("Data '{0}' is null.", 
                                                         data.Name.ToString()));
  }

  ....
}
```

The PVS\-Studio warning: [V3080](https://pvs-studio.com/en/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'data'\. DataManager\.cs 122

The developer checks the parameter for validity\. If the *data* parameter is equal to *null*, an exception will be thrown\. However, when the exception message is generated, *data* is used and equal to *null*\. As a result, we get an exception of the *NullReferenceException* type instead of *GameFrameworkException*\. 

**Fragment 4**

```cpp
internal override void Update(float elapseSeconds, float realElapseSeconds)
{
  while (m_RecycleQueue.Count > 0)
  {
    EntityInfo entityInfo = m_RecycleQueue.Dequeue();
    IEntity entity = entityInfo.Entity;
    EntityGroup entityGroup = (EntityGroup)entity.EntityGroup;
    if (entityGroup == null)
    {
      throw new GameFrameworkException("Entity group is invalid.");
    }

    entityInfo.Status = EntityStatus.WillRecycle;
    entity.OnRecycle();
    entityInfo.Status = EntityStatus.Recycled;
    ....
  }

  ....
}
```

The PVS\-Studio warning: [V3008](https://pvs-studio.com/en/docs/warnings/v3008/) The 'entityInfo\.Status' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 176, 174\. EntityManager\.cs 176

Here, you can see the bug from the analyzer side\. It doesn't detect that the *entityInfo* and *entity* variables are connected to each other\. We left a note for future enhancement\.

**Fragment 5**

```cpp
public static FileSystem Load(....)
{
  ....

  for (int i = 0; i < fileSystem.m_BlockDatas.Count; i++)
  {
    BlockData blockData = fileSystem.m_BlockDatas[i];
    if (blockData.Using)
    {
      StringData stringData = fileSystem.ReadStringData(blockData.StringIndex);
      fileSystem.m_StringDatas.Add(blockData.StringIndex, stringData);
      fileSystem.m_FileDatas.Add(stringData.GetString(....);          // <=
    }
    else
    {
      fileSystem.m_FreeBlockIndexes.Add(blockData.Length, i);
    }
  }

  return fileSystem;
}
```

The PVS\-Studio warning: [V3156](https://pvs-studio.com/en/docs/warnings/v3156/) The first argument of the 'Add' method is not expected to be null\. FileSystem\.cs 206

When developing the analyzer, we consider the logic of some popular methods\. This mechanism is called "method annotation"\. Here, the analyzer uses the information from this mechanism\. The analyzer knows that the *Dictionary<TKey, TValue\>\.Add* method can't accept *null*\.

Let's take a look at the *GetString* method\. Its result is passed to the *Add* method\.

```cpp
public string GetString(byte[] encryptBytes)
{
    if (m_Length <= 0)
    {
        return null;
    }

  ....
}
```

We only need the beginning of the method, when *null* is returned\. As a result, if *m\_Length* is less than or equal to 0, an *ArgumentNullException* will be thrown\.

**Fragment 6**

```cpp
private void RefreshCheckInfoStatus()
{
  ....
  IFileSystem fileSystem = m_ResourceManager.GetFileSystem(....);
  if (!fileSystem.SaveAsFile(resourceFullName, resourcePath))
  {
    throw new GameFrameworkException(Utility.Text.Format(
      "Save as file '{0}' to '{1}' from file system '{2}' error.",
      resourceFullName,
      fileSystem.FullPath));
  }

  fileSystem.DeleteFile(resourceFullName);
  ....
}
```

The PVS\-Studio warning: [V3025](https://pvs-studio.com/en/docs/warnings/v3025/) The 1st argument '"Save as file '\{0\}' to '\{1\}' from file system '\{2\}' error\."' is used as incorrect format string inside method\. A different number of format items is expected while calling 'Format' function\. Format items not used: \{2\}\. ResourceManager\.ResourceChecker\.cs 152

Here is an example of how interprocedural analysis works\. The analyzer "follows" a format string with three placeholders for arguments\.

```cpp
public static string Format(string format, object arg0, object arg1)
{
  if (format == null)
  {
    throw new GameFrameworkException("Format is invalid.");
  }

  CheckCachedStringBuilder();
  s_CachedStringBuilder.Length = 0;
  s_CachedStringBuilder.AppendFormat(format, arg0, arg1);  // <=
  return s_CachedStringBuilder.ToString();
}
```

This format string is used as the first argument to the *StringBuilder\.AppendFormat* method\. As we know, there are three placeholders, but only two arguments\. Using this format string will result in a *FormatException*\.

I assume that such an error may have occurred because of a copy\-paste\. There's a similar place in the code below:

```cpp
if (!fileSystem.WriteFile(resourceFullName, resourcePath))
{
  throw new GameFrameworkException(Utility.Text.Format(
    "Write resource '{0}' to file system '{1}' error.",
    resourceFullName,
    fileSystem.FullPath));
}
```

The developer could just copy a similar handling and modify the format string but not change the number of arguments\.

**Fragment 7**

```cpp
public void ShowPreviewTower(TowerData towerData)
{
  ....

  TowerLevelData towerLevelData = towerData.GetTowerLevelData(0);
  if (towerLevelData == null)    // <=
  {
    Log.Error("Tower '{0}' Level '{1}' data is null.", towerData.Name, 0);
  }

  EntityDataRadiusVisualiser entityDataRadiusVisualiser = 
    EntityDataRadiusVisualiser.Create(towerLevelData.Range); // <=
  ....
}
```

The PVS\-Studio warning: [V3125](https://pvs-studio.com/en/docs/warnings/v3125/) The 'towerLevelData' object was used after it was verified against null\. Check lines: 122, 117\. LevelControl\.cs 122

It seems that using a variable after checking it for *null* isn't a good idea\. If *towerLevelData *is *null*, the case is logged\. But that doesn't always happen\. The compiler won't ignore the *Log\.Error* method only when a conditional compilation symbol associated with logging or the *Debug* configuration is defined\. In addition, it doesn't interrupt the execution flow \(and yes, that would be weird\)\. So, the execution will still reach the line indicated by the analyzer\.

**Fragment 8**

```cpp
public DataTableProcessor(....)
{
  ....
  m_Strings = strings.OrderBy(value => value.Key)
                     .OrderByDescending(value => value.Value)
                     .Select(value => value.Key).ToArray();
  ....
}
```

The PVS\-Studio warning: [V3078](https://pvs-studio.com/en/docs/warnings/v3078/) Sorting keys priority will be reversed relative to the order of 'OrderBy\.\.\.' method calls\. Perhaps, 'ThenByDescending' should be used instead\. DataTableProcessor\.cs 179

Unfortunately, some devs may not know that the *\.OrderBy\.OrderBy* sorting pattern operates in reverse\. We have an article on the topic: "[Sorting in C\#: OrderBy\.OrderBy or OrderBy\.ThenBy? What's more effective and why?](https://pvs-studio.com/en/blog/posts/csharp/0991/)"\. It seems that it's better to use *\.OrderBy\.ThenBy*\. This not only makes the code clearer but also more performant\.

**Fragment 9**

```cpp
public bool BuildResources()
{
  ....
  bool isSuccess = false;
  isSuccess = BuildResources(Platform.Windows, ....);
  if (!watchResult || isSuccess)
  {
    isSuccess = BuildResources(Platform.Windows64, ....);
  }

  ....

  if (!watchResult || isSuccess)
  {
    isSuccess = BuildResources(Platform.WebGL, ....);
  }
  ....
}
```

The PVS\-Studio warning: [V3137](https://pvs-studio.com/en/docs/warnings/v3137/) The 'isSuccess' variable is assigned but is not used by the end of the function\. ResourceBuilderController\.cs 748

PVS\-Studio warns that a value is assigned to the *isSuccess* variable\. The variable isn't used after that\. The *BuildResources* method writes its work result to *isSuccess*\. There are six more assignments hidden under the second '\.\.\.\.'\. It's hard to tell if it's really a bug, but if the last return value of a method is to be ignored, it's better to use '\_':

```cpp
_ = BuildResources(Platform.WebGL, ....);
```

It's not just a code to prevent the analyzer from issuing a warning\. This writing style enhances code readability for other developers\.

**Fragment 10**

```cpp
private void RefreshSourceVersionCount()
{
  m_SourceVersionIndexes[m_TargetVersionIndex + 1] = false; // <=
  m_SourceVersionCount = 0;
  if (m_SourceVersionIndexes == null)     // <=
  {
    return;
  }

  for (int i = 0; i < m_SourceVersionIndexes.Length; i++)
  {
    if (m_SourceVersionIndexes[i])
    {
      m_SourceVersionCount++;
    }
  }
}
```

The PVS\-Studio warning: [V3095](https://pvs-studio.com/en/docs/warnings/v3095/) The 'm\_SourceVersionIndexes' object was used before it was verified against null\. Check lines: 338, 340\. ResourcePackBuilder\.cs 338

This warning is difficult to describe, somehow :\) The* m\_SourceVersionIndexes* field is checked for *null* after accessing array elements by index\. It's better to move the check to the beginning of the method\.

## Conclusion

That's all :\) We didn't waste much time, and it was easy to look into the bugs\. Now you know how to quickly find errors in code\. It's worth remembering that the Best Warnings mechanism is a good choice for newcomers to the static analysis world\. It's better to use static analysis [regularly](https://habr.com/en/articles/440610/)\. For example, you can check commits or run analysis on developers' computers\. This way, you can fully evaluate the outcomes of using static analysis tools in your project\.

Good luck\!