Barotrauma — игра, в которой вы сможете управлять подводной лодкой, прятаться от монстров и даже играть на аккордеоне, пытаясь не затонуть.
Давайте посмотрим, как проект, начатый инди-студией Undertow Games и продолженный совместно с FakeFish, выглядит изнутри.
Для этого мы проверим исходный код, написанный преимущественно на C#, с помощью статического анализатора PVS-Studio. 
Введение
Barotrauma — многопользовательский 2D-симулятор космической подводной лодки в жанре Survival Horror. Игроку предстоит управлять подводной лодкой, отдавать приказы, устранять утечки и противостоять опасностям.Barotrauma не является проектом с открытым исходным кодом в обычном понимании.
Ранняя версия игры доступна бесплатно, текущая версия доступна по адресу Пар .
Разработчики опубликовали исходный код на GitHub чтобы сообщество игроков могло свободно разрабатывать более полные моды и находить существующие ошибки.
Результаты теста
Ошибки в if
В3001 Слева и справа от '||' имеются идентичные подвыражения 'string.IsNullOrEmpty(EndPoint)'.оператор.
BanList.cs 41
Значение Конечная точка проверяется дважды.public bool CompareTo(string endpointCompare) { if (string.IsNullOrEmpty(EndPoint) || string.IsNullOrEmpty(EndPoint)) { return false; } .}
Разработчик скорее всего забыл изменить параметр Конечная точка на конечная точкаСравнить при копировании метода строка.
IsNullOrEmpty .
Вообще программисты очень часто допускают ошибки в методах сравнения.
Я рекомендую прочитать это статья коллеги по этому поводу.
В3004 Оператор then эквивалентен оператору else. 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 сделаю то же самое.
Возможно, его следует удалить еще -разветвить или переработать его поведение.
Следующий фрагмент кода выдал два предупреждения:
- В3021 Есть два оператора if с одинаковыми условными выражениями.
Первый оператор if содержит возврат метода.
Это означает, что второй оператор if бессмысленен.
DebugConsole.cs 2177
- В3022 Выражение '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;
}
);
.
}
Два одинаковых чека.
Если первое условие выполнено если метод завершится, в противном случае оба затем -ветви не будут выполняться.
При работе таким способом сообщение будет отправлено, но метод записи ошибки БросокError не случится.
Два тела должны быть объединены если или изменить состояние второго.
В3022 Выражение «newPrice > 0» всегда истинно.
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); } .
}
Если новая цена меньше или равно 0, тело первого должно быть выполнено если .
После этого выполнение метода заканчивается.
Поэтому условие второго если всегда будет правдой.
Поэтому вы можете добавить тело второго *if *in еще - ветку первого или удалить его вообще.
Опечатки
В3005 ПеременнаяarrowIcon.PressedColor присваивается сама себе.ЧатБокс.
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* есть свойство ВыбранныйЦвет .
Скорее всего, разработчик хотел его использовать, но опечатался.
В3005 Переменная «Проникновение» присваивается сама себе.
Атака.
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; // <=
}
Еще одна похожая ошибка.
В данном случае программист хотел инициализировать свойства объекта, но вместо параметра проникновение закреплен за имуществом Проникновение свой собственный смысл.
В3025 Неправильный формат. При вызове функции «Формат» ожидается другое количество элементов формата.
Не используемые аргументы: 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 .
Однако там будет пустая строка.
Ошибка, скорее всего, является следствием неудачной попытки копирования и вставки предыдущего использования метода.
GameMain.Server.SendTraitorMessage : GameMain.Server.SendTraitorMessage(client,
"There are no traitors at the moment.", "", TraitorMessageType.Console);
Может возникнуть исключение NullReferenceException
В3153 Перечисление результата оператора условного доступа со значением NULL может привести к исключению NullReferenceException. Голосование.
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 будет равен нулевой , то результат всего выражения также будет равен нулевой .
В этом случае при попытке перебора будет выброшено исключение *NullReferenceException *.
для каждого .
Подробности об использовании оператора ? .
В для каждого можно прочитать в Эта статья .
В3027 Переменная spawnPosition использовалась в логическом выражении до того, как она была проверена на соответствие нулю в том же логическом выражении.
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));
}
.
}
Из кода видно, что метод вызывается первым.
Длина в квадрате у поля Нормальный переменная позиция появления и сравнить его с заданным значением, а затем переменная проверяется на соответствие нулевой .
Если позиция появления будет равен нулевой , то будет выброшено исключение NullReferenceException .
Самым простым решением было бы поставить галочку на нулевой к началу состояния.
В3095 Объект «уровень» использовался до того, как было проверено его соответствие нулю.
Проверьте строки: 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 назначается, а затем используется оператор ?.
в выражении уровень?.
LevelData .
При этом возможны ситуации, когда уровень будет равен нулевой - и будет выброшено исключение NullReferenceException или уровень никогда нулевой - и проверка лишняя.
Выходя за границы
В3106 Возможно, индекс вышел за пределы.Индекс «0» указывает за границу «Спрайтов».
ЧастицаПрефаб.
cs 303 public ParticlePrefab(XElement element, ContentFile file)
{
.
if (CollisionRadius <= 0.0f) CollisionRadius = Sprites.Count > 0 ? 1 : Sprites[0].
SourceRect.Width / 2.0f;
}
Когда условие тернарного оператора выполнено, значение переменной СтолкновениеРадиус станет равным 1. В противном случае значение Спрайты.
Количество равно 0, и при доступе к первому элементу коллекции будет выдано исключение.
IndexOutOfRangeException .
Ранее в коде есть проверка: пуста ли коллекция.
if (Sprites.Count == 0)
{
DebugConsole.ThrowError($"Particle prefab \"{Name}\" in the file \"{file}\"
has no sprites defined!");
}
Однако выполнение дальнейшего кода не блокируется.
Разработчику следует пересмотреть условие тернарного оператора.
Ненужные действия
В3107 Идентичное выражение «власть» слева и справа от составного присваивания.
РелеКомпонент.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);
}
}
}
Программист пытается добавить переменную власть переменная Максимальная мощность и разница между переменными currМощностьПотребление И власть .
В расширенной версии выражение будет выглядеть так: power = power + MaxPower + (currPowerConsumption - power);
Выражение можно упростить, поскольку переменная власть вычитается из самого себя.
Упрощенный код будет выглядеть так: power = MaxPower + currPowerConsumption;
Всегда ложь
В3009 Странно, что этот метод всегда возвращает одно и то же значение 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;
}
Странный метод, который всегда возвращает ЛОЖЬ .
Возможно здесь нет ошибки и так задумано, или одна из возвращаться должен вернуться истинный .
Потерянный смысл
В3010 Необходимо использовать возвращаемое значение функции Trim. GameServer.cs 1589 private void ClientWriteInitial(Client c, IWriteMessage outmsg)
{
.
if (gameStarted)
{
.
if (ownedSubmarineIndexes.Length > 0)
{
ownedSubmarineIndexes.Trim(';');
}
outmsg.Write(ownedSubmarineIndexes);
}
}
Метод Подрезать не меняет смысла ownSubmarineIndexes , поэтому нет смысла вызывать его без сохранения результата.
Правильный вариант: ownedSubmarineIndexes = ownedSubmarineIndexes.Trim(';');
Заключение
PVS-Studio обнаружил ряд ошибок, опечаток и недостатков в исходном коде Baratrauma. Многие из них довольно легко реализовать во время разработки и остаются незамеченными при код-ревью.Статический анализ может помочь разработчикам сэкономить время, которое в противном случае они потратили бы на поиск и исправление ошибок, и потратить больше времени на разработку нового контента.
Однако один раз проверить код недостаточно.
Максимальная эффективность достигается при регулярном использовании анализатора при написании кода.
О проверке других проектов с помощью статического анализатора PVS-Studio вы можете прочитать в нашей статье.
блог .
Если вы хотите поделиться этой статьей с англоязычной аудиторией, воспользуйтесь ссылкой для перевода: Михаил Евтихевич.
Проверка Barotrauma статическим анализатором PVS-Studio .
Теги: #Разработка игр #gamedev #C++ #pvs-studio #pvs #статический анализ
-
Вы Когда-Нибудь Писали Парсеры?
19 Oct, 24 -
Зарядка Электромобилей С Поверхности Дороги
19 Oct, 24 -
Я Самозванец
19 Oct, 24 -
Как Долго Мы Будем Жить На Свалке?
19 Oct, 24
