13 лет плохого кода в играх

Одиноким пятничным вечером в поисках вдохновения вы решаете припомнить свои былые победы на программистском фронте. Архив со старого жесткого диска неторопливо открывается и вот перед вами разворачивается код славных далеких времен…

О нет. Это совсем не то, что вы ожидали увидеть. Правда, что ли, все было настолько плохо? Почему вам никто не сказал? Как можно было до такого докатиться? Такое количество операторов go-to в одной-единственной функции – это вообще законно? Вы поспешно закрываете проект. На секунду вас одолевает искушение удалить файл, и всё содержимое жёсткого диска заодно.

Ниже вы найдете коллекцию уроков, поучительных примеров и предостережений, которые я вынес из собственного путешествия в прошлое. Все имена приводятся без изменений, чтобы обличить виновных.

2004

Мне было тринадцать лет. Проект под называнием «Red Moon» представлял собой крайне амбициозную игру про воздушные битвы от третьего лица. Те немногочисленные фрагменты кода, которые я не скопировал символ в символ из пособия «Developing Games in Java», были позор позором. Давайте убедимся на конкретных примерах.

Я хотел, чтобы у игроков было несколько вариантов оружия и возможность выбирать. В планах это все выглядело так: модель оружия перемещается вниз и внутрь модели игрока, заменяются на следующую и так далее. Вот код анимации. Лучше сильно не вникайте.

public void updateAnimation(long eTime) {
	if(group.getGroup("gun") == null) {
		group.addGroup((PolygonGroup)gun.clone());
	}
	changeTime -= eTime;
	if(changing && changeTime <= 0) {
		group.removeGroup("gun");
		group.addGroup((PolygonGroup)gun.clone());
		weaponGroup = group.getGroup("gun");
		weaponGroup.xform.velocityAngleX.set(.003f, 250);
		changing = false;
	}
}

Хотелось бы обратить ваше внимание на пару занимательных фактов. Во-первых, оцените, сколько здесь переменных:

• changeTime
• changing
• weaponGroup
• weaponGroup.xform.velocityAngleX

Но при всем при этом чего-то как будто бы не хватает… ах да, нужна еще переменная, чтобы отслеживать, какое оружие задействовано в данный момент. И, само собой, для этого необходим отдельный файл.

И еще одно: в итоге я так и не собрался сделать больше одной модели оружия. То есть для каждого из этих типов использовалась одна и та же модель. От всего этого кода не было абсолютно никакого прока.

Как это исправить

Убрать лишние переменные. В данном случае состояние оружия можно было описать при помощи всего двух: weaponSwitchTimer и weaponCurrent. Всю остальную информацию можно было бы извлечь из них.

Явно инициализируйте все, что только можно. Эта функция проверяет, является ли оружие null и при необходимости запускает процесс инициализации. Поразмыслив хоть секунд тридцать, я мог бы сообразить, что в этой игре у пользователя всегда есть какое-то оружие, а если нет, то играть попросту невозможно и можно со спокойной совестью крашить всю программу.

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

Проявляйте инициативу и принимайте решения самостоятельно! Не предоставляйте компьютеру разбираться самому.

Названия

boolean noenemies = true; // why oh why

Называйте переменные типа Boolean без использования отрицания. Если вы пишете в коде что-то подобное, значит пришла пора переосмыслить свой подход к жизни:

if (!noenemies) {
	// are there enemies or not??
}

Обработка ошибок

Подобные вещи попадаются в коде сплошь и рядом:

static {
	try {
		gun = Resources.parseModel("images/gun.txt");
	} catch (FileNotFoundException e) {} // *shrug*
	catch (IOException e) {}
}

Вы, наверное, сейчас думаете: «Надо как-то поизящней обработать эту ошибку! Ну, хотя бы сообщение пользователю вывести или что-то в этом духе». Но я, честно говоря, придерживаюсь противоположной точки зрения.

Устранять ошибки – это действительно никогда не лишнее, но вот с обработкой легко можно перестараться. В данном случае без модели оружия играть все равно невозможно, так что можно с тем же успехом крашить игру. Не пытайтесь с достоинством выйти из положения, если оно заведомо безвыходное.

