Мы используем куки, чтобы пользоваться сайтом
было удобно.
Наконец, состоялся релиз Unity 6! Разработчики называют эту версию самой стабильной версией за всё время существования движка. Почему бы нам не убедиться в этом с помощью статического анализатора кода? А заодно проведём краткий обзор основных фич и улучшений, привнесённых обновлением.
Unity 6 — это глобальное обновление, нацеленное на улучшение производительности и стабильности движка, но далеко не ограничивающееся этим. Улучшения затронули самые разные аспекты: инструментарий мультиплеера, систему освещения, рендеринг, постобработку, XR-инструментарий, некоторые визуальные эффекты и пр. Отдельно хочу отметить, что была добавлена система для интеграции нейросетей.
Хотя preview-версия Unity 6 уже была доступна с 15 мая 2024 года, возможно вы, как и я, лишь недавно заинтересовались что же особенного в этом обновлении. Поэтому в первой части статьи я хочу провести краткий обзор наиболее значительных, на мой взгляд, улучшений, но не буду сильно углубляться в детали. Вместо этого я поделюсь ссылками, по которым вы сможете получить больше информации.
Во второй части статьи вы сможете испытать свои навыки анализа кода и попробовать отыскать потенциальные ошибки в нескольких фрагментах кода, найденных статическим анализатором в Open Source части движка. Такая тренировка — хорошая возможность научиться на чужих ошибках и минимизировать вероятность их допущения в собственном коде.
Приятного чтения!
Итак, давайте узнаем, что же команда Unity приготовила для нас на этот раз.
Добавлен фреймворк Sentis. Он позволяет интегрировать предварительно обученные модели нейронных сетей в среду выполнения и использовать их для реализации таких функций как распознавание речи и объектов, создания интеллектуальных NPC и т. д.
Рисунок N1 – Multiplayer Play Mode на практике
В левой части изображения эффект озонового слоя не используется в отличии от правой.
Эффект подводного тумана
Перечисленные здесь изменения показались мне основными, но это вовсе не полный список. Более полный список улучшений, а также их подробное описание вы можете найти здесь.
Теперь, когда мы познакомились с положительными изменениями, настало время узнать не появились ли вместе с обновлением новые ошибки в исходном коде движка.
Итак, вооружившись анализатором кода PVS-Studio, я исследовал C#-часть наиболее актуальной на момент написания статьи версии движка (6000.0.21f1) и нашёл несколько интересных как потенциальных, так и явных ошибок. Однако вместо того, чтобы просто описать вам их я бы хотел предложить вам испытать свои силы в роли статического анализатора кода. Попробуйте отыскать по одной ошибке в приведённых фрагментах кода и придумать способы их исправления. Под каждым из фрагментов есть полный ответ, с помощью которого вы можете проверить своё предположение.
Да начнутся голодные игры испытания и пусть программистская смекалка всегда будет с вами! Кстати, будет круто, если вы поделитесь результатами в комментариях :)
public bool propagationHasStopped { get; }
public bool immediatePropagationHasStopped { get; }
public bool defaultHasBeenPrevented { get; }
public EventDebuggerCallTrace(IPanel panel, EventBase evt,
int cbHashCode, string cbName,
bool propagationHasStopped,
bool immediatePropagationHasStopped,
long duration,
IEventHandler mouseCapture): base(....)
{
this.callbackHashCode = cbHashCode;
this.callbackName = cbName;
this.propagationHasStopped = propagationHasStopped;
this.immediatePropagationHasStopped = immediatePropagationHasStopped;
this.defaultHasBeenPrevented = defaultHasBeenPrevented;
}Сообщение анализатора:
V3005. The 'this.defaultHasBeenPrevented' variable is assigned to itself. EventDebuggerTrace.cs 42.
По какой-то причине при инициализации свойство defaultHasBeenPrevented присваивается само себе. Так как у свойства отсутствует сеттер, его значение больше никогда не изменится и всегда будет равно значению по умолчанию — false.
Возможное исправление: добавить в конструктор новый параметр для инициализации свойства.
Слишком просто? Будем считать это разминкой, посмотрим, как вы справитесь дальше!
public void ParsingPhase(....)
{
....
SpriteCharacter sprite =
(SpriteCharacter)textInfo.textElementInfo[m_CharacterCount]
.textElement;
m_CurrentSpriteAsset = sprite.textAsset as SpriteAsset;
m_SpriteIndex = (int)sprite.glyphIndex;
if (sprite == null)
continue;
if (charCode == '<')
charCode = 57344 + (uint)m_SpriteIndex;
else
m_SpriteColor = Color.white;
}Сообщение анализатора:
V3095. The 'sprite' object was used before it was verified against null. Check lines: 310, 312. TextGeneratorParsing.cs 310.
Переменная sprite разыменовывается прямо перед проверкой на null:
m_CurrentSpriteAsset = sprite.textAsset as SpriteAsset;
m_SpriteIndex = (int)sprite.glyphIndex;
if (sprite == null)
continue;В случае если sprite действительно окажется равным null, это неизбежно приведёт к исключению. Вероятно, при добавлении строк c разыменованием в метод не обратили внимание на важность порядка выполнения операций. Таким образом, избежать проблемы можно, если перенести разыменования под проверку на null:
if (sprite == null)
continue;
m_CurrentSpriteAsset = sprite.textAsset as SpriteAsset;
m_SpriteIndex = (int)sprite.glyphIndex;private static void CompileBackgroundPosition(....)
{
....
else if (valCount == 2)
{
if (((val1.handle.valueType == StyleValueType.Dimension) ||
(val1.handle.valueType == StyleValueType.Float)) &&
((val1.handle.valueType == StyleValueType.Dimension) ||
(val1.handle.valueType == StyleValueType.Float)))
{
.... = new BackgroundPosition(...., val1.sheet
.ReadDimension(val1.handle)
.ToLength());
.... = new BackgroundPosition(...., val2.sheet
.ReadDimension(val2.handle)
.ToLength());
}
else if ((val1.handle.valueType == StyleValueType.Enum)) &&
(val2.handle.valueType == StyleValueType.Enum)
....
{
}Сообщение анализатора:
V3001. There are identical sub-expressions to the left and to the right of the '&&' operator. StyleSheetApplicator.cs 169.
Анализатор подсказывает, что в одном из условий через оператор && выполняются два одинаковых выражения:
if (((val1.handle.valueType == StyleValueType.Dimension) ||
(val1.handle.valueType == StyleValueType.Float)) &&
((val1.handle.valueType == StyleValueType.Dimension) ||
(val1.handle.valueType == StyleValueType.Float)))Здесь явно допущена ошибка. Более того очевидно и её исправление. Обратите внимание на остальной код. В нём повсеместно выполняются аналогичные операции сначала с использованием val1, а потом с использованием val2. Например, следующее за рассматриваемым условие, состоит из двух аналогичных проверок:
else if ((val1.handle.valueType == StyleValueType.Enum)) &&
(val2.handle.valueType == StyleValueType.Enum)Разница между ними заключается лишь в том, что в первой используется значение val1, а во второй — val2.
Таким образом, исправление ошибки, вероятнее всего, будет заключаться в замене всех val1 на val2 в повторяющемся выражении некорректного условия:
if (((val1.handle.valueType == StyleValueType.Dimension) ||
(val1.handle.valueType == StyleValueType.Float)) &&
((val2.handle.valueType == StyleValueType.Dimension) ||
(val2.handle.valueType == StyleValueType.Float)))public partial class BuildPlayerWindow : EditorWindow
{
....
internal static event Action<BuildProfile>
drawingMultiplayerBuildOptions;
....
}
internal static class EditorMultiplayerManager
{
....
public static event Action<NamedBuildTarget> drawingMultiplayerBuildOptions
{
add => BuildPlayerWindow.drawingMultiplayerBuildOptions +=
(profile) => ....;
remove => BuildPlayerWindow.drawingMultiplayerBuildOptions -=
(profile) => ....;
}
....
}Сообщение анализатора:
V3084. Anonymous function is used to unsubscribe from 'drawingMultiplayerBuildOptions' event. No handlers will be unsubscribed, as a separate delegate instance is created for each anonymous function declaration. EditorMultiplayerManager.bindings.cs 48.
Простая, но распространённая ошибка. Здесь для подписки на событие и отписки от него используются анонимные функции. Независимо от того, насколько визуально они похожи, эти функции всё равно будут разными объектами. В результате подписка на событие выполнится корректно, а вот отписка уже не сработает.
Один из вариантов решения — вместо анонимной функции реализовать полноценный метод и использовать его для подписки/отписки на/от события.
public void DoRenderPreview(Rect previewRect, GUIStyle background)
{
....
Matrix4x4 shadowMatrix;
RenderTexture shadowMap = RenderPreviewShadowmap(....);
if (previewUtility.lights[0].intensity != kDefaultIntensity ||
previewUtility.lights[0].intensity != kDefaultIntensity)
{
SetupPreviewLightingAndFx(probe);
}
float tempZoomFactor = (is2D ? 1.0f : m_ZoomFactor);
previewUtility.camera.orthographic = is2D;
if (is2D)
previewUtility.camera.orthographicSize = 2.0f * m_ZoomFactor;
....
}
private void SetupPreviewLightingAndFx(SphericalHarmonicsL2 probe)
{
previewUtility.lights[0].intensity = kDefaultIntensity;
previewUtility.lights[0].transform.rotation = ....;
previewUtility.lights[1].intensity = kDefaultIntensity;
....
}Сообщение анализатора:
V3001. There are identical sub-expressions 'previewUtility.lights[0].intensity != kDefaultIntensity' to the left and to the right of the '||' operator. AvatarPreview.cs 721.
Условие первого if-оператора в методе DoRenderPreview состоит из двух одинаковых подвыражений. Это может быть как ошибкой, так и просто избыточным кодом, оставленным по невнимательности.
На наличие именно ошибки здесь намекает реализация метода SetupPreviewLightingAndFx, в которой используется не только previewUtility.lights[0], но и previewUtility.lights[1].
Таким образом, исправление ошибки может выглядеть следующим образом:
if (previewUtility.lights[0].intensity != kDefaultIntensity ||
previewUtility.lights[1].intensity != kDefaultIntensity)
{
....
}void UpdateInfo()
{
....
var infoLine3_format = "<color=\"white\">CurrentElement:" +
" Visible:{0}" +
" Enable:{1}" +
" EnableInHierarchy:{2}" +
" YogaNodeDirty:{3}";
m_InfoLine3.text = string.Format(infoLine3_format,
m_LastDrawElement.visible,
m_LastDrawElement.enable,
m_LastDrawElement.enabledInHierarchy,
m_LastDrawElement.isDirty);
var infoLine4_format = "<color=\"white\">" +
"Count of ZeroSize Element:{0} {1}%" +
" Count of Out of Root Element:{0} {1}%";
m_InfoLine4.text = string.Format(infoLine4_format,
countOfZeroSizeElement,
100.0f * countOfZeroSizeElement / count,
outOfRootVE,
100.0f * outOfRootVE / count);
....
}Сообщение анализатора:
V3025. Incorrect format. A different number of format items is expected while calling 'Format' function. Arguments not used: 3rd, 4th. UILayoutDebugger.cs 179.
Обратите внимание на второй string.Format(....). В его строке формата (первый аргумент) имеется 4 слота для вставки. Значений для вставки (2-5 аргумент) также передаётся 4. Проблема в том, что слоты содержат только номера 0 и 1. В результате в них будут вставлены только первое и второе значение, тогда как остальные два не будут использованы вовсе.
Исправленная строка формата выглядит следующим образом:
var infoLine4_format = "<color=\"white\">" +
"Count of ZeroSize Element:{0} {1}%" +
" Count of Out of Root Element:{2} {3}%";protected static bool IsFinite(float f)
{
if ( f == Mathf.Infinity
|| f == Mathf.NegativeInfinity
|| f == float.NaN)
{
return false;
}
return true;
}Сообщение анализатора:
V3076. Comparison of 'f' with 'float.NaN' is meaningless. Use 'float.IsNaN()' method instead. PhysicsDebugWindowQueries.cs 87.
Ошибка здесь связана с не очень очевидным поведением. Сравнение двух значений равных NaN всегда ложно. Поэтому, как и советует анализатор, вместо выражения f == float.NaN здесь следует использовать float.IsNaN(f).
public readonly struct SearchField : IEquatable<SearchField>
{
....
public override bool Equals(object other)
{
return other is SearchIndexEntry l && Equals(l);
}
public bool Equals(SearchField other)
{
return string.Equals(name, other.name, StringComparison.Ordinal);
}
}Сообщение анализатора:
V3197. The compared value inside the 'Object.Equals' override is converted to the 'SearchIndexEntry' type instead of 'SearchField' that contains the override. SearchItem.cs 634.
Как сказано в сообщении анализатора, в первом методе Equals параметр other ошибочно приводится к типу SearchIndexEntry вместо SearchField. Из-за чего при последующем вызове Equals(l) будет вызвана та же самая перегрузка метода. Если же вдруг окажется, что other действительно имеет тип SearchIndexEntry, произойдёт зацикливание кода. Это приведёт к StackOverflowException.
private void DrawRenderTargetToolbar()
{
float blackMinLevel = ....;
float blackLevel = ....;
float whiteLevel = ....;
EditorGUILayout.MinMaxSlider(....);
float whiteMaxLevel = ....;
if (blackMinLevel < whiteMaxLevel && whiteMaxLevel > blackMinLevel)
{
m_RTBlackMinLevel = blackMinLevel;
m_RTWhiteMaxLevel = whiteMaxLevel;
m_RTBlackLevel = Mathf.Clamp(blackLevel,
m_RTBlackMinLevel,
whiteLevel);
m_RTWhiteLevel = Mathf.Clamp(whiteLevel,
blackLevel,
m_RTWhiteMaxLevel);
}
}Сообщение анализатора:
V3001. There are identical sub-expressions 'blackMinLevel < whiteMaxLevel' to the left and to the right of the '&&' operator. FrameDebuggerEventDetailsView.cs 364.
Снова перед нами if-оператор, условие которого состоит из двух, по сути, одинаковых выражений, которые отличаются друг от друга лишь порядком операндов. Вероятно, вместо blackMinLevel во втором выражении должно быть что-то другое. Рассмотрев окружающий код, наиболее логичным вариантом кажется whiteLevel. Таким образом, исправленное условие могло бы выглядеть следующим образом:
if (blackMinLevel < whiteMaxLevel && whiteMaxLevel > whiteLevel)
{
....
}internal static IEnumerable<Sample> FindByPackage(PackageInfo package, ....)
{
if (string.IsNullOrEmpty(package?.upmReserved) &&
string.IsNullOrEmpty(package.resolvedPath))
{
return Enumerable.Empty<Sample>();
}
try
{
IEnumerable<IDictionary<string, object>> samples = null;
var upmReserved = upmCache.ParseUpmReserved(package);
if (upmReserved != null)
samples = upmReserved.GetList<....>("samples");
....
}
....
}Сообщение анализатора:
V3042. Possible NullReferenceException. The '?.' and '.' operators are used for accessing members of the 'package' object. PackageSample.cs 102.
Проверкой в начале метода разработчик попытался обработать сразу несколько случаев:
Однако в большинстве случаев она будет работать некорректно:
Возможно, разработчик по невнимательности использовал оператор && вместо ||. Таким образом, исправленная версия проверки может выглядеть так:
if (string.IsNullOrEmpty(package?.upmReserved) ||
string.IsNullOrEmpty(package.resolvedPath))
{
return Enumerable.Empty<Sample>();
}[RequiredByNativeCode]
internal static void InvalidateAll()
{
lock (s_Instances)
{
foreach (var kvp in s_Instances)
{
WeakReference wr = kvp.Value;
if (wr.IsAlive)
(wr.Target as TextGenerator).Invalidate();
}
}
}Сообщение анализатора:
V3145. Unsafe dereference of a WeakReference target. The object could have been garbage collected between checking 'IsAlive' and accessing the 'Target' property. TextGenerator.cs 140.
Несмотря на небольшое количество кода, пожалуй, это самый сложный кейс в статье. Причин несколько:
Так как же защититься в данном случае? Чтобы наверняка избежать исключения NullReferenceException, следует создать сильную ссылку на объект. В таком случае сборщик мусора уже не сможет его удалить, пока ссылка остается актуальной. Проще говоря, нужно создать простую локальную переменную, ссылающуюся на этот объект, и дальше работать уже с ней. Таким образом, безопасная версия метода может выглядеть так:
[RequiredByNativeCode]
internal static void InvalidateAll()
{
lock (s_Instances)
{
foreach (var kvp in s_Instances)
{
WeakReference wr = kvp.Value;
var target = wr.Target;
If (target != null)
(target as TextGenerator).Invalidate();
}
}
}Ну как, справились? Не устали? Что же, пришло время отдохнуть, ведь статья подходит к концу. Надеюсь, она показалась вам не только полезной, но и увлекательной.
Искать ошибки даже в таких небольших фрагментах кода бывает не просто, но найти их в кодовой базе такого большого проекта как редактор Unity практически невозможно. Конечно, если не использовать специальные программные инструменты.
Инструмент, с помощью которого были найдены потенциальные проблемы, описанные в статье, вы можете бесплатно попробовать самостоятельно, запросив триал на сайте PVS-Studio.
Кстати, с помощью анализатора кода PVS-Studio можно проанализировать не только сам Unity, но и игры, реализованные с помощью Unity. Больше информации об этом вы сможете найти в документации.
До встречи в следующих статьях!
0
0
0
28
0
37
0
131
0
Попробуйте PVS-Studio бесплатно
Мы ответим вам на
Если вы так и не получили ответ, пожалуйста, проверьте, отфильтровано ли письмо в одну из следующих стандартных папок:
0