Quickly and easily find bugs in Unity games (for C# developers)
We all encounter bugs in games. In some, even more often than we would like. Many of these bugs are related to errors in the code. Today we’ll see how you can easily find errors and improve your project using static analysis.
The gaming industry is getting bigger and bigger. Every year the cost of development increases, and along with it the price of the games themselves. We have an article on this topic: “Why should Unity game developers use static analysis?“.
Once you've read it and understand how game developers benefit from static code analysis tools, let's talk about Best Warnings. This is one of the ways to quickly get acquainted with the static analyzer. “Why is this acquaintance necessary at all?” – you ask.
We answer. Often on large projects, the analyzer report can contain hundreds or even thousands of warnings. Dealing with them can be quite difficult. Once you've already chosen a static analyzer, you can simply suppress irrelevant warnings and see only new ones. But what if you're just getting started with the instrument? For such situations, the Best Warnings mechanism was created.
How to get Best Warnings?
For example, let's take the Open Source project TowerDefense-GameFramework-Demo. Its source code is available at GitHub. Unfortunately, very few Unity games are open source, which is exactly what we need to demonstrate. But even on this small project we will still see Best Warnings working.
First of all, you should already have the analyzer installed and the license entered. If you haven't done this yet, this will help you. page.
Open your project in Unity Hub. Next in Unity click: Assets > Open C# Project. You will see the IDE specified in the Unity editor settings. It is indicated here:
I will use the Visual Studio 2022 IDE, which I am used to working with. To analyze a project in this version of the IDE, you can use the menu item Extensions > PVS-Studio > Check Solution.
After the analysis, you just need to click on the button Best. After this, you will see a selection of 10 analyzer warnings:
Let's look at the warnings
As we already know, the Best Warnings section contains 10 warnings indicating code that most likely contains a bug. Let's look at all 10. Spoiler: there will be real errors, and simply suspicious code, and, of course, False Positive warnings. Naturally, false positives are normal for static analysis technology. The main thing is that the percentage of such warnings is small. Well, let's get started!
Fragment 1
public void OnRecycle()
{
transform.SetParent(initRoot, false);
transform.localPosition = transform.localPosition;
transform.eulerAngles = initRotation;
transform.localScale = initScale;
....
}
PVS-Studio warning: V3005 The 'transform.localPosition' variable is assigned to itself. Item.cs 150
As you can see from the code, the property transform.localPosition assigned to itself. This is a fairly common mistake that we often see in the projects we analyze.
Fragment 2
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;
}
PVS-Studio warning: V3134 Shift by [32..34] bits is greater than the size of 'Int32' type of expression '(b & 0x7f)'. BinaryExtension.cs 37
But in this case, the analyzer made a mistake. He's talking about an incorrect shift. Unfortunately, the analyzer did not see that shift each time increases by 7, i.e. It cannot have the values 32, 33 and 34. The code works as it should, but I can add that the check shift >= 35 can be replaced by shift >= 32. Then other developers and the analyzer will understand the possible ranges of values.
If you do not know how data flow analysis works, or you are interested in learning about the technologies used in PVS-Studio, then I suggest reading our article.
Fragment 3
public void RemoveData(Data data)
{
if (data == null)
{
throw new GameFrameworkException(Utility.Text.Format("Data '{0}' is null.",
data.Name.ToString()));
}
....
}
PVS-Studio warning: V3080 Possible null dereference. Consider inspecting 'data'. DataManager.cs 122
The developer checks the parameter for validity. If the parameter data equals null, then an exception is thrown. It’s just that when generating an exception message it is used datawhich is exactly equal null. As a result, instead GameFrameworkException we get an exception like NullReferenceException.
Fragment 4
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;
....
}
....
}
PVS-Studio warning: V3008 The 'entityInfo.Status' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 176, 174. EntityManager.cs 176
In this case, the analyzer made an error. He didn't see that the variables entityInfo And entity connected to each other. They wrote it out for revision.
Fragment 5
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;
}
PVS-Studio warning: V3156 The first argument of the 'Add' method is not expected to be null. FileSystem.cs 206
When developing the analyzer, the operating logic of some popular methods is taken into account. We call this mechanism “method annotation”. This is where the analyzer uses the information obtained using this mechanism. The analyzer knows that the method Dictionary
Let's take a look at the method GetString. The result of its work is passed to the method Add.
public string GetString(byte[] encryptBytes)
{
if (m_Length <= 0)
{
return null;
}
....
}
Of the entire method, we are only interested in the beginning, when it returns null. As a result, if m_Length will have a value less than or equal to 0, an exception of type will be thrown ArgumentNullException.
Fragment 6
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);
....
}
PVS-Studio warning: 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
And here you can see the work of interprocedural analysis. The analyzer “tracked” a format string with three placeholders for arguments.
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 of the method StringBuilder.AppendFormat. As we already know, there are three placeholders, but only two arguments. As a result of using such a format string, an exception like FormatException.
I assume that such an error could arise due to copy-paste. There is a similar place in the code below:
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 simply copy similar processing, change the format string, but not change the number of arguments.
Fragment 7
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); // <=
....
}
PVS-Studio warning: V3125 The 'towerLevelData' object was used after it was verified against null. Check lines: 122, 117. LevelControl.cs 122
It seems to use the variable after checking for null not a very good idea. If towerLevelData equals null, then the situation is logged. But this doesn't always happen. Method Log.Error will not be ignored by the compiler only if a conditional compilation symbol related to logging or configuration is defined Debug. In addition, it does not interrupt the flow of execution (and that would be strange). So execution will still reach the line pointed to by the analyzer.
Fragment 8
public DataTableProcessor(....)
{
....
m_Strings = strings.OrderBy(value => value.Key)
.OrderByDescending(value => value.Value)
.Select(value => value.Key).ToArray();
....
}
PVS-Studio warning: V3078 Sorting keys priority will be reversed relative to the order of 'OrderBy…' method calls. Perhaps, 'ThenByDescending' should be used instead. DataTableProcessor.cs 179
Not all developers know that the sorting pattern .OrderBy.OrderBy works in reverse order. We have a separate article on this topic “Sorting in C#: OrderBy.OrderBy or OrderBy.ThenBy? Let's figure out what is more effective and why“. It seems better to use .OrderBy.ThenBy. This not only makes the code more understandable, but also more performant.
Fragment 9
public bool BuildResources()
{
....
bool isSuccess = false;
isSuccess = BuildResources(Platform.Windows, ....);
if (!watchResult || isSuccess)
{
isSuccess = BuildResources(Platform.Windows64, ....);
}
....
if (!watchResult || isSuccess)
{
isSuccess = BuildResources(Platform.WebGL, ....);
}
....
}
PVS-Studio warning: V3137 The 'isSuccess' variable is assigned but is not used by the end of the function. ResourceBuilderController.cs 748
PVS-Studio warns that the variable isSuccess value is assigned. The variable itself is not used after this. IN isSuccess the result of the method is written BuildResources. There are six more assignments hidden under the second '….'. It's hard to say whether this is actually an error, but if the last return value of a method should be ignored, then it is better to use '_':
_ = BuildResources(Platform.WebGL, ....);
And this is not just code so that the analyzer does not issue a warning. Writing this way makes the code more readable for other developers.
Fragment 10
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++;
}
}
}
PVS-Studio warning: V3095 The 'm_SourceVersionIndexes' object was used before it was verified against null. Check lines: 338, 340. ResourcePackBuilder.cs 338
But this warning is difficult to describe somehow 🙂 Field m_SourceVersionIndexes check for null already after accessing the array elements by index. The check should be moved to the beginning of the method.
Conclusion
That's all 🙂 In my opinion, we didn't spend much time and it was pretty easy to sort out these errors. Now you know how to quickly find errors in your code. But it is worth remembering that the Best Warnings mechanism is more suitable for a first acquaintance. Static analysis should be used on an ongoing basis. To do this, you can use commit checks and run analysis on developer computers. This is how static analysis tools come into their own.
Good luck!
If you want to share this article with an English-speaking audience, please use the translation link: Artem Rovenskii. Simple & quick search for bugs in Unity games (for C# developers).