К тому же, возвращаясь к сказанному выше, здесь вам нужно самостоятельно решить, какие ошибки считать исправимыми, а какие – нет. К несчастью, в Sun считают, что практически все ошибки на Java просто обязаны быть исправимыми, и в результате мы имеем случаи ленивой обработки – один из них приведен выше.

2005-2006

К этому времени я уже освоил C++ и DirectX. Я задумал создать переиспользуемый движок, чтобы человечество могло с пользой для себя припасть к кладезю мудрости и опыта, которые я скопил за свою долгую четырнадцатилетнюю жизнь.

Думаете, на первый трэйлер было больно смотреть? Вы ещё ничего не видели.

На тот момент я уже знал что объектно ориентированное программирование – это круто. Это знание привело к ужасам такого рода:

class Mesh {
public:
	static std::list<Mesh*> meshes; // Static list of meshes; used for caching and rendering
	Mesh(LPCSTR file); // Loads the x file specified
	Mesh();
	Mesh(const Mesh& vMesh);
	~Mesh();
	void LoadMesh(LPCSTR xfile); // Loads the x file specified
	void DrawSubset(DWORD index); // Draws the specified subset of the mesh
	DWORD GetNumFaces(); // Returns the number of faces (triangles) in the mesh
	DWORD GetNumVertices(); // Returns the number of vertices (points) in the mesh
	DWORD GetFVF(); // Returns the Flexible Vertex Format of the mesh
	int GetNumSubsets(); // Returns the number of subsets (materials) in the mesh
	Transform transform; // World transform
	std::vector<Material>* GetMaterials(); // Gets the list of materials in this mesh
	std::vector<Cell*>* GetCells(); // Gets the list of cells this mesh is inside
	D3DXVECTOR3 GetCenter(); // Gets the center of the mesh
	float GetRadius(); // Gets the distance from the center to the outermost vertex of the mesh
	bool IsAlpha(); // Returns true if this mesh has alpha information
	bool IsTranslucent(); // Returns true if this mesh needs access to the back buffer
	void AddCell(Cell* cell); // Adds a cell to the list of cells this mesh is inside
	void ClearCells(); // Clears the list of cells this mesh is inside
protected:
	ID3DXMesh* d3dmesh; // Actual mesh data
	LPCSTR filename; // Mesh file name; used for caching
	DWORD numSubsets; // Number of subsets (materials) in the mesh
	std::vector<Material> materials; // List of materials; loaded from X file
	std::vector<Cell*> cells; // List of cells this mesh is inside
	D3DXVECTOR3 center; // The center of the mesh
	float radius; // The distance from the center to the outermost vertex of the mesh
	bool alpha; // True if this mesh has alpha information
	bool translucent; // True if this mesh needs access to the back buffer
	void SetTo(Mesh* mesh);
}

Вдобавок я узнал, что комментарии – это круто, и это сподвигло меня оставлять изречения типа:

D3DXVECTOR3 GetCenter(); // Gets the center of the mesh

Впрочем, с этим классом есть проблемы и посерьезнее. Концепт Mesh – это какая-то туманная абстракция, которой не подберешь эквивалента из реальной действительности. Я даже когда её писал, ничего не понимал. Что она такое – контейнер, который содержит в себе вершины, индексы и прочие данные? Менеджер ресурсов, который загружает и выгружает данные на диск? Bнструмент для рендеринга, которые отправляет данные в GPU? Всё и сразу.

Как это исправить

Класс Mesh должен представлять собой простую структуру данных. В нем не должно быть умных типов. А это значит, что можно со спокойной душой выкинуть все геттеры и сеттеры и сделать все поля публичными.

Далее, можно отделить управление ресурсами и рендеринг в отдельные системы, которые работают с инертными данными. Да, именно системы, а не объекты. Не пытайтесь подстроить каждую проблему под объектно-ориентированную абстракцию, если абстракция другого типа может подойти лучше.

