Для получения триального ключа
заполните форму ниже
Team license
Enterprise license
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
RUB
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Бесплатная лицензия PVS-Studio для специалистов Microsoft MVP
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Мне интересно попробовать плагин на:
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

>
>
>
Проверка Barotrauma статическим анализа…

Проверка Barotrauma статическим анализатором PVS-Studio

31 Мар 2022

Barotrauma – игра, в которой можно поуправлять подлодкой, попрятаться от монстров и даже поиграть на аккордеоне в попытке не пойти ко дну. Посмотрим, как проект, начатый инди-студией Undertow Games и продолженный совместно с FakeFish, выглядит изнутри. Для этого исследуем исходный код, преимущественно написанный на языке C#, с помощью статического анализатора PVS-Studio.

0930_Barotrauma_ru/image1.png

Введение

Barotrauma – многопользовательский космический 2D-симулятор подлодки в жанре survival horror. Игроку предстоит управлять подлодкой, отдавать приказы, устранять протечки и противостоять опасностям.

Barotrauma не является Open Source проектом в обычном понимании. Ранняя версия игры доступна бесплатно, текущая версия доступна в Steam. Разработчики опубликовали исходный код на GitHub для того, чтобы сообщество игроков могло свободно разрабатывать более комплексные моды и находить существующие ошибки.

Результаты проверки

Ошибки в if

V3001 There are identical sub-expressions 'string.IsNullOrEmpty(EndPoint)' to the left and to the right of the '||' operator. BanList.cs 41

public bool CompareTo(string endpointCompare)
{
  if (string.IsNullOrEmpty(EndPoint) || string.IsNullOrEmpty(EndPoint)) 
  { return false; }
  ....
}

Значение EndPoint проверяется дважды. Разработчик, скорее всего, забыл поменять параметр EndPoint на endpointCompare при копировании метода string.IsNullOrEmpty. Вообще программисты очень часто ошибаются в методах сравнения. Рекомендую почитать статью коллеги об этом.

V3004 The 'then' statement is equivalent to the 'else' statement. ServerEntityEventManager.cs 314

public void Write(Client client, IWriteMessage msg, 
                  out List<NetEntityEvent> sentEvents)
{
  List<NetEntityEvent> eventsToSync = null;
  if (client.NeedsMidRoundSync)
  {
    eventsToSync = GetEventsToSync(client);
  }
  else
  {
    eventsToSync = GetEventsToSync(client);
  }
  ....
}

Вне зависимости от значения client.NeedsMidRoundSync будет выполняться одно и то же. Возможно следует убрать else-ветвь или переработать её поведение.

Следующий фрагмент кода вызвал два предупреждения:

  • V3021 There are two 'if' statements with identical conditional expressions. The first 'if' statement contains method return. This means that the second 'if' statement is senseless DebugConsole.cs 2177
  • V3022 Expression 'args.Length < 2' is always false. DebugConsole.cs 2183
