Пишіть код так, як ніби супроводжувати його буде схильний до насильства психопат, який знає, де ви живете

..., навіть якщо проект не планується розвивати і ви не збираєтеся ділитися вихідними кодами, тому що через 20 років який-небудь маніяк буде вивчати і допрацьовувати машинний код вашого продукту, і він може захотіти знайти вас.

Розробка Need For Speed III Modern Patch

Взагалі я досить рідко граю в комп'ютерні ігри. Бувало, не грав по кілька років поспіль. Але іноді в мені прокидається маленький реверс-інженер, який мотивує мене забратися в машинний код якої-небудь улюбленої іграшки з минулого. В останній рік я займався доопрацюванням Need For Speed III: Hot Pursuit (1998 року). Це моя улюблена гра в жанрі, але тепер я, до свого жаль, знаю про те, наскільки огидно вона написана. Велика кількість маленьких багів в найнесподіваніших місцях — прямий наслідок низької якості коду.

Наскільки все погано?
У виконуваному файлі величезна кількість коду, який залишився у спадок від попередніх частин гри і не використовується, тобто застарілий код не віддалявся розробниками, причому мені зустрічалися випадки, коли якийсь застарілий код викликався, але результати його роботи ігнорувалися, бо оновленому кодом вони вже були не потрібні. У грі всюди використовуються статичні масиви фіксованого розміру, у багатьох випадках відсутні перевірки виходу за межі масиву, що призводить до падінь, коли якогось зарезервованого обсягу пам'яті не вистачає. У грі використовується велика кількість брудних хаків. Наприклад, функція гонки по відбитому варіанту траси реалізована не відображенням моделі траси при її завантаженні, а переворотом кожного кадру при рендерінгу, з інверсією кнопок «ліво» і «право» та ряду аналогічних підмін там, де розробники не забули додати відповідний код. З-за цього написи на машинах (наприклад, копів) відображені, а під час перегонів копи плутають «право» і «ліво» при переговорах по рації. Є і помилки, явно пов'язані з використанням магічних чисел в коді замість іменованих констант, коли значення константи в процесі розробки було змінено, але залишився код, який використовує старе значення і працює неправильно. Але один випадок виявився цікавим настільки, що мені захотілося поділитися ним в рамках невеликої замітки.

Скільки помилок може бути в 4 варіантах одного й того ж коду?
У грі присутній один незначний баг, пов'язаний з відступами в випадаючих меню.

imageimage

Наприклад, якщо вибрано режим «Hot Pursuit», лівий відступ всіх верхніх випадаючих списків значно збільшувався без явної на те необхідності. Я припустив, що це пов'язано з помилкою в коді, який додає простір для виведення іконок проходження у випадаючому списку трас, які якраз і повинні відображатися тільки в режимі «Hot Pursuit».

imageimage

Звичайні випадаючі списки теж мали проблеми з відступами залежно від режиму, але не в такій явній формі. Оскільки я якраз працював поруч з відповідним кодом, вирішив заразом виправити і це дивна поведінка.

Тут варто зазначити, що в коді гри не зустрічаються сліди речей на зразок наслідування, так що швидше за все вона писалася на чистому C. У грі реалізований код, який читає списки елементів меню та їх властивості зовнішніх текстових файлів, але розробники часто лінувалися додавати підтримку якогось нового властивості, і писали додатковий код, спираючись просто на ім'я елемента. Наприклад, код виведення випадаючих списків сам перевіряє назва поточного елемента і якісь інші змінні, і в залежності від цього може застосовувати якусь додаткову логіку.

Так було і тут. Оскільки в меню використовується два різних типу випадаючих списків (звичайний і верхній), весь код роботи з ними був цілком повторюється двічі. Крім цього виявилося, що лівий відступ вибирається різними фрагментами коду при обчисленні лівої і правої межі випадаючого списку. Разом — по ідеї в нас повинно бути 4 копії одного і того ж коду. Як би не так!

Варіант 1 (при обчисленні відступу лівої межі звичайної выпадашки):
image
На C це виглядало приблизно так:
dw_padding = (stricmp(str_element_name, "tracks") == 0 && dw_cfg_race_type == 3) ? 35 : 15;

Варіант 2 (при обчисленні відступу правої межі звичайної выпадашки):
image
На C це виглядало приблизно так:
dw_padding = (stricmp(str_element_name, "tracks") == 0 || stricmp(str_element_name, "rectrk") == 0) ? 35 : 15;

Варіанти 3 і 4 (при обчисленні відступу лівої і правої меж выпадашки під заголовком):
imageimage
На C це виглядало приблизно так:
dw_padding = (dw_cfg_race_type == 3) ? 35 : 15;

Всі варіанти разом (для наочності):
dw_padding = (stricmp(str_element_name, "tracks") == 0 && dw_cfg_race_type == 3) ? 35 : 15;
dw_padding = (stricmp(str_element_name, "tracks") == 0 || stricmp(str_element_name, "rectrk") == 0) ? 35 : 15;
dw_padding = (dw_cfg_race_type == 3) ? 35 : 15;

Який же варіант правильний? Відповідь: ні один! Тільки якщо ми об'єднаємо всі перевірки разом, ми можемо отримати єдино вірний варіант коду, і виглядав він приблизно так:
dw_padding = (dw_cfg_race_type == 3 && (stricmp(str_element_name, "tracks") == 0 || stricmp(str_element_name, "rectrk") == 0)) ? 35 : 15;

Разом у нас 4 варіанти одного і того ж коду, при цьому в різних варіантах допущено 3 різні помилки! Просто унікальний випадок і відмінна демонстрація того, чому копіпаста — це погана ідея.

Як це було виправлено?
Про те, як вносяться зміни в машинний код, я писав раніше. Для виправлення проблеми я написав одну функцію:
image
Всі наведені вище фрагменти коду були замінені на виклик цієї функції. Тепер відступи в списках вибираються правильно :)

Висновки?
А тепер подумайте. Якщо тут знайшовся самозванець, який взявся виправляти без дозволу чужі баги в програмі без вихідних кодів, може знайтися і той, хто в підсумку буде настільки злий, що у нього з'явиться бажання відшукати розробника або його родичів. А воно вам треба? Пишіть якісний код :)
Джерело: Хабрахабр

0 коментарів

Тільки зареєстровані та авторизовані користувачі можуть залишати коментарі.