Самый верный способ исправить комментарии – как правило, удалить их. Комментарии быстро становятся неактуальным белым шумом, который только сбивает с толку – компилятор на них все равно не смотрит. Я настаиваю, что от комментариев нужно избавляться, если только они не принадлежат к одной из следующих групп:

• Комментарии, которые отвечают на вопрос «почему?», а не «что?». От них больше всего толку.
• Комментарии, которые в нескольких словах описывают, что делает огромный кусок кода, который последует далее. Они облегчают процесс чтения и навигации;
• Комментарии в декларации структуры данных, поясняющие, что содержит каждое поле. Зачастую они излишни, но в некоторых случаях интуитивно не понятно, как структура расположена в памяти, и тогда комментарии с подобным описанием необходимы.

2007-2008

Этот период своей жизни я называю «тёмные века PHP».

2009-2010

Я уже учусь в колледже. Работаю над шутером-мультиплеером на Python от третьего лица под названием «Acquire, Attack, Asplode, Pwn». Ничего не могу сказать в свое оправдание. Все ещё более позорно, только теперь еще с пикантным послевкусием нарушения авторских прав.

Когда я писал эту игру, меня только-только просветили, что глобальные переменные – это зло. Они превращают код в мешанину. Они позволяют функции А, меняя глобальное состояние, ломать/нарушать функцию Б, которая не имеет к ней никакого отношения. Они не работают с потоками.

Однако практически всему коду геймплея необходим доступ к состоянию мировой матрицы целиком. Я «разрешил» эту проблему тем, что сохранил всё в этом объекте и передал его в каждую из функций. И никаких вам глобальных переменных! Мне казалось, что я здорово придумал, ведь получается, что в теории можно запускать несколько автономных мировых матриц одновременно.

На деле world, по факту, являлся контейнером глобального состояния. Концепция с несколькими world была, разумеется, совершенно бессмысленна, никто её никогда не тестировал, и я сильно подозреваю, что работать она могла бы только при условии серьезного рефакторинга.
Те, кто вступает в секту противников глобальных переменных, открывают для себя целый мир креативных методов самообмана. Самый худший из них – это singleton.

class Thing
{
	static Thing i = null;
	public static Thing Instance()
	{
		if (i == null)
			i = new Thing();
		return i;
	}
}

Thing thing = Thing.Instance();

Крибле крабле бумс! Ни единой глобальной переменной в поле зрения! Но тут есть одно «но»: singleton куда хуже по следующим причинам:

• Все потенциальные недостатки глобальных переменных сохраняются и здесь. Если вы думаете, что singleton не глобален, то хватит уже лгать себе;
• В лучшем случае singleton добавляет в вашу программу ресурсозатратный оператор ветвления. В худшем это полноценный вызов функции;
• Пока не запустишь программу, неизвестно, когда инициализируется singleton. Это еще один пример того, как ленивые программисты предоставляют системе решать то, что следовало спланировать еще на стадии проектирования.

Как это исправить

Если что-то должно быть глобальным, пусть будет. Принимая это решение, учитывайте, как оно скажется на проекте в целом. С опытом это становится проще.

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

Параметры типа Boolean

class ObjectEntity:
	def delete(self, killed, local):
		# ...
		if killed:
			# ...
		if local:
			# ...

Возможно, вам приходилось писать код вроде такого:

class ObjectEntity:
	def delete(self, killed, local):
		# ...
		if killed:
			# ...
		if local:
			# ...

Здесь мы имеем четыре отдельных операций по удалению, которые очень похожи между собой — все незначительные различия завязаны на двух параметрах типа Boolean. Вроде бы всё логично. А теперь давайте посмотрим на клиентский код, который вызывает эту функцию:

obj.delete(True, False)

Выглядит не особо читабельно, да?

Как это исправить

Тут нужно смотреть на конкретный случай. Но есть один совет от Casey Muratori, который актуален всегда: начинайте с клиентского кода. Я убеждён, что никакой человек в своём уме такого клиентского кода, какой приводился выше, не напишет. Скорее уж напишет следующее:

obj.killLocal()

А уже потом пропишет имплементацию функции killLocal().

Названия

Может показаться странным, что я так напираю на названия, но, как говорится в старой шутке, это одна из двух неразрешенных проблем в программировании (вторая — инвалидация кэша и ошибки на единицу).

Внимательно посмотрите на эти функции:

class TeamEntityController(Controller):

	def buildSpawnPacket(self):
		# ...
	
	def readSpawnPacket(self):
		# ...
	
	def serverUpdate(self):
		# ...

	def clientUpdate(self):
		# ...

Ясно, что первые две функции связаны между собой, как и последние две. Но их названия никак не указывают на этот факт. Если я начну печатать self в IDE, эти функции не отобразятся одна за другой в меню автозаполнения.

Соответственно, лучше начинать название с общего, а заканчивать частным. Вот так:

class TeamEntityController(Controller):

	def packetSpawnBuild(self):
		# ...
	
	def packetSpawnRead(self):
		# ...
	
	def updateServer(self):
		# ...

	def updateClient(self):
		# ...

С таким кодом меню автозаполнения будет выглядеть намного логичнее.

2010-2015

Каких-то 12 лет работы — и я закончил игру. Пусть я многому научился в процессе, в конечной версии все-таки сохранились серьезные проколы.

Связывание данных

В то время только-только начиналась лихорадка реактивных UI фреймворков типа MVVM от Microsoft и Angular от Google. Сегодня подобный стиль программирования сохраняется преимущественно в React.

Все эти фреймворки работают по одной схеме. Вы видите текстовое поле для HTML, пустой элемент и одну-единственную строчку кода, которая неразрывно их связывает. Введите текст в поле — и вуаля! волшебным образом обновится.

В контексте игры это будет выглядеть примерно так:

public class Player
{
	public Property<string> Name = new Property<string> { Value = "Ryu" };
}

public class TextElement : UIComponent
{
	public Property<string> Text = new Property<string> { Value = "" };
}

label.add(new Binding<string>(label.Text, player.Name));

Ух ты, интерфейс теперь автоматически обновляется при введении имени игрока! Интерфейс и код игры могут быть совершенно независимы друг от друга. Это заманчиво: мы избавляемся от состояния интерфейса, выводя его из состояния игры.

Однако были некоторые тревожные звоночки. Мне пришлось превратить каждое поле в игре в Propert-объект, который включал целый ряд зависимых связей.

public class Property<Type> : IProperty
{
	protected Type _value;
	protected List<IPropertyBinding> bindings; 

	public Type Value
	{
		get { return this._value; }
		set
		{
			this._value = value;
			
			for (int i = this.bindings.Count - 1; i >= 0; i = Math.Min(this.bindings.Count - 1, i - 1))
				this.bindings[i].OnChanged(this);
		}
	}
}

Абсолютно за каждым полем, вплоть до последнего Boolean, был закреплен громоздкий динамический массив.

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

Тем не менее, я так проникся связыванием данных, что выстроил на нем всю игру. Я разбил объекты на компоненты и связал их свойства. Ситуация стала выходить из-под контроля.