private static void InitProjectSpecific()
{
  ....
  AssignOnClientRequestExecute(
    "setclientcharacter",
    (Client senderClient, Vector2 cursorWorldPos, string[] args) =>
    {
      if (args.Length < 2)
      {
        GameMain.Server.SendConsoleMessage("....", senderClient);
        return;
      }

      if (args.Length < 2)
      {
        ThrowError("....");
        return;
      }
    );
  ....
}

Две одинаковые проверки. В случае выполнения условия первого if метод завершит работу, иначе же обе then-ветви не будут выполнены.

При такой работе сообщение будет отправляться, но запись ошибки методом ThrowError не произойдёт. Следует объединить два тела if или изменить условие второго.

V3022 Expression 'newPrice > 0' is always true. DebugConsole.cs 3310

private static void PrintItemCosts(....)
{
  if (newPrice < 1)
  {
    NewMessage(depth + materialPrefab.Name + 
    " cannot be adjusted to this price, because it would become less than 1.");
    return;
  }

  ....

  if (newPrice > 0)
  {
    newPrices.TryAdd(materialPrefab, newPrice);
  }
  ....
}

Если newPrice меньше или равен 0, следует выполнение тела первого if. После этого исполнение метода завершается. Следовательно условие второго if всегда будет истинно. Поэтому можно добавить тело второго if в else-ветвь первого или же вовсе его убрать.

Опечатки

V3005 The 'arrowIcon.PressedColor' variable is assigned to itself. ChatBox.cs 164

public ChatBox(GUIComponent parent, bool isSinglePlayer)
{
  ....
  arrowIcon = new GUIImage(....)
  {
    Color = new Color(51, 59, 46)
  };
  arrowIcon.HoverColor = arrowIcon.PressedColor = 
  arrowIcon.PressedColor = arrowIcon.Color;
  ....  
}

Значение переменной arrowIcon.PressedColor присваивается самой себе. В то же время внутри класса GUIIMage содержится свойство SelectedColor. Скорее всего, разработчик хотел использовать его, но опечатался.

V3005 The 'Penetration' variable is assigned to itself. Attack.cs 324

public Attack(float damage, 
              float bleedingDamage, 
              float burnDamage, 
              float structureDamage,
              float itemDamage, 
              float range = 0.0f, 
              float penetration = 0f)
{
   ....
   Range = range;
   DamageRange = range;
   StructureDamage = LevelWallDamage = structureDamage;
   ItemDamage = itemDamage;     
   Penetration = Penetration;                // <=
}

Ещё одна подобная ошибка. В этом случае программист хотел проинициализировать свойства объекта, но вместо параметра penetration присвоил свойству Penetration своё же значение.

V3025 Incorrect format. A different number of format items is expected while calling 'Format' function. Arguments not used: t.Character.Name. DebugConsole.cs 1123

private static void InitProjectSpecific()
{
  AssignOnClientRequestExecute("traitorlist", 
      (Client client, Vector2 cursorPos, string[] args) =>
  {
    ....
    GameMain.Server.SendTraitorMessage(
     client, 
     string.Format("- Traitor {0} has no current objective.",            // <=
                   "",                                                   // <=
                   t.Character.Name),                                    // <=
     "",
     TraitorMessageType.Console);   
  });
}

Исходя из смысла использованной в методе GameMain.Server.SendTraitorMessage фразы, логично предположить, что спецификатор ввода {0} должен был содержать t.Character.Name. Однако же там окажется пустая строка.

Ошибка, скорее всего, является следствием неудачного copy-paste предыдущего использования метода GameMain.Server.SendTraitorMessage:

GameMain.Server.SendTraitorMessage(client, 
"There are no traitors at the moment.", "", TraitorMessageType.Console);

Возможно возникновение NullReferenceException

V3153 Enumerating the result of null-conditional access operator can lead to NullReferenceException. Voting.cs 181

public void ClientRead(IReadMessage inc)
{
  ....
  foreach (GUIComponent item in
           GameMain.NetLobbyScreen?.SubList?.Content?.Children)    // <=
  {
    if (item.UserData != null && item.UserData is SubmarineInfo) 
    {
      serversubs.Add(item.UserData as SubmarineInfo); 
    }
  }
  ....
}

Если хотя бы один компонент из последовательности GameMain.NetLobbyScreen?.SubList?.Content?.Children будет равен null, то результат всего выражения тоже будет равен null. В таком случае будет выброшено исключение NullReferenceException при попытке перебора в foreach.

Подробно про использование оператора ?. в foreach можно прочитать в этой статье.

V3027 The variable 'spawnPosition' was utilized in the logical expression before it was verified against null in the same logical expression. LevelObjectManager.cs 274

private void PlaceObject(LevelObjectPrefab prefab, 
                         SpawnPosition spawnPosition, 
                         Level level, Level.Cave parentCave = null)
{
  float rotation = 0.0f;
  if (   prefab.AlignWithSurface 
      && spawnPosition.Normal.LengthSquared() > 0.001f          // <=
      && spawnPosition != null)                                 // <=
  {
    rotation = MathUtils.VectorToAngle(new Vector2(spawnPosition.Normal.Y, 
                                                   spawnPosition.Normal.X));
  }
  ....
}

Из кода видно, что сначала идёт вызов метода LengthSquared у поля Normal переменной spawnPosition и сравнение его с заданным значением, а затем переменная проверяется на null. Если spawnPosition будет равна null, то возникнет исключение NullReferenceException.

Простейшим решением будет поместить проверку на null в начало условия.

V3095 The 'level' object was used before it was verified against null. Check lines: 107, 115. BeaconMission.cs 107

public override void End()
{
  completed = level.CheckBeaconActive();                        // <=
  if (completed)
  {
    if (Prefab.LocationTypeChangeOnCompleted != null)
    {
      ChangeLocationType(Prefab.LocationTypeChangeOnCompleted);
    }
    GiveReward();
    if (level?.LevelData != null)                               // <=
    {
      level.LevelData.IsBeaconActive = true;
    }
  }
}

Сначала значение level.CheckBeaconActive присваивается, а затем используется оператор ?. в выражении level?.LevelData. В этом случае возможны ситуации, когда level будет равен null — и будет выброшено исключение NullReferenceException либо level никогда не будет null — и проверка избыточна.

Выход за границы

V3106 Possibly index is out of bound. The '0' index is pointing beyond 'Sprites' bound. ParticlePrefab.cs 303

public ParticlePrefab(XElement element, ContentFile file)
{
  ....
  if (CollisionRadius <= 0.0f) 
    CollisionRadius = Sprites.Count > 0 ? 1 : 
                                          Sprites[0].SourceRect.Width / 2.0f;
}

При выполнении условия тернарного оператора значение переменной CollisionRadius станет равно 1. В противном случае значение Sprites.Count равно 0, и при обращении к первому элементу коллекции возникнет исключение IndexOutOfRangeException.

Ранее по коду встречается проверка: является ли коллекция пустой.

if (Sprites.Count == 0)
{
  DebugConsole.ThrowError($"Particle prefab \"{Name}\" in the file \"{file}\"
                            has no sprites defined!");
}

Однако блокирование выполнения дальнейшего кода не происходит. Разработчику следует пересмотреть условие тернарного оператора.

Лишние действия

V3107 Identical expression 'power' to the left and to the right of compound assignment. RelayComponent.cs 150

public override void ReceivePowerProbeSignal(Connection connection, 
                                             Item source, float power)
{
  ....
  if (power < 0.0f)
  {
    ....
  }
  else
  {
    if (connection.IsOutput || powerOut == null) { return; }

    if (currPowerConsumption - power < -MaxPower)
    {
      power += MaxPower + (currPowerConsumption - power);
    }
  }
}

Программист пытается прибавить переменной power переменную MaxPower и разницу между переменными currPowerConsumption и power. В разложенном варианте выражение будет иметь вид:

power = power + MaxPower + (currPowerConsumption - power);

Выражение можно упростить, так как переменная power вычитается сама из себя. Упрощённый код будет выглядеть так:

power = MaxPower + currPowerConsumption;

Всегда false

V3009 It's odd that this method always returns one and the same value of 'false'. FileSelection.cs 395

public static bool MoveToParentDirectory(GUIButton button, object userdata)
{
  string dir = CurrentDirectory;
  if (dir.EndsWith("/")) { dir = dir.Substring(0, dir.Length - 1); }
  int index = dir.LastIndexOf("/");
  if (index < 0) { return false; }
  CurrentDirectory = CurrentDirectory.Substring(0, index+1);

  return false;
}

Странный метод, всегда возвращающий false. Возможно, здесь нет ошибки и так задумано, либо один из return должен возвращать true.

Утраченное значение

V3010 The return value of function 'Trim' is required to be utilized. GameServer.cs 1589

private void ClientWriteInitial(Client c, IWriteMessage outmsg)
{
  ....

  if (gameStarted)
  {
    ....

    if (ownedSubmarineIndexes.Length > 0)
    {
      ownedSubmarineIndexes.Trim(';');
    }
    outmsg.Write(ownedSubmarineIndexes);
  }
}

Метод Trim не меняет значение ownedSubmarineIndexes, поэтому нет смысла вызывать его, не сохраняя результат. Правильный вариант:

ownedSubmarineIndexes = ownedSubmarineIndexes.Trim(';');

Заключение

PVS-Studio обнаружил ряд ошибок, опечаток и недочётов в исходном коде Baratrauma. Многие из них довольно легко допустить при разработке и оставить незамеченными на code-review.

Статический анализ может помочь разработчикам сохранить время, которое они бы потратили на поиск и исправление багов, и уделить его разработке нового контента. Однако мало проверить код один раз. Максимальная эффективность достигается при регулярном использовании анализатора в процессе написания кода.

О проверке других проектов с помощью статического анализатора PVS-Studio можно прочесть в нашем блоге.

Популярные статьи по теме
Зачем разработчикам игр на Unity использовать статический анализ?

Дата: 11 Май 2022

Автор: Артём Ровенский

С годами стоимость создания игр стала больше, вырос их масштаб, а следовательно, и их кодовая база. Разработчикам становится всё сложнее уследить за ошибками. А забагованная игра влечёт финансовые и …
Повторная проверка Unity статическим анализатором PVS-Studio

Дата: 08 Апр 2022

Автор: Артём Ровенский

Unity – один из самых популярных игровых движков. С его помощью создаётся множество отличных межплатформенных проектов. С нашей последней проверки его исходного кода прошло почти 4 года. Пришло время…
Игра с null: проверка MonoGame статическим анализатором PVS-Studio

Дата: 04 Фев 2022

Автор: Вадим Кулешов

Анализатор PVS-Studio уже не раз был использован для анализа кода библиотек, фреймворков и движков для разработки игр. Пришло время добавить к их списку MonoGame – низкоуровневый gamedev-фреймворк, н…
Nintendo Switch: Drop Test исходников эмулятора Ryujinx

Дата: 30 Июн 2021

Автор: Данила Карпов

Такие компании, как Sony, Microsoft и Nintendo, каждое поколение радуют своих потребителей новыми консолями и различными играми на них. Но вот беда, часть игр является эксклюзивами своих платформ, а …
Анализ Unity проектов: The solution file has two projects named "UnityEngine.UI"

Дата: 09 Июн 2021

Автор: Сергей Васильев

При анализе Unity проекта с помощью PVS-Studio может возникнуть ошибка следующего вида: Error was encountered while trying to open solution file '...': The solution file has two projects named "Unity…

Комментарии (0)

Следующие комментарии
Unicorn with delicious cookie
Мы используем куки, чтобы пользоваться сайтом было удобно. Хотите узнать подробнее?
Принять