# Анализ кода WolvenKit: что нужно знать перед созданием модов для Cyberpunk 2077

Все мы любим игры, но есть люди, которые любят в них не только играть, но ещё и создавать различные модификации для них\. Сегодня посмотрим на WolvenKit — один из инструментов для создания модов для Cyberpunk 2077\.

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

## Введение

Я думаю, что большинство из вас ставили моды в свои любимые игры\. И на это есть множество причин\. Добавить новые геймплейные механики, скины, улучшить графику, да и просто привнести больше веселья в игру\. Например, у меня такой игрой был The Elder Scrolls V: Skyrim\. И я точно такой не один\. Это же как раз та игра, где Драконорождённый мог сражаться с паровозиком Томасом, а не с драконами\.

![1165_WolvenKit_ru/image2.gif](https://import.viva64.com/docx/blog/1165_WolvenKit_ru/image2.gif)

Кто\-то даже сам создавал модификации для игр\. Сегодня мы посмотрим на проект для создания модов для игр с точки зрения кода\. А конкретнее, взглянем на ошибки и подозрительные места в коде, обнаруженные статическим анализатором\. Этот проект был найден мной на просторах GitHub\. [WolvenKit](https://github.com/WolvenKit/WolvenKit) — это инструмент для моддинга Cyberpunk 2077 с открытым исходным кодом, написанный на C\#\.

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

Для анализа же мы будем использовать статический анализатор кода PVS\-Studio 7\.32\. Получить триальный ключ и попробовать последнюю версию анализатора можно с помощью этой [страницы](https://pvs-studio.ru/ru/pvs-studio/try-free/)\.

Готовы? Тогда поехали\!

## Ошибки и странности

**Issue 1**

Любая программа начинается с установки, так и WolvenKit не исключение\. На GitHub также доступен [WolvenKit\.Installer](https://github.com/WolvenKit/WolvenKit.Installer)\. Он тоже написан на C\#\. Я решил не оставлять его в стороне и проанализировать\. Да, кода там мало, но это не помешало найти одну ошибку\.

```cpp
public async Task<bool> InstallAsync(....)
{
  ....
  try
  {
    ....
  }
  catch (Octokit.ApiException)
  {
    var apiInfo = ghClient.GetLastApiInfo();
    var rateLimit = apiInfo?.RateLimit;
    var howManyRequestsCanIMakePerHour = rateLimit?.Limit;
    var howManyRequestsDoIHaveLeft = rateLimit?.Remaining;
    var whenDoesTheLimitReset = rateLimit?.Reset;

     _logger.LogInformation(
    $"[Update] {howManyRequestsDoIHaveLeft}/{howManyRequestsCanIMakePerHour}" +
    $" - reset: {whenDoesTheLimitReset 
    ?? whenDoesTheLimitReset.Value.ToLocalTime()}");
    return false;
  }
}
```

Предупреждение PVS\-Studio: [V3105](https://pvs-studio.ru/ru/docs/warnings/v3105/) The 'whenDoesTheLimitReset' variable was used after it was assigned through null\-conditional operator\. NullReferenceException is possible\. LibraryService\.cs 332

Ошибка закралась в обработчик исключений :\) Анализатор говорит нам о том, что переменная _whenDoesTheLimitReset_ используется, хотя получила значение с помощью оператора '?\.', а значит может быть равна _null_\. При этом мы видим проверку на _null_ с помощью оператора '??'\. Вот только переменная как раз используется, если её значение равно _null_\. В итоге получим исключение типа _NullReferenceException_\. Интересно, из\-за чего возникла данная ошибка\. Может разработчик не знал, как работает оператор '??', или эта ошибка возникла из\-за невнимательности\.

**Issue 2**

А теперь переходим к ошибкам основного проекта\.

```cpp
public AppImpl()
{
  ....
  // load oodle
  var settingsManager = Locator.Current.GetService<ISettingsManager>();
  if (   settingsManager.IsHealthy() 
      && !Oodle.Load(settingsManager?.GetRED4OodleDll()))
  {
    throw new FileNotFoundException($"{Core.Constants.Oodle} not found.");
  }
}
```

Предупреждение PVS\-Studio: [V3042](https://pvs-studio.ru/ru/docs/warnings/v3042/) Possible NullReferenceException\. The '?\.' and '\.' operators are used for accessing members of the 'settingsManager' object App\.xaml\.cs 48

Ещё одна ошибка с _null_\. Переменная _settingsManager_ дважды используется в условии оператора _if_\. Причём сначала без проверки на _null_, а во второй раз с проверкой, с помощью '?\.' Можно подумать, что _IsHealthy_ — это метод расширения, и исключения не возникнет, но нет, это обычный экземплярный метод\.

**Issue 3**

```cpp
public EFileReadErrorCodes ReadBuffer(RedBuffer buffer)
{
  ....
  data.Uk1 = _reader.ReadUInt32();
  data.Uk2 = _reader.ReadUInt32();
  data.Uk3 = _reader.ReadUInt32();
  var numBrck = _reader.ReadUInt32();
  var numSurf = _reader.ReadUInt32();
  var numProb = _reader.ReadUInt32();
  var numFact = _reader.ReadUInt32();
  var numTetr = _reader.ReadUInt32();
  data.Uk4 = _reader.ReadUInt32();
  data.Uk5 = _reader.ReadUInt32();
  data.Uk5 = _reader.ReadUInt32();      // <=

  data.Bounds.Min.X = _reader.ReadSingle();
  ....
}
```

Предупреждение PVS\-Studio: [V3008](https://pvs-studio.ru/ru/docs/warnings/v3008/) The 'data\.Uk5' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 35, 34\. CGIDataReader\.cs 35

А вот пример часто встречающейся ошибки\. Свойству _data\.Uk5_ дважды присваивается значение\. Видимо, разработчик просто копировал одну строчку и изменял цифру, а в последней строчке забыл\. И вряд ли это лишняя строчка, так как имеется свойство _data\.Uk6\._

**Issue 4**

```cpp
public void Load()
{
  ....
  for (....; sectorIndex < pgc.BufferTableSectors.Count; ....)  // <=
  {
    if (   pgc.SectorEntries == null 
        || pgc.SectorEntries.Count <= sectorIndex 
        || pgc.SectorEntries[sectorIndex] == null)
    {
      throw new ArgumentNullException();
    }

    var sectorHash = pgc.SectorEntries[sectorIndex]!.SectorHash;
    if (!_entries.ContainsKey(sectorHash))
    {
      _entries[sectorHash] = new();
    }

    if (   pgc.BufferTableSectors == null                    // <=
        || pgc.BufferTableSectors.Count <= sectorIndex 
        || pgc.BufferTableSectors[sectorIndex] == null)
    {
      throw new ArgumentNullException();
    }
  }
}
```

Предупреждение PVS\-Studio: [V3095](https://pvs-studio.ru/ru/docs/warnings/v3095/) The 'pgc\.BufferTableSectors' object was used before it was verified against null\. Check lines: 43, 56\. GeometryCacheService\.cs 43

Анализатор обнаружил использование свойства _pgc\.BufferTableSectors_ перед проверкой этого же свойства на равенство _null_\.

**Issue 5**

```cpp
private void LoadInfo()
{
  if (_projectManager.ActiveProject is null)
  {
    return;
  }

  var path = Path.Combine(Environment.CurrentDirectory,
                          "Resources",
                          "soundEvents.json");
  path = Path.Combine(_projectManager.ActiveProject.ResourcesDirectory,
                      "info.json");
    if (File.Exists(path))
    {
      ....
    }
}
```

Предупреждение PVS\-Studio: [V3008](https://pvs-studio.ru/ru/docs/warnings/v3008/) The 'path' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 82, 81\. SoundModdingViewModel\.cs 82

А вот это вряд ли ошибка, но точно является подозрительным фрагментом кода\. Переменной _path_ два раза присваивается значение\. Возможно, одно из них осталось после отладки или это просто артефакт разработки\. 

**Issue 6**

```cpp
private bool InsertArrayItem(IRedArray ira, int index, IRedType item)
{
  var iraType = ira.GetType();
  if (iraType.IsGenericType)
  {
    var arrayType = iraType.GetGenericTypeDefinition();
    if (   arrayType == typeof(CArray<>) 
        || arrayType == typeof(CStatic<>) 
        && ira.Count < ira.MaxSize)
    {
      ....
    }
  }
}
```

Предупреждение PVS\-Studio: [V3130](https://pvs-studio.ru/ru/docs/warnings/v3130/) Priority of the '&&' operator is higher than that of the '\|\|' operator\. Possible missing parentheses\. ChunkViewModel\.cs 3053

Ещё один подозрительный фрагмент\. Сложно сказать — ошибка или нет, но стоит задуматься над этим кодом\. Анализатор сообщает про возможную ошибку с приоритетом операторов\. Как мы знаем, приоритет оператора '&&' выше, чем '\|\|'\. Получается, что выражение _ira\.Count < ira\.MaxSize_ вычисляется только с _arrayType \=\= typeof\(CStatic<\>\)_, а не со всем OR выражением \. Если разработчик действительно хотел такой логики, то для читаемости стоит взять AND выражение в скобки\.

**Issue 7**

```cpp
private void AddCurrent(worldNodeData current)
{
  ....
  if (Parent?.Data is DataBuffer db && db.Buffer.Data is IRedType irt)
  {
    if (irt is IRedArray ira && ira.InnerType.IsAssignableTo(current.GetType()))
    {
      var indexx = Parent.GetIndexOf(this) + 1;
      if (indexx == -1 || indexx > ira.Count)      // <=
      {
        indexx = ira.Count;
      }
      ira.Insert(indexx, current);
    }
  }
}
```

Предупреждение PVS\-Studio: [V3063](https://pvs-studio.ru/ru/docs/warnings/v3063/) A part of conditional expression is always false if it is evaluated: indexx \=\= \-1\. ChunkViewModel\.cs 4201

Анализатор говорит о том, что выражение _indexx \=\= \-1_ всегда _false_\. Почему он так решил и правильно ли это? Давайте разбираться\. Переменная _indexx_ получает своё значение из выражения _Parent\.GetIndexOf\(this\) \+ 1_\. Взглянем на метод _GetIndexOf_\.

```cpp
public int GetIndexOf(ChunkViewModel child)
{
  if (child.NodeIdxInParent > -1)
  {
    return child.NodeIdxInParent;
  }
  
  for (var i = 0; i < Properties.Count; i++)
  {
    if (ReferenceEquals(Properties[i], child))
    {
      child.NodeIdxInParent = i;
      return i;
    }
  }
  return -1;
}
```

Как видим, самое минимальное, что метод может вернуть — это \-1\. А в нашем выражении к возвращаемому значению метода прибавляют 1\. Получается, что анализатор прав\. Тут либо не нужно прибавлять 1, либо нужно как\-то условие изменить\.

Анализатор по ходу кода подсчитывает возможные значения для переменных и накладываемые ограничения\. Этот механизм называется [Data Flow](https://pvs-studio.ru/ru/blog/terms/7004/)\. Именно благодаря этому анализатор и нашёл такую ошибку\. 

**Issue 8**

```cpp
public static Dictionary<string, string> GetPropertiesFor(....)
{
  ....
  foreach (var appearance in appearancesArr)
  {
    details[....] = appearance.AppearanceName.ToString()!;
    details[....] = appearance.IsPlayer == true ? "True" : "False";
    details[....] = ParseGameEntityReference(appearance?.PuppetRef);

    counter++;
  }
  ....
}
```

Предупреждение PVS\-Studio: [V3095](https://pvs-studio.ru/ru/docs/warnings/v3095/) The 'appearance' object was used before it was verified against null\. Check lines: 547, 548\. NodeProperties\.cs 547

Простая ошибка\. Есть 3 строчки, в которых используется переменная _appearance_\. При этом в последней строчке _appearance_ используется с оператором '?\.', то есть потенциально переменная может иметь значение _null_\.

**Issue 9**

```cpp
public int UncookTask(FileSystemInfo[] paths, UncookTaskOptions options)
{
  ....
  if (options.outpath == null)       // <=
  {
    _loggerService.Error("Please fill in an output path.");
    return ERROR_BAD_ARGUMENTS;
  }

  if (   options.meshExportType != null
      && string.IsNullOrEmpty(options.meshExportMaterialRepo) 
      && options.outpath is null)     // <=
  {
    _loggerService.Error("When using --mesh-export-type, the --outpath" +
    " or the --mesh-export-material-repo must be specified.");
    return ERROR_INVALID_COMMAND_LINE;
  }
}
```

Предупреждение PVS\-Studio: [V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) Expression 'options\.meshExportType \!\= null && string\.IsNullOrEmpty\(options\.meshExportMaterialRepo\) && options\.outpath is null' is always false\. UncookTask\.cs 44

В данном случае анализатор считает, что всё это большое выражение всегда _false_\. И это действительно так\. Все подвыражения соединены через '&&', и при этом последнее подвыражение уже было в предыдущем операторе _if_ с последующим выходом из метода\.

**Issue 10**

```cpp
private static string TryGetGameInstallDir()
{
  if (   programName?.ToString() is string n 
      && installLocation?.ToString() is string l)
  {
    if (n.Contains(gameName) || n.Contains(gameName))  // <= 
    {
      exePath = Directory.GetFiles(l, exeName, SearchOption.AllDirectories)
                         .First();
    }
  }
}
```

Предупреждение PVS\-Studio: [V3001](https://pvs-studio.ru/ru/docs/warnings/v3001/) There are identical sub\-expressions 'n\.Contains\(gameName\)' to the left and to the right of the '\|\|' operator\. Oodle\.cs 488

Анализатор обнаружил одинаковые подвыражения слева и справа от оператора '\|\|'\. Возможно, во втором подвыражении нужно использовать переменную _l_\.

**Issue 11**

```cpp
public void Extract(Stream output, bool decompressBuffers)
{
  if (Archive is not { } ar)
  {
    throw new InvalidParsingException(
      $"{Archive.ArchiveAbsolutePath} is not a cyberpunk77 archive.");
  }

  ar.CopyFileToStream(output, NameHash64, decompressBuffers);
}
```

Предупреждение PVS\-Studio: [V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'Archive'\. FileEntry\.cs 87

По сути, _Archive is not \{ \}_ — это то же самое, что и _Archive is not object_ или _Archive is null_\. И если эта проверка в результате даёт _true_, то при создании исключения об ошибке получим _NullReferenceExceprion_\. 

**Issue 12**

```cpp
public ButtonCursorStateView()
{
  HoverStateName = "Hover";
  PressStateName = "Hover";
  DefaultStateName = "Default";

  PostConstruct();
}
```

Предупреждение PVS\-Studio: [V3091](https://pvs-studio.ru/ru/docs/warnings/v3091/) Empirical analysis\. It is possible that a typo is present inside the string literal: "Hover"\. The 'Hover' word is suspicious\. ButtonCursorStateView\.cs 34

Немного эвристического анализа\. Кажется, анализатор нашёл неудачный copy\-paste и в PressStateName нужно присвоить _"Press"_\.

**Issue 13**

```cpp
public static int GetCompressedBufferSizeNeeded(int count)
{
  var n = (((count + 0x3ffff + ((uint)((count + 0x3ffff) >> 0x3f) & 0x3ffff))
    >> 0x12) * 0x112) + count;
  //var n  = OodleNative.GetCompressedBufferSizeNeeded((long)count);
  return (int)n;
}
```

Предупреждение PVS\-Studio: [V3134](https://pvs-studio.ru/ru/docs/warnings/v3134/) Shift by 63 bits is greater than the size of 'Int32' type of expression '\(count \+ 0x3ffff\)'\. Oodle\.cs 397

И последний подозрительный фрагмент\. В данном случае разработчик хочет выполнить сдвиг на 0x3f, что эквивалентно 63\. Вот только это больше, чем размер типа выражения, к которому применяется сдвиг\. Это выражение имеет тип _Int32_ с размером 32\. Так как в C\# сдвиги выполняются циклически, сдвиг на 63 будет эквивалентен сдвигу на 31\. 

## Заключение

Вот мы и прошлись по всем ошибкам и подозрительным местам в проекте WolvenKit\. В статье я привёл не все ошибки, т\.к\. многие из них повторяются, но я обязательно сообщу обо всех разработчикам с помощью issue на GitHub\.

Удачи\!