# Быстро и легко ищем баги в играх на Unity \(для C\# разработчиков\) Все мы сталкиваемся с багами в играх\. В некоторых даже чаще, чем хотелось бы\. Множество этих багов связаны с ошибками в коде\. Сегодня посмотрим, как с помощью статического анализа можно легко находить ошибки и улучшать ваш проект\. ![1111_TowerDefenceDemo_ru/image1.png](https://import.viva64.com/docx/blog/1111_TowerDefenceDemo_ru/image1.png) Игровая индустрия становится всё больше и больше\. С каждым годом растёт стоимость разработки, и вместе с ней цена на сами игры\. У нас есть статья на эту тему: "[Зачем разработчикам игр на Unity использовать статический анализ?](https://pvs-studio.ru/ru/blog/posts/csharp/0945/)"\. Когда вы её прочитали и поняли, какую пользу разработчики игр получают от инструментов статического анализа кода, поговорим про Best Warnings\. Это один из способов быстрого знакомства со статическим анализатором\. "А зачем это знакомство вообще нужно?" — спросите вы\. Отвечаем\. Часто на крупных проектах отчёт анализатора может содержать сотни или даже тысячи предупреждений\. Разбираться с ними бывает довольно трудно\. Когда вы уже выбрали статический анализатор, неактуальные предупреждения можно просто подавить и видеть только новые\. Но что, если вы только знакомитесь с инструментом? Для таких ситуации и создавался механизм Best Warnings\. ## Как получить Best Warnings? Для примера возьмём Open Source проект TowerDefense\-GameFramework\-Demo\. Его исходный код доступен на [GitHub](https://github.com/DrFlower/TowerDefense-GameFramework-Demo)\. К сожалению, очень мало игр на Unity имеют открытый исходный код, который нам как раз и нужен для демонстрации\. Но даже на этом небольшом проекте мы всё равно увидим работу Best Warnings\. Первым делом у вас уже должен быть установлен анализатор и введена лицензия\. Если же вы ещё этого не сделали, то вам поможет эта [страница](https://pvs-studio.ru/ru/pvs-studio/try-free/)\. Откройте ваш проект в Unity Hub\. Далее в Unity нажмите: **Assets \> Open C\# Project**\. У вас откроется IDE, заданная в настройках редактора Unity\. Она указывается здесь: ![1111_TowerDefenceDemo_ru/image2.png](https://import.viva64.com/docx/blog/1111_TowerDefenceDemo_ru/image2.png) Я буду использовать IDE Visual Studio 2022, в которой привык работать\. Чтобы проанализировать проект в этой версии IDE, можно использовать пункт меню **Extensions \> PVS\-Studio \> Check Solution**\. ![1111_TowerDefenceDemo_ru/image3.png](https://import.viva64.com/docx/blog/1111_TowerDefenceDemo_ru/image3.png) После анализа потребуется лишь нажать на кнопку **Best**\. После этого вы увидите выборку из 10 предупреждений анализатора: ![1111_TowerDefenceDemo_ru/image4.png](https://import.viva64.com/docx/blog/1111_TowerDefenceDemo_ru/image4.png) ## Разберём предупреждения Как мы уже знаем, раздел Best Warnings содержит 10 предупреждений, указывающих на код, который, вероятнее всего, содержит ошибку\. Рассмотрим все 10\. Спойлер: будут и реальные ошибки, и просто подозрительный код, и, конечно же, False Positive предупреждения\. Естественно, что ложноположительные срабатывания — это нормально для технологии статического анализа\. Главное, чтобы процент подобных предупреждений был небольшим\. Ну что ж, приступим\! **Фрагмент 1** ```cpp public void OnRecycle() { transform.SetParent(initRoot, false); transform.localPosition = transform.localPosition; transform.eulerAngles = initRotation; transform.localScale = initScale; .... } ``` Предупреждение PVS\-Studio: [V3005](https://pvs-studio.ru/ru/docs/warnings/v3005/) The 'transform\.localPosition' variable is assigned to itself\. Item\.cs 150 Как видно из кода, свойство _transform\.localPosition_ присваивается само себе\. Это довольно распространённая ошибка, которую мы часто видим в анализируемых проектах\. **Фрагмент 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; } ``` Предупреждение PVS\-Studio: [V3134](https://pvs-studio.ru/ru/docs/warnings/v3134/) Shift by \[32\.\.34\] bits is greater than the size of 'Int32' type of expression '\(b & 0x7f\)'\. BinaryExtension\.cs 37 А вот в этом случае анализатор ошибся\. Он говорит о неправильном сдвиге\. К сожалению, анализатор не увидел, что _shift_ каждый раз увеличивается на 7, т\.е\. значений 32, 33 и 34 у него быть не может\. Код работает как надо, но могу добавить, что проверку _shift \>\= 35_ можно заменить на _shift \>\= 32_\. Тогда другим разработчикам и анализатору будут понятны возможные диапазоны значений\. Если вы не знаете, как работает анализ потока данных, или вам интересно узнать про технологии, используемые в PVS\-Studio, то предлагаю прочитать нашу [статью](https://pvs-studio.ru/ru/blog/posts/0908/)\. **Фрагмент 3** ```cpp public void RemoveData(Data data) { if (data == null) { throw new GameFrameworkException(Utility.Text.Format("Data '{0}' is null.", data.Name.ToString())); } .... } ``` Предупреждение PVS\-Studio: [V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'data'\. DataManager\.cs 122 Разработчик проверяет параметр на валидность\. Если параметр _data_ равен _null_, то генерируется исключение\. Вот только при формировании сообщения исключения используется _data_, который как раз равен _null_\. В итоге вместо _GameFrameworkException_ получаем исключение типа _NullReferenceException_\. **Фрагмент 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; .... } .... } ``` Предупреждение PVS\-Studio: [V3008](https://pvs-studio.ru/ru/docs/warnings/v3008/) The 'entityInfo\.Status' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 176, 174\. EntityManager\.cs 176 В этом случае анализатор ошибся\. Он не увидел, что переменные _entityInfo_ и _entity_ связаны друг с другом\. Выписали себе на доработку\. **Фрагмент 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; } ``` Предупреждение PVS\-Studio: [V3156](https://pvs-studio.ru/ru/docs/warnings/v3156/) The first argument of the 'Add' method is not expected to be null\. FileSystem\.cs 206 При разработке анализатора учитывается логика работы некоторых популярных методов\. Этот механизм мы называем "аннотирование методов"\. Как раз здесь анализатор использует информацию, полученную с помощью этого механизма\. Анализатор знает, что метод _Dictionary\.Add_ не может принимать _null_\. Взглянем на метод _GetString_\. Результат его работы передаётся в метод _Add_\. ```cpp public string GetString(byte[] encryptBytes) { if (m_Length <= 0) { return null; } .... } ``` Из всего метода нам интересно только начало, когда возвращается _null_\. В результате, если _m\_Length_ будет иметь значение меньше или равное 0, то будет выброшено исключение типа _ArgumentNullException_\. **Фрагмент 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); .... } ``` Предупреждение PVS\-Studio: [V3025](https://pvs-studio.ru/ru/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 А здесь видно работу межпроцедурного анализа\. Анализатор "проследил" за строкой формата с тремя плейсхолдерами для аргументов\. ```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(); } ``` Эта строка формата используется в качестве первого аргумента метода _StringBuilder\.AppendFormat\._ Как мы уже знаем, плейсхолдера три, а вот аргументов всего два\. В результате использования такой строки формата будет выброшено исключение типа _FormatException_\. Предположу, что подобная ошибка могла возникнуть из\-за copy\-paste\. Ниже в коде есть похожее место: ```cpp if (!fileSystem.WriteFile(resourceFullName, resourcePath)) { throw new GameFrameworkException(Utility.Text.Format( "Write resource '{0}' to file system '{1}' error.", resourceFullName, fileSystem.FullPath)); } ``` Разработчик мог просто скопировать похожую обработку, изменить строку формата, но не изменить количество аргументов\. **Фрагмент 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); // <= .... } ``` Предупреждение PVS\-Studio: [V3125](https://pvs-studio.ru/ru/docs/warnings/v3125/) The 'towerLevelData' object was used after it was verified against null\. Check lines: 122, 117\. LevelControl\.cs 122 Кажется, что использовать переменную после проверки на _null_ не очень хорошая идея\. Если _towerLevelData_ равен _null_, то ситуация логируется\. Но это происходит не всегда\. Метод _Log\.Error_ не будет проигнорирован компилятором только в том случае, когда определён символ условной компиляции, связанный с логированием или конфигурацией _Debug_\. Вдобавок он не прерывает поток выполнения \(да и это было бы странно\)\. Так что до строчки, на которую указывает анализатор, исполнение всё равно дойдёт\. **Фрагмент 8** ```cpp public DataTableProcessor(....) { .... m_Strings = strings.OrderBy(value => value.Key) .OrderByDescending(value => value.Value) .Select(value => value.Key).ToArray(); .... } ``` Предупреждение PVS\-Studio: [V3078](https://pvs-studio.ru/ru/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 Не все разработчики знают, что паттерн сортировки _\.OrderBy\.OrderBy_ работает "в обратном" порядке\. На эту тему у нас есть отдельная статья "[Сортировки в C\#: OrderBy\.OrderBy или OrderBy\.ThenBy? Разбираемся, что эффективнее и почему](https://pvs-studio.ru/ru/blog/posts/csharp/0991/)"\. Кажется, что лучше использовать _\.OrderBy\.ThenBy_\. Это не только делает код более понятным, но и более производительным\. **Фрагмент 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, ....); } .... } ``` Предупреждение PVS\-Studio: [V3137](https://pvs-studio.ru/ru/docs/warnings/v3137/) The 'isSuccess' variable is assigned but is not used by the end of the function\. ResourceBuilderController\.cs 748 PVS\-Studio предупреждает о том, что переменной _isSuccess_ присваивается значение\. Сама переменная после этого не используется\. В _isSuccess_ записывается результат работы метода _BuildResources_\. Под вторым '\.\.\.\.' скрыты ещё шесть присваиваний\. Сложно сказать, действительно ли это ошибка, но если последнее возвращаемое значение метода должно игнорироваться, то лучше использовать '\_': ```cpp _ = BuildResources(Platform.WebGL, ....); ``` И это не просто код, чтобы анализатор не выдавал предупреждение\. Подобное написание делает код более читаемым для других разработчиков\. **Фрагмент 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++; } } } ``` Предупреждение PVS\-Studio: [V3095](https://pvs-studio.ru/ru/docs/warnings/v3095/) The 'm\_SourceVersionIndexes' object was used before it was verified against null\. Check lines: 338, 340\. ResourcePackBuilder\.cs 338 А вот это предупреждение сложно как\-то расписать :\) Поле _m\_SourceVersionIndexes_ проверяют на _null_ уже после доступа к элементам массива по индексу\. Проверку стоит перенести в начало метода\. ## Заключение Вот и всё :\) Как по мне, мы не потратили много времени, и было довольно легко разобрать эти ошибки\. Теперь вы знаете, как можно быстро найти ошибки в коде\. Но стоит помнить, что механизм Best Warnings больше подходит для первого знакомства\. Статический анализ нужно использовать на [постоянной основе](https://habr.com/ru/articles/436868/)\. Для этого можно использовать проверки коммитов и запуски анализа на компьютерах разработчиков\. Именно так инструменты статического анализа раскрываются в полной мере\. Удачи\!