development Conventions

Disclaimer: this page is made of existing openkore source and common sense. These are only guidelines which may not always apply.

Overview

Ultimately, the goal is to write code that fits in with the other code around it and the system as a whole. If the file you are editing already deviates from these guidelines, do what it does. After you edit a file, a reader should not be able to tell just from coding style which parts you worked on. - from Plan 9 coding conventions for C

Feature or Plugin?

Many features, old and new, are better implemented as plugins. It helps with managing development and usage. We can bundle some "core" plugins with openkore so they would be available and enabled by default.

Backwards compatibility

OpenKore used to have excellent backwards compatibility both for users and developers, and generally still does, unless non-caring let-s-break-this-I-dont-care contributors prevail. It's so excellent that almost all commands and configuration options of Skore, the predecessor of OpenKore back from 2003, still may be used exactly in the same way as before. That's one of the greatest advantages of Kore - regardless of features added, bugs fixed and systems reworked, you're still good with it as you were some day.

New versions of openkore shouldn't break old configurations and plugins. It means to avoid unwarranted changes to any existing behaviour, any default behaviour or any internal programming interfaces and names. It doesn't help users only, but developers familiar with various parts of codebase as well.

Recently implemented or highly experimental stuff may depend less or backwards compatibility.

Justified compatibility breaking examples:

• removal of already deprecated APIs which caused problems (%field hash - but even it could have been tie'd instead of removal, and this change caused many problems regardless)
• rework of broken and hardly functioning systems (XKore transition to be more dependent on serverType system)
• non-dangerous name changes of utility commands, to get more uniform command naming overall (cl is chat list now, not clear log)

Unjustified compatibility breaking examples:

• customizable per-server item/skill/monster names, which broke all configurations - broke compatibility and interoperability hard
• hook argument changes just for the sake of it (r7726) - changing existing hook arguments is never a good idea, just add more arguments or add more hooks instead (adding arguments to existing hooks is fine); anyway, all incompatible hook changes MUST be added to Porting Plugins documentation
• breaking existing behaviour of commands (r7653 - fixed)
• obscure and not properly tested (unittests are necessary) changes of core systems, which may break many random things (map instances - fixed; line of sight)

Compatibility workaround examples:

• storageAuto_npc_type would tell you if you're trying to use deprecated (and removed) 0 value

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.