Преследуя Диабло, нефалем попадает в очередную западню Белиала. Погруженный в смертельный кошмар владыкой обмана, герой должен повторить былые победы над воплощениями зла, чтобы пробудиться. Однако в этот раз из-за необъяснимых искажений реальности, свойственных кошмарам, он обречён на поражение. Но вдруг к нему на помощь является загадочное существо, а вместе с ним возвращается и надежда.
"Чего только не найдёшь на GitHub!" – невольно думаю я, путешествуя по его просторам и каждый раз находя какой-то интересный и необычный проект. В этот раз мне попался эмулятор игрового сервера Diablo 3 на C#. Diablo 3 — это игра от компании Blizzard Entertainment, которая обзавелась немалым количеством поклонников, как и прочие игры серии.
Как разработчик статического анализатора кода, я не мог не опробовать наш инструмент на исходном коде — пусть и не оригинальном — одной из моих любимых игр. В результате мне удалось найти несколько фрагментов кода как с потенциальными, так и с вполне однозначными ошибками. В этой статье я предлагаю разобрать их.
Чтобы сделать материал более увлекательным и разнообразным, часть багов будут рассмотрены в рамках небольшого тематического повествования :) Нам предстоит сыграть роль загадочного существа, помогающего герою выбраться из кошмара, навеянного Белиалом. Действия повествования происходят спустя много лет после победы над ангелом смерти Молтаэлем и последующим освобождением воплощений зла. За это время некоторые из них, включая владыку обмана Белиала, вновь вернули себе большую часть былого могущества.
Хочу обратить ваше внимание, что связь между описываемыми ситуациями в сюжете и потенциальными ошибками в приведённых фрагментах кода по большей части надуманна и добавлена для того, чтобы сделать статью более захватывающей. Относитесь к подразделам с обзором кода как к своеобразной мини игре, вроде тех, что мы проходим, когда взламываем замки, шифры или обезвреживаем ловушки в видеоиграх :) Однако сами фрагменты кода c допущенными в нём потенциальными ошибками реальны и присутствуют в исходном коде эмулятора на момент написания статьи.
Очутившись в кошмаре, нефалем обнаружил, что находится посреди Колдея — города, называемого "Жемчужиной востока".
Именно во дворце Колдея герой некогда сразил Белиала. Задача казалась очевидной: проникнуть во дворец и повторить свою прошлую победу над владыкой лжи.
Пробиваясь сквозь прислужников воплощения зла, нефалем плутал по улицам города, безуспешно ища путь ко дворцу. В кошмаре этот город выглядел как огромный незнакомый лабиринт из городских переулков. Так прошёл час, другой, третий... И вот, наконец, герою попался свёрнутый пергамент — карта города. Развернув ее, он обнаружил, что та... пуста. "Что за издевательство!" — только подумал он, как вдруг из яркого света рядом с героем появилось странное существо. На вид его очертания походили на что-то среднее между человеком и единорогом (кого только не встретишь в кошмаре, правда?), но из-за света, исходившего от него, разглядеть ещё что-то было сложно. Казалось, что оно не желает герою зла, даже напротив, хочет помочь. Подойдя к герою, существо использовало рядом с картой загадочный артефакт (назовём его "детектор искажений").
Развеиваем искажение
Сколько бы не была реалистичной иллюзия, в ней всегда найдутся искажения — ошибки в её исходном коде. Похоже, отсутствие изображения на карте — следствие одного из таких искажений.
public MapRevealSceneMessage MapRevealMessage(Player plr)
{
if (
.... ||
SceneSNO.Id == 183556 ||
SceneSNO.Id == 183557 ||
SceneSNO.Id == 183502 ||
SceneSNO.Id == 183505 ||
SceneSNO.Id == 183557 ||
SceneSNO.Id == 183519 ||
....
)
{
return new MapRevealSceneMessage
{
....
MiniMapVisibility = true
};
}
else
{
return new MapRevealSceneMessage
{
....
MiniMapVisibility = false
};
}
}Чтобы развеять искажение и помочь герою, попробуйте найти ошибку в приведённом фрагменте кода, а после сравните свой вариант с ответом ниже.
Сообщение детектора искажений:
V3001 There are identical sub-expressions 'SceneSNO.Id == 183557' to the left and to the right of the '||' operator. Scene.cs 581
В приведённом выше условии через оператор ИЛИ, вероятно, перечисляются идентификаторы сцен (вариантов локации) города, мини-карта для которых должна быть видима. Логично предположить, что все идентификаторы в условии должны быть уникальны, однако детектор искажений обращает наше внимание на наличие двух одинаковых идентификаторов (констант) со значением 183557. Вероятно, один из них был указан по ошибке вместо уникального идентификатора другой сцены, и теперь свойство MiniMapVisibility для неё ошибочно будет иметь значение false.
После того как существо исправило искажение, изображение появилось на карте. С ней нефалем без труда отыскал путь ко дворцу, где он вновь сразил Белиала (вряд ли то был настоящий Белиал, скорее его кошмарная копия).
Покидая дворец, герой необъяснимым образом — как это частенько бывает в кошмарах — попадает прямиком в Ад и обнаруживает, что по какой-то причине он полностью обездвижен. Место, в котором оказался нефалем, было ему знакомо: именно здесь он некогда сразил ещё одно воплощение зла — Азмодана. Стоило нефалему вспомнить о нём, как вдруг откуда-то сзади раздался чудовищный голос владыки греха, стремительно надвигающегося на беззащитного героя.
В этот момент абсолютного отчаяния к нему на помощь вновь явилось загадочное существо. Стало ясно, что оцепенение героя — следствие искажений.
Существо пытается освободить нефалема, но успеет ли?
Развеиваем искажения
Как и в прошлый раз, попробуйте отыскать по одной ошибке в этих фрагментах кода. Помните, от вашего успеха зависит жизнь нефалема!
Искажение 1
public override bool Reveal(....)
{
....
if (!randomed && Destination.DestLevelAreaSNO != 19794)
....
if (World.IsPvP && Destination != null && ....)
....
if (Destination != null)
....
if (Destination == null || ....)
....
}Сообщение детектора искажений:
V3095 The 'Destination' object was used before it was verified against null. Check lines: 1241, 1244. Portal.cs 1241
Детектор подсказывает, что свойство Destination в первом условном операторе потенциально может иметь значение null, на что указывают все последующие проверки этого свойства. Если Destination равно null, то при разыменовании Destination.DestLevelAreaSNO будет выброшено исключение, что приведёт к остановке выполнения всей программы.
Для нейтрализации потенциальной проблемы нужно добавить соответствующую проверку в первый условный оператор:
if (!randomed &&
Destination != null &&
Destination.DestLevelAreaSNO != 19794)Искажение 2
public override void Encode(GameBitBuffer buffer)
{
....
buffer.WriteBool(EngagedWithRareTime.HasValue);
if (EngagedWithRareTime.HasValue)
{
buffer.WriteInt(32, EngagedWithGoblinTime.Value);
}
buffer.WriteBool(EngagedWithGoblinTime.HasValue);
if (EngagedWithGoblinTime.HasValue)
{
buffer.WriteInt(32, EngagedWithGoblinTime.Value);
}
buffer.WriteBool(InCombat.HasValue);
if (InCombat.HasValue)
{
buffer.WriteBool(InCombat.Value);
}
....
}Сообщение детектора искажений:
V3095 The 'EngagedWithGoblinTime' object was used before it was verified against null. Check lines: 190, 192. TrickleMessage.cs 190
Здесь детектор указывает на аналогичную проблему. В buffer записывается значение свойства EngagedWithGoblinTime.HasValue, что указывает на то, что EngagedWithGoblinTime может быть равен null. При этом парой строк выше выполняется разыменование EngagedWithGoblinTime.Value, что потенциально может привести к исключению.
Если посмотреть на условие оператора if, внутри которого выполняется опасное разыменование, нетрудно заметить, что в нём на наличие значения проверяется свойство, название которого очень схоже с EngagedWithGoblinTime. Можно было бы прийти к выводу, что вместо значения EngagedWithRareTime в этом условии должно проверяться значение EngagedWithGoblinTime. Однако, взглянув на следующий условный оператор, становится понятно, что EngagedWithGoblinTime и EngagedWithRareTime действительно перепутаны, но не в условии первого if, а в его then-блоке при разыменовании. Таким образом, решение проблемы выглядит так:
if (EngagedWithRareTime.HasValue)
{
buffer.WriteInt(32, EngagedWithRareTime.Value);
}
buffer.WriteBool(EngagedWithGoblinTime.HasValue);Всего за несколько мгновений до удара Азмодана существу удалось освободить героя от оцепенения. Он, в свою очередь, тут же увернулся от атаки и, не теряя времени, нанёс мощный контрудар. Завязался не самый лёгкий бой: Азмодан призвал на помощь прислужников, но нефалем всё же смог одержать победу.
Стоило владыке греха пасть, как обстановка вокруг героя начала стремительно меняться. Пейзажи пылающего Ада сменились пейзажами... пылающих Небес.
Нефалем стоял у входа к Кристальной Арке — источнику силы всех ангелов. Именно там однажды он одолел самого коварного из воплощений зла — Диабло, когда тот, впитав в себя силы всех прочих воплощений, вознамерился осквернить Арку и тем самым одержать заключительную победу в войне с Небесами.
Нетрудно было догадаться, что ждало его по ту сторону входа, но другого выхода не было. В конце концов, он уже победил в этом бою однажды, справится и сейчас. С этой мыслью нефалем пошёл к Арке.
С первых же минут боя всё пошло не так. Атаки героя не наносили воплощению ужаса никакого заметного вреда, а каждая атака Диабло, напротив, казалась смертельной. Даже жар, исходивший от него, ощущался настолько сильно, будто на герое и вовсе не было никакой брони.
Несомненно, здесь не обошлось без искажения. Но искажение вовсе не сделало Диабло непобедимым. Оно изменило свойства оружия и брони самого героя, сделав их сродни самому посредственному вооружению. С таким снаряжением и на ожившего скелета идти было бы опасно, что уж говорить про единое воплощение зла.
Очевидно, этот бой был безнадёжен. Нефалем как мог уклонялся от атак, но ему не могло везти вечно. В конце концов он пропустил не самый сильный удар, который чуть не оказался смертельным. Герой упал на землю, тогда как воплощение ужаса готовилось нанести добивающий удар. В этот момент — как всегда вовремя — появился загадочный помощник нефалема и принял атаку на себя. Получив серьёзное ранение, существо отлетело в сторону и упало рядом с героем. Из последних сил оно использовало свой артефакт, в надежде успеть развеять искажение.
public static void Generate(....)
{
....
filteredList = filteredList.Where(
a =>
!a.Name.Contains("FireD") &&
!a.Name.Contains("PoisonD") &&
!a.Name.Contains("HolyD") &&
!a.Name.Contains("ColdD") &&
!a.Name.Contains("LightningD") &&
!a.Name.Contains("ArcaneD") &&
!a.Name.Contains("MinMaxDam") &&
isCrafting ? !a.Name.ToLower().Contains(....)
: !a.Name.Contains(....)
);
....
}Сообщение детектора искажений:
V3123 Perhaps the '?:' operator works in a different way than it was expected. Its priority is lower than priority of other operators in its condition. AffixGenerator.cs 207
Обратите внимание на левую часть тернарного выражения. На первый взгляд кажется, что она выглядит следующим образом: "isCrafting ? ....". Но первое впечатление часто бывает обманчиво. Приоритет оператора ? ниже, чем у оператора &&, а потому левая часть тернарного оператора в действительности выглядит так:
!a.Name.Contains("FireD") &&
....
!a.Name.Contains("MinMaxDam") &&
isCrafting ? ....Кажется, в этом коде не учли этого факта, а потому условное выражение, передаваемое в Where, может работать совсем не так, как ожидалось. Решить потенциальную проблему можно, добавив круглые скобки вокруг тернарного выражения:
filteredList = filteredList.Where(
a =>
....
!a.Name.Contains("MinMaxDam") &&
(isCrafting ? ....)
);Ощущая своё абсолютное превосходство, Диабло снова нанёс герою удар, на этот раз смертельный... по крайне мере, так ему казалось. Неожиданно атака отскочила от доспеха, оставив на нём лишь царапину. Искажение рассеялось, а снаряжение нефалема обрело свою былую мощь! Сам герой воспрял духом, резким движением поднялся и набросился на демона. Как и в тот день, Небеса долго содрогались от этого сражения. Как и в тот день, Диабло пал. Проклятье Белиала начало рассеиваться. Тут герой обратил внимание на своего напарника. Герой хотел было подойти и помочь ему, узнать кто он, как здесь оказался и почему помогал... вопросов было много, а времени мало. Кошмар рассеялся, и герой очнулся.
Это небольшое сочинение подошло к концу. Однако не все фрагменты кода с ошибками, которые я хотел бы рассмотреть в этой статье, были разобраны в предыдущем разделе. Хочу устранить эту несправедливость и описать их здесь. Всех, кого мучает желание ещё немного покопаться в коде, приглашаю к ознакомлению.
Чтобы прокачать свои аналитические способности, попробуйте найти в каждом фрагменте кода по одной ошибке самостоятельно, а потом сверьтесь с ответами.
Case 1
public static void GrantCriteria(....)
{
....
else
{
....
uint alcount = alreadycrits.CriteriaId32AndFlags8;
var newrecord = D3.Achievements
....
.SetQuantity32(alcount++) // <=
.Build();
int critCount = UnserializeBytes(achievement.Criteria).Count;
if (critCount >= 5)
{
GrantCriteria(client, 74987246353740);
}
client.Account.GameAccount.AchievementCriteria
.Remove(alreadycrits);
client.Account.GameAccount.AchievementCriteria
.Add(newrecord);
}
....
}Сообщение анализатора PVS-Studio:
V3159 Modified value of the 'alcount' operand is not used after the postfix increment operation. AchievementManager.cs 360
Ожидается, что alcount должно быть увеличено на 1, после чего полученное значение должно быть передано в метод SetQuantity32. В действительности же всё происходит наоборот: сначала значение alcount передаётся в метод, и лишь после этого значение переменной увеличится на 1.
Исправить эту ошибку очень просто: нужно лишь заменить выражение alcount++ на выражение alcount + 1 или ++alcount.
Case 2
public class LooterBrain : Brain
{
....
public override void Think(int tickCounter)
{
....
if (LootLegendaries)
targets.Concat(....);
....
}
....
}Сообщение анализатора PVS-Studio:
V3010 The return value of function 'Concat' is required to be utilized. LooterBrain.cs 154
Даже если LootLegendaries равно true, в targets не будут добавлены новые объекты, ведь метод Concat не изменяет какое-либо из исходных перечислений, а создаёт новое. Чтобы исправить ошибку, нужно присвоить переменной targets результат выполнения метода Concat.
Кстати, в самой игре эта ошибка будет проявляться в неспособности собирающих лут питомцев подбирать легендарные предметы. На это указывает контекст, в котором используется класс LooterBrain со свойством LootLegendaries, равным true, значение которого задаётся через второй аргумент конструктора:
public LooterPetAnniversary(....): base(....)
{
....
SetBrain(new LooterBrain(this, true));
....
}Case 3
public override bool Reveal(....)
{
bool isGolem = false;
if (this is BaseGolem ||
this is IceGolem ||
this is BoneGolem ||
this is DecayGolem ||
this is ConsumeFleshGolem ||
this is BloodGolem)
{
....
isGolem = false;
}
....
if (....)
player.InGameClient.SendMessage(new PetMessage()
{
....
Index = isGolem ? 9 : player.CountFollowers(SNO) + PlusIndex,
....
});
}Сообщение анализатора PVS-Studio:
V3022 Expression 'isGolem' is always false. Minion.cs 230
Логично предположить, что внутри первого условного выражения переменной isGolem должно присваиваться значение true, но вместо этого ей повторно присваивается false. В результате свойству Index может быть присвоено неверное значение.
Надеюсь, что в этой статье мне удалось освежить ваши воспоминая о былых приключениях в Diablo 3 или, если их ещё не было, навеять желание исправить это. Кроме того, эта статья предоставила вам возможность немного поупражняться в анализе C# кода. Не секрет, что, изучая чужие ошибки, вы минимизируете риск допустить их в собственном коде.
Кстати, если вас заинтересовал инструмент, с помощью которого эти ошибки были найдены, вы можете скачать его с официального сайта и бесплатно опробовать самостоятельно.
До встречи в следующих статьях!