jump.Add(new Binding<bool>(jump.Crouched, player.Character.Crouched));
jump.Add(new TwoWayBinding<bool>(player.Character.IsSupported, jump.IsSupported));
jump.Add(new TwoWayBinding<bool>(player.Character.HasTraction, jump.HasTraction));
jump.Add(new TwoWayBinding<Vector3>(player.Character.LinearVelocity, jump.LinearVelocity));
jump.Add(new TwoWayBinding<BEPUphysics.Entities.Entity>(jump.SupportEntity, player.Character.SupportEntity));
jump.Add(new TwoWayBinding<Vector3>(jump.SupportVelocity, player.Character.SupportVelocity));
jump.Add(new Binding<Vector2>(jump.AbsoluteMovementDirection, player.Character.MovementDirection));
jump.Add(new Binding<WallRun.State>(jump.WallRunState, wallRun.CurrentState));
jump.Add(new Binding<float>(jump.Rotation, rotation.Rotation));
jump.Add(new Binding<Vector3>(jump.Position, transform.Position));
jump.Add(new Binding<Vector3>(jump.FloorPosition, floor));
jump.Add(new Binding<float>(jump.MaxSpeed, player.Character.MaxSpeed));
jump.Add(new Binding<float>(jump.JumpSpeed, player.Character.JumpSpeed));
jump.Add(new Binding<float>(jump.Mass, player.Character.Mass));
jump.Add(new Binding<float>(jump.LastRollKickEnded, rollKickSlide.LastRollKickEnded));
jump.Add(new Binding<Voxel>(jump.WallRunMap, wallRun.WallRunVoxel));
jump.Add(new Binding<Direction>(jump.WallDirection, wallRun.WallDirection));
jump.Add(new CommandBinding<Voxel, Voxel.Coord, Direction>(jump.WalkedOn, footsteps.WalkedOn));
jump.Add(new CommandBinding(jump.DeactivateWallRun, (Action)wallRun.Deactivate));
jump.FallDamage = fallDamage;
jump.Predictor = predictor;
jump.Bind(model);
jump.Add(new TwoWayBinding<Voxel>(wallRun.LastWallRunMap, jump.LastWallRunMap));
jump.Add(new TwoWayBinding<Direction>(wallRun.LastWallDirection, jump.LastWallDirection));
jump.Add(new TwoWayBinding<bool>(rollKickSlide.CanKick, jump.CanKick));
jump.Add(new TwoWayBinding<float>(player.Character.LastSupportedSpeed, jump.LastSupportedSpeed));

wallRun.Add(new Binding<bool>(wallRun.IsSwimming, player.Character.IsSwimming));
wallRun.Add(new TwoWayBinding<Vector3>(player.Character.LinearVelocity, wallRun.LinearVelocity));
wallRun.Add(new TwoWayBinding<Vector3>(transform.Position, wallRun.Position));
wallRun.Add(new TwoWayBinding<bool>(player.Character.IsSupported, wallRun.IsSupported));
wallRun.Add(new CommandBinding(wallRun.LockRotation, (Action)rotation.Lock));
wallRun.Add(new CommandBinding<float>(wallRun.UpdateLockedRotation, rotation.UpdateLockedRotation));
vault.Add(new CommandBinding(wallRun.Vault, delegate() { vault.Go(true); }));
wallRun.Predictor = predictor;
wallRun.Add(new Binding<float>(wallRun.Height, player.Character.Height));
wallRun.Add(new Binding<float>(wallRun.JumpSpeed, player.Character.JumpSpeed));
wallRun.Add(new Binding<float>(wallRun.MaxSpeed, player.Character.MaxSpeed));
wallRun.Add(new TwoWayBinding<float>(rotation.Rotation, wallRun.Rotation));
wallRun.Add(new TwoWayBinding<bool>(player.Character.AllowUncrouch, wallRun.AllowUncrouch));
wallRun.Add(new TwoWayBinding<bool>(player.Character.HasTraction, wallRun.HasTraction));
wallRun.Add(new Binding<float>(wallRun.LastWallJump, jump.LastWallJump));
wallRun.Add(new Binding<float>(player.Character.LastSupportedSpeed, wallRun.LastSupportedSpeed));
player.Add(new Binding<WallRun.State>(player.Character.WallRunState, wallRun.CurrentState));

