Соглашение об оформлении кода

Материал из Руководство по OpenKore
Версия от 14:20, 22 января 2019; 4epT (обсуждение | вклад) (4epT переименовал страницу Соглашения по разработке в Соглашение об оформлении кода без оставления перенаправления)
Перейти к навигации Перейти к поиску

Отказ от ответственности: эта страница сделана из существующего источника openkore и здравого смысла. Это только рекомендации, которые не всегда применимы.

Обзор

Основная цель состоит в том, чтобы написать код, который бы соответствовал другому коду вокруг него и системе в целом. Если файл, который Вы редактируете, уже отклоняется от этих рекомендаций, делайте также. После того, как Вы отредактируете файл, читатель не должен определить над какими частями вы работали по стилю кодирования. - из Соглашения об оформлении кода для C (пункт 9)

Функция или Плагин?

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

Обратная совместимость

В OpenKore была отличная обратная совместимость как для пользователей, так и для разработчиков, и, как правило, она сохраняется, если не вмешиваются горе-разработчики: давай-ка-всё-сломаем-мне-всё-равно. Это так прекрасно, что почти все команды и параметры конфигурации Skore (предшественника OpenKore с 2003 года), всё ещё могут использоваться точно так же, как и раньше. Это одно из величайших преимуществ Kore - независимо от того, какие функции добавлены, исправлены ошибки, оно всё ещё работает.

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

Недавно реализованные или экспериментальные функции могут меньше зависеть от обратной совместимости.

Обоснованные примеры нарушения совместимости:

  • удаление уже устаревших API, которые вызывали проблемы (хэш %field - но даже это могло остаться совместимым, вместо удаления, и это изменение вызвало много независимых проблем)
  • переделка сломанных и плохо функционирующих систем (режим XKore очень сильно зависит от системы serverType)
  • неопасные изменения имени служебных команд, чтобы получить более единообразное наименование команд в целом (cl теперь список чата (chat list), а не очистка журнала (clear log))

Необоснованные примеры нарушения совместимости:

  • настраиваемые для каждого сервера названия предметов/умений/монстров, которые ломают все конфигурации - нарушают совместимость
  • изменения аргументов хука без необходимости (r7726) - изменение существующих аргументов хука - плохая идея, просто добавьте новые аргументы или добавьте больше хуков (добавление аргументов к существующим хукам - это нормально); в любом случае все несовместимые изменения хуков ДОЛЖНЫ быть добавлены в документацию Porting Plugins
  • нарушение существующего поведения команд (r7653 - исправлено)
  • неясные и не должным образом протестированные (необходимы юнит-тесты) изменения основных систем, которые могут привести к поломке многих случайных вещей (экземпляры карт - исправлено; LOS (прямая видимость))

Примеры совместимости:

  • storageAuto_npc_type сообщит вам, пытаетесь ли вы использовать устаревшее (и удаленное) значение '0'

Behaviour changes

While some changes may not really break compatibility, they may introduce some unwanted behaviour changes. Treat this situation the same as compatibility changes.

Unjustified behaviour breaking examples:

  • removing secure-by-default behaviour which many users rely on, making it a non-default option when motives for such behaviour in the first place weren't changed (r8118, shopAuto_open and lockMap - fixed)

Consistency

Generally, new features and names should follow the general ideas of naming of existing stuff; be guessable so you don't have to look documentation up every time you're need them; and reflect what named features actually do.

Bad examples:

  • introcuding contradiction in naming of the same thing across different features (unknown_09A1/sync_received_characters/sync_received_characters_09A1, received_characters_info/characters_slots_info for the same packets in different serverTypes, and subsequent XKore "fixes" which broke all serverTypes except the one being fixed; skill_post_delay packet was added in ServerType0, leaving already existing skill_postdelay in kRO as is); for packets and other things with native identifiers always grep all source to find out how it's already called, and make one common name for it
  • total mismatch between name and feature (teleportAuto_search isn't for teleporting and won't really do any search by itself; buy_bulk_vender packet is not for buying, but for selling items)
  • hijacking of names in existing name theme (pl, ml, vl etc listing commands, but cl clear log - fixed)
  • mismatch in naming scope (shop_LockOnly affects only shopAuto_* options, compare with shop_random which affects both auto and manually opened shops - fixed)
  • not following general name uppercasing theme (shop_LockOnly, while all other options use lowercase letter after an underscore; GANSI_RANK and ISVR_DISCONNECT packets, while all other packets prefer lowercase names - fixed)
  • not following similar name theme across different features (shop_LockOnly, while there already were inLockOnly, attackAuto_inLockOnly, avoidList_inLockOnly - fixed)
  • using random abbreviations, especially in a set where no other member is abbreviated (Glt. Cross job name), especially when it's used in configurations (such names are usually unguessable unless you look them up); our own conventional abbreviations, like LOS, KS, Dmg are fine to stick to. If you name doesn't fit into some interface's display, it's a problem of interface only and should be dealt in it (and we have many different ones) - names shouldn't be distorted just because of some interface

If you're unsure of how to name your new options or other stuff, just ask at forums for opinions.

Formatting

K&R style, with opening brace on the same line with all declarations.

Indent with TABs. Don't use TABs for other means. There is no defined column size for TAB, each developer can have its own preference, respect that.

sub test {
<TAB> if ($a == $b) {
<TAB> <TAB> something();
<TAB> <TAB> something_else($a, $b);
<TAB> }
<TAB> $twenty = 20;
<TAB> $ten    = 10; # use spaces if you want to align (not indent) code like this for some reason
}

Naming

Language: only english

Constants: UPPER_CASE

Variables: $camelCase or $underscore_separated

Packet "names": underscore_separated_words

Config option names: camelCase, regular words like "auto" at the end, however underscore may separate a common prefix (teleportAuto). Underscore is also internally used to represent config blocks

Comments

Language: only english, with no translations to other languages; quotations from sources on another languages may be included if it's necessary

Keywords: FIXME, TODO

Notes

This is a work of fiction. Names, characters, places and incidents either are products of the author's imagination or are used fictitiously. Any resemblance to actual code or data or persons, living or dead, is entirely coincidental.

You should not just go and "fix" names for teleportAuto_search and Glt. Cross, as just renaming them would break existing users' configurations.