input.Bind(rollKickSlide.RollKickButton, settings.RollKick);
rollKickSlide.Add(new Binding<bool>(rollKickSlide.EnableCrouch, player.EnableCrouch));
rollKickSlide.Add(new Binding<float>(rollKickSlide.Rotation, rotation.Rotation));
rollKickSlide.Add(new Binding<bool>(rollKickSlide.IsSwimming, player.Character.IsSwimming));
rollKickSlide.Add(new Binding<bool>(rollKickSlide.IsSupported, player.Character.IsSupported));
rollKickSlide.Add(new Binding<Vector3>(rollKickSlide.FloorPosition, floor));
rollKickSlide.Add(new Binding<float>(rollKickSlide.Height, player.Character.Height));
rollKickSlide.Add(new Binding<float>(rollKickSlide.MaxSpeed, player.Character.MaxSpeed));
rollKickSlide.Add(new Binding<float>(rollKickSlide.JumpSpeed, player.Character.JumpSpeed));
rollKickSlide.Add(new Binding<Vector3>(rollKickSlide.SupportVelocity, player.Character.SupportVelocity));
rollKickSlide.Add(new TwoWayBinding<bool>(wallRun.EnableEnhancedWallRun, rollKickSlide.EnableEnhancedRollSlide));
rollKickSlide.Add(new TwoWayBinding<bool>(player.Character.AllowUncrouch, rollKickSlide.AllowUncrouch));
rollKickSlide.Add(new TwoWayBinding<bool>(player.Character.Crouched, rollKickSlide.Crouched));
rollKickSlide.Add(new TwoWayBinding<bool>(player.Character.EnableWalking, rollKickSlide.EnableWalking));
rollKickSlide.Add(new TwoWayBinding<Vector3>(player.Character.LinearVelocity, rollKickSlide.LinearVelocity));
rollKickSlide.Add(new TwoWayBinding<Vector3>(transform.Position, rollKickSlide.Position));
rollKickSlide.Predictor = predictor;
rollKickSlide.Bind(model);
rollKickSlide.VoxelTools = voxelTools;
rollKickSlide.Add(new CommandBinding(rollKickSlide.DeactivateWallRun, (Action)wallRun.Deactivate));
rollKickSlide.Add(new CommandBinding(rollKickSlide.Footstep, footsteps.Footstep));

Это вызвало массу проблем. Я создавал циклы связей, которые закольцовывались. Выяснилось, что порядок инициализации часто имеет большое значение, а при связывании данных инициализация становится настоящим адом, так как по мере добавления связей некоторые свойства инициализируются по несколько раз.

Когда пришло время добавлять анимацию, оказалось, что со связыванием данных анимировать переход между двумя состояниями — сложный и неинтуитивный процесс. И не я один так думаю. Можете посмотреть это видео с программистом Netflix, который рассыпается в похвалах React, а потом рассказывает, как его приходится отключать всякий раз, когда они запускают анимацию.

Я тоже догадался применить всю мощь отключения связей. И добавил еще одно поле:

class Binding<T>
{
	public bool Enabled;
}

К сожалению, от этого терялся весь смысл связывания. Я же хотел избавиться от состояний, а с таким кодом их только больше стало. Вот как убрать это состояние?

Знаю! При помощи связывания!

class Binding<T>
{
	public Property<bool> Enabled = new Property<bool> { Value = true };
}

Да, был момент, когда я серьёзно пытался что-то подобное реализовать. Связывал всё, что связывается. Но я быстро опомнился и понял, что это безумие.

Как можно исправить ситуацию со связыванием? Постарайтесь сделать так, чтобы ваш интерфейс нормально функционировал без состояний. Хороший пример — dear imgui. Разделяйте поведение и состояние, насколько это возможно. Избегайте техник, с которыми легко создавать состояния. Создание состояние должно быть для вас мучительным шагом.

Заключение

Можно было бы продолжать, это ещё далеко не все мои глупейшие ошибки. Я сочинил еще один «оригинальный» метод избегать глобальных переменных. Был период, когда я помешался на замыканиях. Я создал так называемую систему сущность-объект, и это было что угодно, но не система сущность-объект. Я попытался реализовать многопоточность в воксельном движке, щедро наставив блокировок.

Вот выводы, к которым я пришел:

• Принимайте решения самостоятельно, не делегируйте их компьютеру;
• Разделяйте поведение и состояние;
• Пишите чистые функции;
• Начинайте с клиентского кода;
• Пишите скучный код.

Вот вам моя история. А у вас есть тёмное прошлое, которым вы готовы поделиться?

Автор: Everyday Tools

Источник